From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753417AbbCJOw2 (ORCPT ); Tue, 10 Mar 2015 10:52:28 -0400 Received: from e31.co.us.ibm.com ([32.97.110.149]:48534 "EHLO e31.co.us.ibm.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752717AbbCJOw0 (ORCPT ); Tue, 10 Mar 2015 10:52:26 -0400 Date: Tue, 10 Mar 2015 07:52:21 -0700 From: "Paul E. McKenney" To: Alexander Gordeev Cc: linux-kernel@vger.kernel.org Subject: Re: [PATCH 00/10] rcu: Cleanup RCU tree initialization Message-ID: <20150310145220.GM5708@linux.vnet.ibm.com> Reply-To: paulmck@linux.vnet.ibm.com References: <20150309214021.GD5236@linux.vnet.ibm.com> <20150309233947.GA16995@linux.vnet.ibm.com> <20150309234943.GA23932@linux.vnet.ibm.com> <20150310044318.GA9439@linux.vnet.ibm.com> <20150310143939.GB5084@agordeev.usersys.redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: <20150310143939.GB5084@agordeev.usersys.redhat.com> User-Agent: Mutt/1.5.21 (2010-09-15) X-TM-AS-MML: disable X-Content-Scanned: Fidelis XPS MAILER x-cbid: 15031014-8236-0000-0000-00000A100D8B Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Tue, Mar 10, 2015 at 02:39:39PM +0000, Alexander Gordeev wrote: > On Mon, Mar 09, 2015 at 09:43:18PM -0700, Paul E. McKenney wrote: > > On Mon, Mar 09, 2015 at 04:49:43PM -0700, Paul E. McKenney wrote: > > > On Mon, Mar 09, 2015 at 04:39:47PM -0700, Paul E. McKenney wrote: > > > > On Mon, Mar 09, 2015 at 02:40:21PM -0700, Paul E. McKenney wrote: > > > > > On Mon, Mar 09, 2015 at 09:34:04AM +0100, Alexander Gordeev wrote: > > > > > > Hi Paul, > > > > > > > > > > > > Here is cleanup of RCU tree initialization rebased on linux-rcu rcu/next > > > > > > repo, as you requested. Please, note an extra patch #10 that was not > > > > > > present in the first post. > > > > > > > > > > > > The series successfully passes kernel build test with CONFIG_RCU_FANOUT > > > > > > and CONFIG_RCU_FANOUT_LEAF equal to 5. > > > > > > > > > > I queued up 1-9, as discussed and have started testing. I will let you > > > > > know how it goes. > > > > > > > > Initial testing went well except for the following warning: > > > > > > > > /home/paulmck/public_git/linux-rcu/kernel/rcu/tree.c: In function ‘rcu_init_one.isra.63’: > > > > /home/paulmck/public_git/linux-rcu/kernel/rcu/tree.c:3961:3: warning: ‘levelcnt[0]’ may be used uninitialized in this function [-Wmaybe-uninitialized] > > > > for (j = 0; j < levelcnt[i]; j++, rnp++) { > > > > > > > > This warning looks like a false positive to me, given that the loop > > > > near the beginning of the function initializes levelcnt[0]. Am I > > > > missing something here, and either way, what is the best way to shut > > > > this warning up? > > > > > > My suggestion is the following: > > > > > > if (rcu_num_lvls <= 0) > > > panic("rcu_init_one: rcu_num_lvls underflow"); > > > > > > Just following the other panic() in rcu_init_one(). > > > > As in the following patch. > > > > Thanx, Paul > > > > ------------------------------------------------------------------------ > > > > rcu: Shut up spurious gcc uninitialized-variable warning > > > > Because gcc doesn't realize that rcu_num_lvls must be strictly greater > > than zero, some versions give a spurious warning about levelcnt[0] being > > uninitialized in rcu_init_one(). This commit adds a panic() in that > > case in order to educate gcc on this point. > > > > Signed-off-by: Paul E. McKenney > > > > diff --git a/kernel/rcu/tree.c b/kernel/rcu/tree.c > > index bd5a9a1db048..5b42d94335e3 100644 > > --- a/kernel/rcu/tree.c > > +++ b/kernel/rcu/tree.c > > @@ -3942,6 +3942,8 @@ static void __init rcu_init_one(struct rcu_state *rsp, > > /* Silence gcc 4.8 warning about array index out of range. */ > > if (rcu_num_lvls > RCU_NUM_LVLS) > > panic("rcu_init_one: rcu_num_lvls overflow"); > > + if (rcu_num_lvls <= 0) > > + panic("rcu_init_one: rcu_num_lvls underflow"); > > I believe '... else if (rcu_num_lvls <= 0)' is more appropriate here. Good point. > But do you think keeping two static strings just to shut the compiler > worth it? May be a single complain would be enough? As in the following? if (rcu_num_lvls <= 0 || rcu_num_lvls > RCU_NUM_LVLS) panic("rcu_init_one: rcu_num_lvls out of range"); For Tree RCU, I doubt that the memory size matters, but I do like having two lines of code instead of four lines. I took this approach. Thanx, Paul > if (rcu_num_lvls <= 0 || rcu_num_lvls > RCU_NUM_LVLS) > panic("rcu_init_one: rcu_num_lvls is out of range"); > > > /* Initialize the level-tracking arrays. */ > > > > > > -- > Regards, > Alexander Gordeev > agordeev@redhat.com >