OpenAFS Master Repository branch, master, updated. openafs-devel-1_5_76-4306-ga5866b3
Gerrit Code Review
gerrit@openafs.org
Wed, 6 Aug 2014 10:16:53 -0400
The following commit has been merged in the master branch:
commit a5866b3a7c21551a8aa40fc6141cca3a65fea563
Author: Andrew Deason <adeason@sinenomine.net>
Date: Wed Jul 23 11:54:47 2014 -0500
LINUX: Drop dentry if lookup returns new file
Background: when an entry is looked up after its parent changes,
afs_linux_dentry_revalidate re-looks-up the entry name in its parent.
If we get an ENOENT back, we d_drop the dentry, and in any other
situation we just d_invalidate it. As discussed in prior commits
997f7fce437787a45ae0584beaae43affbd37cce and
389473032cf0b200c2c39fd5ace108bdc05c9d97, we cannot simply d_drop the
dentry in all cases, because that would cause legitimate directories
to be reported as "deleted" if we just failed to lookup the entry due
to e.g. transient network errors (this causes, among other things,
'getcwd' to fail with ENOENT).
However, this logic has problems if the dentry name still exists, but
points to a different file; the case where 'tvc != vcp' in
afs_linux_dentry_revalidate. If that case happens, and the dentry is
still held open by some process, we will continue to try to reference
the vcache pointed to by the 'old' dcache entry, which is incorrect.
To maybe more clearly illustrate the issue, consider the following
cases:
$ sleep 9999 < /afs/localcell/testvol.ro/dir1/file1 &
$ rm -rf /afs/localcell/testvol.rw/dir1
$ mkdir /afs/localcell/testvol.rw/dir1
$ vos release testvol
$ ls -l /afs/localcell/testvol.ro
ls: cannot access /afs/localcell/testvol.ro/dir1: No such file or directory
total 0
d????????? ? ? ? ? ? dir1
Here, on the last 'ls', afs_linux_dentry_revalidate will afs_lookup
'dir1', and notice that it points to a different file (tvc != vcp),
and will d_invalidate the dentry. But since the file is still held
open, the dentry doesn't go away, and so we are still pointing to the
vcache for the old, deleted 'dir1'. That file doesn't exist anymore on
the fileserver, so we get an ENOENT when actually trying to stat() it
(we get a VNOVNODE from the fileserver, whcih gets translated to an
ENOENT).
A possibly more serious case is when the file is just renamed:
$ sleep 9999 < /afs/localcell/testvol.ro/dir1/file1 &
$ mv /afs/localcell/testvol.rw/dir1 /afs/localcell/testvol.rw/dir1.moved
$ mkdir /afs/localcell/testvol.rw/dir1
$ touch /afs/localcell/testvol.rw/dir1/file2
$ vos release testvol
$ ls -l /afs/localcell/testvol.ro/dir1
total 0
-rw-rw-r--. 1 1235 adeason 0 Jul 23 11:09 file1
$ kill %1
$ ls -l /afs/localcell/testvol.ro/dir1
total 0
-rw-rw-r--. 1 1235 adeason 0 Jul 23 11:10 file2
In this situation, the same code path applies, but the old file still
exists, so we will continue to use it without error. But since we are
still pointing at the old file, of course the results are incorrect.
Once we kill the process holding the file open, the bad dentry finally
goes away and the results are valid again.
To fix this behavior, d_drop the dentry in all cases, except when we
encounter an error preventing the lookup from being done. This ensures
that the dentry is unhashed from the parent directory in the scenarios
above, and so cannot be used for a subsequent lookup.
With this change, the only afs_lookup response that causes a simple
d_invalidate is when we encounter actual errors during the lookup
(such as transient network failures). This is correct, since in those
cases we don't _know_ that the dentry is wrong. For all other cases,
we do know that the dentry is wrong and so we must force it to be
unhashed.
Change-Id: I11a2db1e05d68a755a77815ec5e8d01ac7b36129
Reviewed-on: http://gerrit.openafs.org/11320
Reviewed-by: Chas Williams - CONTRACTOR <chas@cmf.nrl.navy.mil>
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Marc Dionne <marc.c.dionne@gmail.com>
Reviewed-by: Andrew Deason <adeason@sinenomine.net>
Reviewed-by: D Brashear <shadow@your-file-system.com>
src/afs/LINUX/osi_vnodeops.c | 11 ++++++++---
1 files changed, 8 insertions(+), 3 deletions(-)
--
OpenAFS Master Repository