mirror of
https://git.openafs.org/openafs.git
synced 2025-01-31 21:47:45 +00:00
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>
This commit is contained in:
parent
2d89d447c8
commit
a5866b3a7c
@ -1304,9 +1304,14 @@ afs_linux_dentry_revalidate(struct dentry *dp, int flags)
|
||||
code = afs_lookup(pvcp, (char *)dp->d_name.name, &tvc, credp);
|
||||
if (!tvc || tvc != vcp) {
|
||||
dput(parent);
|
||||
/* Force unhash if name is known not to exist. */
|
||||
if (code == ENOENT)
|
||||
force_drop = 1;
|
||||
/* Force unhash; the name doesn't point to this file
|
||||
* anymore. */
|
||||
force_drop = 1;
|
||||
if (code && code != ENOENT) {
|
||||
/* ...except if we couldn't perform the actual lookup,
|
||||
* we don't know if the name points to this file or not. */
|
||||
force_drop = 0;
|
||||
}
|
||||
goto bad_dentry;
|
||||
}
|
||||
|
||||
|
Loading…
x
Reference in New Issue
Block a user