DF-0843 / fix.diff
1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 27 28 29 30 31 32 33 34 35 36 37 38 39 40 41 42 43 44 45 46 47 48 49 50 51 52 53 54 55 56 57 58 59 60 61 62 63 64 65 66 67 68 69 70 71 72 73 74 75 76 77 78 79 80 81 82 83 84 85 86 87 88 89 90 91 92 93 94 95 96 97 98 99 100 101 102 103 104 105 106 107 108 109 110 111 112 113 114 115 116 117 118 119 120 121 122 123 124 125 126 127 128 129 130 131 132 133 134 135 136 137 138 139 140 141 142 143 144 145 146 147 148 149 150 151 152 153 | 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); |