From mboxrd@z Thu Jan 1 00:00:00 1970 From: "J. Bruce Fields" Subject: Re: lifetime of DCACHE_DISCONECTED dentries Date: Mon, 15 Nov 2010 12:48:37 -0500 Message-ID: <20101115174837.GB10044@fieldses.org> References: <20101112184353.GA32745@fieldses.org> Mime-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: QUOTED-PRINTABLE Cc: linux-nfs@vger.kernel.org, linux-fsdevel@vger.kernel.org To: Nick Piggin Return-path: Received: from fieldses.org ([174.143.236.118]:54552 "EHLO fieldses.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1756163Ab0KORsi (ORCPT ); Mon, 15 Nov 2010 12:48:38 -0500 Content-Disposition: inline In-Reply-To: Sender: linux-fsdevel-owner@vger.kernel.org List-ID: On Sat, Nov 13, 2010 at 10:53:12PM +1100, Nick Piggin wrote: > On Sat, Nov 13, 2010 at 5:43 AM, J. Bruce Fields wrote: > > DCACHE_DISCONECTED dentries aren't always getting destroyed as soon= as > > I'd have expected in the NFSv4 case. > > > > I'm not sure what the right fix is; any ideas? =C2=A0The below at l= east > > demonstrates the problem. > > > > --b. > > > > commit bb125ffbe5fc3f80ac7a5b20f51cc542c175cd49 > > Author: J. Bruce Fields > > Date: =C2=A0 Thu Nov 11 19:22:00 2010 -0500 > > > > =C2=A0 =C2=A0dput: free DCACHE_DISCONNECTED dentries sooner > > > > =C2=A0 =C2=A0DCACHE_DISCONECTED dentries are normally left around f= or the benefit of > > =C2=A0 =C2=A0future nfsd operations. =C2=A0But there's no point kee= ping them around once > > =C2=A0 =C2=A0the inode has been deleted. > > > > =C2=A0 =C2=A0Without this patch > > > > =C2=A0 =C2=A0 =C2=A0 =C2=A0client$ mount -tnfs4 server:/export/ /mn= t/ > > =C2=A0 =C2=A0 =C2=A0 =C2=A0client$ tail -f /mnt/FOO > > =C2=A0 =C2=A0 =C2=A0 =C2=A0... > > =C2=A0 =C2=A0 =C2=A0 =C2=A0server$ df -i /export > > =C2=A0 =C2=A0 =C2=A0 =C2=A0server$ rm /export/FOO > > =C2=A0 =C2=A0 =C2=A0 =C2=A0(^C the tail -f) > > =C2=A0 =C2=A0 =C2=A0 =C2=A0server$ df -i /export > > =C2=A0 =C2=A0 =C2=A0 =C2=A0server$ echo 2 >/proc/sys/vm/drop_caches > > =C2=A0 =C2=A0 =C2=A0 =C2=A0server$ df -i /export > > > > =C2=A0 =C2=A0the df's will show that the inode is not freed on the = filesystem until > > =C2=A0 =C2=A0the last step, when it could have been freed after kil= ling the client's > > =C2=A0 =C2=A0tail -f. =C2=A0On-disk data won't be deallocated eithe= r, leading to possible > > =C2=A0 =C2=A0spurious ENOSPC. > > > > =C2=A0 =C2=A0This occurs because when the client does the close, it= arrives in a > > =C2=A0 =C2=A0compound with a putfh and a close, processed like: > > > > =C2=A0 =C2=A0 =C2=A0 =C2=A0- putfh: look up the filehandle. =C2=A0T= he only alias found for the > > =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0inode will be DCACHE_UNHASHED ali= as referenced by the filp > > =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0associated with the nfsd open. =C2= =A0d_obtain_alias() doesn't like > > =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0this, so it creates a new DCACHE_= DISCONECTED dentry and > > =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0returns that instead. >=20 > This seems to be where the thing goes wrong. It isn't a hashed dentry= at > this point here, so d_obtain_alias should not be making one. Sounds sensible. (But can you think of any actual bugs that will resul= t from trying to add a new hashed dentry in this case?) > I think the inode i_nlink games are much more appropriate on this sid= e of > the equation, rather than the dput side (after all, d_obtain_alias is= setting > up an alias for the inode). >=20 > Can you even put the link check into __d_find_alias? >=20 > - if (S_ISDIR(inode->i_mode) || !d_unhashed(alias)) { > + if (S_ISDIR(inode->i_mode) || !inode->i_nlink || > !d_unhashed(alias)) { >=20 > Something like that? The immediate result of that would be for the close rpc (or any rpc's sent after the file was unlinked) to fail with ESTALE. But nfsd already holds an open file in this case, and you could argue that it should be using that from the start. So, we could modify nfsd to add a hash mapping filehandles to the filp'= s that it knows about, and have nfsd consult that hash before calling dentry_to_fh. --b. -- 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