From 5071e67073f4a8fd6ef3165ca4e417d13a6d1e3e Mon Sep 17 00:00:00 2001 From: Matthew Dillon Date: Wed, 5 Dec 2018 22:42:31 -0800 Subject: [PATCH] hammer2 - stabilization * Adjust the chain lock a bit to bump lockcnt prior to acquiring a non-blocking shared lock, instead of afterwords, which cleans up a live-loop case in the chain unlock code. * Cleanup misc debugging. Add some inode debugging code (default disabled). * Add some crash-dump (or live) debug utilities for tracking down chains, dio's, and inodes. kgdb's macros are too slow for iterating a million chains. --- sys/vfs/hammer2/hammer2.h | 1 + sys/vfs/hammer2/hammer2_chain.c | 33 ++++++------ sys/vfs/hammer2/hammer2_flush.c | 2 - sys/vfs/hammer2/hammer2_io.c | 5 +- sys/vfs/hammer2/hammer2_vfsops.c | 3 ++ sys/vfs/hammer2/hammer2_vnops.c | 19 +++++++ test/debug/h2chains.c | 11 ++-- test/debug/{h2chains.c => h2dio.c} | 68 +++++++------------------ test/debug/{h2chains.c => h2inodes.c} | 72 ++++++++------------------- 9 files changed, 93 insertions(+), 121 deletions(-) copy test/debug/{h2chains.c => h2dio.c} (65%) copy test/debug/{h2chains.c => h2inodes.c} (64%) diff --git a/sys/vfs/hammer2/hammer2.h b/sys/vfs/hammer2/hammer2.h index 2b1c9e58f4..97a2e328fc 100644 --- a/sys/vfs/hammer2/hammer2.h +++ b/sys/vfs/hammer2/hammer2.h @@ -1438,6 +1438,7 @@ extern struct lock hammer2_mntlk; extern int hammer2_debug; +extern long hammer2_debug_inode; extern int hammer2_cluster_meta_read; extern int hammer2_cluster_data_read; extern int hammer2_cluster_write; diff --git a/sys/vfs/hammer2/hammer2_chain.c b/sys/vfs/hammer2/hammer2_chain.c index ba35bc5d01..c0489416d1 100644 --- a/sys/vfs/hammer2/hammer2_chain.c +++ b/sys/vfs/hammer2/hammer2_chain.c @@ -1003,33 +1003,37 @@ hammer2_chain_lock(hammer2_chain_t *chain, int how) if (how & HAMMER2_RESOLVE_NONBLOCK) { /* - * For non-blocking operation attempt to get the lock - * before bumping lockcnt, just so we don't have to deal - * with dropping lockcnt (and dealing with the underlying - * data) if we fail. + * We still have to bump lockcnt before acquiring the lock, + * even for non-blocking operation, because the unlock code + * live-loops on lockcnt == 1 when dropping the last lock. * - * NOTE: LOCKAGAIN must always succeed without blocking. + * If the non-blocking operation fails we have to use a + * ref+drop+unhold sequence to undo the mess (or write a + * hammer2_chain_unhold() function that doesn't drop). + * + * NOTE: LOCKAGAIN must always succeed without blocking, + * even if NONBLOCK is specified. */ + atomic_add_int(&chain->lockcnt, 1); if (how & HAMMER2_RESOLVE_SHARED) { if (how & HAMMER2_RESOLVE_LOCKAGAIN) { hammer2_mtx_sh_again(&chain->lock); } else { - if (hammer2_mtx_sh_try(&chain->lock) != 0) + if (hammer2_mtx_sh_try(&chain->lock) != 0) { + hammer2_chain_ref(chain); + hammer2_chain_drop_unhold(chain); return EAGAIN; + } } } else { - if (hammer2_mtx_ex_try(&chain->lock) != 0) + if (hammer2_mtx_ex_try(&chain->lock) != 0) { + hammer2_chain_ref(chain); + hammer2_chain_drop_unhold(chain); return EAGAIN; + } } - atomic_add_int(&chain->lockcnt, 1); ++curthread->td_tracker; } else { - /* - * Lock the element. Recursive locks are allowed. lockcnt - * ensures that data is left intact. - */ - atomic_add_int(&chain->lockcnt, 1); - /* * Get the appropriate lock. If LOCKAGAIN is flagged with * SHARED the caller expects a shared lock to already be @@ -1037,6 +1041,7 @@ hammer2_chain_lock(hammer2_chain_t *chain, int how) * importantly not block if there is a pending exclusive lock * request. */ + atomic_add_int(&chain->lockcnt, 1); if (how & HAMMER2_RESOLVE_SHARED) { if (how & HAMMER2_RESOLVE_LOCKAGAIN) { hammer2_mtx_sh_again(&chain->lock); diff --git a/sys/vfs/hammer2/hammer2_flush.c b/sys/vfs/hammer2/hammer2_flush.c index aeacd7d9cb..630b590ce0 100644 --- a/sys/vfs/hammer2/hammer2_flush.c +++ b/sys/vfs/hammer2/hammer2_flush.c @@ -1268,8 +1268,6 @@ hammer2_flush_recurse(hammer2_chain_t *child, void *data) if (child->flags & HAMMER2_CHAIN_FLUSH_MASK) { hammer2_chain_setflush(parent); } - kprintf("inum %ld do not dive root inode\n", - (long)parent->bref.key); goto done; } } diff --git a/sys/vfs/hammer2/hammer2_io.c b/sys/vfs/hammer2/hammer2_io.c index eddc9fc157..d1ea75fa36 100644 --- a/sys/vfs/hammer2/hammer2_io.c +++ b/sys/vfs/hammer2/hammer2_io.c @@ -415,7 +415,8 @@ hammer2_io_putblk(hammer2_io_t **diop) if ((orefs & HAMMER2_DIO_MASK) == 1 && (orefs & HAMMER2_DIO_INPROG) == 0) { /* - * Lastdrop case, INPROG can be set. + * Lastdrop case, INPROG can be set. GOOD must be + * cleared to prevent the getblk shortcut. */ nrefs = orefs - 1; nrefs &= ~(HAMMER2_DIO_GOOD | HAMMER2_DIO_DIRTY); @@ -524,7 +525,7 @@ hammer2_io_putblk(hammer2_io_t **diop) atomic_add_int(&hmp->iofree_count, 1); /* - * Clear INPROG, GOOD, and WAITING + * Clear INPROG, GOOD, and WAITING (GOOD should already be clear). */ for (;;) { orefs = dio->refs; diff --git a/sys/vfs/hammer2/hammer2_vfsops.c b/sys/vfs/hammer2/hammer2_vfsops.c index 95ea1fb972..1a51d22b6d 100644 --- a/sys/vfs/hammer2/hammer2_vfsops.c +++ b/sys/vfs/hammer2/hammer2_vfsops.c @@ -81,6 +81,7 @@ struct lock hammer2_mntlk; int hammer2_supported_version = HAMMER2_VOL_VERSION_DEFAULT; int hammer2_debug; +long hammer2_debug_inode; int hammer2_cluster_meta_read = 1; /* physical read-ahead */ int hammer2_cluster_data_read = 4; /* physical read-ahead */ int hammer2_cluster_write = 0; /* physical write clustering */ @@ -127,6 +128,8 @@ SYSCTL_INT(_vfs_hammer2, OID_AUTO, supported_version, CTLFLAG_RD, &hammer2_supported_version, 0, ""); SYSCTL_INT(_vfs_hammer2, OID_AUTO, debug, CTLFLAG_RW, &hammer2_debug, 0, ""); +SYSCTL_LONG(_vfs_hammer2, OID_AUTO, debug_inode, CTLFLAG_RW, + &hammer2_debug_inode, 0, ""); SYSCTL_INT(_vfs_hammer2, OID_AUTO, cluster_meta_read, CTLFLAG_RW, &hammer2_cluster_meta_read, 0, ""); SYSCTL_INT(_vfs_hammer2, OID_AUTO, cluster_data_read, CTLFLAG_RW, diff --git a/sys/vfs/hammer2/hammer2_vnops.c b/sys/vfs/hammer2/hammer2_vnops.c index 08ddaecee4..332d290cc3 100644 --- a/sys/vfs/hammer2/hammer2_vnops.c +++ b/sys/vfs/hammer2/hammer2_vnops.c @@ -1872,6 +1872,15 @@ hammer2_vop_nremove(struct vop_nremove_args *ap) ncp = ap->a_nch->ncp; + if (hammer2_debug_inode && dip->meta.inum == hammer2_debug_inode) { + kprintf("hammer2: attempt to delete inside debug inode: %s\n", + ncp->nc_name); + while (hammer2_debug_inode && + dip->meta.inum == hammer2_debug_inode) { + tsleep(&hammer2_debug_inode, 0, "h2debug", hz*5); + } + } + /*hammer2_pfs_memory_wait(dip->pmp);*/ hammer2_trans_init(dip->pmp, 0); hammer2_inode_lock(dip, 0); @@ -1909,6 +1918,16 @@ hammer2_vop_nremove(struct vop_nremove_args *ap) ip = hammer2_inode_get(dip->pmp, &xop->head, -1, -1); hammer2_xop_retire(&xop->head, HAMMER2_XOPMASK_VOP); if (ip) { + if (hammer2_debug_inode && + ip->meta.inum == hammer2_debug_inode) { + kprintf("hammer2: attempt to delete debug " + "inode!\n"); + while (hammer2_debug_inode && + ip->meta.inum == hammer2_debug_inode) { + tsleep(&hammer2_debug_inode, 0, + "h2debug", hz*5); + } + } hammer2_inode_unlink_finisher(ip, isopen); hammer2_inode_depend(dip, ip); /* after modified */ hammer2_inode_unlock(ip); diff --git a/test/debug/h2chains.c b/test/debug/h2chains.c index bf19b79fe3..37528df44b 100644 --- a/test/debug/h2chains.c +++ b/test/debug/h2chains.c @@ -1,7 +1,7 @@ /* * H2CHAINS.C * - * cc -I/usr/src/sys h2chains.c -o /usr/local/bin/h2chains -lkvm + * cc -I/usr/src/sys h2chains.c -o ~/bin/h2chains -lkvm * * h2chains * @@ -137,8 +137,13 @@ h2chainscan(kvm_t *kd, int tab, uintptr_t cp, uintptr_t pcp, int pflags) h2chainscan(kd, tab, (uintptr_t)chain.rbnode.rbe_left, pcp, pflags); - printf("%*.*s chain %p %08x type %02x ", tab, tab, "", - (void *)cp, chain.flags, chain.bref.type); + printf("%*.*s chain %p type %02x lock %p dio %p " + "off/pbase=%016jx/%016jx flags %08x ", + tab, tab, "", + (void *)cp, chain.bref.type, chain.lock.mtx_owner, chain.dio, + chain.bref.data_off & ~(hammer2_off_t)0x0F, + chain.bref.data_off & ~(hammer2_off_t)65535, + chain.flags); if (chain.flags & HAMMER2_CHAIN_ONFLUSH) printf("F"); if (chain.flags & HAMMER2_CHAIN_MODIFIED) diff --git a/test/debug/h2chains.c b/test/debug/h2dio.c similarity index 65% copy from test/debug/h2chains.c copy to test/debug/h2dio.c index bf19b79fe3..31df176ecc 100644 --- a/test/debug/h2chains.c +++ b/test/debug/h2dio.c @@ -1,11 +1,11 @@ /* - * H2CHAINS.C + * H2DIO.C * - * cc -I/usr/src/sys h2chains.c -o /usr/local/bin/h2chains -lkvm + * cc -I/usr/src/sys h2dio.c -o ~/bin/h2dio -lkvm * - * h2chains + * h2dio * - * Copyright (c) 2017 The DragonFly Project. All rights reserved. + * Copyright (c) 2018 The DragonFly Project. All rights reserved. * * This code is derived from software contributed to The DragonFly Project * by Matthew Dillon @@ -53,8 +53,7 @@ #include #include -static void h2chainscan(kvm_t *kd, int tab, uintptr_t cp, - uintptr_t pcp, int pflags); +static void h2dioscan(kvm_t *kd, int tab, uintptr_t cp); static void kkread(kvm_t *kd, u_long addr, void *buf, size_t nbytes); #if 0 @@ -108,57 +107,28 @@ main(int ac, char **av) base = strtoul(av[0], NULL, 0); kkread(kd, base, &hmp, sizeof(hmp)); - cp = (uintptr_t)hmp.vchain.core.rbtree.rbh_root; - printf("VCHAIN %08x\n", hmp.vchain.flags); + cp = (uintptr_t)hmp.iotree.rbh_root; if (cp) - h2chainscan(kd, 4, cp, - base + offsetof(struct hammer2_dev, vchain), - hmp.vchain.flags); + h2dioscan(kd, 4, cp); printf("\n"); - - cp = (uintptr_t)hmp.fchain.core.rbtree.rbh_root; - printf("FCHAIN %08x\n", hmp.fchain.flags); - if (cp) - h2chainscan(kd, 4, cp, - base + offsetof(struct hammer2_dev, fchain), - hmp.fchain.flags); - return 0; } static void -h2chainscan(kvm_t *kd, int tab, uintptr_t cp, uintptr_t pcp, int pflags) +h2dioscan(kvm_t *kd, int tab, uintptr_t cp) { - hammer2_chain_t chain; - - kkread(kd, cp, &chain, sizeof(chain)); - if (chain.rbnode.rbe_left) - h2chainscan(kd, tab, (uintptr_t)chain.rbnode.rbe_left, - pcp, pflags); - - printf("%*.*s chain %p %08x type %02x ", tab, tab, "", - (void *)cp, chain.flags, chain.bref.type); - if (chain.flags & HAMMER2_CHAIN_ONFLUSH) - printf("F"); - if (chain.flags & HAMMER2_CHAIN_MODIFIED) - printf("M"); - if (chain.flags & (HAMMER2_CHAIN_ONFLUSH|HAMMER2_CHAIN_MODIFIED)) { - if ((pflags & HAMMER2_CHAIN_ONFLUSH) == 0) - printf(" FAIL"); - } - if ((uintptr_t)chain.parent != pcp) - printf(" FAIL2"); - printf("\n"); - if (chain.core.rbtree.rbh_root) - h2chainscan(kd, tab + 4, - (uintptr_t)chain.core.rbtree.rbh_root, - cp, chain.flags); - - if (chain.rbnode.rbe_right) - h2chainscan(kd, tab, (uintptr_t)chain.rbnode.rbe_right, - pcp, pflags); - + hammer2_io_t dio; + + kkread(kd, cp, &dio, sizeof(dio)); + if (dio.rbnode.rbe_left) + h2dioscan(kd, tab, (uintptr_t)dio.rbnode.rbe_left); + printf("%*.*s dio %p pbase=%016lx refs=%08x " + "bp=%p psize=%d ticks=%d err=%d\n", + tab, tab, "", (void *)cp, dio.pbase, dio.refs, + dio.bp, dio.psize, dio.ticks, dio.error); + if (dio.rbnode.rbe_right) + h2dioscan(kd, tab, (uintptr_t)dio.rbnode.rbe_right); } static void diff --git a/test/debug/h2chains.c b/test/debug/h2inodes.c similarity index 64% copy from test/debug/h2chains.c copy to test/debug/h2inodes.c index bf19b79fe3..3192214e58 100644 --- a/test/debug/h2chains.c +++ b/test/debug/h2inodes.c @@ -1,11 +1,11 @@ /* - * H2CHAINS.C + * H2INODES.C * - * cc -I/usr/src/sys h2chains.c -o /usr/local/bin/h2chains -lkvm + * cc -I/usr/src/sys h2inodes.c -o ~/bin/h2inodes -lkvm * - * h2chains + * h2inodes * - * Copyright (c) 2017 The DragonFly Project. All rights reserved. + * Copyright (c) 2018 The DragonFly Project. All rights reserved. * * This code is derived from software contributed to The DragonFly Project * by Matthew Dillon @@ -53,8 +53,7 @@ #include #include -static void h2chainscan(kvm_t *kd, int tab, uintptr_t cp, - uintptr_t pcp, int pflags); +static void h2inumscan(kvm_t *kd, int tab, uintptr_t cp); static void kkread(kvm_t *kd, u_long addr, void *buf, size_t nbytes); #if 0 @@ -71,7 +70,7 @@ main(int ac, char **av) kvm_t *kd; const char *corefile = NULL; const char *sysfile = NULL; - hammer2_dev_t hmp; + hammer2_pfs_t pmp; uintptr_t base; uintptr_t cp; int ch; @@ -107,58 +106,29 @@ main(int ac, char **av) base = strtoul(av[0], NULL, 0); - kkread(kd, base, &hmp, sizeof(hmp)); - cp = (uintptr_t)hmp.vchain.core.rbtree.rbh_root; - printf("VCHAIN %08x\n", hmp.vchain.flags); + kkread(kd, base, &pmp, sizeof(pmp)); + cp = (uintptr_t)pmp.inum_tree.rbh_root; if (cp) - h2chainscan(kd, 4, cp, - base + offsetof(struct hammer2_dev, vchain), - hmp.vchain.flags); + h2inumscan(kd, 4, cp); printf("\n"); - - cp = (uintptr_t)hmp.fchain.core.rbtree.rbh_root; - printf("FCHAIN %08x\n", hmp.fchain.flags); - if (cp) - h2chainscan(kd, 4, cp, - base + offsetof(struct hammer2_dev, fchain), - hmp.fchain.flags); - return 0; } static void -h2chainscan(kvm_t *kd, int tab, uintptr_t cp, uintptr_t pcp, int pflags) +h2inumscan(kvm_t *kd, int tab, uintptr_t cp) { - hammer2_chain_t chain; - - kkread(kd, cp, &chain, sizeof(chain)); - if (chain.rbnode.rbe_left) - h2chainscan(kd, tab, (uintptr_t)chain.rbnode.rbe_left, - pcp, pflags); - - printf("%*.*s chain %p %08x type %02x ", tab, tab, "", - (void *)cp, chain.flags, chain.bref.type); - if (chain.flags & HAMMER2_CHAIN_ONFLUSH) - printf("F"); - if (chain.flags & HAMMER2_CHAIN_MODIFIED) - printf("M"); - if (chain.flags & (HAMMER2_CHAIN_ONFLUSH|HAMMER2_CHAIN_MODIFIED)) { - if ((pflags & HAMMER2_CHAIN_ONFLUSH) == 0) - printf(" FAIL"); - } - if ((uintptr_t)chain.parent != pcp) - printf(" FAIL2"); - printf("\n"); - if (chain.core.rbtree.rbh_root) - h2chainscan(kd, tab + 4, - (uintptr_t)chain.core.rbtree.rbh_root, - cp, chain.flags); - - if (chain.rbnode.rbe_right) - h2chainscan(kd, tab, (uintptr_t)chain.rbnode.rbe_right, - pcp, pflags); - + hammer2_inode_t ip; + + kkread(kd, cp, &ip, sizeof(ip)); + if (ip.rbnode.rbe_left) + h2inumscan(kd, tab, (uintptr_t)ip.rbnode.rbe_left); + printf("%*.*s ip %p inum=%ld refs=%08x " + "flags=%08x chain0=%p vp=%p\n", + tab, tab, "", (void *)cp, ip.meta.inum, ip.refs, + ip.flags, ip.cluster.array[0].chain, ip.vp); + if (ip.rbnode.rbe_right) + h2inumscan(kd, tab, (uintptr_t)ip.rbnode.rbe_right); } static void -- 2.41.0