Re: CVS commit: src/sys

[Chuck Silvers]

On Wed, Jul 07, 2010 at 02:38:58PM +0100, Mindaugas Rasiukevicius wrote:
> Hi,
> 
> > Module Name:    src
> > Committed By:   chs
> > Date:           Wed Jul  7 01:30:38 UTC 2010
> > 
> > Modified Files:
> > ...
> > ...
> > ...
> > cvs rdiff -u -r1.150 -r1.151 src/sys/kern/kern_lwp.c
> > cvs rdiff -u -r1.167 -r1.168 src/sys/kern/kern_proc.c
> > ...
> 
> I have added assert on proc_lock into proc_find(), but not proc_find_raw(),
> because the later is for special cases, including DDB, where we do not hold
> the locks.  DDB would fire asserts now.

oops, I was fooled by the comment right before it.
the attached patch has proc_find_raw() check for either proc_lock being held
or DDB being active, does that look better?


> Also, from lwp_exit():
> 
> +     if ((l->l_pflag & LP_PIDLID) != 0 && l->l_lid != p->p_pid) {
> +             proc_free_pid(l->l_lid);
> +     }
> 
> The lid != pid check is a micro-optimisation for one-LWP case, right?

no, it's to avoid freeing the pid twice (since proc_free() will free it too).

-Chuck

Index: src/sys/kern/kern_proc.c
===================================================================
RCS file: /cvsroot/src/sys/kern/kern_proc.c,v
retrieving revision 1.168
diff -u -p -r1.168 kern_proc.c
--- src/sys/kern/kern_proc.c    7 Jul 2010 01:30:37 -0000       1.168
+++ src/sys/kern/kern_proc.c    9 Jul 2010 16:27:52 -0000
@@ -68,6 +68,7 @@ __KERNEL_RCSID(0, "$NetBSD: kern_proc.c,
 #include "opt_kstack.h"
 #include "opt_maxuprc.h"
 #include "opt_dtrace.h"
+#include "opt_ddb.h"
 #endif
 
 #include <sys/param.h>
@@ -101,6 +102,13 @@ __KERNEL_RCSID(0, "$NetBSD: kern_proc.c,
 #include <uvm/uvm.h>
 #include <uvm/uvm_extern.h>
 
+#ifdef DDB
+extern int db_active;
+#define DB_ACTIVE db_active
+#else
+#define DB_ACTIVE 0
+#endif
+
 /*
  * Other process lists
  */
@@ -506,9 +514,9 @@ p_inferior(struct proc *p, struct proc *
 }
 
 /*
- * proc_find: locate a process by the ID.
+ * proc_find_raw: locate a process by PID.
  *
- * => Must be called with proc_lock held.
+ * => Must be called with proc_lock held or from DDB.
  */
 proc_t *
 proc_find_raw(pid_t pid)
@@ -516,7 +524,7 @@ proc_find_raw(pid_t pid)
        struct pid_table *pt;
        proc_t *p;
 
-       KASSERT(mutex_owned(proc_lock));
+       KASSERT(mutex_owned(proc_lock) || DB_ACTIVE);
        pt = &pid_table[pid & pid_tbl_mask];
        p = pt->pt_proc;
        if (__predict_false(!P_VALID(p) || pt->pt_pid != pid)) {
@@ -525,11 +533,17 @@ proc_find_raw(pid_t pid)
        return p;
 }
 
+/*
+ * proc_find: locate a live process by PID.
+ *
+ * => Must be called with proc_lock held.
+ */
 proc_t *
 proc_find(pid_t pid)
 {
        proc_t *p;
 
+       KASSERT(mutex_owned(proc_lock));
        p = proc_find_raw(pid);
        if (__predict_false(p == NULL)) {
                return NULL;