Date: Sat, 20 Jul 2002 14:39:27 -0700 (PDT) From: Don Lewis <dl-slicex@catspoiler.org> To: bde@zeta.org.au Cc: arch@FreeBSD.ORG Subject: Re: wiring the sysctl output buffer Message-ID: <200207202139.g6KLdRwr041480@gw.catspoiler.org> In-Reply-To: <200207170755.g6H7t1wr029257@gw.catspoiler.org>
next in thread | previous in thread | raw e-mail | index | archive | help
On 17 Jul, Don Lewis wrote:
> My initial solution was to allocate a temporary buffer, but that
> complicates the code and the similar code might have to be added to
> other parts of the kernel where similar situations might arise. It was
> not until later that I thought of pre-wiring the buffer, which looks
> like a much cleaner solution. In the latter case, I decided that it
> would be best to abstact the interface so that the handler doesn't need
> to become intimate with the internals of the sysctl system. If this
> solution allows us to avoid the call to vslock() in the majority of
> cases, then so much the better.
>
> The initial implementation of the interface to vslock() would probably
> just ignore the size argument, but defining the API this way would allow
> it to be used later if necessary without the necessity of retrofitting
> the handlers that use it.
Attached is another iteration of the patch. I added a length parameter
to the vslock() wrapper, though this parameter is currently ignored. I
modified the standard handlers to either make a temporary copy of the
data or to wire the buffer as appropriate. I also added an assertion to
sysctl_old_user() to catch calls to SYSCTL_OUT() that may block while
locks are held. This assertion has already turned up some other calls
to SYSCTL_OUT() where locks are held.
This patch also includes the sysctl_kern_function_list() fix.
What's the scoop with CTLFLAG_NOLOCK? Should it be used to retrieve
static data? If so, should the default handlers skip the extra copy if
this flag is set?
Index: kern_sysctl.c
===================================================================
RCS file: /home/ncvs/src/sys/kern/kern_sysctl.c,v
retrieving revision 1.127
diff -u -r1.127 kern_sysctl.c
--- kern_sysctl.c 15 Jul 2002 17:28:34 -0000 1.127
+++ kern_sysctl.c 20 Jul 2002 21:10:42 -0000
@@ -57,6 +57,7 @@
static MALLOC_DEFINE(M_SYSCTL, "sysctl", "sysctl internal magic");
static MALLOC_DEFINE(M_SYSCTLOID, "sysctloid", "sysctl dynamic oids");
+static MALLOC_DEFINE(M_SYSCTLTMP, "sysctltmp", "sysctl temp output buffer");
/*
* Locking - this locks the sysctl tree in memory.
@@ -751,12 +752,16 @@
int
sysctl_handle_int(SYSCTL_HANDLER_ARGS)
{
- int error = 0;
+ int tmpout, error = 0;
+ /*
+ * Attempt to get a coherent snapshot by making a copy of the data.
+ */
if (arg1)
- error = SYSCTL_OUT(req, arg1, sizeof(int));
+ tmpout = *(int *)arg1;
else
- error = SYSCTL_OUT(req, &arg2, sizeof(int));
+ tmpout = arg2;
+ error = SYSCTL_OUT(req, &tmpout, sizeof(int));
if (error || !req->newptr)
return (error);
@@ -776,10 +781,15 @@
sysctl_handle_long(SYSCTL_HANDLER_ARGS)
{
int error = 0;
+ long tmpout;
+ /*
+ * Attempt to get a coherent snapshot by making a copy of the data.
+ */
if (!arg1)
return (EINVAL);
- error = SYSCTL_OUT(req, arg1, sizeof(long));
+ tmpout = *(long *)arg1;
+ error = SYSCTL_OUT(req, &tmpout, sizeof(long));
if (error || !req->newptr)
return (error);
@@ -799,8 +809,23 @@
sysctl_handle_string(SYSCTL_HANDLER_ARGS)
{
int error=0;
+ char *tmparg;
+ size_t outlen;
- error = SYSCTL_OUT(req, arg1, strlen((char *)arg1)+1);
+ /*
+ * Attempt to get a coherent snapshot by copying to a
+ * temporary kernel buffer.
+ */
+retry:
+ outlen = strlen((char *)arg1)+1;
+ tmparg = malloc(outlen, M_SYSCTLTMP, M_WAITOK);
+ strncpy(tmparg, (char *)arg1, outlen);
+ if (tmparg[outlen-1] != '\0') {
+ free(tmparg, M_SYSCTLTMP);
+ goto retry;
+ }
+ error = SYSCTL_OUT(req, tmparg, outlen);
+ free(tmparg, M_SYSCTLTMP);
if (error || !req->newptr)
return (error);
@@ -825,8 +850,23 @@
sysctl_handle_opaque(SYSCTL_HANDLER_ARGS)
{
int error;
+ void *tmparg;
- error = SYSCTL_OUT(req, arg1, arg2);
+ /*
+ * Attempt to get a coherent snapshot, either by wiring the
+ * user space buffer or copying to a temporary kernel buffer
+ * depending on the size of the data.
+ */
+ if (arg2 > PAGE_SIZE) {
+ sysctl_wire_old_buffer(req, arg2);
+ error = SYSCTL_OUT(req, arg1, arg2);
+ } else {
+ tmparg = malloc(arg2, M_SYSCTLTMP, M_WAITOK);
+ strcpy(tmparg, (char *)arg1);
+ bcopy(arg1, tmparg, arg2);
+ error = SYSCTL_OUT(req, tmparg, arg2);
+ free(tmparg, M_SYSCTLTMP);
+ }
if (error || !req->newptr)
return (error);
@@ -953,10 +993,8 @@
int error = 0;
size_t i = 0;
- if (req->lock == 1 && req->oldptr) {
- vslock(req->oldptr, req->oldlen);
- req->lock = 2;
- }
+ if (req->lock == 1 && req->oldptr)
+ WITNESS_SLEEP(1, NULL);
if (req->oldptr) {
i = l;
if (req->oldlen <= req->oldidx)
@@ -988,6 +1026,23 @@
error = copyin((char *)req->newptr + req->newidx, p, l);
req->newidx += l;
return (error);
+}
+
+/*
+ * Wire the user space destination buffer. If set to a value greater than
+ * zero, the len parameter limits the maximum amount of wired memory.
+ *
+ * XXX - The len parameter is currently ignored due to the lack of
+ * a place to save it in the sysctl_req structure so that the matching
+ * amount of memory can be unwired in the sysctl exit code.
+ */
+void
+sysctl_wire_old_buffer(struct sysctl_req *req, size_t len)
+{
+ if (req->lock == 1 && req->oldptr && req->oldfunc == sysctl_old_user) {
+ vslock(req->oldptr, req->oldlen);
+ req->lock = 2;
+ }
}
int
Index: kern_linker.c
===================================================================
RCS file: /home/ncvs/src/sys/kern/kern_linker.c,v
retrieving revision 1.91
diff -u -r1.91 kern_linker.c
--- kern_linker.c 7 Jul 2002 22:35:47 -0000 1.91
+++ kern_linker.c 17 Jul 2002 10:37:50 -0000
@@ -1794,6 +1794,7 @@
linker_file_t lf;
int error;
+ sysctl_wire_old_buffer(req, 0);
mtx_lock(&kld_mtx);
TAILQ_FOREACH(lf, &linker_files, link) {
error = LINKER_EACH_FUNCTION_NAME(lf,
To Unsubscribe: send mail to majordomo@FreeBSD.org
with "unsubscribe freebsd-arch" in the body of the message
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?200207202139.g6KLdRwr041480>