From owner-svn-src-all@FreeBSD.ORG Thu Aug 28 08:47:16 2014 Return-Path: Delivered-To: svn-src-all@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:1900:2254:206a::19:1]) (using TLSv1 with cipher ADH-AES256-SHA (256/256 bits)) (No client certificate requested) by hub.freebsd.org (Postfix) with ESMTPS id 933421C0; Thu, 28 Aug 2014 08:47:16 +0000 (UTC) Received: from mail-we0-x22a.google.com (mail-we0-x22a.google.com [IPv6:2a00:1450:400c:c03::22a]) (using TLSv1 with cipher ECDHE-RSA-RC4-SHA (128/128 bits)) (Client CN "smtp.gmail.com", Issuer "Google Internet Authority G2" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id 85C1113A2; Thu, 28 Aug 2014 08:47:15 +0000 (UTC) Received: by mail-we0-f170.google.com with SMTP id p10so425504wes.29 for ; Thu, 28 Aug 2014 01:47:13 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20120113; h=date:from:to:cc:subject:message-id:references:mime-version :content-type:content-disposition:in-reply-to:user-agent; bh=pNFmtlZ6tuMqMwUXeu485hvKN0KHvGNcJ5PdoPiKjU8=; b=fCp/fuBd3w7QVXqyWq4MsGzqNFi6WzQeHmi2dcnRfz8qgmdivY+DoIWS/3Qe0jr9wo IGGYnNbU2aY3AxlcN2nlI8HsdMCrpR35MTZnsm616edn0gWpN07Sai3pY3L5vB7RCOFQ coS0RW6p3rwFP1jkKHlpGOXsPvMXJDaneFRPgDSpal62PTcrO8rjsdLafBZIGFSjmimr kmmGbMCxpepgz2UgprIzf1rey0ByY9Sy9Rqnv57CmcUXYDEOCrWNzIXpCINeu96E4SK+ euht2R54k4lVgzCkCW8C8KOnht7DbsMGbbTwF9NH/iWyLI1PcBStktA6VXsxjoKBwv5F YYzw== X-Received: by 10.195.11.234 with SMTP id el10mr3197877wjd.95.1409215633651; Thu, 28 Aug 2014 01:47:13 -0700 (PDT) Received: from dft-labs.eu (n1x0n-1-pt.tunnel.tserv5.lon1.ipv6.he.net. [2001:470:1f08:1f7::2]) by mx.google.com with ESMTPSA id cj2sm33136078wid.23.2014.08.28.01.47.12 for (version=TLSv1.2 cipher=RC4-SHA bits=128/128); Thu, 28 Aug 2014 01:47:12 -0700 (PDT) Date: Thu, 28 Aug 2014 10:47:10 +0200 From: Mateusz Guzik To: Konstantin Belousov Subject: Re: svn commit: r270444 - in head/sys: kern sys Message-ID: <20140828084709.GB29429@dft-labs.eu> 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> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline In-Reply-To: <20140828082139.GK2737@kib.kiev.ua> User-Agent: Mutt/1.5.21 (2010-09-15) Cc: src-committers@freebsd.org, John Baldwin , Mateusz Guzik , svn-src-all@freebsd.org, svn-src-head@freebsd.org, John-Mark Gurney X-BeenThere: svn-src-all@freebsd.org X-Mailman-Version: 2.1.18-1 Precedence: list List-Id: "SVN commit messages for the entire src tree \(except for " user" and " projects" \)" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 28 Aug 2014 08:47:16 -0000 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