Date: Sun, 16 Jun 2002 04:10:23 -0700 From: Mike Makonnen <makonnen@pacbell.net> To: John Baldwin <jhb@FreeBSD.ORG> Cc: hiten@uk.FreeBSD.org, jrh@lab.it.uc3m.es, freebsd-current@FreeBSD.ORG Subject: Re: Fixing "could sleeep..." was (Re: ../../../vm/uma_core.c:132 Message-ID: <20020616041023.47491ea3.makonnen@pacbell.net> In-Reply-To: <XFMail.20020611183332.jhb@FreeBSD.org> References: <20020611152747.091c2377.makonnen@pacbell.net> <XFMail.20020611183332.jhb@FreeBSD.org>
next in thread | previous in thread | raw e-mail | index | archive | help
On Tue, 11 Jun 2002 18:33:32 -0400 (EDT)
John Baldwin <jhb@FreeBSD.ORG> wrote:
>
> Well, it shouldn't be but it is. :-P However, one should try to avoid holding
> locks except when necessary to maximize concurrency. Thus it is better to do
> things like malloc() and free() while not holding locks if possible.
>
> > On which way to go:
> > I like your idea better, because it is less work and less bloat. Sometimes
> > I have to keep reminding myself: "Choose the simplest design that works."
>
> Fair enough. Thanks for working on this.
>
np. It's much fun!
I don't know if you recieved my earlier email about a bug that I found in
execve() while working on fixing the "malloc w/ process lock held" bugs.
Here's a simpler patch.
It fixes possible resource leaks and failure to unlock a lock, introduced
by nectar@ in rev. 1.162 of kern/kern_exec.c, in the case where the call
to fdcheckstd() fails. Basically it fails to deallocate resources and unlock the
process lock.
Cheers,
Mike Makonnen
Index: sys/kern/kern_exec.c
===================================================================
RCS file: /home/ncvs/src/sys/kern/kern_exec.c,v
retrieving revision 1.164
diff -u -r1.164 kern_exec.c
--- sys/kern/kern_exec.c 7 Jun 2002 05:41:27 -0000 1.164
+++ sys/kern/kern_exec.c 14 Jun 2002 14:06:14 -0000
@@ -133,7 +133,7 @@
struct image_params image_params, *imgp;
struct vattr attr;
int (*img_first)(struct image_params *);
- struct pargs *oldargs, *newargs = NULL;
+ struct pargs *oldargs=NULL, *newargs = NULL;
struct procsig *oldprocsig, *newprocsig;
#ifdef KTRACE
struct vnode *tracevp = NULL;
@@ -383,8 +383,10 @@
#endif
/* Make sure file descriptors 0..2 are in use. */
error = fdcheckstd(td);
- if (error != 0)
- goto exec_fail_dealloc;
+ if (error != 0) {
+ oldcred = NULL;
+ goto done1;
+ }
/*
* Set the new credentials.
*/
@@ -467,6 +469,7 @@
p->p_args = newargs;
newargs = NULL;
}
+done1:
PROC_UNLOCK(p);
/*
@@ -486,7 +489,8 @@
if (tracevp != NULL)
vrele(tracevp);
#endif
- pargs_drop(oldargs);
+ if (oldargs != NULL)
+ pargs_drop(oldargs);
exec_fail_dealloc:
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?20020616041023.47491ea3.makonnen>