On Sun, Sep 24, 2023 at 11:03:54PM +0300, Vitaliy Makkoveev wrote:
> Please test this diff, I have no midi(4) devices.
>
> midi(4) already uses `audio_lock' mutex(9) for filterops, but they are
> still kernel locked. Wipe out old selwakeup API and make them MP safe.
> knote_locked(9) will not grab kernel lock, so call it directly from
> interrupt handlers instead of scheduling software interrupts.
>
Works for me (tested with a usb device), pulling the usb cable
properly wakes up any process waiting for input/output.
ok ratchov@ with the two changes below
> @@ -531,20 +518,8 @@ midiattach(struct device *parent, struct
> }
> #endif
>
> - sc->inbuf.softintr = softintr_establish(IPL_SOFTMIDI,
> - midi_buf_wakeup, &sc->inbuf);
> - if (sc->inbuf.softintr == NULL) {
> - printf("%s: can't establish input softintr\n", DEVNAME(sc));
> - return;
> - }
> -
> - sc->outbuf.softintr = softintr_establish(IPL_SOFTMIDI,
> - midi_buf_wakeup, &sc->outbuf);
> - if (sc->outbuf.softintr == NULL) {
> - printf("%s: can't establish output softintr\n", DEVNAME(sc));
> - softintr_disestablish(sc->inbuf.softintr);
> - return;
> - }
> + klist_init_mutex(&sc->inbuf.klist, &audio_lock);
> + klist_init_mutex(&sc->outbuf.klist, &audio_lock);
>
> sc->hw_if = hwif;
> sc->hw_hdl = hdl;
> @@ -580,27 +555,19 @@ mididetach(struct device *self, int flag
> KERNEL_ASSERT_LOCKED();
> if (sc->flags & FREAD) {
> wakeup(&sc->inbuf.blocking);
> - mtx_enter(&audio_lock);
> - selwakeup(&sc->inbuf.sel);
> - mtx_leave(&audio_lock);
> + knote(&sc->inbuf.klist, 0);
AFAIU, the knote() calls are not necessary because klist_invalidate()
already does the job.
> }
> if (sc->flags & FWRITE) {
> wakeup(&sc->outbuf.blocking);
> - mtx_enter(&audio_lock);
> - selwakeup(&sc->outbuf.sel);
> - mtx_leave(&audio_lock);
> + knote(&sc->outbuf.klist, 0);
> }
> sc->hw_if->close(sc->hw_hdl);
> sc->flags = 0;
> }
>
> - klist_invalidate(&sc->inbuf.sel.si_note);
> - klist_invalidate(&sc->outbuf.sel.si_note);
> + klist_invalidate(&sc->inbuf.klist);
> + klist_invalidate(&sc->outbuf.klist);
>
Could you add two klist_free() calls here? They are simple asserts,
but they'd make the attach/detach functions more midi(4) would look
more like audio(4).
