Re: [PATCH] [REPOST] fuse: drop dentry on failed revalidate

[Ric Wheeler <rwheeler@redhat.com>]

On 07/30/2013 12:16 PM, Miklos Szeredi wrote:
>>>    fs/fuse/dir.c | 4 ++++
>>>    1 file changed, 4 insertions(+)
>>>
>>> diff --git a/fs/fuse/dir.c b/fs/fuse/dir.c
>>> index a1d9047..83c217e 100644
>>> --- a/fs/fuse/dir.c
>>> +++ b/fs/fuse/dir.c
>>> @@ -226,6 +226,10 @@ static int fuse_dentry_revalidate(struct dentry
>>> *entry, unsigned int flags)
>>>                  if (!err) {
>>>                          struct fuse_inode *fi = get_fuse_inode(inode);
>>>                          if (outarg.nodeid != get_node_id(inode)) {
>>> +                               if (!have_submounts(entry)) {
>>> +                                       shrink_dcache_parent(entry);
>>> +                                       d_drop(entry);
>>> +                               }
> Doing d_drop() on a subtree really has problems.  Take this example:
>
> cd /mnt/foo/bar
> mkdir my-mount-point
> [->d_revalidate() drops "/mnt/foo"]
> mount whatever on ./my-mount-point
> cd /
>
> At this point that mount is not accessible in any way.  The only way
> to umount it is to lazy umount the parent mount.  If the parent mount
> was root, then that's not a practical thing to do.
>
> AFAICS nothing prevents this from happening on NFS and root privs are
> not required (e.g. mounting a fuse fs on an NFS home dir).
>
> The other problem is that, unlike NFS, fuse doesn't currently
> reconnect these subtrees when it finds them at a different point in
> the tree.  d_drop on it just makes things worse because at that point
> that subtree will not be accessible anymore (while the fuse fs is
> mounted, that is).  This could be fixed pretty easily by using the
> d_materialise_*() helpers.
>
> But issues with mounting over an unhashed subtree *should* be addressed.
>
> Recursively d_drop()-ing might still be the simplest way.  Need to
> make sure the subtree doesn't change in the meantime.  Holding
> rename_lock might do the trick.  But then there's still an unlikely
> race with mount()...
>
> Thanks,
> Miklos


Hi Miklos,

Do you have a respin of the proposed patch that we can put through testing?  We 
see this with hadoop on top of gluster and have lots of testers ready and waiting :)

Al, do you have any thoughts on this?

Thanks!

ric