HAMMER 34/many: Stabilization pass
authorMatthew Dillon <dillon@dragonflybsd.org>
Sat, 22 Mar 2008 02:06:55 +0000 (02:06 +0000)
committerMatthew Dillon <dillon@dragonflybsd.org>
Sat, 22 Mar 2008 02:06:55 +0000 (02:06 +0000)
* Fix a node reference count leak in hammer_btree_lock_children().  This
  was resulting in a panic at umount time.

* Fix a misordered lock in hammer_cursor_up() which could result in
  an assertion in the B-Tree iteration code.  The cursor-up code was
  resolving the parent index prior to acquiring a shared lock on the
  parent node.  An insertion by another thread could do a split and
  change the parent index.  The shared lock must be acquired first.

* Add additional debugging output and add additional assertions.  HAMMER
  now has 205 assert lines in the code.

Reported-by: YONETANI Tomokazu <qhwt+dfly@les.ath.cx> (umount panic)
sys/vfs/hammer/hammer_btree.c
sys/vfs/hammer/hammer_cursor.c

index c636c33..f161813 100644 (file)
@@ -31,7 +31,7 @@
  * OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF
  * SUCH DAMAGE.
  * 
- * $DragonFly: src/sys/vfs/hammer/hammer_btree.c,v 1.34 2008/03/19 20:49:46 dillon Exp $
+ * $DragonFly: src/sys/vfs/hammer/hammer_btree.c,v 1.35 2008/03/22 02:06:55 dillon Exp $
  */
 
 /*
@@ -153,6 +153,15 @@ hammer_btree_iterate(hammer_cursor_t cursor)
                 * up our scan.
                 */
                if (cursor->index == node->count) {
+                       if (hammer_debug_btree) {
+                               kprintf("BRACKETU %016llx[%d] -> %016llx[%d] (td=%p)\n",
+                                       cursor->node->node_offset,
+                                       cursor->index,
+                                       (cursor->parent ? cursor->parent->node_offset : -1),
+                                       cursor->parent_index,
+                                       curthread);
+                       }
+                       KKASSERT(cursor->parent == NULL || cursor->parent->ondisk->elms[cursor->parent_index].internal.subtree_offset == cursor->node->node_offset);
                        error = hammer_cursor_up(cursor);
                        if (error)
                                break;
@@ -174,13 +183,14 @@ hammer_btree_iterate(hammer_cursor_t cursor)
                        r = hammer_btree_cmp(&cursor->key_end, &elm[0].base);
                        s = hammer_btree_cmp(&cursor->key_beg, &elm[1].base);
                        if (hammer_debug_btree) {
-                               kprintf("BRACKETL %016llx[%d] %016llx %02x %016llx %d\n",
+                               kprintf("BRACKETL %016llx[%d] %016llx %02x %016llx %d (td=%p)\n",
                                        cursor->node->node_offset,
                                        cursor->index,
                                        elm[0].internal.base.obj_id,
                                        elm[0].internal.base.rec_type,
                                        elm[0].internal.base.key,
-                                       r
+                                       r,
+                                       curthread
                                );
                                kprintf("BRACKETR %016llx[%d] %016llx %02x %016llx %d\n",
                                        cursor->node->node_offset,
@@ -209,6 +219,7 @@ hammer_btree_iterate(hammer_cursor_t cursor)
                         * deadlocks, but it is ok if we can't.
                         */
                        if (elm->internal.subtree_offset == 0) {
+                               kprintf("REMOVE DELETED ELEMENT\n");
                                btree_remove_deleted_element(cursor);
                                /* note: elm also invalid */
                        } else if (elm->internal.subtree_offset != 0) {
@@ -828,14 +839,27 @@ btree_search(hammer_cursor_t cursor, int flags)
        flags |= cursor->flags;
 
        if (hammer_debug_btree) {
-               kprintf("SEARCH   %016llx[%d] %016llx %02x key=%016llx cre=%016llx\n",
+               kprintf("SEARCH   %016llx[%d] %016llx %02x key=%016llx cre=%016llx (td = %p)\n",
                        cursor->node->node_offset, 
                        cursor->index,
                        cursor->key_beg.obj_id,
                        cursor->key_beg.rec_type,
                        cursor->key_beg.key,
-                       cursor->key_beg.create_tid
+                       cursor->key_beg.create_tid, 
+                       curthread
                );
+               if (cursor->parent)
+                   kprintf("SEARCHP %016llx[%d] (%016llx/%016llx %016llx/%016llx) (%p/%p %p/%p)\n",
+                       cursor->parent->node_offset, cursor->parent_index,
+                       cursor->left_bound->obj_id,
+                       cursor->parent->ondisk->elms[cursor->parent_index].internal.base.obj_id,
+                       cursor->right_bound->obj_id,
+                       cursor->parent->ondisk->elms[cursor->parent_index+1].internal.base.obj_id,
+                       cursor->left_bound,
+                       &cursor->parent->ondisk->elms[cursor->parent_index],
+                       cursor->right_bound,
+                       &cursor->parent->ondisk->elms[cursor->parent_index+1]
+                   );
        }
 
        /*
@@ -2098,6 +2122,7 @@ hammer_btree_lock_children(hammer_cursor_t cursor,
                                        hammer_ref_node(cursor->deadlk_node);
                                }
                                error = EDEADLK;
+                               hammer_rel_node(child);
                        } else {
                                item = kmalloc(sizeof(*item),
                                                M_HAMMER, M_WAITOK);
index db2407b..6e1b36b 100644 (file)
@@ -31,7 +31,7 @@
  * OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF
  * SUCH DAMAGE.
  * 
- * $DragonFly: src/sys/vfs/hammer/hammer_cursor.c,v 1.19 2008/03/19 20:18:17 dillon Exp $
+ * $DragonFly: src/sys/vfs/hammer/hammer_cursor.c,v 1.20 2008/03/22 02:06:55 dillon Exp $
  */
 
 /*
@@ -102,6 +102,7 @@ hammer_init_cursor(hammer_transaction_t trans, hammer_cursor_t cursor,
        if (error == 0)
                error = hammer_load_cursor_parent(cursor);
        KKASSERT(error == 0);
+       /* if (error) hammer_done_cursor(cursor); */
        return(error);
 }
 
@@ -213,7 +214,6 @@ hammer_cursor_seek(hammer_cursor_t cursor, hammer_node_t node, int index)
                hammer_unlock(&cursor->node->lock);
                hammer_rel_node(cursor->node);
                cursor->node = node;
-               cursor->index = index;
                hammer_ref_node(node);
                hammer_lock_sh(&node->lock);
 
@@ -225,6 +225,7 @@ hammer_cursor_seek(hammer_cursor_t cursor, hammer_node_t node, int index)
                }
                error = hammer_load_cursor_parent(cursor);
        }
+       cursor->index = index;
        return (error);
 }
 
@@ -249,6 +250,7 @@ hammer_load_cursor_parent(hammer_cursor_t cursor)
                parent = hammer_get_node(hmp, node->ondisk->parent, &error);
                if (error)
                        return(error);
+               hammer_lock_sh(&parent->lock);
                elm = NULL;
                for (i = 0; i < parent->ondisk->count; ++i) {
                        elm = &parent->ondisk->elms[i];
@@ -257,15 +259,15 @@ hammer_load_cursor_parent(hammer_cursor_t cursor)
                                break;
                        }
                }
-               if (i == parent->ondisk->count)
+               if (i == parent->ondisk->count) {
+                       hammer_unlock(&parent->lock);
                        panic("Bad B-Tree link: parent %p node %p\n", parent, node);
+               }
                KKASSERT(i != parent->ondisk->count);
                cursor->parent = parent;
                cursor->parent_index = i;
                cursor->left_bound = &elm[0].internal.base;
                cursor->right_bound = &elm[1].internal.base;
-
-               hammer_lock_sh(&parent->lock);
                return(error);
        } else {
                cursor->parent = NULL;