From mboxrd@z Thu Jan 1 00:00:00 1970 From: Jan Harkes Subject: Re: [PATCH 2.4.19pre8][RFC] remove-NFS-close-to-open from VFS (was Re: [PATCHSET] 2.4.19-pre8-jp12) Date: Thu, 17 Oct 2002 16:38:04 -0400 Sender: linux-fsdevel-owner@vger.kernel.org Message-ID: <20021017203804.GA24523@ravel.coda.cs.cmu.edu> References: <200205162142.AWF00051@netmail.netcologne.de> <20020517034357.GA18449@ravel.coda.cs.cmu.edu> <200205162142.AWF00051@netmail.netcologne.de> <200205162142.AWF00051@netmail.netcologne.de> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Cc: Marcelo Tosatti , alan@lxorguk.ukuu.org.uk, Alexander Viro , Trond Myklebust Return-path: To: linux-fsdevel@vger.kernel.org Content-Disposition: inline In-Reply-To: <20020517034357.GA18449@ravel.coda.cs.cmu.edu> <200205162142.AWF00051@netmail.netcologne.de> List-Id: linux-fsdevel.vger.kernel.org Hi, I'm pulling up this old thread again, because there are some serious issues with a patch that was merged in 2.4.19-pre3. Basically it boils down to this, NFS had problems with '.' and '..' lookups as they were bypassing the dcache revalidation/lookup. However, the patch that got merged in 2.4.19-pre3 to fix this is causing some significant problems, it changed the semantics of d_revalidate. Originally, returning a error from d_revalidate in cached_lookup would force a subsequent real_lookup. After the patch, returning an error from d_revalidate propagates (in some cases) ESTALE back up to userspace. NFS seems to do it's own replacement for real_lookup in dentry_revalidate, but most other filesystems (like Samba) 'fixed' the problem by simply revalidating the cached attributes of the object with the server. This ofcourse breaks when an object is simply renamed, as the revalidation will reinstantiate the old and incorrect lookup path for the object in the dcache. Now either every filesystem will have to follow NFS's lead and implement some form of real_lookup inside of the dentry_revalidate function which is not the prettiest solution. Or the VFS patch should be reverted/fixed to actually walk the tree for '.' and '..' lookups. btw. the patch also leaks dentries when a 'stale dentry' happens to have children because it doesn't properly check and handle the d_invalidate returncode. Jan For reference here is the thread from last May when Joerg Prante tracked down his problems in supermount to the NFS patch. http://marc.theaimsgroup.com/?l=linux-kernel&m=102158542315351&w=2 Here is the 'controversial patch', cut-n-pasted out from linux.bkbits.net. --- 1.19/fs/namei.c Thu Feb 28 05:57:29 2002 +++ 1.20/fs/namei.c Tue Mar 12 07:35:02 2002 @@ -457,7 +457,7 @@ while (*name=='/') name++; if (!*name) - goto return_base; + goto return_reval; inode = nd->dentry->d_inode; if (current->link_count) @@ -576,7 +576,7 @@ inode = nd->dentry->d_inode; /* fallthrough */ case 1: - goto return_base; + goto return_reval; } if (nd->dentry->d_op && nd->dentry->d_op->d_hash) { err = nd->dentry->d_op->d_hash(nd->dentry, &this); @@ -627,6 +627,19 @@ nd->last_type = LAST_DOT; else if (this.len == 2 && this.name[1] == '.') nd->last_type = LAST_DOTDOT; +return_reval: + /* + * We bypassed the ordinary revalidation routines. + * Check the cached dentry for staleness. + */ + dentry = nd->dentry; + if (dentry && dentry->d_op && dentry->d_op->d_revalidate) { + err = -ESTALE; + if (!dentry->d_op->d_revalidate(dentry, 0)) { + d_invalidate(dentry); + break; + } + } return_base: return 0; out_dput: