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 <gang.c...@asianux.com> >>>>> --- >>>>> 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 majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/