Re: CVS commit: src/sys

[Mindaugas Rasiukevicius <rmind%netbsd.org@localhost>]

Chuck Silvers <chuq%chuq.com@localhost> wrote:
> > > ...
> > > 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?

Seems good, except I would avoid such ad hoc #ifdefs.  How about adding
public MI routine, something like:

bool
ddb_active_p(void)
{
#ifdef DDB
        extern int db_active;
        return db_active != 0;
#else
        return false;
#endif
}

> > 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).

OK.  Can you please add a comment about it?

Thanks!

> 
> -Chuck

-- 
Mindaugas