From 3abca35872bd27de36ce910a72f01943d223f423 Mon Sep 17 00:00:00 2001 From: alfred Date: Fri, 27 Apr 2001 22:24:45 +0000 Subject: Address a number of problems with sysctl_vm_zone(). The zone allocator's locks should be leaflocks, meaning that they should never be held when entering into another subsystem, however the sysctl grabs the zone global mutex and individual zone mutexes while holding the lock it calls SYSCTL_OUT which recurses into the VM subsystem in order to wire user memory to do a safe copy. This can block and cause lock order reversals. To fix this: lock zone global. get a count of the number of zones. unlock global. allocate temporary storage. format and SYSCTL_OUT the banner. lock global. traverse list. make sure we haven't looped more than the initial count taken to avoid overflowing the allocated buffer. lock each nodes. read values and format into buffer. unlock individual node. unlock global. format and SYSCTL_OUT the rest of the data. free storage. return. Other problems included not checking for errors when doing sysctl out of the column header. Fixed. Inconsistant termination of the copied string. Fixed. Objected to by: des (for not using sbuf) Since the output is not variable length and I'm actually over allocating signifigantly and I'd like to get this fixed now, I'll work on the sbuf convertion at a later date. I would not object to someone else taking it upon themselves to convert it to sbuf. I hold no MAINTIANER rights to this code (for now). --- sys/vm/vm_zone.c | 40 +++++++++++++++++++++++++++++----------- 1 file changed, 29 insertions(+), 11 deletions(-) diff --git a/sys/vm/vm_zone.c b/sys/vm/vm_zone.c index 65c10de..4cddadc 100644 --- a/sys/vm/vm_zone.c +++ b/sys/vm/vm_zone.c @@ -414,31 +414,49 @@ zfree(vm_zone_t z, void *item) static int sysctl_vm_zone(SYSCTL_HANDLER_ARGS) { - int error, len; - char tmpbuf[128]; + int error, len, cnt; + const int linesize = 128; /* conservative */ + char *tmpbuf, *offset; vm_zone_t z; char *p; + cnt = 0; mtx_lock(&zone_mtx); - len = snprintf(tmpbuf, sizeof(tmpbuf), + SLIST_FOREACH(z, &zlist, zent) + cnt++; + mtx_unlock(&zone_mtx); + MALLOC(tmpbuf, char *, (cnt == 0 ? 1 : cnt) * linesize, + M_TEMP, M_WAITOK); + len = snprintf(tmpbuf, linesize, "\nITEM SIZE LIMIT USED FREE REQUESTS\n\n"); - error = SYSCTL_OUT(req, tmpbuf, SLIST_EMPTY(&zlist) ? len-1 : len); + if (cnt == 0) + tmpbuf[len - 1] = '\0'; + error = SYSCTL_OUT(req, tmpbuf, cnt == 0 ? len-1 : len); + if (error || cnt == 0) + goto out; + offset = tmpbuf; + mtx_lock(&zone_mtx); SLIST_FOREACH(z, &zlist, zent) { + if (cnt == 0) /* list may have changed size */ + break; mtx_lock(&z->zmtx); - len = snprintf(tmpbuf, sizeof(tmpbuf), + len = snprintf(offset, linesize, "%-12.12s %6.6u, %8.8u, %6.6u, %6.6u, %8.8u\n", z->zname, z->zsize, z->zmax, (z->ztotal - z->zfreecnt), z->zfreecnt, z->znalloc); - for (p = tmpbuf + 12; p > tmpbuf && *p == ' '; --p) + mtx_unlock(&z->zmtx); + for (p = offset + 12; p > offset && *p == ' '; --p) /* nothing */ ; p[1] = ':'; - mtx_unlock(&z->zmtx); - if (SLIST_NEXT(z, zent) == NULL) - tmpbuf[len - 1] = 0; - if ((error = SYSCTL_OUT(req, tmpbuf, len)) != 0) - break; + cnt--; + offset += len; } mtx_unlock(&zone_mtx); + offset--; + *offset = '\0'; + error = SYSCTL_OUT(req, tmpbuf, offset - tmpbuf); +out: + FREE(tmpbuf, M_TEMP); return (error); } -- cgit v1.1