Date: Thu, 1 Mar 2012 16:47:41 -0500 From: John Baldwin <jhb@freebsd.org> To: Peter Holm <pho@freebsd.org> Cc: svn-src-head@freebsd.org, svn-src-all@freebsd.org, src-committers@freebsd.org Subject: Re: svn commit: r226967 - head/sys/ufs/ufs Message-ID: <201203011647.41313.jhb@freebsd.org> In-Reply-To: <201110311501.p9VF1lrf020688@svn.freebsd.org> References: <201110311501.p9VF1lrf020688@svn.freebsd.org>
next in thread | previous in thread | raw e-mail | index | archive | help
On Monday, October 31, 2011 11:01:47 am Peter Holm wrote: > Author: pho > Date: Mon Oct 31 15:01:47 2011 > New Revision: 226967 > URL: http://svn.freebsd.org/changeset/base/226967 > > Log: > The kern_renameat() looks up the fvp using the DELETE flag, which causes > the removal of the name cache entry for fvp. > > Reported by: Anton Yuzhaninov <citrin citrin ru> > In collaboration with: kib > MFC after: 1 week > > Modified: > head/sys/ufs/ufs/ufs_vnops.c So I ran into this at work recently, and even this fix applied I was still seeing rename()'s that were seemingly not taking effect. After getting some extra KTR traces, I figured out that the same purge needs to be applied to the destination vnode. Specifically, the issue I ran into was that was renaming 'foo' to 'bar', but lookups for 'bar' were still returning the old file. The reason was that a lookup after the namei(RENAME) of the destination while ufs_rename() had its locks dropped was readding the name cache entry for 'bar', and then a cache_lookup() of 'bar' would return the old vnode as long as that vnode was valid (e.g. if it had a link in another location, or other processes had an open file descriptor for it). I'm currently testing the patch below: Index: ufs/ufs/ufs_vnops.c =================================================================== --- ufs/ufs/ufs_vnops.c (revision 225498) +++ ufs/ufs/ufs_vnops.c (working copy) @@ -1519,8 +1539,15 @@ * ufs_lookup_ino() and then VFS_VGET(), another thread might do a * normal lookup of the from name just before the VFS_VGET() call, * causing the cache entry to be re-instantiated. + * + * The same issue also applies to tvp if it exists as + * otherwise we may have a stale name cache entry for the new + * name that references the old i-node if it has other links + * or open file descriptors. */ cache_purge(fvp); + if (tvp) + cache_purge(tvp); unlockout: vput(fdvp); Index: fs/tmpfs/tmpfs_vnops.c =================================================================== --- fs/tmpfs/tmpfs_vnops.c (revision 225498) +++ fs/tmpfs/tmpfs_vnops.c (working copy) @@ -1090,6 +1090,8 @@ tmpfs_free_dirent(VFS_TO_TMPFS(tvp->v_mount), de, TRUE); } cache_purge(fvp); + if (tvp != NULL) + cache_purge(tvp); error = 0; -- John Baldwin
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?201203011647.41313.jhb>