From mboxrd@z Thu Jan 1 00:00:00 1970 From: "J. Bruce Fields" Subject: Re: find_fh_dentry returned a DISCONNECTED directory Date: Fri, 14 Feb 2014 10:38:34 -0500 Message-ID: <20140214153833.GF21982@fieldses.org> References: <20140213212701.GB21982@fieldses.org> <8738jm1ss3.fsf@xmission.com> <20140214033030.GC21982@fieldses.org> <52FE32A0.9070505@fb.com> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Cc: "Eric W. Biederman" , linux-fsdevel@vger.kernel.org To: Josef Bacik Return-path: Received: from fieldses.org ([174.143.236.118]:45496 "EHLO fieldses.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751065AbaBNPig (ORCPT ); Fri, 14 Feb 2014 10:38:36 -0500 Content-Disposition: inline In-Reply-To: <52FE32A0.9070505@fb.com> Sender: linux-fsdevel-owner@vger.kernel.org List-ID: On Fri, Feb 14, 2014 at 10:13:36AM -0500, Josef Bacik wrote: > -----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" 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(). That's exactly the sort of situation that d_obtain_alias() is meant to help you with. It's just that you don't want DCACHE_DISCONNECTED set. I'm not even sure if that causes any actual bug, but it seems confusing at least. Maybe it would be worth making a d_obtain_alias_for_root() or something for yours and nfs's use. --b. > 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-----