Re: [rfc][possible solution] RCU vfsmounts

[Linus Torvalds]

On Sun, Sep 29, 2013 at 11:10 AM, Al Viro <viro@xxxxxxxxxxxxxxxxxx> 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
--
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/