From 68b321c1c2fcb76069594715a2a617f08aeb59ec Mon Sep 17 00:00:00 2001 From: Matthew Dillon Date: Fri, 16 Mar 2018 11:01:11 -0700 Subject: [PATCH] hammer2 - More involved refactoring of chain_repparent, cleanup * Remove unused locking flags (remove the NOLOCK and NOUNLOCK features). * Add HAMMER2_RESOLVE_NONBLOCK to hammer2_chain_lock() for use only by hammer2_chain_getparent() and hammer2_chain_repparent(). * Refactor hammer2_chain_getparent() and hammer2_chain_repparent(). Add a hot-path that uses HAMMER2_RESOLVE_NONBLOCK. If this fails we now do a much more involved tracking operation via 'reptrack' to deal with races against indirect block deletions. * Cleanup the copyright messages. * Fix an issue where a sync could be held-up indefinitely by ongoing overlapping modifying operations. * Install a proper initial inode count when creating a snapshot. * Fix a deadlock in checkdirempty(). A chain lock was winding up being ordered incorrectly. --- sys/vfs/hammer2/hammer2.h | 33 ++-- sys/vfs/hammer2/hammer2_admin.c | 2 +- sys/vfs/hammer2/hammer2_bulkfree.c | 2 +- sys/vfs/hammer2/hammer2_ccms.c | 2 +- sys/vfs/hammer2/hammer2_ccms.h | 2 +- sys/vfs/hammer2/hammer2_chain.c | 305 ++++++++++++++++++++++++++----------- sys/vfs/hammer2/hammer2_cluster.c | 2 +- sys/vfs/hammer2/hammer2_disk.h | 2 +- sys/vfs/hammer2/hammer2_flush.c | 41 +++-- sys/vfs/hammer2/hammer2_freemap.c | 2 +- sys/vfs/hammer2/hammer2_inode.c | 2 +- sys/vfs/hammer2/hammer2_io.c | 2 +- sys/vfs/hammer2/hammer2_iocom.c | 2 +- sys/vfs/hammer2/hammer2_ioctl.c | 4 +- sys/vfs/hammer2/hammer2_ioctl.h | 2 +- sys/vfs/hammer2/hammer2_mount.h | 2 +- sys/vfs/hammer2/hammer2_msgops.c | 2 +- sys/vfs/hammer2/hammer2_strategy.c | 1 + sys/vfs/hammer2/hammer2_subr.c | 2 +- sys/vfs/hammer2/hammer2_synchro.c | 2 +- sys/vfs/hammer2/hammer2_vnops.c | 5 +- sys/vfs/hammer2/hammer2_xops.c | 40 ++--- sys/vfs/hammer2/hammer2_xxhash.h | 2 +- 23 files changed, 314 insertions(+), 147 deletions(-) diff --git a/sys/vfs/hammer2/hammer2.h b/sys/vfs/hammer2/hammer2.h index da0a69d443..7db8ec9871 100644 --- a/sys/vfs/hammer2/hammer2.h +++ b/sys/vfs/hammer2/hammer2.h @@ -112,7 +112,6 @@ struct hammer2_inode; struct hammer2_dev; struct hammer2_pfs; struct hammer2_span; -struct hammer2_state; struct hammer2_msg; struct hammer2_thread; union hammer2_xop; @@ -129,8 +128,10 @@ typedef mtx_state_t hammer2_mtx_state_t; typedef struct spinlock hammer2_spin_t; #define hammer2_mtx_ex mtx_lock_ex_quick +#define hammer2_mtx_ex_try mtx_lock_ex_try #define hammer2_mtx_sh mtx_lock_sh_quick #define hammer2_mtx_sh_again mtx_lock_sh_again +#define hammer2_mtx_sh_try mtx_lock_sh_try #define hammer2_mtx_unlock mtx_unlock #define hammer2_mtx_downgrade mtx_downgrade #define hammer2_mtx_owned mtx_owned @@ -243,11 +244,18 @@ TAILQ_HEAD(h2_core_list, hammer2_chain); #define CHAIN_CORE_DELETE_BMAP_ENTRIES \ (HAMMER2_PBUFSIZE / sizeof(hammer2_blockref_t) / sizeof(uint32_t)) +struct hammer2_reptrack { + hammer2_spin_t spin; + struct hammer2_reptrack *next; + struct hammer2_chain *chain; +}; + /* * Core topology for chain (embedded in chain). Protected by a spinlock. */ struct hammer2_chain_core { hammer2_spin_t spin; + struct hammer2_reptrack *reptrack; struct hammer2_chain_tree rbtree; /* sub-chains */ int live_zero; /* blockref array opt */ u_int live_count; /* live (not deleted) chains in tree */ @@ -300,7 +308,6 @@ struct hammer2_chain { RB_ENTRY(hammer2_chain) rbnode; /* live chain(s) */ hammer2_blockref_t bref; struct hammer2_chain *parent; - struct hammer2_state *state; /* if active cache msg */ struct hammer2_dev *hmp; struct hammer2_pfs *pmp; /* A PFS or super-root (spmp) */ @@ -421,10 +428,6 @@ RB_PROTOTYPE(hammer2_chain_tree, hammer2_chain, rbnode, hammer2_chain_cmp); * Flags passed to hammer2_chain_lookup() and hammer2_chain_next() * * NOTES: - * NOLOCK - Input and output chains are referenced only and not - * locked. Output chain might be temporarily locked - * internally. - * * NODATA - Asks that the chain->data not be resolved in order * to avoid I/O. * @@ -448,21 +451,15 @@ RB_PROTOTYPE(hammer2_chain_tree, hammer2_chain, rbnode, hammer2_chain_cmp); * ALWAYS - Always resolve the data. If ALWAYS and NODATA are both * missing, bulk file data is not resolved but inodes and * other meta-data will. - * - * NOUNLOCK - Used by hammer2_chain_next() to leave the lock on - * the input chain intact. The chain is still dropped. - * This allows the caller to add a reference to the chain - * and retain it in a locked state (used by the - * XOP/feed/collect code). */ -#define HAMMER2_LOOKUP_NOLOCK 0x00000001 /* ref only */ +#define HAMMER2_LOOKUP_UNUSED0001 0x00000001 #define HAMMER2_LOOKUP_NODATA 0x00000002 /* data left NULL */ #define HAMMER2_LOOKUP_NODIRECT 0x00000004 /* no offset=0 DD */ #define HAMMER2_LOOKUP_SHARED 0x00000100 #define HAMMER2_LOOKUP_MATCHIND 0x00000200 /* return all chains */ #define HAMMER2_LOOKUP_ALLNODES 0x00000400 /* allow NULL focus */ #define HAMMER2_LOOKUP_ALWAYS 0x00000800 /* resolve data */ -#define HAMMER2_LOOKUP_NOUNLOCK 0x00001000 /* leave lock intact */ +#define HAMMER2_LOOKUP_UNUSED1000 0x00001000 /* * Flags passed to hammer2_chain_modify() and hammer2_chain_resize() @@ -481,6 +478,10 @@ RB_PROTOTYPE(hammer2_chain_tree, hammer2_chain, rbnode, hammer2_chain_cmp); * will be made to either the cluster being locked or any underlying * cluster. It allows the cluster to lock and access data for a subset * of available nodes instead of all available nodes. + * + * NOTE: NONBLOCK is only used for hammer2_chain_repparent() and getparent(), + * other functions (e.g. hammer2_chain_lookup(), etc) can't handle its + * operation. */ #define HAMMER2_RESOLVE_NEVER 1 #define HAMMER2_RESOLVE_MAYBE 2 @@ -490,6 +491,7 @@ RB_PROTOTYPE(hammer2_chain_tree, hammer2_chain, rbnode, hammer2_chain_cmp); #define HAMMER2_RESOLVE_SHARED 0x10 /* request shared lock */ #define HAMMER2_RESOLVE_LOCKAGAIN 0x20 /* another shared lock */ #define HAMMER2_RESOLVE_RDONLY 0x40 /* higher level op flag */ +#define HAMMER2_RESOLVE_NONBLOCK 0x80 /* non-blocking */ /* * Flags passed to hammer2_chain_delete() @@ -761,6 +763,7 @@ typedef struct hammer2_inode_sideq hammer2_inode_sideq_t; struct hammer2_trans { uint32_t flags; uint32_t sync_wait; + int fticks; /* FPENDING start */ }; typedef struct hammer2_trans hammer2_trans_t; @@ -1487,7 +1490,7 @@ void hammer2_chain_ref(hammer2_chain_t *chain); void hammer2_chain_ref_hold(hammer2_chain_t *chain); void hammer2_chain_drop(hammer2_chain_t *chain); void hammer2_chain_drop_unhold(hammer2_chain_t *chain); -void hammer2_chain_lock(hammer2_chain_t *chain, int how); +int hammer2_chain_lock(hammer2_chain_t *chain, int how); void hammer2_chain_lock_unhold(hammer2_chain_t *chain, int how); void hammer2_chain_load_data(hammer2_chain_t *chain); const hammer2_media_data_t *hammer2_chain_rdata(hammer2_chain_t *chain); diff --git a/sys/vfs/hammer2/hammer2_admin.c b/sys/vfs/hammer2/hammer2_admin.c index f8692df000..d8b0170d15 100644 --- a/sys/vfs/hammer2/hammer2_admin.c +++ b/sys/vfs/hammer2/hammer2_admin.c @@ -1,5 +1,5 @@ /* - * Copyright (c) 2015 The DragonFly Project. All rights reserved. + * Copyright (c) 2015-2018 The DragonFly Project. All rights reserved. * * This code is derived from software contributed to The DragonFly Project * by Matthew Dillon diff --git a/sys/vfs/hammer2/hammer2_bulkfree.c b/sys/vfs/hammer2/hammer2_bulkfree.c index 6525bfd25d..bf81cd273f 100644 --- a/sys/vfs/hammer2/hammer2_bulkfree.c +++ b/sys/vfs/hammer2/hammer2_bulkfree.c @@ -1,5 +1,5 @@ /* - * Copyright (c) 2013-2015 The DragonFly Project. All rights reserved. + * Copyright (c) 2013-2018 The DragonFly Project. All rights reserved. * * This code is derived from software contributed to The DragonFly Project * by Matthew Dillon diff --git a/sys/vfs/hammer2/hammer2_ccms.c b/sys/vfs/hammer2/hammer2_ccms.c index e202814b8f..3d255ef076 100644 --- a/sys/vfs/hammer2/hammer2_ccms.c +++ b/sys/vfs/hammer2/hammer2_ccms.c @@ -1,5 +1,5 @@ /* - * Copyright (c) 2006,2012-2014 The DragonFly Project. All rights reserved. + * Copyright (c) 2006,2012-2018 The DragonFly Project. All rights reserved. * * This code is derived from software contributed to The DragonFly Project * by Matthew Dillon diff --git a/sys/vfs/hammer2/hammer2_ccms.h b/sys/vfs/hammer2/hammer2_ccms.h index 116c8cdf54..8585e52a36 100644 --- a/sys/vfs/hammer2/hammer2_ccms.h +++ b/sys/vfs/hammer2/hammer2_ccms.h @@ -1,5 +1,5 @@ /* - * Copyright (c) 2006,2014 The DragonFly Project. All rights reserved. + * Copyright (c) 2006,2014-2018 The DragonFly Project. All rights reserved. * * This code is derived from software contributed to The DragonFly Project * by Matthew Dillon diff --git a/sys/vfs/hammer2/hammer2_chain.c b/sys/vfs/hammer2/hammer2_chain.c index 3af4c76040..c9172fa169 100644 --- a/sys/vfs/hammer2/hammer2_chain.c +++ b/sys/vfs/hammer2/hammer2_chain.c @@ -1,5 +1,5 @@ /* - * Copyright (c) 2011-2015 The DragonFly Project. All rights reserved. + * Copyright (c) 2011-2018 The DragonFly Project. All rights reserved. * * This code is derived from software contributed to The DragonFly Project * by Matthew Dillon @@ -400,7 +400,7 @@ hammer2_chain_drop(hammer2_chain_t *chain) KKASSERT(refs > 0); if (refs == 1) { - if (mtx_lock_ex_try(&chain->lock) == 0) + if (hammer2_mtx_ex_try(&chain->lock) == 0) chain = hammer2_chain_lastdrop(chain, 0); /* retry the same chain, or chain from lastdrop */ } else { @@ -431,7 +431,7 @@ hammer2_chain_drop_unhold(hammer2_chain_t *chain) lockcnt, lockcnt - 1)) { break; } - } else if (mtx_lock_ex_try(&chain->lock) == 0) { + } else if (hammer2_mtx_ex_try(&chain->lock) == 0) { hammer2_chain_unlock(chain); break; } else { @@ -933,7 +933,7 @@ again: KKASSERT(refs > 0); if (refs == 1) { - if (mtx_lock_ex_try(&chain->lock) == 0) + if (hammer2_mtx_ex_try(&chain->lock) == 0) chain = hammer2_chain_lastdrop(chain, 1); /* retry the same chain, or chain from lastdrop */ } else { @@ -1006,6 +1006,9 @@ hammer2_chain_drop_data(hammer2_chain_t *chain) * HAMMER2_RESOLVE_SHARED- (flag) The chain is locked shared, otherwise * it will be locked exclusive. * + * HAMMER2_RESOLVE_NONBLOCK- (flag) The chain is locked non-blocking. If + * the lock fails, EAGAIN is returned. + * * NOTE: Embedded elements (volume header, inodes) are always resolved * regardless. * @@ -1018,6 +1021,9 @@ hammer2_chain_drop_data(hammer2_chain_t *chain) * a logical file buffer. However, if ALWAYS is specified the * device buffer will be instantiated anyway. * + * NOTE: The return value is always 0 unless NONBLOCK is specified, in which + * case it can be either 0 or EAGAIN. + * * WARNING! This function blocks on I/O if data needs to be fetched. This * blocking can run concurrent with other compatible lock holders * who do not need data returning. The lock is not upgraded to @@ -1026,31 +1032,58 @@ hammer2_chain_drop_data(hammer2_chain_t *chain) * on being interlocked against an I/O fetch managed by a shared * lock holder. */ -void +int hammer2_chain_lock(hammer2_chain_t *chain, int how) { - /* - * Ref and lock the element. Recursive locks are allowed. - */ KKASSERT(chain->refs > 0); - 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 present and we - * are giving it another ref. This case must importantly not block - * if there is a pending exclusive lock request. - */ - if (how & HAMMER2_RESOLVE_SHARED) { - if (how & HAMMER2_RESOLVE_LOCKAGAIN) { - hammer2_mtx_sh_again(&chain->lock); + 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. + * + * NOTE: LOCKAGAIN must always succeed without blocking. + */ + 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) + return EAGAIN; + } } else { - hammer2_mtx_sh(&chain->lock); + if (hammer2_mtx_ex_try(&chain->lock) != 0) + return EAGAIN; } + atomic_add_int(&chain->lockcnt, 1); + ++curthread->td_tracker; } else { - hammer2_mtx_ex(&chain->lock); + /* + * 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 + * present and we are giving it another ref. This case must + * importantly not block if there is a pending exclusive lock + * request. + */ + if (how & HAMMER2_RESOLVE_SHARED) { + if (how & HAMMER2_RESOLVE_LOCKAGAIN) { + hammer2_mtx_sh_again(&chain->lock); + } else { + hammer2_mtx_sh(&chain->lock); + } + } else { + hammer2_mtx_ex(&chain->lock); + } + ++curthread->td_tracker; } - ++curthread->td_tracker; /* * If we already have a valid data pointer make sure the data is @@ -1060,7 +1093,7 @@ hammer2_chain_lock(hammer2_chain_t *chain, int how) if (chain->data) { if (chain->dio) hammer2_io_bkvasync(chain->dio); - return; + return 0; } /* @@ -1070,17 +1103,17 @@ hammer2_chain_lock(hammer2_chain_t *chain, int how) */ switch(how & HAMMER2_RESOLVE_MASK) { case HAMMER2_RESOLVE_NEVER: - return; + return 0; case HAMMER2_RESOLVE_MAYBE: if (chain->flags & HAMMER2_CHAIN_INITIAL) - return; + return 0; if (chain->bref.type == HAMMER2_BREF_TYPE_DATA) - return; + return 0; #if 0 if (chain->bref.type == HAMMER2_BREF_TYPE_FREEMAP_NODE) - return; + return 0; if (chain->bref.type == HAMMER2_BREF_TYPE_FREEMAP_LEAF) - return; + return 0; #endif /* fall through */ case HAMMER2_RESOLVE_ALWAYS: @@ -1092,6 +1125,8 @@ hammer2_chain_lock(hammer2_chain_t *chain, int how) * Caller requires data */ hammer2_chain_load_data(chain); + + return 0; } /* @@ -2298,7 +2333,9 @@ hammer2_chain_lookup_done(hammer2_chain_t *parent) /* * Take the locked chain and return a locked parent. The chain remains * locked on return, but may have to be temporarily unlocked to acquire - * the parent. Can return NULL. + * the parent. Because of this, (chain) must be stable and cannot be + * deleted while it was temporarily unlocked (typically means that (chain) + * is an inode). * * Pass HAMMER2_RESOLVE_* flags in flags. * @@ -2314,32 +2351,33 @@ hammer2_chain_getparent(hammer2_chain_t *chain, int flags) /* * 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. + * is locked below to avoid a deadlock. Try it trivially first. */ -again: - hammer2_spin_ex(&chain->core.spin); parent = chain->parent; - if (parent == NULL) { - hammer2_spin_unex(&chain->core.spin); + if (parent == NULL) panic("hammer2_chain_getparent: no parent"); - } hammer2_chain_ref(parent); - hammer2_spin_unex(&chain->core.spin); + if (hammer2_chain_lock(parent, flags|HAMMER2_RESOLVE_NONBLOCK) == 0) + return parent; - hammer2_chain_unlock(chain); - hammer2_chain_lock(parent, flags); - hammer2_chain_lock(chain, flags); + for (;;) { + hammer2_chain_unlock(chain); + hammer2_chain_lock(parent, flags); + hammer2_chain_lock(chain, flags); - /* - * Parent relinking races are quite common. We have to get it right - * or we will blow up the block table. - */ - if (chain->parent != parent) { + /* + * Parent relinking races are quite common. We have to get + * it right or we will blow up the block table. + */ + if (chain->parent == parent) + break; hammer2_chain_unlock(parent); hammer2_chain_drop(parent); - goto again; + cpu_ccfence(); + parent = chain->parent; + if (parent == NULL) + panic("hammer2_chain_getparent: no parent"); + hammer2_chain_ref(parent); } return parent; } @@ -2352,6 +2390,9 @@ again: * This will work even if the chain is errored, and the caller can check * parent->error on return if desired since the parent will be locked. * + * The chain does NOT need to be stable. We use a tracking structure + * to track the expected parent if the chain is deleted out from under us. + * * This function handles the lock order reversal. */ hammer2_chain_t * @@ -2359,44 +2400,141 @@ hammer2_chain_repparent(hammer2_chain_t **chainp, int flags) { hammer2_chain_t *chain; hammer2_chain_t *parent; + struct hammer2_reptrack reptrack; + struct hammer2_reptrack **repp; /* * 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. + * is locked below to avoid a deadlock. Try it trivially first. */ 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"); + panic("hammer2_chain_repparent: no parent"); } hammer2_chain_ref(parent); - hammer2_spin_unex(&chain->core.spin); + if (hammer2_chain_lock(parent, flags|HAMMER2_RESOLVE_NONBLOCK) == 0) { + hammer2_chain_unlock(chain); + hammer2_chain_drop(chain); + *chainp = parent; + + return parent; + } + + /* + * Ok, now it gets a bit nasty. There are multiple situations where + * the parent might be in the middle of a deletion, or where the child + * (chain) might be deleted the instant we let go of its lock. + * We can potentially end up in a no-win situation! + * + * In particular, the indirect_maintenance() case can cause these + * situations. + * + * To deal with this we install a reptrack structure in the parent + * This reptrack structure 'owns' the parent ref and will automatically + * migrate to the parent's parent if the parent is deleted permanently. + */ + hammer2_spin_init(&reptrack.spin, "h2reptrk"); + reptrack.chain = parent; + hammer2_chain_ref(parent); /* for the reptrack */ + + hammer2_spin_ex(&parent->core.spin); + reptrack.next = parent->core.reptrack; + parent->core.reptrack = &reptrack; + hammer2_spin_unex(&parent->core.spin); hammer2_chain_unlock(chain); - hammer2_chain_lock(parent, flags); + hammer2_chain_drop(chain); + chain = NULL; /* gone */ /* - * Parent relinking races are quite common. We have to get it right - * or we will blow up the block table. + * At the top of this loop, chain is gone and parent is refd both + * by us explicitly AND via our reptrack. We are attempting to + * lock parent. */ - if (chain->parent != parent) { - hammer2_chain_lock(chain, flags); + for (;;) { + hammer2_chain_lock(parent, flags); + + if (reptrack.chain == parent) + break; hammer2_chain_unlock(parent); hammer2_chain_drop(parent); - goto again; + + kprintf("hammer2: debug REPTRACK %p->%p\n", + parent, reptrack.chain); + hammer2_spin_ex(&reptrack.spin); + parent = reptrack.chain; + hammer2_chain_ref(parent); + hammer2_spin_unex(&reptrack.spin); } - hammer2_chain_drop(chain); - *chainp = parent; + + /* + * Once parent is locked and matches our reptrack, our reptrack + * will be stable and we have our parent. We can unlink our + * reptrack. + * + * WARNING! Remember that the chain lock might be shared. Chains + * locked shared have stable parent linkages. + */ + hammer2_spin_ex(&parent->core.spin); + repp = &parent->core.reptrack; + while (*repp != &reptrack) + repp = &(*repp)->next; + *repp = reptrack.next; + hammer2_spin_unex(&parent->core.spin); + + hammer2_chain_drop(parent); /* reptrack ref */ + *chainp = parent; /* return parent lock+ref */ return parent; } /* + * Dispose of any linked reptrack structures in (chain) by shifting them to + * (parent). Both (chain) and (parent) must be exclusively locked. + * + * This is interlocked against any children of (chain) on the other side. + * No children so remain as-of when this is called so we can test + * core.reptrack without holding the spin-lock. + * + * Used whenever the caller intends to permanently delete chains related + * to topological recursions (BREF_TYPE_INDIRECT, BREF_TYPE_FREEMAP_NODE), + * where the chains underneath the node being deleted are given a new parent + * above the node being deleted. + */ +static +void +hammer2_chain_repchange(hammer2_chain_t *parent, hammer2_chain_t *chain) +{ + struct hammer2_reptrack *reptrack; + + KKASSERT(chain->core.live_count == 0 && RB_EMPTY(&chain->core.rbtree)); + while (chain->core.reptrack) { + hammer2_spin_ex(&parent->core.spin); + hammer2_spin_ex(&chain->core.spin); + reptrack = chain->core.reptrack; + if (reptrack == NULL) { + hammer2_spin_unex(&chain->core.spin); + hammer2_spin_unex(&parent->core.spin); + break; + } + hammer2_spin_ex(&reptrack->spin); + chain->core.reptrack = reptrack->next; + reptrack->chain = parent; + reptrack->next = parent->core.reptrack; + parent->core.reptrack = reptrack; + hammer2_chain_ref(parent); /* reptrack */ + + hammer2_spin_unex(&chain->core.spin); + hammer2_spin_unex(&parent->core.spin); + kprintf("hammer2: debug repchange %p %p->%p\n", + reptrack, chain, parent); + hammer2_chain_drop(chain); /* reptrack */ + } +} + +/* * Locate the first chain whos key range overlaps (key_beg, key_end) inclusive. * (*parentp) typically points to an inode but can also point to a related * indirect block and this function will recurse upwards and find the inode @@ -2423,15 +2561,9 @@ again: * wishes to insert a new chain, (*parentp) will be set properly even if NULL * is returned, as long as no error occurred. * - * The matching chain will be returned locked according to flags. If NOLOCK - * is requested the chain will be returned only referenced. Note that the - * parent chain must always be locked shared or exclusive, matching the - * HAMMER2_LOOKUP_SHARED flag. We can conceivably lock it SHARED temporarily - * when NOLOCK is specified but that complicates matters if *parentp must - * inherit the chain. + * The matching chain will be returned locked according to flags. * - * NOLOCK also implies NODATA, since an unlocked chain usually has a NULL - * data pointer or can otherwise be in flux. + * -- * * NULL is returned if no match was found, but (*parentp) will still * potentially be adjusted. @@ -2474,7 +2606,7 @@ hammer2_chain_lookup(hammer2_chain_t **parentp, hammer2_key_t *key_nextp, if (flags & HAMMER2_LOOKUP_ALWAYS) { how_maybe = how_always; how = HAMMER2_RESOLVE_ALWAYS; - } else if (flags & (HAMMER2_LOOKUP_NODATA | HAMMER2_LOOKUP_NOLOCK)) { + } else if (flags & HAMMER2_LOOKUP_NODATA) { how = HAMMER2_RESOLVE_NEVER; } else { how = HAMMER2_RESOLVE_MAYBE; @@ -2508,7 +2640,6 @@ hammer2_chain_lookup(hammer2_chain_t **parentp, hammer2_key_t *key_nextp, parent = hammer2_chain_repparent(parentp, how_maybe); } again: - if (--maxloops == 0) panic("hammer2_chain_lookup: maxloops"); /* @@ -2537,10 +2668,8 @@ again: goto done; } hammer2_chain_ref(parent); - if ((flags & HAMMER2_LOOKUP_NOLOCK) == 0) - hammer2_chain_lock(parent, - how_always | - HAMMER2_RESOLVE_LOCKAGAIN); + hammer2_chain_lock(parent, how_always | + HAMMER2_RESOLVE_LOCKAGAIN); *key_nextp = key_end + 1; return (parent); } @@ -2754,7 +2883,7 @@ again: } done: /* - * All done, return the chain. + * All done, return the locked chain. * * If the caller does not want a locked chain, replace the lock with * a ref. Perhaps this can eventually be optimized to not obtain the @@ -2765,10 +2894,6 @@ done: * *errorp is only set based on issues which occur while * trying to reach the chain. */ - if (chain) { - if (flags & HAMMER2_LOOKUP_NOLOCK) - hammer2_chain_unlock(chain); - } return (chain); } @@ -2816,10 +2941,7 @@ hammer2_chain_next(hammer2_chain_t **parentp, hammer2_chain_t *chain, if (chain) { key_beg = chain->bref.key + ((hammer2_key_t)1 << chain->bref.keybits); - if ((flags & (HAMMER2_LOOKUP_NOLOCK | - HAMMER2_LOOKUP_NOUNLOCK)) == 0) { - hammer2_chain_unlock(chain); - } + hammer2_chain_unlock(chain); hammer2_chain_drop(chain); /* @@ -2917,7 +3039,7 @@ hammer2_chain_scan(hammer2_chain_t *parent, hammer2_chain_t **chainp, if (flags & HAMMER2_LOOKUP_ALWAYS) { how_maybe = how_always; how = HAMMER2_RESOLVE_ALWAYS; - } else if (flags & (HAMMER2_LOOKUP_NODATA | HAMMER2_LOOKUP_NOLOCK)) { + } else if (flags & HAMMER2_LOOKUP_NODATA) { how = HAMMER2_RESOLVE_NEVER; } else { how = HAMMER2_RESOLVE_MAYBE; @@ -4003,9 +4125,11 @@ hammer2_chain_create_indirect(hammer2_chain_t *parent, (chain->flags & HAMMER2_CHAIN_DELETED)) { hammer2_chain_unlock(chain); hammer2_chain_drop(chain); - kprintf("hammer2_chain_create_indirect " + if (hammer2_debug & 0x0040) { + kprintf("LOST PARENT RETRY " "RETRY (%p,%p)->%p %08x\n", parent, chain->parent, chain, chain->flags); + } hammer2_spin_ex(&parent->core.spin); continue; } @@ -4023,6 +4147,12 @@ hammer2_chain_create_indirect(hammer2_chain_t *parent, * * WARNING! Parent must already be marked modified, so we * can assume that chain_delete always suceeds. + * + * WARNING! hammer2_chain_repchange() does not have to be + * called (and doesn't work anyway because we are + * only doing a partial shift). A recursion that is + * in-progress can continue at the current parent + * and will be able to properly find its next key. */ error = hammer2_chain_delete(parent, chain, mtid, 0); KKASSERT(error == 0); @@ -4133,6 +4263,7 @@ hammer2_chain_indirect_maintenance(hammer2_chain_t *parent, hammer2_chain_delete(parent, chain, chain->bref.modify_tid, HAMMER2_DELETE_PERMANENT); + hammer2_chain_repchange(parent, chain); return 1; } @@ -4260,6 +4391,8 @@ hammer2_chain_indirect_maintenance(hammer2_chain_t *parent, } hammer2_spin_unex(&chain->core.spin); + hammer2_chain_repchange(parent, chain); + return 1; } diff --git a/sys/vfs/hammer2/hammer2_cluster.c b/sys/vfs/hammer2/hammer2_cluster.c index e72470507a..82d005a35e 100644 --- a/sys/vfs/hammer2/hammer2_cluster.c +++ b/sys/vfs/hammer2/hammer2_cluster.c @@ -1,5 +1,5 @@ /* - * Copyright (c) 2013-2015 The DragonFly Project. All rights reserved. + * Copyright (c) 2013-2018 The DragonFly Project. All rights reserved. * * This code is derived from software contributed to The DragonFly Project * by Matthew Dillon diff --git a/sys/vfs/hammer2/hammer2_disk.h b/sys/vfs/hammer2/hammer2_disk.h index 4f90081ce2..283ce747f5 100644 --- a/sys/vfs/hammer2/hammer2_disk.h +++ b/sys/vfs/hammer2/hammer2_disk.h @@ -1,5 +1,5 @@ /* - * Copyright (c) 2011-2014 The DragonFly Project. All rights reserved. + * Copyright (c) 2011-2018 The DragonFly Project. All rights reserved. * * This code is derived from software contributed to The DragonFly Project * by Matthew Dillon diff --git a/sys/vfs/hammer2/hammer2_flush.c b/sys/vfs/hammer2/hammer2_flush.c index 0b458e897d..01e4a7a245 100644 --- a/sys/vfs/hammer2/hammer2_flush.c +++ b/sys/vfs/hammer2/hammer2_flush.c @@ -1,5 +1,5 @@ /* - * Copyright (c) 2011-2015 The DragonFly Project. All rights reserved. + * Copyright (c) 2011-2018 The DragonFly Project. All rights reserved. * * This code is derived from software contributed to The DragonFly Project * by Matthew Dillon @@ -176,12 +176,24 @@ hammer2_trans_init(hammer2_pfs_t *pmp, uint32_t flags) #endif } else { /* - * Requesting normal modifying transaction (read-only - * operations do not use transactions). Waits for - * any flush to finish before allowing. Multiple - * modifying transactions can run concurrently. + * Requesting a normal modifying transaction. + * Waits for any flush to finish before allowing. + * Multiple modifying transactions can run + * concurrently. + * + * If a flush is pending for more than one second + * but can't run because many modifying transactions + * are active, we wait for the flush to be granted. + * + * NOTE: Remember that non-modifying operations + * such as read, stat, readdir, etc, do + * not use transactions. */ - if (oflags & HAMMER2_TRANS_ISFLUSH) { + if ((oflags & HAMMER2_TRANS_FPENDING) && + (u_int)(ticks - pmp->trans.fticks) >= (u_int)hz) { + nflags = oflags | HAMMER2_TRANS_WAITING; + dowait = 1; + } else if (oflags & HAMMER2_TRANS_ISFLUSH) { nflags = oflags | HAMMER2_TRANS_WAITING; dowait = 1; } else { @@ -191,6 +203,10 @@ hammer2_trans_init(hammer2_pfs_t *pmp, uint32_t flags) if (dowait) tsleep_interlock(&pmp->trans.sync_wait, 0); if (atomic_cmpset_int(&pmp->trans.flags, oflags, nflags)) { + if ((oflags & HAMMER2_TRANS_FPENDING) == 0 && + (nflags & HAMMER2_TRANS_FPENDING)) { + pmp->trans.fticks = ticks; + } if (dowait == 0) break; tsleep(&pmp->trans.sync_wait, PINTERLOCKED, @@ -449,8 +465,11 @@ hammer2_flush(hammer2_chain_t *chain, int flags) */ info.diddeferral = 0; if (info.parent != chain->parent) { - kprintf("LOST CHILD4 %p->%p (actual parent %p)\n", - info.parent, chain, chain->parent); + if (hammer2_debug & 0x0040) { + kprintf("LOST CHILD4 %p->%p " + "(actual parent %p)\n", + info.parent, chain, chain->parent); + } hammer2_chain_drop(info.parent); info.parent = chain->parent; hammer2_chain_ref(info.parent); @@ -746,8 +765,10 @@ hammer2_flush_core(hammer2_flush_info_t *info, hammer2_chain_t *chain, } if (chain->parent != parent) { - kprintf("LOST CHILD3 %p->%p (actual parent %p)\n", - parent, chain, chain->parent); + if (hammer2_debug & 0x0040) { + kprintf("LOST CHILD3 %p->%p (actual parent %p)\n", + parent, chain, chain->parent); + } KKASSERT(parent != NULL); hammer2_chain_unlock(parent); if ((chain->flags & HAMMER2_CHAIN_DELAYED) == 0) { diff --git a/sys/vfs/hammer2/hammer2_freemap.c b/sys/vfs/hammer2/hammer2_freemap.c index aa4081d7ea..356f10d446 100644 --- a/sys/vfs/hammer2/hammer2_freemap.c +++ b/sys/vfs/hammer2/hammer2_freemap.c @@ -1,5 +1,5 @@ /* - * Copyright (c) 2011-2014 The DragonFly Project. All rights reserved. + * Copyright (c) 2011-2018 The DragonFly Project. All rights reserved. * * This code is derived from software contributed to The DragonFly Project * by Matthew Dillon diff --git a/sys/vfs/hammer2/hammer2_inode.c b/sys/vfs/hammer2/hammer2_inode.c index cf544ae23f..927b004041 100644 --- a/sys/vfs/hammer2/hammer2_inode.c +++ b/sys/vfs/hammer2/hammer2_inode.c @@ -1,5 +1,5 @@ /* - * Copyright (c) 2011-2014 The DragonFly Project. All rights reserved. + * Copyright (c) 2011-2018 The DragonFly Project. All rights reserved. * * This code is derived from software contributed to The DragonFly Project * by Matthew Dillon diff --git a/sys/vfs/hammer2/hammer2_io.c b/sys/vfs/hammer2/hammer2_io.c index 4e2bb6d0be..138fb411f4 100644 --- a/sys/vfs/hammer2/hammer2_io.c +++ b/sys/vfs/hammer2/hammer2_io.c @@ -1,5 +1,5 @@ /* - * Copyright (c) 2013-2017 The DragonFly Project. All rights reserved. + * Copyright (c) 2013-2018 The DragonFly Project. All rights reserved. * * This code is derived from software contributed to The DragonFly Project * by Matthew Dillon diff --git a/sys/vfs/hammer2/hammer2_iocom.c b/sys/vfs/hammer2/hammer2_iocom.c index 6b1a6251b9..c3d9f76ad9 100644 --- a/sys/vfs/hammer2/hammer2_iocom.c +++ b/sys/vfs/hammer2/hammer2_iocom.c @@ -1,5 +1,5 @@ /* - * Copyright (c) 2011-2015 The DragonFly Project. All rights reserved. + * Copyright (c) 2011-2018 The DragonFly Project. All rights reserved. * * This code is derived from software contributed to The DragonFly Project * by Matthew Dillon diff --git a/sys/vfs/hammer2/hammer2_ioctl.c b/sys/vfs/hammer2/hammer2_ioctl.c index 00b4ae11a2..638baeb484 100644 --- a/sys/vfs/hammer2/hammer2_ioctl.c +++ b/sys/vfs/hammer2/hammer2_ioctl.c @@ -1,5 +1,5 @@ /* - * Copyright (c) 2011-2015 The DragonFly Project. All rights reserved. + * Copyright (c) 2011-2018 The DragonFly Project. All rights reserved. * * This code is derived from software contributed to The DragonFly Project * by Matthew Dillon @@ -880,6 +880,8 @@ hammer2_ioctl_pfs_snapshot(hammer2_inode_t *ip, void *data) nip->meta.pfs_type = HAMMER2_PFSTYPE_MASTER; nip->meta.pfs_subtype = HAMMER2_PFSSUBTYPE_SNAPSHOT; nip->meta.op_flags |= HAMMER2_OPFLAG_PFSROOT; + nchain->bref.embed.stats = chain->bref.embed.stats; + kern_uuidgen(&nip->meta.pfs_fsid, 1); #if 0 diff --git a/sys/vfs/hammer2/hammer2_ioctl.h b/sys/vfs/hammer2/hammer2_ioctl.h index 36a51b1058..8d9e3921c2 100644 --- a/sys/vfs/hammer2/hammer2_ioctl.h +++ b/sys/vfs/hammer2/hammer2_ioctl.h @@ -1,5 +1,5 @@ /* - * Copyright (c) 2011-2014 The DragonFly Project. All rights reserved. + * Copyright (c) 2011-2018 The DragonFly Project. All rights reserved. * * This code is derived from software contributed to The DragonFly Project * by Matthew Dillon diff --git a/sys/vfs/hammer2/hammer2_mount.h b/sys/vfs/hammer2/hammer2_mount.h index ae7492c080..3c6b408fdd 100644 --- a/sys/vfs/hammer2/hammer2_mount.h +++ b/sys/vfs/hammer2/hammer2_mount.h @@ -1,5 +1,5 @@ /* - * Copyright (c) 2011-2014 The DragonFly Project. All rights reserved. + * Copyright (c) 2011-2018 The DragonFly Project. All rights reserved. * * This code is derived from software contributed to The DragonFly Project * by Matthew Dillon diff --git a/sys/vfs/hammer2/hammer2_msgops.c b/sys/vfs/hammer2/hammer2_msgops.c index e7e7042695..408ced9eda 100644 --- a/sys/vfs/hammer2/hammer2_msgops.c +++ b/sys/vfs/hammer2/hammer2_msgops.c @@ -1,5 +1,5 @@ /* - * Copyright (c) 2012-2014 The DragonFly Project. All rights reserved. + * Copyright (c) 2012-2018 The DragonFly Project. All rights reserved. * * This code is derived from software contributed to The DragonFly Project * by Matthew Dillon diff --git a/sys/vfs/hammer2/hammer2_strategy.c b/sys/vfs/hammer2/hammer2_strategy.c index a64a5cbf08..dafd2a0b30 100644 --- a/sys/vfs/hammer2/hammer2_strategy.c +++ b/sys/vfs/hammer2/hammer2_strategy.c @@ -596,6 +596,7 @@ hammer2_strategy_xop_write(hammer2_thread_t *thr, hammer2_xop_t *arg) lblksize = hammer2_calc_logical(ip, bio->bio_offset, &lbase, NULL); pblksize = hammer2_calc_physical(ip, lbase); bkvasync(bp); + KKASSERT(lblksize <= MAXPHYS); bcopy(bp->b_data, bio_data, lblksize); hammer2_mtx_unlock(&xop->lock); diff --git a/sys/vfs/hammer2/hammer2_subr.c b/sys/vfs/hammer2/hammer2_subr.c index d9796e0fa2..fa554a9e5a 100644 --- a/sys/vfs/hammer2/hammer2_subr.c +++ b/sys/vfs/hammer2/hammer2_subr.c @@ -1,5 +1,5 @@ /* - * Copyright (c) 2011-2014 The DragonFly Project. All rights reserved. + * Copyright (c) 2011-2018 The DragonFly Project. All rights reserved. * * This code is derived from software contributed to The DragonFly Project * by Matthew Dillon diff --git a/sys/vfs/hammer2/hammer2_synchro.c b/sys/vfs/hammer2/hammer2_synchro.c index f04f26f9e9..81b0b0f42f 100644 --- a/sys/vfs/hammer2/hammer2_synchro.c +++ b/sys/vfs/hammer2/hammer2_synchro.c @@ -1,5 +1,5 @@ /* - * Copyright (c) 2015-2017 The DragonFly Project. All rights reserved. + * Copyright (c) 2015-2018 The DragonFly Project. All rights reserved. * * This code is derived from software contributed to The DragonFly Project * by Matthew Dillon diff --git a/sys/vfs/hammer2/hammer2_vnops.c b/sys/vfs/hammer2/hammer2_vnops.c index d57dc953a7..bb7c302af5 100644 --- a/sys/vfs/hammer2/hammer2_vnops.c +++ b/sys/vfs/hammer2/hammer2_vnops.c @@ -1,5 +1,5 @@ /* - * Copyright (c) 2011-2015 The DragonFly Project. All rights reserved. + * Copyright (c) 2011-2018 The DragonFly Project. All rights reserved. * * This code is derived from software contributed to The DragonFly Project * by Matthew Dillon @@ -174,7 +174,8 @@ hammer2_vop_reclaim(struct vop_reclaim_args *ap) */ if ((ip->flags & (HAMMER2_INODE_ISUNLINKED | HAMMER2_INODE_MODIFIED | - HAMMER2_INODE_RESIZED)) && + HAMMER2_INODE_RESIZED | + HAMMER2_INODE_DIRTYDATA)) && (ip->flags & HAMMER2_INODE_ISDELETED) == 0) { hammer2_inode_sideq_t *ipul; diff --git a/sys/vfs/hammer2/hammer2_xops.c b/sys/vfs/hammer2/hammer2_xops.c index 13a127c4d8..9b0db75a56 100644 --- a/sys/vfs/hammer2/hammer2_xops.c +++ b/sys/vfs/hammer2/hammer2_xops.c @@ -1,5 +1,5 @@ /* - * Copyright (c) 2011-2015 The DragonFly Project. All rights reserved. + * Copyright (c) 2011-2018 The DragonFly Project. All rights reserved. * * This code is derived from software contributed to The DragonFly Project * by Matthew Dillon @@ -62,6 +62,8 @@ /* * Determine if the specified directory is empty. Returns 0 on success. + * + * Caller assumes that we do not cycle the lock on oparent and ochain. */ static int @@ -74,30 +76,34 @@ checkdirempty(hammer2_chain_t *oparent, hammer2_chain_t *ochain, int clindex) int error; error = 0; - chain = hammer2_chain_lookup_init(ochain, 0); - if (chain->bref.type == HAMMER2_BREF_TYPE_DIRENT) { - if (oparent) - hammer2_chain_unlock(oparent); - inum = chain->bref.embed.dirent.inum; + if (ochain->bref.type == HAMMER2_BREF_TYPE_DIRENT) { + /* + * Forward the directory entry to the directory inode. + * This is not strictly heirarchical so the locks kinda + * violate top-down lock order. + * + * Define an allowance for directory-entry -> dinode + * chain lock ordering, since directory entries have only + * one link the directory inode is kinda topologically + * underneath the directory entry, even though it isn't + * really. XXX + */ + inum = ochain->bref.embed.dirent.inum; parent = NULL; - error = hammer2_chain_inode_find(chain->pmp, inum, + chain = NULL; + error = hammer2_chain_inode_find(ochain->pmp, inum, clindex, 0, &parent, &chain); if (parent) { hammer2_chain_unlock(parent); hammer2_chain_drop(parent); } - if (oparent) { - hammer2_chain_lock(oparent, HAMMER2_RESOLVE_ALWAYS); - if (ochain->parent != oparent) { - if (chain) { - hammer2_chain_unlock(chain); - hammer2_chain_drop(chain); - } - return HAMMER2_ERROR_EAGAIN; - } - } + } else { + /* + * The directory entry *is* the directory inode + */ + chain = hammer2_chain_lookup_init(ochain, 0); } /* diff --git a/sys/vfs/hammer2/hammer2_xxhash.h b/sys/vfs/hammer2/hammer2_xxhash.h index 5304bea671..9250397e32 100644 --- a/sys/vfs/hammer2/hammer2_xxhash.h +++ b/sys/vfs/hammer2/hammer2_xxhash.h @@ -1,5 +1,5 @@ /* - * Copyright (c) 2016 The DragonFly Project. All rights reserved. + * Copyright (c) 2016-2018 The DragonFly Project. All rights reserved. * * This code is derived from software contributed to The DragonFly Project * by Matthew Dillon -- 2.11.4.GIT