DAFS: large volume support - fileserver crash after "addled bitmap"

Any DAFS fileserver operation that allocates a new vnode but fails
to update the vnode index will crash:

"Fatal Rx error: assertion failed: --vp->nWaiters >= 0,
				file: ../vol/volume.c, line: nnnn"

Note: This crash was exposed by other bugs (to be addressed in future
commits) in OpenAFS large volume support.  However, there may
be other failure paths (unrelated to large volumes) that expose
this error as well.

When VAllocVnode() must allocate a new vnode but fails while
updating the vnode index file (e.g. an "addled bitmap" due to other
bugs in working with a vnode index larger than 2^31 bytes), it branches
to common recovery logic at label error_encountered:.

Part of this recovery is to call VFreeBitmapEntry_r().  Commit
08ffe3e81d875b58ae5fe4c5733845d5132913a0 added a VOL_FREE_BITMAP_WAIT
flag to VFreeBitmapEntry() in order to prevent races with VAllocBitmapEntry().
If the caller specifies VOL_FREE_BITMAP_WAIT, VFreeBitmapEntry_r will
call VCreateReservation_r() and VWaitExclusiveState_r().  However, the
exit from VFreeBitmapEntry_r() calls VCancelReservation_r() unconditionally.
This works correctly with the majority of callers to VFreeBitmapEntry_r,
which do specify the VOL_FREE_BITMAP_WAIT flag.

However, the VAllocVnode() error_encountered logic must specify 0 for
this flag because the thread is already in an exclusive state
(VOL_STATE_VNODE_ALLOC).  This correctly causes VFreeBitmapEntry_r() to
forgo both the reservation and wait-for-exclusive-state.  However, before
exit it erroneously calls VCancelReservation_r().  We now have unbalanced
reservations (nWaiters); this causes an assert when the VAllocVnode()
error_encountered recovery code later calls VCancelReservation_r()
for what it believes is its own prior reservation.

Modify VFreeBitmapEntry_r() to make its final VCancelReservation_r()
conditional on flag VOL_FREE_BITMAP_WAIT.

Reviewed-on: http://gerrit.openafs.org/11983
Reviewed-by: Perry Ruiter <pruiter@sinenomine.net>
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>
Reviewed-by: Michael Meffie <mmeffie@sinenomine.net>
Reviewed-by: Jeffrey Altman <jaltman@auristor.com>
Tested-by: BuildBot <buildbot@rampaginggeek.com>
(cherry picked from commit d833ba768064a32a19c6b0b94ffb0d8a3a40a089)

Change-Id: Ia146ca55b1c0497d475357e61eaeb061a11bd597
Reviewed-on: https://gerrit.openafs.org/12209
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Mark Vitale <mvitale@sinenomine.net>
Reviewed-by: Stephan Wiesand <stephan.wiesand@desy.de>

diff --git a/src/vol/volume.c b/src/vol/volume.c
index 700c4357128315f5e24a047dff27a33efa13e1b0..33e9ee510af69bc9d2125d3b2d3b9aa2d6d0d1d9 100644 (file)
--- a/src/vol/volume.c
+++ b/src/vol/volume.c
@@ -6504,7 +6504,9 @@ VFreeBitMapEntry_r(Error * ec, Volume *vp, struct vnodeIndex *index,
 
  done:
 #ifdef AFS_DEMAND_ATTACH_FS
-    VCancelReservation_r(vp);
+    if (flags & VOL_FREE_BITMAP_WAIT) {
+       VCancelReservation_r(vp);
+    }
 #endif
     return; /* make the compiler happy for non-DAFS */
 }