2.6.36: Sound stop working
in [Kernel]
|
Prev: [tip:perf/urgent] perf: Add back list_head data types
Next: [tip:x86/uv] x86, UV: Make kdump avoid stack dumps - fix !CONFIG_KEXEC breakage
From: Takashi Iwai on 12 Aug 2010 17:50 At Thu, 12 Aug 2010 23:24:43 +0200, Jiri Slaby wrote: > > On 08/12/2010 11:18 PM, Linus Torvalds wrote: > > On Thu, Aug 12, 2010 at 2:01 PM, Jiri Slaby <jirislaby(a)gmail.com> wrote: > >> Probably I got into this problem yesterday. Found out that PA fails to > >> open /dev/snd/pcmC0D0p _second_ time. It opens it, then closes, then > >> opens it again and gets EBUSY. aplay is OK. > > Perfectly reproducible in qemu-kvm with ac97 soundhw, i.e. intel8x0 > driver. Just in case you want to debug that easily. And the below is a minimal test case to simulate the situation PulseAudio does. Takashi === #include <stdio.h> #include <sys/types.h> #include <sys/stat.h> #include <fcntl.h> #include <sys/inotify.h> #include <unistd.h> int main() { int fd; inotify_add_watch(inotify_init(), "/dev/snd", IN_CLOSE_WRITE); fd = open("/dev/snd/pcmC0D0p", O_RDWR | O_NONBLOCK); if (fd < 0) perror("open1"); else close(fd); fd = open("/dev/snd/pcmC0D0p", O_RDWR | O_NONBLOCK); if (fd < 0) perror("open2"); else close(fd); return 0; } -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo(a)vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
From: Linus Torvalds on 12 Aug 2010 17:50 On Thu, Aug 12, 2010 at 2:17 PM, Takashi Iwai <tiwai(a)suse.de> wrote: > At Thu, 12 Aug 2010 23:01:04 +0200, > Jiri Slaby wrote: >> >> Probably I got into this problem yesterday. Found out that PA fails to >> open /dev/snd/pcmC0D0p _second_ time. It opens it, then closes, then >> opens it again and gets EBUSY. aplay is OK. > > Yes, I can also confirm that it's broken on my machine in the same > way now :) PA log shows that the succeeding open failed. > > PA tries to do quick open/close of the same device to figure out which > configuration is available at start-up. This implies that the > fs/notify commits touching the open/close stuff can be the culprit. Yeah. The f_count stuff is disgusting. This revert patch makes things work for me again. And I suspect it's the right thing to do regardless. I reacted to that ugly __fput() hackery when I pulled the fanotify tree, but I let it slide. I clearly should have let my taste guide me more. fsnotify playing games with fput/fget is almost certainly totally wrong. Al, what was the problem that caused you to think that we want to have a 'struct file' here? Is it just the fact that some of those fsnotify things use that 'path' structure that is embedded in the parent? But isn't the simplest fix for that to just _copy_ the "struct path" rather than pass the "struct file" around? Linus
From: Linus Torvalds on 12 Aug 2010 18:00 On Thu, Aug 12, 2010 at 2:41 PM, Linus Torvalds <torvalds(a)linux-foundation.org> wrote: > > Al, what was the problem that caused you to think that we want to have > a 'struct file' here? Is it just the fact that some of those fsnotify > things use that 'path' structure that is embedded in the parent? But > isn't the simplest fix for that to just _copy_ the "struct path" > rather than pass the "struct file" around? Btw, that's exactly what the fsnotify code does seem to do, in fsnotify_create_event(). So as far as I can tell, it's all ok - we only use that file->f_path pointer while we hold the file open, and then we copy it to event->path and increment the counts properly. Of course, maybe I'm missing some other case where we _don't_ copy the data, and pass a pointer to a file->f_path around that could get stale. Or maybe I'm missing some other worry entirely. But those games with f_count are just disgusting. The path-based thing seems to be much more straightforward. Linus -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo(a)vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
From: Takashi Iwai on 12 Aug 2010 18:10 At Thu, 12 Aug 2010 14:41:51 -0700, Linus Torvalds wrote: > > On Thu, Aug 12, 2010 at 2:17 PM, Takashi Iwai <tiwai(a)suse.de> wrote: > > At Thu, 12 Aug 2010 23:01:04 +0200, > > Jiri Slaby wrote: > >> > >> Probably I got into this problem yesterday. Found out that PA fails to > >> open /dev/snd/pcmC0D0p _second_ time. It opens it, then closes, then > >> opens it again and gets EBUSY. aplay is OK. > > > > Yes, I can also confirm that it's broken on my machine in the same > > way now :) PA log shows that the succeeding open failed. > > > > PA tries to do quick open/close of the same device to figure out which > > configuration is available at start-up. This implies that the > > fs/notify commits touching the open/close stuff can be the culprit. > > Yeah. The f_count stuff is disgusting. This revert patch makes things > work for me again. It makes working for me, too. thanks, Takashi -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo(a)vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
From: Eric Paris on 12 Aug 2010 22:00
On Thu, 2010-08-12 at 15:19 -0700, Linus Torvalds wrote: > On Thu, Aug 12, 2010 at 2:52 PM, Al Viro <viro(a)zeniv.linux.org.uk> wrote: > > On Thu, Aug 12, 2010 at 02:41:51PM -0700, Linus Torvalds wrote: > > I'll see what can be done to fix that mess more or less right way... > > .. but I assume you don't wan tto keep those "struct file" games in > fanotify regardless, right? So we can fix the sound issue by the > revert I sent out, no? Sorry I'm just coming into this now that it is 'solved.' I just got back to my hotel from Linuxcon and haven't been checking e-mail. I guess noone (including me) is testing sound in linux-next :(. I wonder what kind of tom foolery it must be doing with f_count (like I am) to have hit problems. I certainly agree the revert patch you sent should be applied if the worst artifact for calling dentry_open() with an 'arbitrary dentry' is that it fails. An easier long term fix might be a 'dentry_open_as' which I can call in the context of the original opening process but which will be done (for security/accounting/fd table/etc purposes) as the process which will ultimately consume the new struct file. I don't know which would be better/easier for the VFS, Al? -Eric -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo(a)vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/ |