Date: Thu, 28 Aug 2014 10:47:10 +0200 From: Mateusz Guzik <mjguzik@gmail.com> To: Konstantin Belousov <kostikbel@gmail.com> Cc: src-committers@freebsd.org, John Baldwin <jhb@freebsd.org>, Mateusz Guzik <mjg@freebsd.org>, svn-src-all@freebsd.org, svn-src-head@freebsd.org, John-Mark Gurney <jmg@funkthat.com> Subject: Re: svn commit: r270444 - in head/sys: kern sys Message-ID: <20140828084709.GB29429@dft-labs.eu> In-Reply-To: <20140828082139.GK2737@kib.kiev.ua> References: <201408240904.s7O949sI083660@svn.freebsd.org> <201408261509.26815.jhb@freebsd.org> <20140826193210.GL71691@funkthat.com> <201408261723.10854.jhb@freebsd.org> <20140826215522.GG2737@kib.kiev.ua> <20140827165432.GA28581@dft-labs.eu> <20140827185903.GJ2737@kib.kiev.ua> <20140828033009.GA29429@dft-labs.eu> <20140828082139.GK2737@kib.kiev.ua>
next in thread | previous in thread | raw e-mail | index | archive | help
On Thu, Aug 28, 2014 at 11:21:39AM +0300, Konstantin Belousov wrote: > On Thu, Aug 28, 2014 at 05:30:09AM +0200, Mateusz Guzik wrote: > > @@ -791,6 +791,8 @@ fill_kinfo_proc_only(struct proc *p, struct kinfo_proc *kp) > > struct ucred *cred; > > struct sigacts *ps; > > > > + /* For proc_realparent. */ > > + sx_assert(&proctree_lock, SX_LOCKED); > > PROC_LOCK_ASSERT(p, MA_OWNED); > > bzero(kp, sizeof(*kp)); > > > > @@ -920,7 +922,9 @@ fill_kinfo_proc_only(struct proc *p, struct kinfo_proc *kp) > > kp->ki_acflag = p->p_acflag; > > kp->ki_lock = p->p_lock; > > if (p->p_pptr) > > - kp->ki_ppid = p->p_pptr->p_pid; > > + kp->ki_ppid = proc_realparent(p)->p_pid; > Is the check for p_pptr != NULL still needed for the call to > proc_realparent() ? If yes, I think it indicates a bug in > proc_realparent(), which should incorporate the check, instead of > enforcing it on the callers. It seems to be there for the kernel process > (pid 0). As it is proc_realparent cannot fail, so after this change callers like this one would have to have some checks anyway. On the other hand other consumers don't need to worry about this edge case, so it would only add an unnecessary branch. I have no strong opinion either way, for now I decided to just commit the patch in its current form. > > If the test can be removed, and proc_realparent() called unconditionally, > I suggest to remove assert about proctree_lock at the start of > fill_kinfo_proc_only(), since the check is done in proc_realparent(). > > Whatever decision is made there, it can be implemented after your > change is landed. The patch looks fine for me. Thanks, committed as r270745. -- Mateusz Guzik <mjguzik gmail.com>
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20140828084709.GB29429>