From: Matthew Dillon Date: Sat, 22 Mar 2008 02:06:55 +0000 (+0000) Subject: HAMMER 34/many: Stabilization pass X-Git-Tag: v2.0.1~948 X-Git-Url: https://gitweb.dragonflybsd.org/dragonfly.git/commitdiff_plain/a84a197d08b6a87160750dfaef79a0ca13fed0d3 HAMMER 34/many: Stabilization pass * 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 (umount panic) --- diff --git a/sys/vfs/hammer/hammer_btree.c b/sys/vfs/hammer/hammer_btree.c index c636c332f6..f1618138c1 100644 --- a/sys/vfs/hammer/hammer_btree.c +++ b/sys/vfs/hammer/hammer_btree.c @@ -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); diff --git a/sys/vfs/hammer/hammer_cursor.c b/sys/vfs/hammer/hammer_cursor.c index db2407b7b3..6e1b36b094 100644 --- a/sys/vfs/hammer/hammer_cursor.c +++ b/sys/vfs/hammer/hammer_cursor.c @@ -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;