Author: jah Date: Fri Aug 14 21:37:38 2020 New Revision: 364239 URL: https://svnweb.freebsd.org/changeset/base/364239
Log: kenv: avoid sleepable alloc for integer tunables Avoid performing a potentially-blocking malloc for kenv lookups that will only perform non-destructive integer conversions on the returned buffer. Instead, perform the strtoq() in-place with the kenv lock held. While here, factor the logic around kenv_lock acquire and release into kenv_acquire() and kenv_release(), and use these functions for some light cleanup. Collapse getenv_string_buffer() into kern_getenv(), as the former no longer has any other callers and the only additional task performed by the latter is a WITNESS check that hasn't been useful since r362231. PR: 248250 Reported by: gbe Reviewed by: mjg Tested by: gbe Differential Revision: https://reviews.freebsd.org/D26010 Modified: head/sys/kern/kern_environment.c Modified: head/sys/kern/kern_environment.c ============================================================================== --- head/sys/kern/kern_environment.c Fri Aug 14 21:29:56 2020 (r364238) +++ head/sys/kern/kern_environment.c Fri Aug 14 21:37:38 2020 (r364239) @@ -59,6 +59,9 @@ __FBSDID("$FreeBSD$"); static char *_getenv_dynamic_locked(const char *name, int *idx); static char *_getenv_dynamic(const char *name, int *idx); +static char *kenv_acquire(const char *name); +static void kenv_release(const char *buf); + static MALLOC_DEFINE(M_KENV, "kenv", "kernel environment"); #define KENV_SIZE 512 /* Maximum number of environment strings */ @@ -88,8 +91,6 @@ bool dynamic_kenv; #define KENV_CHECK if (!dynamic_kenv) \ panic("%s: called before SI_SUB_KMEM", __func__) -static char *getenv_string_buffer(const char *); - int sys_kenv(td, uap) struct thread *td; @@ -482,16 +483,24 @@ _getenv_static(const char *name) char * kern_getenv(const char *name) { - char *ret; + char *cp, *ret; + int len; if (dynamic_kenv) { - ret = getenv_string_buffer(name); - if (ret == NULL) { - WITNESS_WARN(WARN_GIANTOK | WARN_SLEEPOK, NULL, - "getenv"); + len = KENV_MNAMELEN + 1 + kenv_mvallen + 1; + ret = uma_zalloc(kenv_zone, M_WAITOK | M_ZERO); + mtx_lock(&kenv_lock); + cp = _getenv_dynamic(name, NULL); + if (cp != NULL) + strlcpy(ret, cp, len); + mtx_unlock(&kenv_lock); + if (cp == NULL) { + uma_zfree(kenv_zone, ret); + ret = NULL; } } else ret = _getenv_static(name); + return (ret); } @@ -503,12 +512,9 @@ testenv(const char *name) { char *cp; - if (dynamic_kenv) { - mtx_lock(&kenv_lock); - cp = _getenv_dynamic(name, NULL); - mtx_unlock(&kenv_lock); - } else - cp = _getenv_static(name); + cp = kenv_acquire(name); + kenv_release(cp); + if (cp != NULL) return (1); return (0); @@ -615,30 +621,33 @@ kern_unsetenv(const char *name) } /* - * Return a buffer containing the string value from an environment variable + * Return the internal kenv buffer for the variable name, if it exists. + * If the dynamic kenv is initialized and the name is present, return + * with kenv_lock held. */ static char * -getenv_string_buffer(const char *name) +kenv_acquire(const char *name) { - char *cp, *ret; - int len; + char *value; if (dynamic_kenv) { - len = KENV_MNAMELEN + 1 + kenv_mvallen + 1; - ret = uma_zalloc(kenv_zone, M_WAITOK | M_ZERO); mtx_lock(&kenv_lock); - cp = _getenv_dynamic(name, NULL); - if (cp != NULL) - strlcpy(ret, cp, len); - mtx_unlock(&kenv_lock); - if (cp == NULL) { - uma_zfree(kenv_zone, ret); - ret = NULL; - } + value = _getenv_dynamic(name, NULL); + if (value == NULL) + mtx_unlock(&kenv_lock); + return (value); } else - ret = _getenv_static(name); + return (_getenv_static(name)); +} - return (ret); +/* + * Undo a previous kenv_acquire() operation + */ +static void +kenv_release(const char *buf) +{ + if ((buf != NULL) && dynamic_kenv) + mtx_unlock(&kenv_lock); } /* @@ -649,17 +658,13 @@ getenv_string(const char *name, char *data, int size) { char *cp; - if (dynamic_kenv) { - mtx_lock(&kenv_lock); - cp = _getenv_dynamic(name, NULL); - if (cp != NULL) - strlcpy(data, cp, size); - mtx_unlock(&kenv_lock); - } else { - cp = _getenv_static(name); - if (cp != NULL) - strlcpy(data, cp, size); - } + cp = kenv_acquire(name); + + if (cp != NULL) + strlcpy(data, cp, size); + + kenv_release(cp); + return (cp != NULL); } @@ -673,16 +678,18 @@ getenv_array(const char *name, void *pdata, int size, uint8_t shift; int64_t value; int64_t old; - char *buf; + const char *buf; char *end; - char *ptr; + const char *ptr; int n; int rc; - if ((buf = getenv_string_buffer(name)) == NULL) - return (0); - rc = 0; /* assume failure */ + + buf = kenv_acquire(name); + if (buf == NULL) + goto error; + /* get maximum number of elements */ size /= type_size; @@ -797,8 +804,7 @@ getenv_array(const char *name, void *pdata, int size, if (n != 0) rc = 1; /* success */ error: - if (dynamic_kenv) - uma_zfree(kenv_zone, buf); + kenv_release(buf); return (rc); } @@ -898,18 +904,21 @@ getenv_ulong(const char *name, unsigned long *data) int getenv_quad(const char *name, quad_t *data) { - char *value, *vtp; - quad_t iv; + const char *value; + char suffix, *vtp; + quad_t iv; - value = getenv_string_buffer(name); - if (value == NULL) - return (0); + value = kenv_acquire(name); + if (value == NULL) { + goto error; + } iv = strtoq(value, &vtp, 0); if (vtp == value || (vtp[0] != '\0' && vtp[1] != '\0')) { - freeenv(value); - return (0); + goto error; } - switch (vtp[0]) { + suffix = vtp[0]; + kenv_release(value); + switch (suffix) { case 't': case 'T': iv *= 1024; /* FALLTHROUGH */ @@ -924,12 +933,13 @@ getenv_quad(const char *name, quad_t *data) case '\0': break; default: - freeenv(value); return (0); } - freeenv(value); *data = iv; return (1); +error: + kenv_release(value); + return (0); } /* _______________________________________________ svn-src-all@freebsd.org mailing list https://lists.freebsd.org/mailman/listinfo/svn-src-all To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"
