Date: Mon, 17 Jun 2002 23:17:21 +1000 (EST) From: Bruce Evans <bde@zeta.org.au> To: Peter Jeremy <peter.jeremy@alcatel.com.au> Cc: freebsd-current@FreeBSD.ORG Subject: Re: proc-args (M_PARGS) leakage Message-ID: <20020617230109.I360-100000@gamplex.bde.org> In-Reply-To: <20020617155522.X3493-100000@gamplex.bde.org>
next in thread | previous in thread | raw e-mail | index | archive | help
On Mon, 17 Jun 2002, Bruce Evans wrote:
> On Mon, 17 Jun 2002, Peter Jeremy wrote:
>
> > Having noticed that my system is paging far more than I would have
> > expected, I went looking and found that the 'proc-args' pool was
> > far larger than I expected. And is growing over time:
> >
> > gsmx07# vmstat -m|grep proc-args
> > proc-args701802 70634K 70634K 1589264 16,32,64,128,256
> > [about 10 minutes delay]
> > gsmx07# vmstat -m|grep proc-args;vmstat -m|grep proc-args
> > proc-args702048 70652K 70652K 1589557 16,32,64,128,256
> > proc-args702047 70652K 70652K 1589558 16,32,64,128,256
> > gsmx07#
>
> I see a relatively slow growth on a fairly idle machine (4031K after
> 3 days of uptime).
> > [bugs in sysctl_kern_proc_args]
> ...
> Otherwise OK until here, as above. We return if we are just reading
> the proc table. I would have though that this was the usual case, but
> on my fairly idle system it is 100% unusual: this sysctl is only used
> by sendmail to write sometime.
sendmail was just doing setproctitle() to usually the same value every
few seconds. Doing sysproctitle() in a loop exhausts kernel memory in a
few minutes (it should only take a few seconds, but bot the user and
kernel parts of setproctitle() are slow).
> Other bugs:
>
> if (req->oldptr && pa != NULL) {
> error = SYSCTL_OUT(req, pa->ar_args, pa->ar_length);
> }
>
> This fails to set the output parameters in the req->oldptr != NULL case.
> It also has some style bugs (only one pointer explicitly compared with NULL,
> and verbose braces).
Setting the output parameters in the req->oldptr != NULL case is not a bug
(since SYSCTL_OUT() just advances the data pointer and higher level code
sets the count later).
The following patch seems to fix all the bugs that I noticed except minor
style bugs. It was tested mainly with setproctitle() in a loop.
%%%
Index: kern_proc.c
===================================================================
RCS file: /home/ncvs/src/sys/kern/kern_proc.c,v
retrieving revision 1.136
diff -u -2 -r1.136 kern_proc.c
--- kern_proc.c 7 Jun 2002 05:41:27 -0000 1.136
+++ kern_proc.c 17 Jun 2002 13:15:39 -0000
@@ -998,6 +988,6 @@
int *name = (int*) arg1;
u_int namelen = arg2;
+ struct pargs *newpa, *pa;
struct proc *p;
- struct pargs *pa;
int error = 0;
@@ -1022,29 +1012,22 @@
pargs_hold(pa);
PROC_UNLOCK(p);
- if (req->oldptr && pa != NULL) {
+ if (req->oldptr != NULL && pa != NULL)
error = SYSCTL_OUT(req, pa->ar_args, pa->ar_length);
- }
- if (req->newptr == NULL) {
- pargs_drop(pa);
- return (error);
- }
-
- PROC_LOCK(p);
- pa = p->p_args;
- p->p_args = NULL;
- PROC_UNLOCK(p);
pargs_drop(pa);
-
- if (req->newlen + sizeof(struct pargs) > ps_arg_cache_limit)
+ if (error != 0 || req->newptr == NULL)
return (error);
- pa = pargs_alloc(req->newlen);
- error = SYSCTL_IN(req, pa->ar_args, req->newlen);
- if (!error) {
+ if (req->newlen + sizeof(struct pargs) > ps_arg_cache_limit)
+ return (ENOMEM);
+ newpa = pargs_alloc(req->newlen);
+ error = SYSCTL_IN(req, newpa->ar_args, req->newlen);
+ if (error == 0) {
PROC_LOCK(p);
- p->p_args = pa;
+ pa = p->p_args;
+ p->p_args = newpa;
PROC_UNLOCK(p);
+ pargs_drop(pa);
} else
- pargs_free(pa);
+ pargs_free(newpa);
return (error);
}
%%%
Bruce
To Unsubscribe: send mail to majordomo@FreeBSD.org
with "unsubscribe freebsd-current" in the body of the message
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20020617230109.I360-100000>