OpenAFS Master Repository branch, openafs-stable-1_8_x, updated. openafs-stable-1_8_0pre4-12-ge9419dc
Gerrit Code Review
gerrit@openafs.org
Fri, 9 Feb 2018 09:04:09 -0500
The following commit has been merged in the openafs-stable-1_8_x branch:
commit e9419dc895801bda6962f2ac0f5981631a0d95c7
Author: Andrew Deason <adeason@sinenomine.net>
Date: Thu Jan 11 21:27:28 2018 -0600
LINUX: Avoid locking inode in check_dentry_race
Currently, check_dentry_race locks the parent inode in order to ensure
it is not running in parallel with d_splice_alias for the same inode.
(For old Linux kernel versions; see commit b0461f2d: "LINUX:
Workaround d_splice_alias/d_lookup race".)
However, it is possible to hit this area of code when the parent inode
is already locked. When someone tries to create a file, directory, or
symlink, Linux tries to lookup the dentry for the target path, to see
if it already exists. While looking up the last component of the path,
Linux locks the directory, and if it finds a dentry for the target
name, it calls d_invalidate on it while the parent directory is
locked.
For a dentry with a NULL inode, we'll then try to lock the parent
inode in check_dentry_race. But since the inode is already locked, we
will deadlock.
From a user's point of view, the hang can be reproduced by doing
something similar to:
$ mkdir dir # succeeds
$ rmdir dir
$ ls -l dir
ls: cannot access dir: No such file or directory
$ mkdir dir # hangs
To avoid this, we can just change which lock we're using to avoid
check_dentry_race/d_splice_alias from running in parallel. Instead of
locking the parent inode, introduce a new global lock (called
dentry_race_sem), and lock that in check_dentry_race and around our
d_splice_alias call. We know that those are the only two users of this
new lock, so this should avoid any such deadlocks.
This does potentially reduce performance, since all tasks that hit
check_dentry_race or d_splice_alias will take the same global lock.
However, this at least still allows us to make use of negative
dentries, and this entire code path only applies to older Linux
kernels. It could be possible to add a new lock into struct vcache
instead, but using a global lock like this commit does is much
simpler.
Reviewed-on: https://gerrit.openafs.org/12868
Tested-by: Benjamin Kaduk <kaduk@mit.edu>
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>
(cherry picked from commit ef1d4c8d328e9b9affc9864fd084257e9fa08445)
Change-Id: Ia8e28519fff36baca7dc4061ceef6719a2a738d4
Reviewed-on: https://gerrit.openafs.org/12881
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>
src/afs/LINUX/osi_vnodeops.c | 40 ++++++++++++++++++++++++++++++----------
1 files changed, 30 insertions(+), 10 deletions(-)
--
OpenAFS Master Repository