hammer2 - Correct allocator race and related corruption
authorMatthew Dillon <dillon@apollo.backplane.com>
Fri, 12 Apr 2019 02:04:56 +0000 (19:04 -0700)
committerMatthew Dillon <dillon@apollo.backplane.com>
Fri, 12 Apr 2019 02:04:56 +0000 (19:04 -0700)
* When allocating fragments (below 16KB), for example 1K directory
  entries, 1K inodes, compressed file blocks that happen to be
  fragments, or end-of-file fragments, the allocator must ensure
  that any partially freed block is set back to fully allocated.

* In this specific case the allocator was not setting the
  correct bits in the freemap.  The situation never occurs
  on a block boundary (different code is executed which does
  the correct calculation), so the related block will always
  be in a minimally allocated state (either partially allocated
  or fully allocated).

  This means that the corruption can only happen under the specific
  circumstance where a fragment is allocated out of a block that
  the bulkfree code is simultaneously trying to free (marking it
  partially-allocated).  Because the wrong bits are set, the NEXT
  bulkfree pass can also miss the fact that the fragment is
  allocated and finish transitioning the block from partially-
  allocated to fully-free.

  A later allocation then corrupts the block, resulting in CHECK
  errors on the console.

* Because the bulkfree code always comes in and in ALL SITUATIONS OTHER
  THAN THIS SPECIFIC RACE will re-mark the blocks fully-allocated,
  the corruption can ONLY occur during heavy write activity during
  a bulkfree operation, typically when heavy manipulation of directory
  entries or inodes occurs.

* Correct the fragmentary bitmap calculation to set the proper
  bits.

sys/vfs/hammer2/hammer2_bulkfree.c
sys/vfs/hammer2/hammer2_freemap.c

index 9a55f34..73c0b99 100644 (file)
@@ -1071,30 +1071,36 @@ h2_bulkfree_sync_adjust(hammer2_bulkfree_info_t *cbinfo,
                ++cbinfo->count_l0cleans;
        } else if (bindex < 7) {
                /*
                ++cbinfo->count_l0cleans;
        } else if (bindex < 7) {
                /*
-                * Partially full, bitmapq[bindex] != 0.  The live->linear
-                * offset can legitimately be just about anything, but
-                * our bulkfree pass doesn't record enough information to
-                * set it exactly.  Just make sure that it is set to a
-                * safe value that also works in our match code above (the
-                * bcmp and linear test).
+                * Partially full, bitmapq[bindex] != 0.  Our bulkfree pass
+                * does not record enough information to set live->linear
+                * exactly.
                 *
                 *
-                * We cannot safely leave live->linear at a sub-block offset
-                * unless it is already in the same block as bmap->linear.
+                * NOTE: Setting live->linear to a sub-block (16K) boundary
+                *       forces the live code to iterate to the next fully
+                *       free block.  It does NOT mean that all blocks above
+                *       live->linear are available.
                 *
                 *
-                * If it is not in the same block, we cannot assume that
-                * we can set it to bmap->linear on a sub-block boundary,
-                * because the live system could have bounced it around.
-                * In that situation we satisfy our bcmp/skip requirement
-                * above by setting it to the nearest higher block boundary.
-                * This alignment effectively kills any partial allocation it
-                * might have been tracking before.
+                *       Setting live->linear to a fragmentary (less than
+                *       16K) boundary allows allocations to iterate within
+                *       that sub-block.
                 */
                if (live->linear < bmap->linear &&
                    ((live->linear ^ bmap->linear) &
                     ~HAMMER2_FREEMAP_BLOCK_MASK) == 0) {
                 */
                if (live->linear < bmap->linear &&
                    ((live->linear ^ bmap->linear) &
                     ~HAMMER2_FREEMAP_BLOCK_MASK) == 0) {
+                       /*
+                        * If greater than but still within the same
+                        * sub-block as live we can adjust linear upward.
+                        */
                        live->linear = bmap->linear;
                        ++cbinfo->count_linadjusts;
                } else {
                        live->linear = bmap->linear;
                        ++cbinfo->count_linadjusts;
                } else {
+                       /*
+                        * Otherwise adjust to the nearest higher or same
+                        * sub-block boundary.  The live system may have
+                        * bounced live->linear around so we cannot make any
+                        * assumptions with regards to available fragmentary
+                        * allocations.
+                        */
                        live->linear =
                                (bmap->linear + HAMMER2_FREEMAP_BLOCK_MASK) &
                                ~HAMMER2_FREEMAP_BLOCK_MASK;
                        live->linear =
                                (bmap->linear + HAMMER2_FREEMAP_BLOCK_MASK) &
                                ~HAMMER2_FREEMAP_BLOCK_MASK;
index 30e87b4..6b54c5f 100644 (file)
@@ -644,13 +644,20 @@ hammer2_bmap_alloc(hammer2_dev_t *hmp, hammer2_bmap_data_t *bmap,
                /*
                 * Use linear iterator if it is not block-aligned to avoid
                 * wasting space.
                /*
                 * Use linear iterator if it is not block-aligned to avoid
                 * wasting space.
+                *
+                * Calculate the bitmapq[] index (i) and calculate the
+                * shift count within the 64-bit bitmapq[] entry.
+                *
+                * The freemap block size is 16KB, but each bitmap
+                * entry is two bits so use a little trick to get
+                * a (j) shift of 0, 2, 4, ... 62 in 16KB chunks.
                 */
                KKASSERT(bmap->linear >= 0 &&
                         bmap->linear + size <= HAMMER2_SEGSIZE &&
                         (bmap->linear & (HAMMER2_ALLOC_MIN - 1)) == 0);
                offset = bmap->linear;
                 */
                KKASSERT(bmap->linear >= 0 &&
                         bmap->linear + size <= HAMMER2_SEGSIZE &&
                         (bmap->linear & (HAMMER2_ALLOC_MIN - 1)) == 0);
                offset = bmap->linear;
-               i = offset / (HAMMER2_SEGSIZE / 8);
-               j = (offset / (HAMMER2_FREEMAP_BLOCK_SIZE / 2)) & 30;
+               i = offset / (HAMMER2_SEGSIZE / HAMMER2_BMAP_ELEMENTS);
+               j = (offset / (HAMMER2_FREEMAP_BLOCK_SIZE / 2)) & 62;
                bmmask = (bmradix == HAMMER2_BMAP_BITS_PER_ELEMENT) ?
                         HAMMER2_BMAP_ALLONES :
                         ((hammer2_bitmap_t)1 << bmradix) - 1;
                bmmask = (bmradix == HAMMER2_BMAP_BITS_PER_ELEMENT) ?
                         HAMMER2_BMAP_ALLONES :
                         ((hammer2_bitmap_t)1 << bmradix) - 1;