From 10252dc74a966f1dbb2cb131b287e03d95807a15 Mon Sep 17 00:00:00 2001 From: Matthew Dillon Date: Thu, 31 Jan 2013 16:03:59 -0800 Subject: [PATCH] hammer2 - serialized flush work part 3 * Clean up hammer2_inode vs hammer2_chain. Remove chain->u.ip (the backpointer from the chain to the inode), which simplifies a lot of things. This was only used to detect when a vnode might have to be recycled but we solve that a different way by adjusting the kernel's cache_unlink() code. * Revamp hammer2_inode_get(). This routine now always kmalloc()'s a new hammer2_inode. In HAMMER2, the hammer2_inode structure is actually an indirect pointer to an underlying chain so it does not have to be unique. There can multiple ip's pointing to the same chain. * Revamp hammer2_inode_put(). The ref on ip->chain is dropped via ip->chain, and the (passed_chain) argument is treated separately and does not necessarily have to match ip->chain. This makes hammer2_inode_put()'s API comparable to hammer2_inode_unlock_ex(), reducing confusion. * Depend on adjustments to cache_unlink() and cache_rename() to detect vnodes which should be recycled. This code (in the proper kernel, not here) now triggers a deactivation if the vnode related to the namecache entry has no refs, allowing HAMMER2 to detect any deletions and vrecycle() the vnode immediately. * Numerous ref/lock fixes. --- sys/vfs/hammer2/hammer2.h | 16 +-- sys/vfs/hammer2/hammer2_chain.c | 54 +-------- sys/vfs/hammer2/hammer2_inode.c | 191 ++++++++++++++++--------------- sys/vfs/hammer2/hammer2_ioctl.c | 6 +- sys/vfs/hammer2/hammer2_subr.c | 8 ++ sys/vfs/hammer2/hammer2_vfsops.c | 14 ++- sys/vfs/hammer2/hammer2_vnops.c | 53 ++++++--- 7 files changed, 165 insertions(+), 177 deletions(-) diff --git a/sys/vfs/hammer2/hammer2.h b/sys/vfs/hammer2/hammer2.h index 4a8a124b81..99de8eae27 100644 --- a/sys/vfs/hammer2/hammer2.h +++ b/sys/vfs/hammer2/hammer2.h @@ -113,10 +113,6 @@ struct hammer2_chain { struct hammer2_state *state; /* if active cache msg */ RB_ENTRY(hammer2_chain) rbnode; TAILQ_ENTRY(hammer2_chain) flush_node; /* flush deferral list */ - union { - void *mem; /* generic */ - struct hammer2_inode *ip; /* high-level h2 structure */ - } u; struct buf *bp; /* buffer cache (ro) */ hammer2_media_data_t *data; /* modified copy of data (rw) */ @@ -243,7 +239,6 @@ struct hammer2_inode { struct vnode *vp; hammer2_chain_t *chain; struct lockf advlock; - struct lock lk; u_int flags; u_int refs; /* +vpref, +flushref */ }; @@ -252,7 +247,6 @@ typedef struct hammer2_inode hammer2_inode_t; #define HAMMER2_INODE_MODIFIED 0x0001 #define HAMMER2_INODE_DIRTYEMBED 0x0002 -#define HAMMER2_INODE_DELETED 0x0004 /* * XXX @@ -282,6 +276,7 @@ struct hammer2_mount { int maxipstacks; hammer2_chain_t vchain; /* anchor chain */ hammer2_chain_t *schain; /* super-root */ + hammer2_inode_t *sroot; /* super-root inode */ struct lock alloclk; /* lockmgr lock */ struct lock voldatalk; /* lockmgr lock */ @@ -399,9 +394,10 @@ struct vnode *hammer2_igetv(hammer2_inode_t *ip, int *errorp); void hammer2_inode_lock_nlinks(hammer2_inode_t *ip); void hammer2_inode_unlock_nlinks(hammer2_inode_t *ip); -hammer2_inode_t *hammer2_inode_get(hammer2_pfsmount_t *pmp, - hammer2_inode_t *dip, hammer2_chain_t *chain); -void hammer2_inode_put(hammer2_inode_t *ip, hammer2_chain_t *chain); +hammer2_inode_t *hammer2_inode_get(hammer2_mount_t *hmp, + hammer2_pfsmount_t *pmp, hammer2_inode_t *dip, + hammer2_chain_t *chain); +void hammer2_inode_put(hammer2_inode_t *ip, hammer2_chain_t *passed_chain); void hammer2_inode_free(hammer2_inode_t *ip); void hammer2_inode_ref(hammer2_inode_t *ip); void hammer2_inode_drop(hammer2_inode_t *ip); @@ -422,7 +418,7 @@ hammer2_inode_t *hammer2_inode_common_parent(hammer2_mount_t *hmp, int hammer2_unlink_file(hammer2_inode_t *dip, const uint8_t *name, size_t name_len, - int isdir, hammer2_inode_t *retain_ip); + int isdir, hammer2_chain_t *retain_chain); int hammer2_hardlink_consolidate(hammer2_inode_t **ipp, hammer2_inode_t *tdip); int hammer2_hardlink_deconsolidate(hammer2_inode_t *dip, hammer2_chain_t **chainp, hammer2_chain_t **ochainp); diff --git a/sys/vfs/hammer2/hammer2_chain.c b/sys/vfs/hammer2/hammer2_chain.c index 5b1d700ef6..eedb990a04 100644 --- a/sys/vfs/hammer2/hammer2_chain.c +++ b/sys/vfs/hammer2/hammer2_chain.c @@ -185,7 +185,6 @@ hammer2_chain_dealloc(hammer2_mount_t *hmp, hammer2_chain_t *chain) KKASSERT(chain->refs == 0); KKASSERT(chain->flushing == 0); - KKASSERT(chain->u.mem == NULL); KKASSERT((chain->flags & (HAMMER2_CHAIN_MOVED | HAMMER2_CHAIN_MODIFIED)) == 0); @@ -239,13 +238,10 @@ hammer2_chain_free(hammer2_mount_t *hmp, hammer2_chain_t *chain) } KKASSERT(chain->bp == NULL); - KKASSERT(chain->bref.type != HAMMER2_BREF_TYPE_INODE || - chain->u.ip == NULL); ccms_thread_unlock(&chain->cst); KKASSERT(chain->cst.count == 0); KKASSERT(chain->cst.upgrade == 0); - KKASSERT(chain->u.mem == NULL); kfree(chain, hmp->mchain); } @@ -827,6 +823,7 @@ hammer2_chain_resize(hammer2_inode_t *ip, hammer2_chain_t *chain, * worry about snapshots. */ if ((chain->flags & HAMMER2_CHAIN_MODIFIED) == 0) { + atomic_set_int(&ip->flags, HAMMER2_INODE_MODIFIED); atomic_set_int(&chain->flags, HAMMER2_CHAIN_MODIFIED | HAMMER2_CHAIN_MODIFY_TID); hammer2_chain_ref(hmp, chain); @@ -1236,26 +1233,6 @@ hammer2_chain_get(hammer2_mount_t *hmp, hammer2_chain_t *parent, atomic_add_int(&parent->refs, 1); /* for red-black entry */ ccms_thread_lock_restore(&parent->cst, ostate); -#if 0 - /* - * Additional linkage for inodes. Reuse the parent pointer to - * find the parent directory. - * - * The ccms_inode is initialized from its parent directory. The - * chain of ccms_inode's is seeded by the mount code. - */ - if (bref->type == HAMMER2_BREF_TYPE_INODE) { - ip = chain->u.ip; - while (parent->bref.type == HAMMER2_BREF_TYPE_INDIRECT) - parent = parent->parent; - if (parent->bref.type == HAMMER2_BREF_TYPE_INODE) { - ip->pip = parent->u.ip; - ip->pmp = parent->u.ip->pmp; - ccms_cst_init(&ip->topo_cst, &ip->chain); - } - } -#endif - /* * Our new chain structure has already been referenced and locked * but the lock code handles the I/O so call it to resolve the data. @@ -1861,33 +1838,6 @@ again: KKASSERT(parent->refs > 0); atomic_add_int(&parent->refs, 1); -#if 0 - /* - * Additional linkage for inodes. Reuse the parent pointer to - * find the parent directory. - * - * Cumulative adjustments are inherited on [re]attach and will - * propagate up the tree on the next flush. - * - * The ccms_inode is initialized from its parent directory. The - * chain of ccms_inode's is seeded by the mount code. - */ - if (chain->bref.type == HAMMER2_BREF_TYPE_INODE) { - hammer2_chain_t *scan = parent; - hammer2_inode_t *ip = chain->u.ip; - - while (scan->bref.type == HAMMER2_BREF_TYPE_INDIRECT) - scan = scan->parent; - if (scan->bref.type == HAMMER2_BREF_TYPE_INODE) { - ip->pip = scan->u.ip; - ip->pmp = scan->u.ip->pmp; - ip->pip->delta_icount += ip->ip_data.inode_count; - ip->pip->delta_dcount += ip->ip_data.data_count; - ++ip->pip->delta_icount; - ccms_cst_init(&ip->topo_cst, &ip->chain); - } - } -#endif /* * (allocated) indicates that this is a newly-created chain element * rather than a renamed chain element. In this situation we want @@ -2486,7 +2436,7 @@ hammer2_chain_delete(hammer2_mount_t *hmp, hammer2_chain_t *parent, */ if ((chain->flags & HAMMER2_CHAIN_DELETED) == 0 && chain->bref.type == HAMMER2_BREF_TYPE_INODE) { - KKASSERT(chain->u.ip == NULL); + /* XXX */ } /* diff --git a/sys/vfs/hammer2/hammer2_inode.c b/sys/vfs/hammer2/hammer2_inode.c index 195d8fb296..07186dfa8f 100644 --- a/sys/vfs/hammer2/hammer2_inode.c +++ b/sys/vfs/hammer2/hammer2_inode.c @@ -58,8 +58,10 @@ hammer2_inode_ref(hammer2_inode_t *ip) void hammer2_inode_drop(hammer2_inode_t *ip) { - u_int refs; hammer2_mount_t *hmp; + hammer2_inode_t *pip; + hammer2_chain_t *chain; + u_int refs; for (;;) { refs = ip->refs; @@ -68,11 +70,26 @@ hammer2_inode_drop(hammer2_inode_t *ip) if (atomic_cmpset_int(&ip->refs, 1, 0)) { kprintf("hammer2_inode_drop: 1->0 %p\n", ip); KKASSERT(ip->topo_cst.count == 0); - KKASSERT(ip->chain == NULL); + hmp = ip->hmp; ip->hmp = NULL; + pip = ip->pip; + ip->pip = NULL; + chain = ip->chain; + ip->chain = NULL; + if (chain) + hammer2_chain_drop(hmp, chain); + + /* + * We have to drop pip (if non-NULL) to + * dispose of our implied reference from + * ip->pip. We can simply loop on it. + */ kfree(ip, hmp->minode); - break; + if (pip == NULL) + break; + ip = pip; + /* continue */ } } else { if (atomic_cmpset_int(&ip->refs, refs, refs - 1)) @@ -212,43 +229,50 @@ hammer2_igetv(hammer2_inode_t *ip, int *errorp) } /* - * Return an exclusively locked inode associated with the specified - * chain. The chain must be a BREF_TYPE_INODE, and (dip) must properly - * specify the inode's position in the topology. - * * The passed-in chain must be locked and the returned inode will also be - * locked. + * locked. A ref is added to both the chain and the inode. + * + * The hammer2_inode structure regulates the interface between the high level + * kernel VNOPS API and the filesystem backend (the chains). + * + * NOTE! This routine allocates the hammer2_inode structure + * unconditionally, and thus there might be several which + * are associated with the same chain. Particularly for hardlinks + * but this can also happen temporarily for normal files and + * directories. * * WARNING! This routine sucks up the chain's lock (makes it part of the - * inode lock), so callers need to be careful. + * inode lock from the point of view of the inode lock API), + * so callers need to be careful. * - * WARNING! The mount code is allowed to pass dip == NULL for iroot. + * WARNING! The mount code is allowed to pass dip == NULL for iroot and + * is allowed to pass pmp == NULL and dip == NULL for sroot. */ hammer2_inode_t * -hammer2_inode_get(hammer2_pfsmount_t *pmp, hammer2_inode_t *dip, - hammer2_chain_t *chain) +hammer2_inode_get(hammer2_mount_t *hmp, hammer2_pfsmount_t *pmp, + hammer2_inode_t *dip, hammer2_chain_t *chain) { - hammer2_mount_t *hmp = pmp->hmp; hammer2_inode_t *nip; KKASSERT(chain->bref.type == HAMMER2_BREF_TYPE_INODE); - if (chain->u.ip) { - nip = chain->u.ip; - KKASSERT(nip->pip == dip); - KKASSERT(nip->pmp == pmp); - } else { - nip = kmalloc(sizeof(*nip), hmp->minode, M_WAITOK | M_ZERO); - nip->chain = chain; - nip->pip = dip; /* can be NULL */ - if (dip) - hammer2_inode_ref(dip); - nip->pmp = pmp; - nip->hmp = hmp; - nip->refs = 1; - ccms_cst_init(&nip->topo_cst, &nip->chain); - hammer2_chain_ref(hmp, chain); - chain->u.ip = nip; - } + + nip = kmalloc(sizeof(*nip), hmp->minode, M_WAITOK | M_ZERO); + + nip->chain = chain; + hammer2_chain_ref(hmp, chain); /* nip->chain */ + nip->pip = dip; /* can be NULL */ + if (dip) + hammer2_inode_ref(dip); /* ref dip for nip->pip */ + + nip->pmp = pmp; + nip->hmp = hmp; + + /* + * ref and lock on nip gives it state compatible to after a + * hammer2_inode_lock_ex() call. + */ + nip->refs = 1; + ccms_cst_init(&nip->topo_cst, &nip->chain); ccms_thread_lock(&nip->topo_cst, CCMS_STATE_EXCLUSIVE); /* combination of thread lock and chain lock == inode lock */ @@ -261,34 +285,41 @@ hammer2_inode_get(hammer2_pfsmount_t *pmp, hammer2_inode_t *dip, * * The inode will be unlocked by this function. Note however that any related * chain returned by the hammer2_inode_lock_*() call will NOT be unlocked - * by this function. + * by this function. The related chain is dropped to undo the ref that + * hammer2_inode_get() put on it. + * + * passed_chain is unlocked normally and does not have to be directly + * associated with (ip). This is simply so the API works the same as + * the hammer2_inode_unlock_ex() API. NULL is ok. */ void -hammer2_inode_put(hammer2_inode_t *ip, hammer2_chain_t *chain) +hammer2_inode_put(hammer2_inode_t *ip, hammer2_chain_t *passed_chain) { hammer2_mount_t *hmp = ip->hmp; hammer2_inode_t *pip; + hammer2_chain_t *chain; - KKASSERT(chain); - KKASSERT(chain->u.ip == ip); + /* + * Disconnect chain + */ + if ((chain = ip->chain) != NULL) { + ip->chain = NULL; + hammer2_chain_drop(hmp, chain); /* from *_get() */ + } KKASSERT(ip->topo_cst.count == -1); /* one excl lock allowed */ - chain->u.ip = NULL; - ip->chain = NULL; - hammer2_chain_drop(hmp, chain); /* ref */ /* - * Disconnect ip from pip & related parent ref. - * - * We have to unlock the chain manually because - * the ip->chain pointer has already been NULL'd out. + * Disconnect pip */ if ((pip = ip->pip) != NULL) { ip->pip = NULL; - hammer2_inode_unlock_ex(ip, chain); hammer2_inode_drop(pip); - } else { - hammer2_inode_unlock_ex(ip, chain); } + + /* + * clean up the ip, we use an inode_unlock_ex-compatible API. + */ + hammer2_inode_unlock_ex(ip, passed_chain); } /* @@ -378,7 +409,7 @@ retry: * * NOTE: *_get() integrates chain's lock into the inode lock. */ - nip = hammer2_inode_get(dip->pmp, dip, chain); + nip = hammer2_inode_get(dip->hmp, dip->pmp, dip, chain); kprintf("nip %p chain %p\n", nip, nip->chain); *nipp = nip; *nchainp = chain; @@ -559,7 +590,7 @@ retry: * * NOTE: *_get() integrates chain's lock into the inode lock. */ - nip = hammer2_inode_get(dip->pmp, dip, chain); + nip = hammer2_inode_get(dip->hmp, dip->pmp, dip, chain); hammer2_chain_modify(hmp, chain, 0); nipdata = &chain->data->ipdata; *nipdata = parent->data->ipdata; @@ -728,8 +759,7 @@ retry: * * NOTE: *_get() integrates chain's lock into the inode lock. */ - KKASSERT(chain->u.ip == NULL); - nip = hammer2_inode_get(dip->pmp, dip, chain); + nip = hammer2_inode_get(dip->hmp, dip->pmp, dip, chain); hammer2_chain_modify(hmp, chain, 0); KKASSERT(name_len < HAMMER2_INODE_MAXNAME); nipdata = &nip->chain->data->ipdata; @@ -748,8 +778,7 @@ retry: * * NOTE: *_get() integrates chain's lock into the inode lock. */ - KKASSERT(chain->u.ip == NULL); - nip = hammer2_inode_get(dip->pmp, dip, chain); + nip = hammer2_inode_get(dip->hmp, dip->pmp, dip, chain); hammer2_chain_modify(hmp, chain, 0); KKASSERT(name_len < HAMMER2_INODE_MAXNAME); nipdata = &nip->chain->data->ipdata; @@ -768,7 +797,6 @@ retry: * We are using oip as chain, already locked by caller, * do not unlock it. */ - KKASSERT(chain->u.ip != NULL); hammer2_chain_modify(hmp, chain, 0); nipdata = &ochain->data->ipdata; @@ -787,18 +815,22 @@ retry: /* * Unlink the file from the specified directory inode. The directory inode - * does not need to be locked. + * does not need to be locked. The caller should pass a non-NULL (ip) + * representing the object being removed only if the related vnode is + * potentially inactive (not referenced in the caller's active path), + * so we can vref/vrele it to trigger the VOP_INACTIVE path and properly + * recycle it. * * isdir determines whether a directory/non-directory check should be made. * No check is made if isdir is set to -1. * - * If retain_ip is non-NULL this function can fail with an EAGAIN if it + * If retain_chain is non-NULL this function can fail with an EAGAIN if it * catches the object in the middle of a flush. */ int hammer2_unlink_file(hammer2_inode_t *dip, const uint8_t *name, size_t name_len, - int isdir, hammer2_inode_t *retain_ip) + int isdir, hammer2_chain_t *retain_chain) { hammer2_inode_data_t *ipdata; hammer2_mount_t *hmp; @@ -807,17 +839,13 @@ hammer2_unlink_file(hammer2_inode_t *dip, hammer2_chain_t *chain; hammer2_chain_t *dparent; hammer2_chain_t *dchain; - hammer2_chain_t *tmpchain; hammer2_key_t lhc; - hammer2_inode_t *ip; - hammer2_inode_t *oip; int error; int parent_ref; uint8_t type; parent_ref = 0; error = 0; - ip = NULL; ochain = NULL; hmp = dip->hmp; lhc = hammer2_dirhash(name, name_len); @@ -935,23 +963,13 @@ hammer2_unlink_file(hammer2_inode_t *dip, hammer2_chain_drop(hmp, parent); } - if (ochain == retain_ip->chain && ochain->flushing) { + if (ochain == retain_chain && ochain->flushing) { hammer2_chain_unlock(hmp, ochain); error = EAGAIN; goto done; } - if ((oip = ochain->u.ip) != NULL) { - tmpchain = hammer2_inode_lock_ex(oip); - oip->flags |= HAMMER2_INODE_DELETED; - if (oip->vp || oip->refs > 1) - hammer2_inode_unlock_ex(oip, tmpchain); - else - hammer2_inode_put(oip, tmpchain); - KKASSERT(tmpchain == ochain); - /* ochain still actively locked */ - } hammer2_chain_delete(hmp, parent, ochain, - (ochain == retain_ip->chain)); + (ochain == retain_chain)); hammer2_chain_unlock(hmp, parent); hammer2_chain_drop(hmp, parent); parent = NULL; @@ -971,15 +989,6 @@ hammer2_unlink_file(hammer2_inode_t *dip, hammer2_chain_drop(hmp, chain); hammer2_chain_modify(hmp, chain, 0); --chain->data->ipdata.nlinks; - if ((ip = chain->u.ip) != NULL) { - parent = hammer2_inode_lock_ex(ip); - ip->flags |= HAMMER2_INODE_DELETED; - if (ip->vp) - hammer2_inode_unlock_ex(ip, parent); - else - hammer2_inode_put(ip, parent); - parent = NULL; - } hammer2_chain_delete(hmp, dparent, chain, 0); hammer2_chain_unlock(hmp, dparent); } else { @@ -993,28 +1002,18 @@ hammer2_unlink_file(hammer2_inode_t *dip, * * NOTE: *_get() integrates chain's lock into the inode lock. */ - ip = hammer2_inode_get(dip->pmp, dip, chain); - ipdata = &ip->chain->data->ipdata; - if (ip == retain_ip && chain->flushing) { - hammer2_inode_unlock_ex(ip, chain); - chain = NULL; /* inode_unlock eats chain */ + ipdata = &chain->data->ipdata; + if (chain == retain_chain && chain->flushing) { error = EAGAIN; goto done; } hammer2_chain_modify(hmp, chain, 0); --ipdata->nlinks; - ip->flags |= HAMMER2_INODE_DELETED; hammer2_chain_delete(hmp, parent, chain, - (retain_ip == ip)); - if (ip->vp) - hammer2_inode_unlock_ex(ip, chain); - else - hammer2_inode_put(ip, chain); - chain = NULL; /* inode_unlock eats chain */ + (retain_chain == chain)); } error = 0; - done: if (chain) hammer2_chain_unlock(hmp, chain); @@ -1082,6 +1081,9 @@ hammer2_hardlink_consolidate(hammer2_inode_t **ipp, hammer2_inode_t *tdip) if (hammer2_hardlink_enable == 0) return (ENOTSUP); + /* + * cdip will be returned with a ref, but not locked. + */ fdip = oip->pip; cdip = hammer2_inode_common_parent(hmp, fdip, tdip); @@ -1180,7 +1182,6 @@ hammer2_hardlink_consolidate(hammer2_inode_t **ipp, hammer2_inode_t *tdip) hammer2_chain_unlock(hmp, parent); hammer2_chain_drop(hmp, parent); } - oip->flags |= HAMMER2_INODE_DELETED; hammer2_chain_delete(hmp, parent, ochain, 0); hammer2_inode_put(oip, ochain); /* unconditional */ hammer2_chain_unlock(hmp, parent); @@ -1255,7 +1256,7 @@ hammer2_hardlink_find(hammer2_inode_t *dip, hammer2_chain_t **chainp, while ((ip = pip) != NULL) { parent = hammer2_inode_lock_ex(ip); - hammer2_inode_drop(ip); + hammer2_inode_drop(ip); /* loop */ KKASSERT(parent->bref.type == HAMMER2_BREF_TYPE_INODE); chain = hammer2_chain_lookup(hmp, &parent, lhc, lhc, 0); hammer2_chain_unlock(hmp, parent); @@ -1263,7 +1264,7 @@ hammer2_hardlink_find(hammer2_inode_t *dip, hammer2_chain_t **chainp, break; pip = ip->pip; /* safe, ip held locked */ if (pip) - hammer2_inode_ref(pip); + hammer2_inode_ref(pip); /* loop */ hammer2_inode_unlock_ex(ip, NULL); } diff --git a/sys/vfs/hammer2/hammer2_ioctl.c b/sys/vfs/hammer2/hammer2_ioctl.c index f2d00884e8..61d8c44882 100644 --- a/sys/vfs/hammer2/hammer2_ioctl.c +++ b/sys/vfs/hammer2/hammer2_ioctl.c @@ -418,7 +418,6 @@ hammer2_ioctl_pfs_lookup(hammer2_inode_t *ip, void *data) HAMMER2_LOOKUP_SHARED); while (chain) { if (chain->bref.type == HAMMER2_BREF_TYPE_INODE && - chain->u.ip && len == chain->data->ipdata.name_len && bcmp(pfs->name, chain->data->ipdata.filename, len) == 0) { break; @@ -466,7 +465,8 @@ hammer2_ioctl_pfs_create(hammer2_inode_t *ip, void *data) nip = NULL; pfs->name[sizeof(pfs->name) - 1] = 0; /* ensure 0-termination */ - error = hammer2_inode_create(hmp->schain->u.ip, NULL, NULL, + + error = hammer2_inode_create(hmp->sroot, NULL, NULL, pfs->name, strlen(pfs->name), &nip, &nchain); if (error == 0) { @@ -490,7 +490,7 @@ hammer2_ioctl_pfs_delete(hammer2_inode_t *ip, void *data) hammer2_ioc_pfs_t *pfs = data; int error; - error = hammer2_unlink_file(hmp->schain->u.ip, + error = hammer2_unlink_file(hmp->sroot, pfs->name, strlen(pfs->name), 0, NULL); return (error); diff --git a/sys/vfs/hammer2/hammer2_subr.c b/sys/vfs/hammer2/hammer2_subr.c index 01dfe47e06..0e3476c68d 100644 --- a/sys/vfs/hammer2/hammer2_subr.c +++ b/sys/vfs/hammer2/hammer2_subr.c @@ -74,6 +74,14 @@ hammer2_inode_lock_ex(hammer2_inode_t *ip) void hammer2_inode_unlock_ex(hammer2_inode_t *ip, hammer2_chain_t *chain) { + /* + * XXX this will catch parent directories too which we don't + * really want. + */ + if (ip->chain && (ip->chain->flags & (HAMMER2_CHAIN_MODIFIED | + HAMMER2_CHAIN_SUBMODIFIED))) { + atomic_set_int(&ip->flags, HAMMER2_INODE_MODIFIED); + } if (chain) hammer2_chain_unlock(ip->hmp, chain); ccms_thread_unlock(&ip->topo_cst); diff --git a/sys/vfs/hammer2/hammer2_vfsops.c b/sys/vfs/hammer2/hammer2_vfsops.c index 201e9c3326..54a4f7c988 100644 --- a/sys/vfs/hammer2/hammer2_vfsops.c +++ b/sys/vfs/hammer2/hammer2_vfsops.c @@ -435,11 +435,17 @@ hammer2_vfs_mount(struct mount *mp, char *path, caddr_t data, } hammer2_chain_ref(hmp, schain); /* for hmp->schain */ hmp->schain = schain; /* left locked */ + hmp->sroot = hammer2_inode_get(hmp, NULL, NULL, schain); + hammer2_inode_ref(hmp->sroot); /* for hmp->sroot */ + hammer2_inode_unlock_ex(hmp->sroot, NULL); } else { schain = hmp->schain; hammer2_chain_lock(hmp, schain, HAMMER2_RESOLVE_ALWAYS); } + /* + * schain left locked at this point, use as basis for PFS search. + */ parent = schain; lhc = hammer2_dirhash(label, strlen(label)); rchain = hammer2_chain_lookup(hmp, &parent, @@ -473,7 +479,7 @@ hammer2_vfs_mount(struct mount *mp, char *path, caddr_t data, */ hammer2_chain_ref(hmp, rchain); /* for pmp->rchain */ pmp->rchain = rchain; /* left held & unlocked */ - pmp->iroot = hammer2_inode_get(pmp, NULL, rchain); + pmp->iroot = hammer2_inode_get(hmp, pmp, NULL, rchain); hammer2_inode_ref(pmp->iroot); /* ref for pmp->iroot */ hammer2_inode_unlock_ex(pmp->iroot, rchain); /* iroot & its chain */ @@ -590,7 +596,7 @@ hammer2_vfs_unmount(struct mount *mp, int mntflags) hammer2_inode_put(pmp->iroot, chain); /* lock destroyed by the put */ KKASSERT(pmp->iroot->refs == 1); - hammer2_inode_drop(pmp->iroot); + hammer2_inode_drop(pmp->iroot); /* ref for pmp->iroot */ pmp->iroot = NULL; } if (pmp->rchain) { @@ -610,6 +616,10 @@ hammer2_vfs_unmount(struct mount *mp, int mntflags) * If no PFS's left drop the master hammer2_mount for the device. */ if (hmp->pmp_count == 0) { + if (hmp->sroot) { + hammer2_inode_drop(hmp->sroot); + hmp->sroot = NULL; + } if (hmp->schain) { KKASSERT(hmp->schain->refs == 1); hammer2_chain_drop(hmp, hmp->schain); diff --git a/sys/vfs/hammer2/hammer2_vnops.c b/sys/vfs/hammer2/hammer2_vnops.c index 40596625b0..6ff831b28d 100644 --- a/sys/vfs/hammer2/hammer2_vnops.c +++ b/sys/vfs/hammer2/hammer2_vnops.c @@ -96,18 +96,20 @@ hammer2_vop_inactive(struct vop_inactive_args *ap) * the strategy code. Simply mark the inode modified so it gets * picked up by our normal flush. */ + chain = hammer2_inode_lock_ex(ip); if (ip->flags & HAMMER2_INODE_DIRTYEMBED) { - chain = hammer2_inode_lock_ex(ip); atomic_clear_int(&ip->flags, HAMMER2_INODE_DIRTYEMBED); hammer2_chain_modify(ip->hmp, chain, 0); - hammer2_inode_unlock_ex(ip, chain); } /* * Check for deleted inodes and recycle immediately. */ - if (ip->flags & HAMMER2_INODE_DELETED) { + if (chain && (chain->flags & HAMMER2_CHAIN_DELETED)) { + hammer2_inode_unlock_ex(ip, chain); vrecycle(vp); + } else { + hammer2_inode_unlock_ex(ip, chain); } return (0); } @@ -138,7 +140,7 @@ hammer2_vop_reclaim(struct vop_reclaim_args *ap) chain = hammer2_inode_lock_ex(ip); vp->v_data = NULL; ip->vp = NULL; - if (ip->flags & HAMMER2_INODE_DELETED) { + if (chain->flags & HAMMER2_CHAIN_DELETED) { KKASSERT(chain->flags & HAMMER2_CHAIN_DELETED); atomic_set_int(&chain->flags, HAMMER2_CHAIN_DESTROYED | HAMMER2_CHAIN_SUBMODIFIED); @@ -193,6 +195,7 @@ hammer2_vop_fsync(struct vop_fsync_args *ap) * which call this function will eventually call chain_flush * on the volume root as a catch-all, which is far more optimal. */ + atomic_clear_int(&ip->flags, HAMMER2_INODE_MODIFIED); if (ap->a_flags & VOP_FSYNC_SYSCALL) hammer2_chain_flush(hmp, chain, 0); hammer2_inode_unlock_ex(ip, chain); @@ -996,7 +999,6 @@ hammer2_assign_physical(hammer2_inode_t *ip, hammer2_key_t lbase, *errorp = 0; retry: parent = hammer2_inode_lock_ex(ip); - chain = hammer2_chain_lookup(hmp, &parent, lbase, lbase, HAMMER2_LOOKUP_NODATA); @@ -1462,14 +1464,18 @@ hammer2_vop_nresolve(struct vop_nresolve_args *ap) * NOTE: For error processing, only ENOENT resolves the namecache * entry to NULL, otherwise we just return the error and * leave the namecache unresolved. + * + * NOTE: multiple hammer2_inode structures can be aliased to the + * same chain element, for example for hardlinks. This + * use case does not 'reattach' inode associations that + * might already exist, but always allocates a new one. */ if (chain) { - ip = hammer2_inode_get(dip->pmp, dip, chain); + ip = hammer2_inode_get(dip->hmp, dip->pmp, dip, chain); vp = hammer2_igetv(ip, &error); if (error == 0) { vn_unlock(vp); cache_setvp(ap->a_nch, vp); - vrele(vp); } else if (error == ENOENT) { cache_setvp(ap->a_nch, NULL); } @@ -1480,6 +1486,14 @@ hammer2_vop_nresolve(struct vop_nresolve_args *ap) */ hammer2_inode_unlock_ex(ip, NULL); hammer2_chain_unlock(hmp, chain); + + /* + * The vp should not be released until after we've disposed + * of our locks, because it might cause vop_inactive() to + * be called. + */ + if (vp) + vrele(vp); } else { error = ENOENT; cache_setvp(ap->a_nch, NULL); @@ -1741,15 +1755,23 @@ hammer2_vop_nlink(struct vop_nlink_args *ap) * XXX this can race against concurrent vnode ops. */ if (oip != ip) { - hammer2_inode_ref(ip); /* vp ref+ */ + hammer2_inode_ref(ip); /* vp ref+ */ chain = hammer2_inode_lock_ex(ip); ochain = hammer2_inode_lock_ex(oip); - ip->vp = ap->a_vp; - ap->a_vp->v_data = ip; - oip->vp = NULL; + if (ip->vp) { + KKASSERT(ip->vp == ap->a_vp); + hammer2_inode_drop(ip); /* vp already ref'd */ + } else { + ip->vp = ap->a_vp; + ap->a_vp->v_data = ip; + } + if (oip->vp) { + KKASSERT(oip->vp == ap->a_vp); + oip->vp = NULL; + hammer2_inode_drop(oip); /* vp ref- */ + } hammer2_inode_unlock_ex(oip, ochain); hammer2_inode_unlock_ex(ip, chain); - hammer2_inode_drop(oip); /* vp ref- */ } /* @@ -1921,7 +1943,6 @@ hammer2_vop_nremove(struct vop_nremove_args *ap) name_len = ncp->nc_nlen; error = hammer2_unlink_file(dip, name, name_len, 0, NULL); - if (error == 0) { cache_unlink(ap->a_nch); } @@ -1952,7 +1973,6 @@ hammer2_vop_nrmdir(struct vop_nrmdir_args *ap) name_len = ncp->nc_nlen; error = hammer2_unlink_file(dip, name, name_len, 1, NULL); - if (error == 0) { cache_unlink(ap->a_nch); } @@ -2039,6 +2059,7 @@ hammer2_vop_nrename(struct vop_nrename_args *ap) * contents of the inode. */ chain = hammer2_inode_lock_sh(ip); + hammer2_chain_ref(hmp, chain); /* for unlink file */ if (chain->data->ipdata.nlinks > 1) { hammer2_inode_unlock_sh(ip, chain); error = hammer2_hardlink_consolidate(&ip, tdip); @@ -2047,18 +2068,20 @@ hammer2_vop_nrename(struct vop_nrename_args *ap) } else { hammer2_inode_unlock_sh(ip, chain); } + /* chain ref still intact */ /* * NOTE! Because we are retaining (ip) the unlink can fail with * an EAGAIN. */ for (;;) { - error = hammer2_unlink_file(fdip, fname, fname_len, -1, ip); + error = hammer2_unlink_file(fdip, fname, fname_len, -1, chain); if (error != EAGAIN) break; kprintf("hammer2_vop_nrename: unlink race %s\n", fname); tsleep(fdip, 0, "h2renr", 1); } + hammer2_chain_drop(hmp, chain); /* drop temporary ref */ if (error) goto done; -- 2.41.0