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>
Tue, 3 Jan 2012 15:59:17 +0000 (07:59 -0800)
commit8e15e16c9e6a5768f31976cc21b48d5bb10617b7
tree5270bf6b3b19b772b9aaee198be653fb99f9ef08
parent7461fa11939556d3b6f3ea38da7ff65607805579
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.

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