lkml.org 
[lkml]   [2013]   [Sep]   [29]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
SubjectRe: [rfc][possible solution] RCU vfsmounts
From
On Sun, Sep 29, 2013 at 11:10 AM, Al Viro <viro@zeniv.linux.org.uk> wrote:
>
> FWIW, right now I'm reviewing the subset of fs code that can be hit in
> RCU mode. Not a pretty sight, that... ;-/ First catch: in
> fuse_dentry_revalidate() we have a case (reachable with LOOKUP_RCU) where
> we do this:
> } else if (inode) {
> fc = get_fuse_conn(inode);
> if (fc->readdirplus_auto) {
> parent = dget_parent(entry);
> fuse_advise_use_readdirplus(parent->d_inode);
> dput(parent);
> }
> }

Ugh, yes, that dget/dput(parent) looks wrong in RCU mode.

That said, in RCU mode you simply shouldn't _need_ it at all, you
should be able to just use dentry->d_parent without any refcount
games. Put an ACCESS_ONCE there to be safe. You might want to make
sure that you do the same for the inode, and check for NULL, to be
safe against racing with a cross-directory rename/rmdir. I don't know
if there are then any internal fuse races with the whole
get_fuse_conn() etc, so...

It does look bad. In practice, of course, it will never hit anything.

> If my reading of that code is right, the proper fix would be to
> turn that else if (inode) into else if (inode && !(flags & LOOKUP_RCU))

That sounds safer, but then the fuse_advise_use_readdirplus() bit
wouldn't get set. But why _is_ that bit set there in the first place?
It sounds stupid. I think the bit should be set in the lookup path (or
the revalidation slow-path when the timeout is over and the thing gets
properly revalidated), why the hell does it do it in the fast-path
revalidation in the first place? That's just odd. Maybe there is some
odd internal fuse logic.

Miklos, please do give that a look..

Linus


\
 
 \ /
  Last update: 2013-09-29 20:41    [W:0.225 / U:0.008 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site