Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752158Ab3HUGAc (ORCPT ); Wed, 21 Aug 2013 02:00:32 -0400 Received: from intranet.asianux.com ([58.214.24.6]:50509 "EHLO intranet.asianux.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752068Ab3HUGAb (ORCPT ); Wed, 21 Aug 2013 02:00:31 -0400 X-Spam-Score: -101.0 Message-ID: <52145741.7000008@asianux.com> Date: Wed, 21 Aug 2013 13:59:29 +0800 From: Chen Gang User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:17.0) Gecko/20130110 Thunderbird/17.0.2 MIME-Version: 1.0 To: paulmck@linux.vnet.ibm.com CC: dipankar@in.ibm.com, "linux-kernel@vger.kernel.org" Subject: Re: [PATCH] kernel/rcutree.c: deem to be lazy if there are no callbacks. References: <5212E76A.40903@asianux.com> <5212E7BB.4020602@asianux.com> <20130820041832.GY29406@linux.vnet.ibm.com> <5212F3E5.7080700@asianux.com> <5212F467.8090407@asianux.com> In-Reply-To: <5212F467.8090407@asianux.com> Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 3837 Lines: 136 If we still doubt about it, but can not find a suitable way to fix it (neither of us are familiar with it). Is it suitable to use BUG_ON() for it (the diff may like below) ? -------------------------------diff begin------------------------------- diff --git a/kernel/rcutree.c b/kernel/rcutree.c index dbf74b5..1d02659 100644 --- a/kernel/rcutree.c +++ b/kernel/rcutree.c @@ -2728,6 +2728,7 @@ static int rcu_cpu_has_callbacks(int cpu, bool *all_lazy) if (rdp->nxtlist) hc = true; } + BUG_ON(!hc && !al); if (all_lazy) *all_lazy = al; return hc; -------------------------------diff end--------------------------------- Thanks. On 08/20/2013 12:45 PM, Chen Gang wrote: > On 08/20/2013 12:43 PM, Chen Gang wrote: >> On 08/20/2013 12:18 PM, Paul E. McKenney wrote: >>> On Tue, Aug 20, 2013 at 11:51:23AM +0800, Chen Gang wrote: >>>> >>>> >>>> If 'hc' is false, 'al' will never be false, either (only need check >>>> "irdp->qlen != rdp->qlen_lazy' when 'rdp->nxtlist' existance). >>>> >>>> Recommend to improve the related code, like the diff below. >>> >>> Are you sure that this represents an improvement? If so, why? >>> >> >> If 'hc' and 'al' really has relationships, better to let 'C code' >> express it, that will make the code clearer. >> >>> Or to put it another way, I see a patch that increases the size of the >>> kernel by three lines. What is the corresponding benefit given common >>> kernel workloads? >>> >> >> For 'al', need not check for each looping, and for 'hc', may save the >> useless looping (so it can make performance better). >> >> For C code, it really increases 3 lines, but may not for assembly code >> (excuse me, I am not check it, I think it is not important, although it >> is easy to give a comparing for binary). >> > > Oh, sorry, I mean: only for our case, "it is not important". > > >>> Thanx, Paul >>> >>>> ----------------------------------diff begin------------------------------------ >>>> >>>> diff --git a/kernel/rcutree.c b/kernel/rcutree.c >>>> index 5b53a89..421caf0 100644 >>>> --- a/kernel/rcutree.c >>>> +++ b/kernel/rcutree.c >>>> @@ -2719,10 +2719,13 @@ static int rcd'_cpu_has_callbacks(int cpu, bool *all_lazy) >>>> >>>> for_each_rcu_flavor(rsp) { >>>> rdp = per_cpu_ptr(rsp->rda, cpu); >>>> - if (rdp->qlen != rdp->qlen_lazy) >>>> - al = false; >>>> - if (rdp->nxtlist) >>>> + if (rdp->nxtlist) { >>>> hc = true; >>>> + if (rdp->qlen != rdp->qlen_lazy) { >>>> + al = false; >>>> + break; >>>> + } >>>> + } >>>> } >>>> if (all_lazy) >>>> *all_lazy = al; >>>> >>>> ----------------------------------diff end-------------------------------------- >>>> >>>> >>>> On 08/20/2013 11:50 AM, Chen Gang wrote: >>>>> According to the comment above rcu_cpu_has_callbacks(): "If there are >>>>> no callbacks, all of them are deemed to be lazy". >>>>> >>>>> So when both 'hc' and 'al' are false, '*all_lazy' should be true, not >>>>> false. >>>>> >>>>> >>>>> Signed-off-by: Chen Gang >>>>> --- >>>>> kernel/rcutree.c | 2 +- >>>>> 1 files changed, 1 insertions(+), 1 deletions(-) >>>>> >>>>> diff --git a/kernel/rcutree.c b/kernel/rcutree.c >>>>> index 5b53a89..9ee9565 100644 >>>>> --- a/kernel/rcutree.c >>>>> +++ b/kernel/rcutree.c >>>>> @@ -2725,7 +2725,7 @@ static int rcu_cpu_has_callbacks(int cpu, bool *all_lazy) >>>>> hc = true; >>>>> } >>>>> if (all_lazy) >>>>> - *all_lazy = al; >>>>> + *all_lazy = !hc ? true : al; >>>>> return hc; >>>>> } >>>>> >>>>> >>>> >>>> >>>> -- >>>> Chen Gang >>>> >>> >>> >>> >> >> > > -- Chen Gang -- 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/