Date: Wed, 17 Feb 2021 17:55:41 GMT From: Alexander Motin <mav@FreeBSD.org> To: src-committers@FreeBSD.org, dev-commits-src-all@FreeBSD.org, dev-commits-src-branches@FreeBSD.org Subject: git: eefc7f388ab6 - stable/12 - kenv: avoid sleepable alloc for integer tunables Message-ID: <202102171755.11HHtfGr044629@gitrepo.freebsd.org>
next in thread | raw e-mail | index | archive | help
The branch stable/12 has been updated by mav: URL: https://cgit.FreeBSD.org/src/commit/?id=eefc7f388ab66a79f99a5f73236e9fbfc5312d4f commit eefc7f388ab66a79f99a5f73236e9fbfc5312d4f Author: Jason A. Harmening <jah@FreeBSD.org> AuthorDate: 2020-08-14 21:37:38 +0000 Commit: Alexander Motin <mav@FreeBSD.org> CommitDate: 2021-02-17 17:52:24 +0000 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 --- sys/kern/kern_environment.c | 124 ++++++++++++++++++++++++-------------------- 1 file changed, 67 insertions(+), 57 deletions(-) diff --git a/sys/kern/kern_environment.c b/sys/kern/kern_environment.c index f2d7dcb1de4d..761113f108ec 100644 --- a/sys/kern/kern_environment.c +++ b/sys/kern/kern_environment.c @@ -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); @@ -616,30 +622,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); } /* @@ -650,17 +659,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); } @@ -674,16 +679,18 @@ getenv_array(const char *name, void *pdata, int size, int *psize, 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; @@ -798,8 +805,7 @@ getenv_array(const char *name, void *pdata, int size, int *psize, if (n != 0) rc = 1; /* success */ error: - if (dynamic_kenv) - uma_zfree(kenv_zone, buf); + kenv_release(buf); return (rc); } @@ -899,18 +905,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 */ @@ -925,12 +934,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); } /*
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?202102171755.11HHtfGr044629>