hammer2 - Fix bulkfree bug, Fix chain->parent bug, refactor dedup a bit
authorMatthew Dillon <dillon@apollo.backplane.com>
Sun, 27 Aug 2017 04:56:14 +0000 (21:56 -0700)
committerMatthew Dillon <dillon@apollo.backplane.com>
Sun, 27 Aug 2017 05:22:14 +0000 (22:22 -0700)
* Fix a bulkfree bug where live->linear was being calculated using
  HAMMER2_FREEMAP_BLOCK_SIZE instead of HAMMER2_BMAP_INDEX_SIZE.
  This calculation was still block-size-aligned and couldn't cause
  any damage.  However, the conditional itself was in error and
  could prevent bulkfree from correcting an unaligned live->linear
  value.

  The new code unconditoinally corrects live->linear for now.
  Bulkfree doesn't collect sufficiently fine-grained data to set
  it properly.

* bulkfree now also requires live->linear to be reasonable when deciding
  to skip bitmaps.

* hammer2_chain_modify() needs to know when a dedup has been registered
  for a (data) block.  We previously cleared the CHAIN_MODIFIED bit to
  force a reallocation, but doing so interferes with the flush code
  (even though UPDATE remains set).

  Fix this by adding a chain flag.  Also properly set the flag only if
  a dedup has actually been registered, and properly clear the flag once
  the underlying storage has been reallocated.

* Refactor hammer2_chain_getparent() and add hammer2_chain_repparent().
  Have these functions deal with all chain->parent race conditions
  themselves.

  This fixes a race condition in two places, most importantly in
  hammer2_xop_nrename() (albeit limited to directory-embedded inodes,
  which would only be the super-root entries).

* Do a better job protecting the chain->parent field.  Require that
  chain->core.spin be held for field modifications in addition to
  parent->core.spin.  This fixes a race against hammer2_chain_setflush()
  that could cause the flush code to miss chains.

* Disable TIMER() debugging for now.

sys/vfs/hammer2/hammer2.h
sys/vfs/hammer2/hammer2_bulkfree.c
sys/vfs/hammer2/hammer2_chain.c
sys/vfs/hammer2/hammer2_freemap.c
sys/vfs/hammer2/hammer2_inode.c
sys/vfs/hammer2/hammer2_strategy.c
sys/vfs/hammer2/hammer2_xops.c

index a5c808d..32e42ae 100644 (file)
@@ -378,7 +378,7 @@ RB_PROTOTYPE(hammer2_chain_tree, hammer2_chain, rbnode, hammer2_chain_cmp);
 #define HAMMER2_CHAIN_MODIFIED         0x00000001      /* dirty chain data */
 #define HAMMER2_CHAIN_ALLOCATED                0x00000002      /* kmalloc'd chain */
 #define HAMMER2_CHAIN_DESTROY          0x00000004
-#define HAMMER2_CHAIN_UNUSED0008       0x00000008
+#define HAMMER2_CHAIN_DEDUPABLE                0x00000008      /* registered w/dedup */
 #define HAMMER2_CHAIN_DELETED          0x00000010      /* deleted chain */
 #define HAMMER2_CHAIN_INITIAL          0x00000020      /* initial create */
 #define HAMMER2_CHAIN_UPDATE           0x00000040      /* need parent update */
@@ -1459,7 +1459,8 @@ hammer2_chain_t *hammer2_chain_get(hammer2_chain_t *parent, int generation,
                                hammer2_blockref_t *bref);
 hammer2_chain_t *hammer2_chain_lookup_init(hammer2_chain_t *parent, int flags);
 void hammer2_chain_lookup_done(hammer2_chain_t *parent);
-hammer2_chain_t *hammer2_chain_getparent(hammer2_chain_t **parentp, int how);
+hammer2_chain_t *hammer2_chain_getparent(hammer2_chain_t *chain, int how);
+hammer2_chain_t *hammer2_chain_repparent(hammer2_chain_t **chainp, int how);
 hammer2_chain_t *hammer2_chain_lookup(hammer2_chain_t **parentp,
                                hammer2_key_t *key_nextp,
                                hammer2_key_t key_beg, hammer2_key_t key_end,
index 45d3213..b457208 100644 (file)
@@ -251,7 +251,7 @@ static int h2_bulkfree_callback(hammer2_bulkfree_info_t *cbinfo,
 static void h2_bulkfree_sync(hammer2_bulkfree_info_t *cbinfo);
 static void h2_bulkfree_sync_adjust(hammer2_bulkfree_info_t *cbinfo,
                        hammer2_off_t data_off, hammer2_bmap_data_t *live,
-                       hammer2_bmap_data_t *bmap, int nofree);
+                       hammer2_bmap_data_t *bmap);
 
 void
 hammer2_bulkfree_init(hammer2_dev_t *hmp)
@@ -579,8 +579,21 @@ h2_bulkfree_callback(hammer2_bulkfree_info_t *cbinfo, hammer2_blockref_t *bref)
                        bref->keybits,
                        class, bmap->class);
        }
-       if (bmap->linear < (int32_t)data_off + (int32_t)bytes)
+
+       /*
+        * Just record the highest byte-granular offset for now.  Do not
+        * match against allocations which are in multiples of whole blocks.
+        *
+        * Make sure that any in-block linear offset at least covers the
+        * data range.  This can cause bmap->linear to become block-aligned.
+        */
+       if (bytes & HAMMER2_FREEMAP_BLOCK_MASK) {
+               if (bmap->linear < (int32_t)data_off + (int32_t)bytes)
+                       bmap->linear = (int32_t)data_off + (int32_t)bytes;
+       } else if (bmap->linear >= (int32_t)data_off &&
+                  bmap->linear < (int32_t)data_off + (int32_t)bytes) {
                bmap->linear = (int32_t)data_off + (int32_t)bytes;
+       }
 
        /*
         * Adjust the hammer2_bitmap_t bitmap[HAMMER2_BMAP_ELEMENTS].
@@ -681,7 +694,6 @@ h2_bulkfree_sync(hammer2_bulkfree_info_t *cbinfo)
                 * The freemap is not used below allocator_beg or beyond
                 * volu_size.
                 */
-               int nofree;
 
                if (data_off < cbinfo->hmp->voldata.allocator_beg)
                        goto next;
@@ -692,7 +704,6 @@ h2_bulkfree_sync(hammer2_bulkfree_info_t *cbinfo)
                 * Locate the freemap leaf on the live filesystem
                 */
                key = (data_off & ~HAMMER2_FREEMAP_LEVEL1_MASK);
-               nofree = 0;
 
                if (live_chain == NULL || live_chain->bref.key != key) {
                        if (live_chain) {
@@ -761,12 +772,21 @@ h2_bulkfree_sync(hammer2_bulkfree_info_t *cbinfo)
                live = &live_chain->data->bmdata[bmapindex];
 
                /*
+                * Shortcut if the bitmaps match and the live linear
+                * indicator is sane.  We can't do a perfect check of
+                * live->linear because the only real requirement is that
+                * if it is not block-aligned, that it not cover the space
+                * within its current block which overlaps one of the data
+                * ranges we scan.  We don't retain enough fine-grained
+                * data in our scan to be able to set it exactly.
+                *
                 * TODO - we could shortcut this by testing that both
                 * live->class and bmap->class are 0, and both avails are
                 * set to HAMMER2_FREEMAP_LEVEL0_SIZE (4MB).
                 */
                if (bcmp(live->bitmapq, bmap->bitmapq,
-                        sizeof(bmap->bitmapq)) == 0) {
+                        sizeof(bmap->bitmapq)) == 0 &&
+                   live->linear >= bmap->linear) {
                        goto next;
                }
                if (hammer2_debug & 1) {
@@ -777,7 +797,7 @@ h2_bulkfree_sync(hammer2_bulkfree_info_t *cbinfo)
                hammer2_chain_modify(live_chain, cbinfo->mtid, 0, 0);
                live = &live_chain->data->bmdata[bmapindex];
 
-               h2_bulkfree_sync_adjust(cbinfo, data_off, live, bmap, nofree);
+               h2_bulkfree_sync_adjust(cbinfo, data_off, live, bmap);
 next:
                data_off += HAMMER2_FREEMAP_LEVEL0_SIZE;
                ++bmap;
@@ -794,15 +814,12 @@ next:
 
 /*
  * Merge the bulkfree bitmap against the existing bitmap.
- *
- * If nofree is non-zero the merge will only mark free blocks as allocated
- * and will refuse to free any blocks.
  */
 static
 void
 h2_bulkfree_sync_adjust(hammer2_bulkfree_info_t *cbinfo,
                        hammer2_off_t data_off, hammer2_bmap_data_t *live,
-                       hammer2_bmap_data_t *bmap, int nofree)
+                       hammer2_bmap_data_t *bmap)
 {
        int bindex;
        int scount;
@@ -840,8 +857,6 @@ h2_bulkfree_sync_adjust(hammer2_bulkfree_info_t *cbinfo,
                                                "transition m=00/l=01\n");
                                        break;
                                case 2: /* 10 -> 00 */
-                                       if (nofree)
-                                               break;
                                        live->bitmapq[bindex] &=
                                            ~((hammer2_bitmap_t)2 << scount);
                                        live->avail +=
@@ -918,43 +933,49 @@ h2_bulkfree_sync_adjust(hammer2_bulkfree_info_t *cbinfo,
                if (live->bitmapq[bindex] != 0)
                        break;
        }
-       if (nofree) {
-               /* do nothing */
-       } else if (bindex < 0) {
+       if (bindex < 0) {
+               /*
+                * Completely empty, reset entire segment
+                */
                live->avail = HAMMER2_FREEMAP_LEVEL0_SIZE;
                live->class = 0;
                live->linear = 0;
                ++cbinfo->count_l0cleans;
-#if 0
-               hammer2_io_dedup_assert(cbinfo->hmp,
-                                    data_off |
-                                    HAMMER2_FREEMAP_LEVEL0_RADIX,
-                                    HAMMER2_FREEMAP_LEVEL0_SIZE);
-#endif
        } else if (bindex < 7) {
-               int32_t nlinear;
-
-               ++bindex;
-
-               if (live->linear > bindex * HAMMER2_FREEMAP_BLOCK_SIZE) {
-                       nlinear = bindex * HAMMER2_FREEMAP_BLOCK_SIZE;
-#if 0
-                       hammer2_io_dedup_assert(cbinfo->hmp,
-                                            data_off + nlinear,
-                                            live->linear - nlinear);
-#endif
-                       live->linear = nlinear;
-                       ++cbinfo->count_linadjusts;
-               }
-
                /*
-                * XXX this fine-grained measure still has some issues.
+                * 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).
+                *
+                * We cannot safely leave live->linear at a sub-block offset
+                * unless it is already in the same block as bmap->linear.
+                *
+                * 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.
                 */
-               if (live->linear < bindex * HAMMER2_FREEMAP_BLOCK_SIZE) {
-                       live->linear = bindex * HAMMER2_FREEMAP_BLOCK_SIZE;
+               if (live->linear < bmap->linear &&
+                   ((live->linear ^ bmap->linear) &
+                    ~HAMMER2_FREEMAP_BLOCK_MASK) == 0) {
+                       live->linear = bmap->linear;
+                       ++cbinfo->count_linadjusts;
+               } else {
+                       live->linear =
+                               (bmap->linear + HAMMER2_FREEMAP_BLOCK_MASK) &
+                               ~HAMMER2_FREEMAP_BLOCK_MASK;
                        ++cbinfo->count_linadjusts;
                }
        } else {
+               /*
+                * Completely full, effectively disable the linear iterator
+                */
                live->linear = HAMMER2_SEGSIZE;
        }
 
index 6ca8016..1c5288f 100644 (file)
@@ -86,6 +86,9 @@ static hammer2_chain_t *hammer2_combined_find(
  */
 RB_GENERATE(hammer2_chain_tree, hammer2_chain, rbnode, hammer2_chain_cmp);
 
+#if 1
+#define TIMER(which)
+#else
 extern int h2timer[32];
 extern int h2last;
 extern int h2lid;
@@ -96,6 +99,7 @@ extern int h2lid;
         h2last = ticks;                                 \
        h2lid = which;                                  \
 } while(0)
+#endif
 
 int
 hammer2_chain_cmp(hammer2_chain_t *chain1, hammer2_chain_t *chain2)
@@ -751,7 +755,8 @@ hammer2_chain_lastdrop(hammer2_chain_t *chain)
 
                /*
                 * 1->0 transition successful, parent spin held to prevent
-                * new lookups.  remove chain from the parent.
+                * new lookups, chain spinlock held to protect parent field.
+                * Remove chain from the parent.
                 */
                if (chain->flags & HAMMER2_CHAIN_ONRBTREE) {
                        RB_REMOVE(hammer2_chain_tree,
@@ -1528,77 +1533,25 @@ hammer2_chain_resize(hammer2_chain_t *chain,
 }
 
 /*
- * Helper for chains already flagged as MODIFIED.  A new allocation may
- * still be required if the existing one has already been used in a de-dup.
- */
-static __inline
-int
-modified_needs_new_allocation(hammer2_chain_t *chain)
-{
-       /*
-        * We only live-dedup data, we do not live-dedup meta-data.
-        */
-       if (chain->bref.type != HAMMER2_BREF_TYPE_DATA &&
-           chain->bref.type != HAMMER2_BREF_TYPE_DIRENT) {
-               return 0;
-       }
-
-       /*
-        * If chain has no data, then there is nothing to live-dedup.
-        */
-       if (chain->bytes == 0)
-               return 0;
-
-       return 0;
-
-#if 0
-       hammer2_io_t *dio;
-       /*
-        * If this flag is not set the current modification has not been
-        * recorded for dedup so a new allocation is not needed.  The
-        * recording occurs when dirty file data is flushed from the frontend
-        * to the backend.
-        */
-       if (chain->flags & HAMMER2_CHAIN_DEDUP)
-               return 1;
-
-       /*
-        * If the DEDUP flag is set we have one final line of defense to
-        * allow re-use of a modified buffer, and that is if the DIO_INVALOK
-        * flag is still set on the underlying DIO.  This flag is only set
-        * for hammer2_io_new() buffers which cover the whole buffer (64KB),
-        * and is cleared when a dedup operation actually decides to use
-        * the buffer.
-        */
-
-       if ((dio = chain->dio) != NULL) {
-               if (dio->refs & HAMMER2_DIO_INVALOK)
-                       return 0;
-       } else {
-               dio = hammer2_io_getquick(chain->hmp, chain->bref.data_off,
-                                         chain->bytes);
-               if (dio) {
-                       if (dio->refs & HAMMER2_DIO_INVALOK) {
-                               hammer2_io_putblk(&dio);
-                               return 0;
-                       }
-                       hammer2_io_putblk(&dio);
-               }
-       }
-       return 1;
-#endif
-}
-
-/*
- * Set the chain modified so its data can be changed by the caller.
+ * Set the chain modified so its data can be changed by the caller, or
+ * install deduplicated data.  The caller must call this routine for each
+ * set of modifications it makes, even if the chain is already flagged
+ * MODIFIED.
  *
  * Sets bref.modify_tid to mtid only if mtid != 0.  Note that bref.modify_tid
  * is a CLC (cluster level change) field and is not updated by parent
  * propagation during a flush.
  *
- * If the caller passes a non-zero dedup_off we assign data_off to that
- * instead of allocating a ne block.  Caller must not modify the data already
- * present at the target offset.
+ *                              Dedup Handling
+ *
+ * If the DEDUPABLE flag is set in the chain the storage must be reallocated
+ * even if the chain is still flagged MODIFIED.  In this case the chain's
+ * DEDUPABLE flag will be cleared once the new storage has been assigned.
+ *
+ * If the caller passes a non-zero dedup_off we will use it to assign the
+ * new storage.  The MODIFIED flag will be *CLEARED* in this case, and
+ * DEDUPABLE will be set (NOTE: the UPDATE flag is always set).  The caller
+ * must not modify the data content upon return.
  */
 void
 hammer2_chain_modify(hammer2_chain_t *chain, hammer2_tid_t mtid,
@@ -1637,17 +1590,16 @@ hammer2_chain_modify(hammer2_chain_t *chain, hammer2_tid_t mtid,
        }
 
        /*
-        * Set MODIFIED to indicate that the chain has been modified.
+        * Set MODIFIED to indicate that the chain has been modified.  A new
+        * allocation is required when modifying a chain.
+        *
         * Set UPDATE to ensure that the blockref is updated in the parent.
         *
+        *
         * If MODIFIED is already set determine if we can reuse the assigned
-        * data block or if we need a new data block.  The assigned data block
-        * can be reused if HAMMER2_DIO_INVALOK is set on the dio.
+        * data block or if we need a new data block.
         */
-       if ((chain->flags & HAMMER2_CHAIN_MODIFIED) &&
-           modified_needs_new_allocation(chain)) {
-               newmod = 1;
-       } else if ((chain->flags & HAMMER2_CHAIN_MODIFIED) == 0) {
+       if ((chain->flags & HAMMER2_CHAIN_MODIFIED) == 0) {
                /*
                 * Must set modified bit.
                 */
@@ -1668,12 +1620,12 @@ hammer2_chain_modify(hammer2_chain_t *chain, hammer2_tid_t mtid,
                if ((chain->bref.type == HAMMER2_BREF_TYPE_DATA ||
                     chain->bref.type == HAMMER2_BREF_TYPE_DIRENT) &&
                    (chain->flags & HAMMER2_CHAIN_INITIAL) == 0 &&
+                   (chain->flags & HAMMER2_CHAIN_DEDUPABLE) == 0 &&
                    HAMMER2_DEC_CHECK(chain->bref.methods) ==
                     HAMMER2_CHECK_NONE &&
                    chain->pmp &&
                    chain->bref.modify_tid >
-                    chain->pmp->iroot->meta.pfs_lsnap_tid &&
-                   modified_needs_new_allocation(chain) == 0) {
+                    chain->pmp->iroot->meta.pfs_lsnap_tid) {
                        /*
                         * Sector overwrite allowed.
                         */
@@ -1684,6 +1636,14 @@ hammer2_chain_modify(hammer2_chain_t *chain, hammer2_tid_t mtid,
                         */
                        newmod = 1;
                }
+       } else if (chain->flags & HAMMER2_CHAIN_DEDUPABLE) {
+               /*
+                * If the modified chain was registered for dedup we need
+                * a new allocation.  This only happens for delayed-flush
+                * chains (i.e. which run through the front-end buffer
+                * cache).
+                */
+               newmod = 1;
        } else {
                /*
                 * Already flagged modified, no new allocation is needed.
@@ -1725,13 +1685,6 @@ hammer2_chain_modify(hammer2_chain_t *chain, hammer2_tid_t mtid,
                         *       allocated and the underlying DIO will
                         *       still be flushed.
                         */
-#if 0
-                       /* removed */
-                       hammer2_io_dedup_delete(chain->hmp,
-                                               chain->bref.type,
-                                               chain->bref.data_off,
-                                               chain->bytes);
-#endif
                        if (dedup_off) {
                                chain->bref.data_off = dedup_off;
                                chain->bytes = 1 << (dedup_off &
@@ -1744,8 +1697,12 @@ hammer2_chain_modify(hammer2_chain_t *chain, hammer2_tid_t mtid,
                                        hammer2_pfs_memory_wakeup(chain->pmp);
                                hammer2_freemap_adjust(hmp, &chain->bref,
                                                HAMMER2_FREEMAP_DORECOVER);
+                               atomic_set_int(&chain->flags,
+                                               HAMMER2_CHAIN_DEDUPABLE);
                        } else {
                                hammer2_freemap_alloc(chain, chain->bytes);
+                               atomic_clear_int(&chain->flags,
+                                               HAMMER2_CHAIN_DEDUPABLE);
                        }
                        /* XXX failed allocation */
                }
@@ -2189,37 +2146,95 @@ hammer2_chain_lookup_done(hammer2_chain_t *parent)
        }
 }
 
+/*
+ * Take the locked chain and return a locked parent.  The chain remains
+ * locked on return.
+ *
+ * This function handles the lock order reversal.
+ */
 hammer2_chain_t *
-hammer2_chain_getparent(hammer2_chain_t **parentp, int how)
+hammer2_chain_getparent(hammer2_chain_t *chain, int how)
 {
-       hammer2_chain_t *oparent;
-       hammer2_chain_t *nparent;
+       hammer2_chain_t *parent;
 
        /*
-        * Be careful of order, oparent must be unlocked before nparent
+        * Be careful of order, chain must be unlocked before parent
         * is locked below to avoid a deadlock.
         *
         * Safe access to fu->parent requires fu's core spinlock.
         */
-       oparent = *parentp;
-       hammer2_spin_ex(&oparent->core.spin);
-       nparent = oparent->parent;
-       if (nparent == NULL) {
-               hammer2_spin_unex(&oparent->core.spin);
+again:
+       hammer2_spin_ex(&chain->core.spin);
+       parent = chain->parent;
+       if (parent == NULL) {
+               hammer2_spin_unex(&chain->core.spin);
                panic("hammer2_chain_getparent: no parent");
        }
-       hammer2_chain_ref(nparent);
-       hammer2_spin_unex(&oparent->core.spin);
-       if (oparent) {
-               hammer2_chain_unlock(oparent);
-               hammer2_chain_drop(oparent);
-               oparent = NULL;
+       hammer2_chain_ref(parent);
+       hammer2_spin_unex(&chain->core.spin);
+
+       hammer2_chain_unlock(chain);
+       hammer2_chain_lock(parent, how);
+       hammer2_chain_lock(chain, how);
+
+       /*
+        * Parent relinking races are quite common.  We have to get it right
+        * or we will blow up the block table.
+        */
+       if (chain->parent != parent) {
+               hammer2_chain_unlock(parent);
+               hammer2_chain_drop(parent);
+               goto again;
+       }
+       return parent;
+}
+
+/*
+ * Take the locked chain and return a locked parent.  The chain is unlocked
+ * and dropped.  *chainp is set to the returned parent as a convenience.
+ *
+ * This function handles the lock order reversal.
+ */
+hammer2_chain_t *
+hammer2_chain_repparent(hammer2_chain_t **chainp, int how)
+{
+       hammer2_chain_t *chain;
+       hammer2_chain_t *parent;
+
+       /*
+        * Be careful of order, chain must be unlocked before parent
+        * is locked below to avoid a deadlock.
+        *
+        * Safe access to fu->parent requires fu's core spinlock.
+        */
+       chain = *chainp;
+again:
+       hammer2_spin_ex(&chain->core.spin);
+       parent = chain->parent;
+       if (parent == NULL) {
+               hammer2_spin_unex(&chain->core.spin);
+               panic("hammer2_chain_getparent: no parent");
        }
+       hammer2_chain_ref(parent);
+       hammer2_spin_unex(&chain->core.spin);
 
-       hammer2_chain_lock(nparent, how);
-       *parentp = nparent;
+       hammer2_chain_unlock(chain);
+       hammer2_chain_lock(parent, how);
 
-       return (nparent);
+       /*
+        * Parent relinking races are quite common.  We have to get it right
+        * or we will blow up the block table.
+        */
+       if (chain->parent != parent) {
+               hammer2_chain_lock(chain, how);
+               hammer2_chain_unlock(parent);
+               hammer2_chain_drop(parent);
+               goto again;
+       }
+       hammer2_chain_drop(chain);
+       *chainp = parent;
+
+       return parent;
 }
 
 /*
@@ -2325,7 +2340,7 @@ hammer2_chain_lookup(hammer2_chain_t **parentp, hammer2_key_t *key_nextp,
                        if (key_beg >= scan_beg && key_end <= scan_end)
                                break;
                }
-               parent = hammer2_chain_getparent(parentp, how_maybe);
+               parent = hammer2_chain_repparent(parentp, how_maybe);
        }
 again:
 
@@ -2475,7 +2490,7 @@ again:
                          ((hammer2_key_t)1 << parent->bref.keybits);
                if (key_beg == 0 || key_beg > key_end)
                        return (NULL);
-               parent = hammer2_chain_getparent(parentp, how_maybe);
+               parent = hammer2_chain_repparent(parentp, how_maybe);
                goto again;
        }
 
@@ -2663,7 +2678,7 @@ hammer2_chain_next(hammer2_chain_t **parentp, hammer2_chain_t *chain,
                          ((hammer2_key_t)1 << parent->bref.keybits);
                if (key_beg == 0 || key_beg > key_end)
                        return (NULL);
-               parent = hammer2_chain_getparent(parentp, how_maybe);
+               parent = hammer2_chain_repparent(parentp, how_maybe);
        }
 
        /*
@@ -3370,12 +3385,13 @@ _hammer2_chain_delete_helper(hammer2_chain_t *parent, hammer2_chain_t *chain,
                KKASSERT(parent != NULL);
                KKASSERT(parent->error == 0);
                KKASSERT((parent->flags & HAMMER2_CHAIN_INITIAL) == 0);
-               hammer2_chain_modify(parent, mtid, 0, HAMMER2_MODIFY_OPTDATA);
+               hammer2_chain_modify(parent, mtid, 0, 0);
 
                /*
                 * Calculate blockmap pointer
                 */
                KKASSERT(chain->flags & HAMMER2_CHAIN_ONRBTREE);
+               hammer2_spin_ex(&chain->core.spin);
                hammer2_spin_ex(&parent->core.spin);
 
                atomic_set_int(&chain->flags, HAMMER2_CHAIN_DELETED);
@@ -3446,6 +3462,7 @@ _hammer2_chain_delete_helper(hammer2_chain_t *parent, hammer2_chain_t *chain,
                                            &cache_index, chain);
                }
                hammer2_spin_unex(&parent->core.spin);
+               hammer2_spin_unex(&chain->core.spin);
        } else if (chain->flags & HAMMER2_CHAIN_ONRBTREE) {
                /*
                 * Chain is not blockmapped but a parent is present.
@@ -3456,6 +3473,7 @@ _hammer2_chain_delete_helper(hammer2_chain_t *parent, hammer2_chain_t *chain,
                 * synchronized, the chain's *_count_up fields contain
                 * inode adjustment statistics which must be undone.
                 */
+               hammer2_spin_ex(&chain->core.spin);
                hammer2_spin_ex(&parent->core.spin);
                atomic_set_int(&chain->flags, HAMMER2_CHAIN_DELETED);
                atomic_add_int(&parent->core.live_count, -1);
@@ -3465,6 +3483,7 @@ _hammer2_chain_delete_helper(hammer2_chain_t *parent, hammer2_chain_t *chain,
                --parent->core.chain_count;
                chain->parent = NULL;
                hammer2_spin_unex(&parent->core.spin);
+               hammer2_spin_unex(&chain->core.spin);
        } else {
                /*
                 * Chain is not blockmapped and has no parent.  This
@@ -4503,12 +4522,9 @@ hammer2_chain_delete(hammer2_chain_t *parent, hammer2_chain_t *chain,
        /*
         * Permanent deletions mark the chain as destroyed.
         */
-       if (flags & HAMMER2_DELETE_PERMANENT) {
+       if (flags & HAMMER2_DELETE_PERMANENT)
                atomic_set_int(&chain->flags, HAMMER2_CHAIN_DESTROY);
-       } else {
-               /* XXX might not be needed */
-               hammer2_chain_setflush(chain);
-       }
+       hammer2_chain_setflush(chain);
 }
 
 /*
index 6dfa999..0eb3366 100644 (file)
@@ -586,11 +586,11 @@ hammer2_bmap_alloc(hammer2_dev_t *hmp, hammer2_bmap_data_t *bmap,
         * once we pack to the boundary.  We adjust it after a bitmap
         * allocation only for sub-16KB allocations (so the perfectly good
         * previous value can still be used for fragments when 16KB+
-        * allocations are made).
+        * allocations are made inbetween fragmentary allocations).
         *
         * Beware of hardware artifacts when bmradix == 64 (intermediate
         * result can wind up being '1' instead of '0' if hardware masks
-        * bit-count & 31).
+        * bit-count & 63).
         *
         * NOTE: j needs to be even in the j= calculation.  As an artifact
         *       of the /2 division, our bitmask has to clear bit 0.
index a14c78b..4141749 100644 (file)
@@ -1459,34 +1459,19 @@ hammer2_inode_xop_destroy(hammer2_thread_t *thr, hammer2_xop_t *arg)
         */
        ip = xop->head.ip1;
        pmp = ip->pmp;
-       chain = NULL;
 
-again:
-       parent = hammer2_inode_chain(ip, thr->clindex, HAMMER2_RESOLVE_ALWAYS);
-       if (parent)
-               hammer2_chain_getparent(&parent, HAMMER2_RESOLVE_ALWAYS);
-       if (parent == NULL) {
-               error = EIO;
-               goto done;
-       }
        chain = hammer2_inode_chain(ip, thr->clindex, HAMMER2_RESOLVE_ALWAYS);
        if (chain == NULL) {
+               parent = NULL;
                error = EIO;
                goto done;
        }
-
-       /*
-        * The inode chain is unlocked so the parent can change inbetween
-        * the two calls.  Detect the situation and retry.  This case can
-        * occur quite often under heavy lods.
-        */
-       if (chain->parent != parent) {
-               hammer2_chain_unlock(parent);
-               hammer2_chain_drop(parent);
-               hammer2_chain_unlock(chain);
-               hammer2_chain_drop(chain);
-               goto again;
+       parent = hammer2_chain_getparent(chain, HAMMER2_RESOLVE_ALWAYS);
+       if (parent == NULL) {
+               error = EIO;
+               goto done;
        }
+       KKASSERT(chain->parent == parent);
 
        /*
         * We have the correct parent, we can issue the deletion.
index 55b4364..6903f58 100644 (file)
@@ -95,6 +95,9 @@ static void hammer2_strategy_read_completion(hammer2_chain_t *chain,
 static hammer2_off_t hammer2_dedup_lookup(hammer2_dev_t *hmp,
                        char **datap, int pblksize);
 
+#if 1
+#define TIMER(which)
+#else
 int h2timer[32];
 int h2last;
 int h2lid;
@@ -105,6 +108,7 @@ int h2lid;
        h2last = ticks;                                 \
        h2lid = which;                                  \
 } while(0)
+#endif
 
 int
 hammer2_vop_strategy(struct vop_strategy_args *ap)
@@ -1412,18 +1416,6 @@ hammer2_dedup_record(hammer2_chain_t *chain, hammer2_io_t *dio, char *data)
                        return;
        }
 
-#if 0
-       /*
-        * Only committed data can be recorded for de-duplication, otherwise
-        * the contents may change out from under us.  So, on read if the
-        * chain is not modified, and on flush when the chain is committed.
-        */
-       if ((chain->flags &
-           (HAMMER2_CHAIN_MODIFIED | HAMMER2_CHAIN_INITIAL)) == 0) {
-               return;
-       }
-#endif
-
        hmp = chain->hmp;
 
        switch(HAMMER2_DEC_CHECK(chain->bref.methods)) {
@@ -1465,6 +1457,9 @@ hammer2_dedup_record(hammer2_chain_t *chain, hammer2_io_t *dio, char *data)
                 */
                return;
        }
+
+       atomic_set_int(&chain->flags, HAMMER2_CHAIN_DEDUPABLE);
+
        dedup = &hmp->heur_dedup[crc & (HAMMER2_DEDUP_HEUR_MASK & ~3)];
        for (i = 0; i < 4; ++i) {
                if (dedup[i].data_crc == crc) {
@@ -1498,6 +1493,11 @@ hammer2_dedup_record(hammer2_chain_t *chain, hammer2_io_t *dio, char *data)
        mask = hammer2_dedup_mask(dio, chain->bref.data_off, chain->bytes);
        atomic_set_64(&dio->dedup_valid, mask);
 
+#if 0
+       /*
+        * XXX removed. MODIFIED is an integral part of the flush code,
+        * lets not just clear it
+        */
        /*
         * Once we record the dedup the chain must be marked clean to
         * prevent reuse of the underlying block.   Remember that this
@@ -1511,6 +1511,7 @@ hammer2_dedup_record(hammer2_chain_t *chain, hammer2_io_t *dio, char *data)
                if (chain->pmp)
                        hammer2_pfs_memory_wakeup(chain->pmp);
        }
+#endif
 }
 
 static
index f4eed82..692b878 100644 (file)
@@ -505,18 +505,15 @@ hammer2_xop_nrename(hammer2_thread_t *thr, hammer2_xop_t *arg)
                /*
                 * Find ip's direct parent chain.
                 */
-               parent = hammer2_inode_chain(ip, thr->clindex,
-                                            HAMMER2_RESOLVE_ALWAYS);
-               if (parent)
-                       hammer2_chain_getparent(&parent,
-                                               HAMMER2_RESOLVE_ALWAYS);
-               if (parent == NULL) {
-                       error = EIO;
-                       goto done;
-               }
                chain = hammer2_inode_chain(ip, thr->clindex,
                                            HAMMER2_RESOLVE_ALWAYS);
                if (chain == NULL) {
+                       error = EIO;
+                       parent = NULL;
+                       goto done;
+               }
+               parent = hammer2_chain_getparent(chain, HAMMER2_RESOLVE_ALWAYS);
+               if (parent == NULL) {
                        error = EIO;
                        goto done;
                }