Re: 2.6.36: Sound stop working

From: Linus Torvalds
Date: Thu Aug 12 2010 - 18:20:18 EST


On Thu, Aug 12, 2010 at 2:52 PM, Al Viro <viro@xxxxxxxxxxxxxxxxxx> wrote:
> On Thu, Aug 12, 2010 at 02:41:51PM -0700, Linus Torvalds wrote:
>
>> 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?
>
> I agree that what this crap is doing to f_count is blatantly wrong,
> of course - no arguments here.  I *do* have a reason to want struct
> file, but not that way, TYVM.
>
> Basically, dentry_open() is not even promised to work on arbitrary
> dentry.  Thanks to !@#!@#!@#!@# intents crap we are not promised
> that the damn thing won't need setup by ->lookup/->d_revalidate.
> We _are_ more or less promised that it'll work while the file is
> opened (provided that /proc/*/fd/* is openable), but that's it.

Hmm. We do basically dentry_open() these days with the whole
path_init() of a dfd. Yes, we have the 'struct file', but the only
thing we take from there is the dentry.

So why not just say that 'dentry_open()' is exactly that path_init()
setup, and then look up an empty path. Allow a non-directory, and
leave it to the filesystems to then say "I can't do that at all" if
they really want to. fanotify seems to be perfectly happy with the
lookup failing.

So I'm not convinced this is really a fanotify problem, as much as a
VFS layer thing.

> It's not an API that can be made to work on an arbitrary dentries.  If
> caller knows what it's dealing with, it's OK, but not in general.  And
> no, I'm not fond of that situation, to put it mildly.

So I think it's probably doable for arbitrary dentries too, although
maybe we need to have a special "reopen()" function. I suspect few
filesystems will care, and if you can fail it, the ones that do care
deeply are home free too. No?

> 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?

Linus
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/