OSS: allow unplug sound cards without apps close devices

[rozhuk.im-gmail.com]

Summary

This patch fix use case: user unplug USB sound card before all apps close /dev/mixer and /dev/dsp and then USB driver hang/wait until all apps closed, no old devices can be removed, no new devices can be added.

dmesg show for mixer:
pcm4: unregister: mixer busy
pcm4: Waiting for sound application to exit!
And almost same for dsp, but also prints app pid.

Test Plan

• apply patch
• rebuild kernel
• install kernel and reboot
• start playing sound/music then unplug USB sound card
• connect sound card and restart player

[• hselasky]

Hi,

Did you check that no memory is used after free?

Did you build a kernel with WITNESS and memory debugging turned on?

--HPS

[rozhuk.im-gmail.com]

No.
How I can check mem use after free and how turn on mem debug?

[• hselasky]

Hi,

I'm afraid that pending read/write/ioctls might use freed structures. I'll try to analyze your patch carefully next week. Currently at bsdcan2017 .

--HPS

[• hselasky]

Hi,

Make a test app like this:

fd = open("/dev/dsp" ...);
while (1) {
if (ioctl(fd, SND_XXXX, XXX) != 0)
break;
}
close(fd);

And run it while you yank the USB audio device. Also try reading(), writing() and mmap() in a loop when detaching.

--HPS

[rozhuk.im-gmail.com]

Thank you.

Im not sure about race condition in case: app read/write/ioctl/close and in same time unplug code started in another thread.
May be destroy_dev() handle this.
If yes then I'm sure that mixer OK, but dsp code match more complicated, so I hope that some one with expert knowledge in OSS and dev subsystem will rewiew/help.

[• hselasky]

destroy_dev() will make sure all system calls have returned. In your patch are you destroying the device first?

--HPS

[rozhuk.im-gmail.com]

This is only mixer unplug fix.

[• hselasky]

I think you approach may work for the mixer case, assuming no IOCTLs are sleeping. Please make sure that destroy_dev() will flush all pending system calls.

sys/dev/sound/usb/uaudio.c

1239

Is this change needed. I think mixer_uninit() will now always succeed?

[• hselasky]

https://cgit.freebsd.org/ports/commit/?id=3bc8803e5c4a96b60ffe5fa6887e2b73e0df7756

FYI

[christos]

D43545 takes care of this change. Should we close this?