Re: [PATCH] ALSA: rawmidi: give up draining output when the device stops draining

[Takashi Iwai <tiwai@suse.de>]

On Sun, 20 Sep 2026 15:06:20 +0200,
Akhil Arul wrote:
> 
> snd_rawmidi_drain_output() waits up to 10 seconds for the output buffer
> to empty.  The wait is unconditional, so a substream that has stopped
> being consumed costs the full timeout on every call.
> 
> The sequencer OSS emulation makes that expensive.  midisynth_unuse() runs
> as the port unuse callback with grp->list_mutex held for write, and calls
> snd_rawmidi_drain_output().  A single teardown closes every OSS midi port
> of the device, so an unresponsive device exposing many ports holds the
> rwsem for minutes.  snd_seq_port_connect() needs the same rwsem, and in
> the OSS path it runs under register_mutex, so every other odev_open()
> queues up behind it until the hung task detector fires:
> 
>   INFO: task syz.4.21:6176 blocked for more than 143 seconds.
>     __mutex_lock
>     odev_open
>     chrdev_open
>     vfs_open
>     path_openat
> 
> Detect a stalled drain by sampling the free space instead of always
> sleeping for the whole timeout, and give up once it has not increased for
> a second.  A substream that is still making progress is given as long as
> it needs, within the same overall 10 second limit as before, and each
> wait is clipped to the remaining time so that limit is not overshot.
> 
> The warning is rate limited because a stalled drain is now detected much
> more often than once per 10 seconds.
> 
> Closing one such device in qemu, with the syzkaller reproducer supplying
> the gadget, took 72-103 seconds before and 9-11 seconds after.
> 
> Reported-by: syzbot+825b7e3a03dd072c187f@syzkaller.appspotmail.com
> Closes: https://syzkaller.appspot.com/bug?extid=825b7e3a03dd072c187f
> Signed-off-by: Akhil Arul <akhilarul324@gmail.com>
> ---
> Went with the adaptive one in the end.
> 
> I tried the fixed cap first and couldn't make it stand up. buffer_size is
> PAGE_SIZE, so 16K or 64K on arm64, and PARAMS goes to 1MiB, so any constant
> I pick is really a guess about the buffer. Test device consuming at the
> MIDI-1 wire rate:
> 
>   buffer    unpatched      2*HZ cap        adaptive
>    4096     ok 1.42s       ok 1.44s        ok 1.43s
>   16384     ok 5.60s       -EIO 2.06s      ok 5.63s
>   65536     -EIO 10.06s    -EIO 2.06s      -EIO 10.06s
> 
> The middle row is a healthy device at full rate failing because the kernel
> has 16K pages. That killed it for me.
> 
> The adaptive version keeps the 10s ceiling and only bails out when avail
> hasn't moved for a second, so those rows stay as they are. It also happens
> to fix the hang better than the cap did. Closing one /dev/sequencer2 with
> the syzkaller gadget attached, three runs each:
> 
>   unpatched   72.0s   102.7s   102.3s
>   adaptive    10.7s     8.7s    11.6s
>   2*HZ cap    18.4s    12.4s    16.5s
> 
> The teardown still calls the drain the same ~31 times either way, 9 to 15
> of which time out; only the cost of each timeout moves. Hung task detector
> at the default 120s stays quiet over three runs of about four minutes, with
> the repro plus two threads opening /dev/sequencer2.
> 
> The cost is that a substream which goes quiet for a second with data still
> queued now gets -EIO. Same device, pausing once and then resuming:
> 
>   pause    unpatched     patched
>    950ms   ok 3.53s      ok 3.53s
>   1050ms   ok 3.83s      ok 3.84s
>   1200ms   ok 4.28s      -EIO 1.30s
>   2000ms   ok 6.68s      -EIO 1.31s
> 
> So it's 1.05-1.2s rather than exactly a second. The poll grid is 200ms and
> the cutoff drifts depending on where the pause lands against it.
> 
> I said 400ms in my last mail. That turned out to cut off a device pausing
> for half a second, which a real one might do, so I widened it to a second.
> 
> Worth saying that avail isn't monotonic here - drain doesn't gate writers,
> it only forces the wakeup in snd_rawmidi_transmit_ack(). That's why the
> test is whether avail increased rather than whether it reached buffer_size.
> I checked a shared append substream with a second writer refilling it, in
> case that read as a stall. It doesn't: avail jitters instead of sitting
> flat, the counter keeps resetting and the drain runs the full deadline.
> With the refill matched to the consumption rate, unpatched gives -EIO at
> 10.07s and patched at 10.06s.
> 
> I couldn't find a way to bound the teardown without putting a cutoff
> somewhere. Whether a second of silence is enough to call a device stopped
> is your call.
> 
> No Fixes: tag, the 10s wait predates git.

I like the idea, but the code is for the core part and non-trivial, I
put this to for-next branch (for 7.4) now.  Thanks!


Takashi