Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S933992AbbENVDL (ORCPT ); Thu, 14 May 2015 17:03:11 -0400 Received: from mx1.redhat.com ([209.132.183.28]:39759 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S933779AbbENVDI (ORCPT ); Thu, 14 May 2015 17:03:08 -0400 Date: Thu, 14 May 2015 22:15:29 +0100 From: Alexander Gordeev To: Steven Rostedt Cc: "Paul E. McKenney" , linux-kernel@vger.kernel.org, mingo@kernel.org, laijs@cn.fujitsu.com, dipankar@in.ibm.com, akpm@linux-foundation.org, mathieu.desnoyers@efficios.com, josh@joshtriplett.org, tglx@linutronix.de, peterz@infradead.org, dhowells@redhat.com, edumazet@google.com, dvhart@linux.intel.com, fweisbec@gmail.com, oleg@redhat.com, bobby.prani@gmail.com Subject: Re: [PATCH tip/core/rcu 06/24] rcu: Cleanup rcu_init_geometry() code and arithmetics Message-ID: <20150514211528.GA25221@agordeev.usersys.redhat.com> References: <20150512223034.GA1396@linux.vnet.ibm.com> <1431469854-3826-1-git-send-email-paulmck@linux.vnet.ibm.com> <1431469854-3826-6-git-send-email-paulmck@linux.vnet.ibm.com> <20150512232232.3f3b23c6@grimm.local.home> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20150512232232.3f3b23c6@grimm.local.home> User-Agent: Mutt/1.5.23 (2014-03-12) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 2504 Lines: 89 On Tue, May 12, 2015 at 11:22:32PM -0400, Steven Rostedt wrote: > On Tue, 12 May 2015 15:30:36 -0700 > "Paul E. McKenney" wrote: > > > @@ -4103,24 +4102,21 @@ static void __init rcu_init_geometry(void) > > return; > > } > > > > + /* Calculate the number of levels in the tree. */ > > + for (i = 0; nr_cpu_ids > rcu_capacity[i]; i++) { > > Should this start at i = 1 as it use to? Also, should there be a safety > check too: > > for (i = 1; i <= MAX_RCU_LVLS && nr_cpu_ids > rcu_capacity[i]; i++) { The safety check is not needed as it indirectly tried few lines above: if (nr_cpu_ids > rcu_capacity[MAX_RCU_LVLS]) panic("rcu_init_geometry: rcu_capacity[] is too small"); Starting at i = 0 appears indeed incorrect. In case NR_CPUS of 1 that might yield rcu_num_lvls = 0, which is wrong. I will check it. > > > + } > > + rcu_num_lvls = i; > > + > > /* Calculate the number of rcu_nodes at each level of the tree. */ > > - for (i = 1; i <= MAX_RCU_LVLS; i++) > > - if (nr_cpu_ids <= rcu_capacity[i]) { > > - for (j = 0; j <= i; j++) { > > - int cap = rcu_capacity[i - j]; > > - num_rcu_lvl[j] = DIV_ROUND_UP(nr_cpu_ids, cap); 1. In case j == i, num_rcu_lvl[j] = nr_cpu_ids > > - } > > - rcu_num_lvls = i; > > - for (j = i + 1; j <= MAX_RCU_LVLS; j++) > > - num_rcu_lvl[j] = 0; > > - break; > > - } > > + for (i = 0; i < rcu_num_lvls; i++) { > > Hmm, up above we have: for (j = 0; j <= i; j++) > > and now we have rcu_num_lvls = i, so shouldn't this be; > > for (i = 0; i <= rcu_num_lvls; i++) > > ? No, it should not. See [1] above and [2] below. > -- Steve > > > + int cap = rcu_capacity[rcu_num_lvls - i]; > > + num_rcu_lvl[i] = DIV_ROUND_UP(nr_cpu_ids, cap); > > + } > > > > /* Calculate the total number of rcu_node structures. */ > > rcu_num_nodes = 0; > > - for (i = 0; i <= MAX_RCU_LVLS; i++) > > + for (i = 0; i < rcu_num_lvls; i++) > > rcu_num_nodes += num_rcu_lvl[i]; > > - rcu_num_nodes -= nr_cpu_ids; So nr_cpu_ids is added rcu_num_nodes in the cycle and subtracted from rcu_num_nodes afterwards. The new version of code does not neigher [1] nor [2]. > > } > > > > void __init rcu_init(void) > -- Regards, Alexander Gordeev agordeev@redhat.com -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/