OSS: allow unplug sound cards without apps close devices
AbandonedPublic
Actions

Authored by rozhuk.im-gmail.com on Jun 10 2017, 8:50 PM.

Details

Reviewers

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

Diff Detail

Lint

Lint Skipped

Unit

Tests Skipped

Event Timeline

rozhuk.im-gmail.com created this revision.Jun 10 2017, 8:50 PM

Herald added a subscriber: imp. · View Herald TranscriptJun 10 2017, 8:50 PM

Hi,

Did you check that no memory is used after free?

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

--HPS

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

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

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

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.

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

--HPS

emaste added a subscriber: emaste.Jul 25 2017, 1:49 AM

This is only mixer unplug fix.

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?

dan.kotowski_a9development.com added a subscriber: dan.kotowski_a9development.com.Aug 12 2021, 10:18 AM

• grahamperrin retitled this revision from OSS: allow unplug soundcars without apps close devices to OSS: allow unplug sound cards without apps close devices.Oct 31 2021, 9:42 AM

• grahamperrin added a subscriber: • grahamperrin.

moonlapse81_gmail.com added a subscriber: moonlapse81_gmail.com.Feb 5 2022, 9:27 AM

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

FYI