diff --git a/sys/vfs/ufs/ufs_dirhash.c b/sys/vfs/ufs/ufs_dirhash.c --- a/sys/vfs/ufs/ufs_dirhash.c +++ b/sys/vfs/ufs/ufs_dirhash.c @@ -91,6 +91,17 @@ static TAILQ_HEAD(, dirhash) ufsdirhash_list; /* + * Global lock guarding ufsdirhash_list membership and the dh_hash lifetime + * of every dirhash on the list. Held shared by ufsdirhash_lookup (and the + * helper readers) so a concurrent ufsdirhash_recycle / ufsdirhash_free -- + * which need it exclusive -- cannot free a dh_hash array out from under a + * reader. Mirrors FreeBSD's ufsdirhash_lock (sx); DragonFly previously had + * no lock here at all. + */ +static struct lock ufsdirhash_lock; + + +/* * Attempt to build up a hash table for the directory contents in * inode 'ip'. Returns 0 on success, or -1 of the operation failed. */ @@ -218,8 +229,10 @@ if (bp != NULL) brelse(bp); + lockmgr(&ufsdirhash_lock, LK_EXCLUSIVE); TAILQ_INSERT_TAIL(&ufsdirhash_list, dh, dh_list); dh->dh_onlist = 1; + lockmgr(&ufsdirhash_lock, LK_RELEASE); return (0); fail: @@ -248,8 +261,11 @@ if ((dh = ip->i_dirhash) == NULL) return; - if (dh->dh_onlist) + if (dh->dh_onlist) { + lockmgr(&ufsdirhash_lock, LK_EXCLUSIVE); TAILQ_REMOVE(&ufsdirhash_list, dh, dh_list); + lockmgr(&ufsdirhash_lock, LK_RELEASE); + } /* The dirhash pointed to by 'dh' is exclusively ours now. */ @@ -294,18 +310,14 @@ if ((dh = ip->i_dirhash) == NULL) return (EJUSTRETURN); /* - * Move this dirhash towards the end of the list if it has a - * score higher than the next entry. - * Optimise the case where it's already the last by performing - * an unlocked read of the TAILQ_NEXT pointer. + * Take the global dirhash lock exclusively for the (rare) score + * reorder of the global list, then downgrade to shared for the + * actual hash lookup. recycle/free need the lock exclusive, so + * while we hold it (either way) dh_hash cannot be freed out from + * under us. Re-validate dh_hash != NULL at every transition. */ + lockmgr(&ufsdirhash_lock, LK_EXCLUSIVE); if (TAILQ_NEXT(dh, dh_list) != NULL) { - /* - * If the new score will be greater than that of the next - * entry, then move this entry past it. With both mutexes - * held, dh_next won't go away, but its dh_score could - * change; that's not important since it is just a hint. - */ if (dh->dh_hash != NULL && (dh_next = TAILQ_NEXT(dh, dh_list)) != NULL && dh->dh_score >= dh_next->dh_score) { @@ -316,9 +328,12 @@ } } if (dh->dh_hash == NULL) { + lockmgr(&ufsdirhash_lock, LK_RELEASE); ufsdirhash_free(ip); return (EJUSTRETURN); } + /* Hold shared across the hash-array derefs: recycle is now excluded. */ + lockmgr(&ufsdirhash_lock, LK_DOWNGRADE); /* Update the score. */ if (dh->dh_score < DH_SCOREMAX) @@ -364,8 +379,10 @@ if (bp != NULL) brelse(bp); blkoff = offset & ~bmask; - if (ffs_blkatoff(vp, (off_t)blkoff, NULL, &bp) != 0) + if (ffs_blkatoff(vp, (off_t)blkoff, NULL, &bp) != 0) { + lockmgr(&ufsdirhash_lock, LK_RELEASE); return (EJUSTRETURN); + } } dp = (struct direct *)(bp->b_data + (offset & bmask)); if (dp->d_reclen == 0 || dp->d_reclen > @@ -397,12 +414,14 @@ *bpp = bp; *offp = offset; + lockmgr(&ufsdirhash_lock, LK_RELEASE); return (0); } if (dh->dh_hash == NULL) { if (bp != NULL) brelse(bp); + lockmgr(&ufsdirhash_lock, LK_RELEASE); ufsdirhash_free(ip); return (EJUSTRETURN); } @@ -417,6 +436,7 @@ } if (bp != NULL) brelse(bp); + lockmgr(&ufsdirhash_lock, LK_RELEASE); return (ENOENT); } @@ -932,15 +952,18 @@ uint8_t *blkfree; int i, mem, narrays; + lockmgr(&ufsdirhash_lock, LK_EXCLUSIVE); while (wanted + ufs_dirhashmem > ufs_dirhashmaxmem) { /* Find a dirhash, and lock it. */ if ((dh = TAILQ_FIRST(&ufsdirhash_list)) == NULL) { + lockmgr(&ufsdirhash_lock, LK_RELEASE); return (-1); } KASSERT(dh->dh_hash != NULL, ("dirhash: NULL hash on list")); /* Decrement the score; only recycle if it becomes zero. */ if (--dh->dh_score > 0) { + lockmgr(&ufsdirhash_lock, LK_RELEASE); return (-1); } @@ -965,6 +988,7 @@ /* Account for the returned memory, and repeat if necessary. */ ufs_dirhashmem -= mem; } + lockmgr(&ufsdirhash_lock, LK_RELEASE); /* Success. */ return (0); } @@ -975,6 +999,7 @@ { ufsdirhash_oc = objcache_create_simple(M_DIRHASH, DH_NBLKOFF * sizeof(daddr_t)); + lockinit(&ufsdirhash_lock, "ufsdirhash", 0, 0); TAILQ_INIT(&ufsdirhash_list); } SYSINIT(ufsdirhash, SI_SUB_PSEUDO, SI_ORDER_ANY, ufsdirhash_init, NULL);