Re: find_fh_dentry returned a DISCONNECTED directory

[ebiederm@xmission.com (Eric W. Biederman)]

"J. Bruce Fields" <bfields@fieldses.org> writes:

> Yesterday you passed on a report of this printk from nfsdfh.c firing:
>
> 	printk("nfsd: find_fh_dentry returned a DISCONNECTED directory: %pd2\n",
> 	                                dentry);
>
> I think the dentry probably comes from the FILEID_ROOT case of:
>
> 	if (fileid_type == FILEID_ROOT)
> 		dentry = dget(exp->ex_path.dentry);
> 	else {
> 		dentry = exportfs_decode_fh(exp->ex_path.mnt, fid,
> 				data_left, fileid_type,
> 				nfsd_acceptable, exp);
>         }
>
> In that case the dentry was found using ordinary filesystem lookups, so
> doesn't go through the same DISCONNECTED-clearing logic as in the case
> of lookups by filehandle.
>
> Probably they have an export root that's not a filesystem root, and the
> lookups happened in the right order?
>
> I suspect that's fine, and that the printk is just stupid, but maybe we
> should clear DISCONNECTED when possible on normal lookups.  The
> following is my attempt, though I'm not sure if d_alloc is the right
> place to do this.  In any case it might help confirm this is what's
> happening.
>
> So if you pass along this patch to the person who was seeing that printk
> I'd be interested in the results.

I have been reading through the dentry code for other reasons and your
patch definitely won't change anything. __d_alloc sets d_flags = 0.
Therefore d_alloc always returns with d_flags == 0.


Eric

> --b.
>
>
> commit 49fc5637ca951d2add808a695768512f8b5bd7ad
> Author: J. Bruce Fields <bfields@redhat.com>
> Date:   Thu Feb 13 14:04:43 2014 -0500
>
>     dcache: clear DCACHE_DISCONNECTED on child when cleared on parent
>     
>     Currently DCACHE_DISCONNECTED is only cleared by the filehandle lookup
>     code.  That's OK, it's only the filehandle code that really cares about
>     this flag.  But it might be kinder to also clear DCACHE_DISCONNECTED on
>     regular lookups when we notice it's appropriate.
>     
>     Signed-off-by: J. Bruce Fields <bfields@redhat.com>
>
> diff --git a/fs/dcache.c b/fs/dcache.c
> index 265e0ce..c4a9478 100644
> --- a/fs/dcache.c
> +++ b/fs/dcache.c
> @@ -1534,6 +1534,16 @@ struct dentry *__d_alloc(struct super_block *sb, const struct qstr *name)
>  	return dentry;
>  }
>  
> +static void d_inherit_connected(struct dentry *dentry)
> +{
> +	/*
> +	 * If the parent is connected, then the child is too.  (But if
> +	 * DISCONNECTED is set, that doesn't tell us anything certain.)
> +	 */
> +	if (!(dentry->d_parent->d_flags & DCACHE_DISCONNECTED))
> +		dentry->d_flags &= ~DCACHE_DISCONNECTED;
> +}
> +
>  /**
>   * d_alloc	-	allocate a dcache entry
>   * @parent: parent of entry to allocate
> @@ -1556,6 +1566,7 @@ struct dentry *d_alloc(struct dentry * parent, const struct qstr *name)
>  	 */
>  	__dget_dlock(parent);
>  	dentry->d_parent = parent;
> +	d_inherit_connected(dentry);
>  	list_add(&dentry->d_u.d_child, &parent->d_subdirs);
>  	spin_unlock(&parent->d_lock);
>  
> --
> To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html