Re: [PATCH] nfs_unlink() race (was: nfs_refresh_inode: inode number mismatch)

From: viro@parcelfarce.linux.theplanet.co.uk
To: Trond Myklebust <trond.myklebust@fys.uio.no>
Cc: Frank Cusack <fcusack@fcusack.com>,
	torvalds@transmeta.com, marcelo@conectiva.com.br,
	lkml <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH] nfs_unlink() race (was: nfs_refresh_inode: inode number mismatch)
Date: Wed, 11 Jun 2003 03:27:54 +0100	[thread overview]
Message-ID: <20030611022754.GC6754@parcelfarce.linux.theplanet.co.uk> (raw)
In-Reply-To: <16102.36078.894833.262461@charged.uio.no>

On Wed, Jun 11, 2003 at 03:59:10AM +0200, Trond Myklebust wrote:
> AFAICS the problem is the following:
> 
>   - NFS sillyrenames dentry 1
>   - Upon return from nfs_unlink(), VFS unhashes dentry 1
> 
>   - Upon next lookup, VFS+NFS conspire to create aliased dentry 2 to
>     sillyrenamed file
>   - Upon last close of files associated with dentry 1, NFS completes
>     sillyrename. File is unlinked on server.
>   - Aliased dentry 2 is still around, but it is now pointing to stale
>     fh.
> 
> IOW we just want to prevent VFS from unhashing the dentry in the first
> place: dentry aliasing cannot work together with sillyrename.

Aliasing could be dealt with.  They would have the same inode, so it's
easy to detect.  The real problem is different: what happens if I take
silly-renamed file and rename it away?  You suddenly get ->dir and ->dentry
if your nfs_unlinkdata having nothing to do with each other.

_If_ we want to be able to work with silly-renamed dentry, we need much
more careful async unlink.  Your current code assumes that these dentries
won't go anywhere.   AFAICS, dcache will not get into inconsistent state,
but it will have very little to do with state of server...