]> git.michaelhowe.org Git - packages/o/openafs.git/commit
DAFS: Atomically re-hash vnode in VGetFreeVnode_r
authorAndrew Deason <adeason@sinenomine.net>
Fri, 18 Nov 2011 16:25:08 +0000 (10:25 -0600)
committerDerrick Brashear <shadow@dementix.org>
Sat, 7 Jan 2012 14:22:17 +0000 (06:22 -0800)
commit6479bb29d494077f31d0ddc3d165164b302b2e3f
treeb8eddcfd96c237df7c46c6cc35a7fd9e04a936ae
parent70fe8ec10943e0ef56b60880064b30ed7804071d
DAFS: Atomically re-hash vnode in VGetFreeVnode_r

VGetFreeVnode_r pulls a vnode off of the vnode LRU, and removes the
vnode from the vnode hash table. In DAFS, we may drop the volume glock
immediately afterwards in order to close the ihandle for the old vnode
structure.

While we have the glock dropped, another thread may try to
VLookupVnode for the new vnode we are creating, find that it is not
hashed, and call VGetFreeVnode_r itself. This can result in two
threads having two separate copies of the same vnode, which bypasses
any mutual exclusion ensured by per-vnode locks, since they will lock
their own version of the vnode. This can result in a variety of
different problems where two threads try to write to the same vnode at
the same time. One example is calling CopyOnWrite on the same file in
parallel, which can cause link undercounts, writes to the wrong vnode
tag, and other CoW-related errors.

To prevent all this, make VGetFreeVnode_r atomically remove the old
vnode structure from the relevant hashes, and add it to the new hashes
before dropping the glock. This ensures that any other thread trying
to load the same vnode will see the new vnode in the hash table,
though it will not yet be valid until the vnode is loaded.

Note that this only solves this race for DAFS. For non-DAFS, the vol
glock is held over the ihandle close, so this race does not exist.
The comments around the callers of VGetFreeVnode_r indicate that
similar extant races exist here for non-DAFS, but they are unsolvable
without significant DAFS-like changes to the vnode package.

Reviewed-on: http://gerrit.openafs.org/6385
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Derrick Brashear <shadow@dementix.org>
(cherry picked from commit 8e15e16c9e6a5768f31976cc21b48d5bb10617b7)

Change-Id: I915d18c4252b698f39fdf65793311a39381096b4
Reviewed-on: http://gerrit.openafs.org/6495
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Derrick Brashear <shadow@dementix.org>
src/vol/vnode.c
src/vol/vnode.h