svn commit: r364239 - head/sys/kern
Jason A. Harmening
jah at FreeBSD.org
Fri Aug 14 21:37:38 UTC 2020
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);
}
/*
More information about the svn-src-all
mailing list