Re: [PATCH] fuse: make fuse_permission() RCU aware

From: Miklos Szeredi
Date: Thu Jan 13 2011 - 06:09:17 EST


On Thu, 13 Jan 2011, Nick Piggin wrote:
> > Â Â Â Âinode = entry->d_inode;
> > Â Â Â Âif (inode && is_bad_inode(inode))
> > Â Â Â Â Â Â Â Âreturn 0;
>
> Now it can be the case that entry->d_inode is not stable -- it can
> go away or even flip between inodes in the case of concurrent
> unlink/creat activity. And you may be using a different inode than
> the namei path walk is using!
> This isn't as scary as it sounds actually, because any such changes
> do get detected and the path walk restarted in that case.
>
> You might be OK becuse you do test for NULL (although it really
> wants an ACCESS_ONCE() so it doesn't load a NULL or different
> inodes, but that's quite theoretical).
>
> But this is a little unfriendly for filesystems, and I do want to impress
> the rule to not touch ->d_parent or ->d_inode in rcu walk mode
> (just to avoid any surprises).
>
> So what I have done in such cases is to update the API to provide
> what the callers want. In this case, we could consider adding an
> inode parameter to .d_revalidate, which callers can be sure matches
> the inode used by vfs, and will not change.

Yes, I think supplying an inode to ->d_revalidate() would be the right
thing.

I see that there's nd->inode, and it's documented in vfs.txt, but IMO
we shouln'd be encouraging new uses of 'nd' inside filesystem
callbacks, rather the opposite. A 'flags' parameter too might make
sense which, for the moment, would only have LOOKUP_RCU instead of all
the lookup flags.

Thanks,
Miklos
--
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/