From: Matthew Dillon Date: Sat, 9 Oct 2010 16:26:46 +0000 (-0700) Subject: libc - Fix livelock in nmalloc X-Git-Tag: v2.8.0~29 X-Git-Url: http://gitweb.dragonflybsd.org/dragonfly.git/commitdiff_plain/4cd64cfec8d5e4d67ed8ee045e6d875a398a7892 libc - Fix livelock in nmalloc * free() -> _slabfree() was turning around and calling _slaballoc() with the depot lock held, which could lead to a livelock. Fix by unlocking the depot lock around the call. * Clean up and document other unrelated bits of nmalloc(). In particular, clean up magazine_alloc(). Reported-by: Siju George , and others --- diff --git a/lib/libc/stdlib/nmalloc.c b/lib/libc/stdlib/nmalloc.c index 829a4cb..b7d5e9b 100644 --- a/lib/libc/stdlib/nmalloc.c +++ b/lib/libc/stdlib/nmalloc.c @@ -879,7 +879,6 @@ _slaballoc(size_t size, int flags) * exhausted pull one off the free list or allocate a new one. */ if ((z = slgd->ZoneAry[zi]) == NULL) { - z = zone_alloc(flags); if (z == NULL) goto fail; @@ -1121,7 +1120,9 @@ _slabrealloc(void *ptr, size_t size) * checking memory limits in malloc. * * flags: - * FASTSLABREALLOC Fast call from realloc + * FASTSLABREALLOC Fast call from realloc, *rbigp already + * unlinked. + * * MPSAFE */ static void @@ -1156,10 +1157,8 @@ _slabfree(void *ptr, int flags, bigalloc_t *rbigp) if ((bigp = bigalloc_check_and_lock(ptr)) != NULL) { while ((big = *bigp) != NULL) { if (big->base == ptr) { - if ((flags & FASTSLABREALLOC) == 0) { - *bigp = big->next; - bigalloc_unlock(ptr); - } + *bigp = big->next; + bigalloc_unlock(ptr); fastslabrealloc: size = big->bytes; _slabfree(big, 0, NULL); @@ -1255,6 +1254,7 @@ fastslabrealloc: z->z_Magic = -1; z->z_Next = NULL; zone_free(z); + /* slgd lock released */ return; } slgd_unlock(slgd); @@ -1294,36 +1294,37 @@ chunk_mark_free(slzone_t z, void *chunk) #endif +/* + * Allocate and return a magazine. NULL is returned and *burst is adjusted + * if the magazine is empty. + */ static __inline void * magazine_alloc(struct magazine *mp, int *burst) { - void *obj = NULL; - - do { - if (mp != NULL && MAGAZINE_NOTEMPTY(mp)) { - obj = mp->objects[--mp->rounds]; - break; - } + void *obj; - /* Return burst factor to caller */ - if ((mp->flags & M_BURST) && (burst != NULL)) { - *burst = mp->burst_factor; - } + if (MAGAZINE_NOTEMPTY(mp)) { + obj = mp->objects[--mp->rounds]; + return(obj); + } - /* Reduce burst factor by NSCALE; if it hits 1, disable BURST */ - if ((mp->flags & M_BURST) && (mp->flags & M_BURST_EARLY) && - (burst != NULL)) { - mp->burst_factor -= M_BURST_NSCALE; - if (mp->burst_factor <= 1) { - mp->burst_factor = 1; - mp->flags &= ~(M_BURST); - mp->flags &= ~(M_BURST_EARLY); - } + /* + * Return burst factor to caller along with NULL + */ + if ((mp->flags & M_BURST) && (burst != NULL)) { + *burst = mp->burst_factor; + } + /* Reduce burst factor by NSCALE; if it hits 1, disable BURST */ + if ((mp->flags & M_BURST) && (mp->flags & M_BURST_EARLY) && + (burst != NULL)) { + mp->burst_factor -= M_BURST_NSCALE; + if (mp->burst_factor <= 1) { + mp->burst_factor = 1; + mp->flags &= ~(M_BURST); + mp->flags &= ~(M_BURST_EARLY); } - - } while (0); - - return obj; + } + return (NULL); } static __inline int @@ -1371,9 +1372,11 @@ mtmagazine_alloc(int zi) continue; } - /* Lock the depot and check if it has any full magazines; if so - * we return the prev to the emptymag list, move loaded to prev - * load a full magazine, and retry */ + /* + * Lock the depot and check if it has any full magazines; if + * so we return the prev to the emptymag list, move loaded + * to prev load a full magazine, and retry + */ d = &depots[zi]; depot_lock(d); @@ -1384,14 +1387,14 @@ mtmagazine_alloc(int zi) SLIST_REMOVE_HEAD(&d->full, nextmagazine); /* Return emptymag to the depot */ - if (emptymag != NULL) - SLIST_INSERT_HEAD(&d->empty, emptymag, nextmagazine); - + if (emptymag != NULL) { + SLIST_INSERT_HEAD(&d->empty, emptymag, + nextmagazine); + } depot_unlock(d); continue; - } else { - depot_unlock(d); } + depot_unlock(d); break; } @@ -1446,26 +1449,29 @@ mtmagazine_free(int zi, void *ptr) SLIST_REMOVE_HEAD(&d->empty, nextmagazine); /* Return loadedmag to the depot */ - if (loadedmag != NULL) + if (loadedmag != NULL) { SLIST_INSERT_HEAD(&d->full, loadedmag, nextmagazine); + } depot_unlock(d); continue; } - /* Allocate an empty magazine, add it to the depot, retry */ + /* + * Allocate an empty magazine, add it to the depot, retry + */ + depot_unlock(d); newmag = _slaballoc(sizeof(struct magazine), SAFLAG_ZERO); if (newmag != NULL) { newmag->capacity = M_MAX_ROUNDS; newmag->rounds = 0; + depot_lock(d); SLIST_INSERT_HEAD(&d->empty, newmag, nextmagazine); depot_unlock(d); continue; - } else { - depot_unlock(d); - rc = -1; } + rc = -1; break; } @@ -1553,30 +1559,23 @@ zone_alloc(int flags) slgd_unlock(slgd); z = magazine_alloc(&zone_magazine, &burst); - if (z == NULL) { - if (burst == 1) - zone_magazine_unlock(); - + if (z == NULL && burst == 1) { + zone_magazine_unlock(); z = _vmem_alloc(ZoneSize * burst, ZoneSize, flags); - if (z == NULL) { - zone_magazine_unlock(); - slgd_lock(slgd); - return (NULL); - } - - for (i = 1; i < burst; i++) { - j = magazine_free(&zone_magazine, - (char *) z + (ZoneSize * i)); - MASSERT(j == 0); + } else if (z == NULL) { + z = _vmem_alloc(ZoneSize * burst, ZoneSize, flags); + if (z) { + for (i = 1; i < burst; i++) { + j = magazine_free(&zone_magazine, + (char *) z + (ZoneSize * i)); + MASSERT(j == 0); + } } - - if (burst != 1) - zone_magazine_unlock(); + zone_magazine_unlock(); } else { z->z_Flags |= SLZF_UNOTZEROD; zone_magazine_unlock(); } - slgd_lock(slgd); return z; } @@ -1584,7 +1583,7 @@ zone_alloc(int flags) /* * zone_free() * - * Releases the slgd lock prior to unmap, if unmapping is necessary + * Release a zone and unlock the slgd lock. */ static void zone_free(void *z) @@ -1603,9 +1602,11 @@ zone_free(void *z) i = magazine_free(&zone_magazine, z); - /* If we failed to free, collect excess magazines; release the zone + /* + * If we failed to free, collect excess magazines; release the zone * magazine lock, and then free to the system via _vmem_free. Re-enable - * BURST mode for the magazine. */ + * BURST mode for the magazine. + */ if (i == -1) { j = zone_magazine.rounds - zone_magazine.low_factor; for (i = 0; i < j; i++) {