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>