Re: find_fh_dentry returned a DISCONNECTED directory

[Josef Bacik <jbacik@fb.com>]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1



On 02/13/2014 10:30 PM, J. Bruce Fields wrote:
> On Thu, Feb 13, 2014 at 03:45:16PM -0800, Eric W. Biederman wrote:
>> "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.
> 
> You're right, of course.  I wasn't thinking straight.
> 
> So the only dentries with DISCONNECTED set are those created with 
> d_obtain_alias, which is normally only used when you're looking up
> by filehandle.
> 
> Except btrfs has a weird use in get_default_root().  So maybe they
> were running into the dentry that created?
> 
> So btrfs should probably be using something else, I'm not sure
> what.
> 

Course now that I look at it I'm not sure what to do.  We know the
location of the inode we want to use as our root dentry, but we could
already have a dentry for this in cache which is why we use
d_obtain_alias().  If we use d_make_root() and then wander into the
directory later we end up with inodes left over.  So should we just
build a path to the location and do the path lookup stuff so we have a
valid dentry?  Thanks,

Josef
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1
Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/

iQIcBAEBAgAGBQJS/jKfAAoJEANb+wAKly3B1q4P/1wgkd/w6QI6A3c40Qw5Oyfk
gTOFcSrCnGpd59wOzCu3B8fKuVygBweJ0vwYMlnNgGjCzqidTt37LCD+rRb1iz0w
BZVSvWS5v8qeaW+qQJJSQH83NX8v+L6zc/5D1stwPIfctXf0WvSijmacV2/BGRgJ
PqZVXd4yQhd24cqonXUzqNswiFOmPUPs8xrSz3NaR4GcRFVMLuRuubpf/pWkotmI
mldm8p+SCrDiQHgXEYYHKW1rHKjxKU4GIc9+Dsx0Jjnv26ITiooM8pY2tWW8w98S
CQv54VQGzzAmyNHQxmv/3sxPg9K4CL+8t7Z4nLQ043K9U9itd+YU/v/vvnzAXvob
KX5amGZc0cWyYHu9e/9i4HXOJmrmEUdwl5megeI47V/LStzkRUggeha+tRzm4xQW
+nT4dpueUK1ezDDPvJdOF9id9WNWaGwxKqg6Mqj9+tsnmOwYll2zaNuoOVUZacWY
Qmv+1JS6gdE8/qLTVjp9I4nTdGN9Cr/sE1EwBVM/GFbCQzAxe4m4uKnXtJ28+Noo
LOg/u27GSssO6oUFA8Mj6dnTEOKtm5QOMDD+4lxRSyWuAHCOvNg6I4SRykHb1o1I
OexIp6DK1Siofj6mAFGG5AZunXYH8uR8aOH4Ctva04vC1D6TlZsI5ycgE6V51NUk
Rv4ICvuwjuu3nZRLLKve
=KK7m
-----END PGP SIGNATURE-----