> From: Peter Jeremy <[EMAIL PROTECTED]>
> To: [EMAIL PROTECTED]
> Subject: proc-args (M_PARGS) leakage
> Date: Mon, 17 Jun 2002 13:15:03 +1000
> Message-id: <[EMAIL PROTECTED]>
> User-Agent: Mutt/1.2.5.1i
> Sender: [EMAIL PROTECTED]
> 
> This is -CURRENT from 7th May so it's possible the bug has been
> fixed, though there's nothing obvious in either the CVS commit
> logs or by diffing the relevant files.
> 
> 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# 
> 
> Unfortunately, M_PARGS is not the easiest pool to track allocations
> and de-allocations.  Having gone through the references to pargs_*()
> and p_args, I can't see any obvious cause of this.

This is definately failure to drop a reference.

> Whilst I'm fairly certain it's not my problem, sysctl_kern_proc_args()
> (1.136) looks dubious:

  [ ... ]

> (And later code shows pa dead at this point).  I don't follow this.
> pargs_drop(pa) deletes a single reference count - which matches the
> line "p->p_args = NULL;" - but I don't see anything to match the
> pargs_hold(pa) above.

You are correct, One reference too few is dropped when a proc's pargs
is changed. This would mean that every call to setproctitle(3) would
cause a leak.

Please let me know if this fixes your problem:

Index: kern_proc.c
===================================================================
RCS file: /home/ncvs/src/sys/kern/kern_proc.c,v
retrieving revision 1.136
diff -u -r1.136 kern_proc.c
--- kern_proc.c 7 Jun 2002 05:41:27 -0000       1.136
+++ kern_proc.c 21 Jun 2002 08:04:17 -0000
@@ -1024,10 +1024,9 @@
        if (req->oldptr && pa != NULL) {
                error = SYSCTL_OUT(req, pa->ar_args, pa->ar_length);
        }
-       if (req->newptr == NULL) {
-               pargs_drop(pa);
+       pargs_drop(pa);
+       if (req->newptr == NULL)
                return (error);
-       }
 
        PROC_LOCK(p);
        pa = p->p_args;

> Additionally, whilst I'm certain it's not my problem,
> fill_kinfo_proc() copys a reference to pargs, but doesn't increment
> the reference counter (using pargs_hold()).

You are correct; this is intentional. The kinfo stuff is terribly broken
currently. Many fields, pargs being one of them, are broken. A kinfo
struct is passed to userland, so it can't hold any references to kernel
objects. Really, kinfo should contain a copy of the string, not a pargs
pointer.

-- 
Jonathan Mini <[EMAIL PROTECTED]>
http://www.freebsd.org/

To Unsubscribe: send mail to [EMAIL PROTECTED]
with "unsubscribe freebsd-current" in the body of the message

Reply via email to