Date: Thu, 6 Jul 2006 03:48:29 GMT From: Scott Long <scottl@FreeBSD.org> To: Perforce Change Reviews <perforce@freebsd.org> Subject: PERFORCE change 100686 for review Message-ID: <200607060348.k663mTHW007992@repoman.freebsd.org>
next in thread | raw e-mail | index | archive | help
http://perforce.freebsd.org/chv.cgi?CH=100686 Change 100686 by scottl@scottl-wv1u on 2006/07/06 03:47:59 Use a sleep mutex to protect kernel environment handling instead of an sx lock. The sx lock seemed to only be used to get around the copyout case in kenv(KENV_DUMP) path. Fix that path to safely use a sleep lock instead. Affected files ... .. //depot/projects/scottl-camlock/src/sys/kern/kern_environment.c#7 edit .. //depot/projects/scottl-camlock/src/sys/kern/subr_hints.c#3 edit .. //depot/projects/scottl-camlock/src/sys/sys/systm.h#7 edit Differences ... ==== //depot/projects/scottl-camlock/src/sys/kern/kern_environment.c#7 (text+ko) ==== @@ -48,7 +48,6 @@ #include <sys/malloc.h> #include <sys/mutex.h> #include <sys/kernel.h> -#include <sys/sx.h> #include <sys/systm.h> #include <sys/sysent.h> #include <sys/sysproto.h> @@ -65,7 +64,7 @@ /* dynamic environment variables */ char **kenvp; -struct sx kenv_lock; +struct mtx kenv_lock; /* * No need to protect this with a mutex @@ -86,7 +85,7 @@ int len; } */ *uap; { - char *name, *value; + char *name, *value, *buffer; size_t len, done, needed; int error, i; @@ -100,7 +99,8 @@ return (error); #endif done = needed = 0; - sx_slock(&kenv_lock); + buffer = malloc(uap->len, M_TEMP, M_WAITOK|M_ZERO); + mtx_lock(&kenv_lock); for (i = 0; kenvp[i] != NULL; i++) { len = strlen(kenvp[i]) + 1; needed += len; @@ -110,14 +110,15 @@ * buffer, just keep computing the required size. */ if (uap->value != NULL && len > 0) { - error = copyout(kenvp[i], uap->value + done, - len); - if (error) + if (done + len > uap->len) break; + bcopy(kenvp[i], buffer + done, len); done += len; } } - sx_sunlock(&kenv_lock); + mtx_unlock(&kenv_lock); + error = copyout(buffer, uap->value, done); + free(buffer, M_TEMP); td->td_retval[0] = ((done == needed) ? 0 : needed); return (error); } @@ -220,7 +221,7 @@ } kenvp[i] = NULL; - sx_init(&kenv_lock, "kernel environment"); + mtx_init(&kenv_lock, "kernel environment", NULL, MTX_DEF); dynamic_kenv = 1; } SYSINIT(kenv, SI_SUB_KMEM, SI_ORDER_ANY, init_dynamic_kenv, NULL); @@ -242,7 +243,7 @@ char *cp; int len, i; - sx_assert(&kenv_lock, SX_LOCKED); + mtx_assert(&kenv_lock, MA_OWNED); len = strlen(name); for (cp = kenvp[0], i = 0; cp != NULL; cp = kenvp[++i]) { if ((strncmp(cp, name, len) == 0) && @@ -288,16 +289,16 @@ int len; if (dynamic_kenv) { - sx_slock(&kenv_lock); + mtx_lock(&kenv_lock); cp = _getenv_dynamic(name, NULL); if (cp != NULL) { strcpy(buf, cp); - sx_sunlock(&kenv_lock); + mtx_unlock(&kenv_lock); len = strlen(buf) + 1; ret = malloc(len, M_KENV, M_WAITOK); strcpy(ret, buf); } else { - sx_sunlock(&kenv_lock); + mtx_unlock(&kenv_lock); ret = NULL; } } else @@ -314,9 +315,9 @@ char *cp; if (dynamic_kenv) { - sx_slock(&kenv_lock); + mtx_lock(&kenv_lock); cp = _getenv_dynamic(name, NULL); - sx_sunlock(&kenv_lock); + mtx_unlock(&kenv_lock); } else cp = _getenv_static(name); if (cp != NULL) @@ -344,12 +345,12 @@ buf = malloc(namelen + vallen, M_KENV, M_WAITOK); sprintf(buf, "%s=%s", name, value); - sx_xlock(&kenv_lock); + mtx_lock(&kenv_lock); cp = _getenv_dynamic(name, &i); if (cp != NULL) { oldenv = kenvp[i]; kenvp[i] = buf; - sx_xunlock(&kenv_lock); + mtx_unlock(&kenv_lock); free(oldenv, M_KENV); } else { /* We add the option if it wasn't found */ @@ -359,13 +360,13 @@ /* Bounds checking */ if (i < 0 || i >= KENV_SIZE) { free(buf, M_KENV); - sx_xunlock(&kenv_lock); + mtx_unlock(&kenv_lock); return (-1); } kenvp[i] = buf; kenvp[i + 1] = NULL; - sx_xunlock(&kenv_lock); + mtx_unlock(&kenv_lock); } return (0); } @@ -381,18 +382,18 @@ KENV_CHECK; - sx_xlock(&kenv_lock); + mtx_lock(&kenv_lock); cp = _getenv_dynamic(name, &i); if (cp != NULL) { oldenv = kenvp[i]; for (j = i + 1; kenvp[j] != NULL; j++) kenvp[i++] = kenvp[j]; kenvp[i] = NULL; - sx_xunlock(&kenv_lock); + mtx_unlock(&kenv_lock); free(oldenv, M_KENV); return (0); } - sx_xunlock(&kenv_lock); + mtx_unlock(&kenv_lock); return (-1); } ==== //depot/projects/scottl-camlock/src/sys/kern/subr_hints.c#3 (text+ko) ==== @@ -29,7 +29,7 @@ #include <sys/param.h> #include <sys/lock.h> -#include <sys/sx.h> +#include <sys/mutex.h> #include <sys/systm.h> #include <sys/bus.h> @@ -72,7 +72,7 @@ break; case 2: /* fallback mode */ if (dynamic_kenv) { - sx_slock(&kenv_lock); + mtx_lock(&kenv_lock); cp = kenvp[0]; for (i = 0; cp != NULL; cp = kenvp[++i]) { if (!strncmp(cp, "hint.", 5)) { @@ -81,7 +81,7 @@ break; } } - sx_sunlock(&kenv_lock); + mtx_unlock(&kenv_lock); } else { cp = kern_envp; while (cp) { @@ -114,11 +114,11 @@ } if (use_kenv) { - sx_slock(&kenv_lock); + mtx_lock(&kenv_lock); i = 0; cp = kenvp[0]; if (cp == NULL) { - sx_sunlock(&kenv_lock); + mtx_unlock(&kenv_lock); return (ENOENT); } } else @@ -165,7 +165,7 @@ } } if (use_kenv) - sx_sunlock(&kenv_lock); + mtx_unlock(&kenv_lock); if (cp == NULL) return ENOENT; ==== //depot/projects/scottl-camlock/src/sys/sys/systm.h#7 (text+ko) ==== @@ -102,7 +102,7 @@ extern int envmode; extern int hintmode; /* 0 = off. 1 = config, 2 = fallback */ extern int dynamic_kenv; -extern struct sx kenv_lock; +extern struct mtx kenv_lock; extern char *kern_envp; extern char static_env[]; extern char static_hints[]; /* by config for now */
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?200607060348.k663mTHW007992>