public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: "Paul E. McKenney" <paulmck@linux.vnet.ibm.com>
To: Lai Jiangshan <laijs@cn.fujitsu.com>
Cc: Andrew Morton <akpm@linux-foundation.org>,
	Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
	mingo@elte.hu
Subject: Re: [PATCH]rcu classic: update qlen when cpu offline
Date: Thu, 26 Jun 2008 07:13:24 -0700	[thread overview]
Message-ID: <20080626141324.GA10859@linux.vnet.ibm.com> (raw)
In-Reply-To: <4862F9B3.5030300@cn.fujitsu.com>

On Thu, Jun 26, 2008 at 10:06:43AM +0800, Lai Jiangshan wrote:
> 
> When callbacks are moved from offline cpu to this cpu,
> the qlen field of this rdp should be updated.

Good catch!!!

The effect of this bug would be for force_quiescent_state() to be invoked
when it should not and vice versa -- wasting cycles in the first case
and letting RCU callbacks remain piled up in the second case.  The bug
is thus "benign" in that it does not result in premature grace-period
termination, but should of course be fixed nonetheless.

Preemption is disabled by the caller's get_cpu_var(), so we are guaranteed
to remain on the same CPU, as required.  The local_irq_disable() is indeed
needed, otherwise, an interrupt might invoke call_rcu() or call_rcu_bh(),
which could cause that interrupt's increment of ->qlen to be lost.

So this patch looks correct to me.  Good job, Jiangshan!!!

Ingo, would you be willing to add this patch to tip/core/rcu?

Reviewed-by: Paul E. McKenney <paulmck@linux.vnet.ibm.com>

> Signed-off-by: Lai Jiangshan <laijs@cn.fujitsu.com>
> ---
> diff --git a/kernel/rcuclassic.c b/kernel/rcuclassic.c
> index f4ffbd0..42aead6 100644
> --- a/kernel/rcuclassic.c
> +++ b/kernel/rcuclassic.c
> @@ -373,6 +373,10 @@ static void __rcu_offline_cpu(struct rcu_data *this_rdp,
>  	rcu_move_batch(this_rdp, rdp->donelist, rdp->donetail);
>  	rcu_move_batch(this_rdp, rdp->curlist, rdp->curtail);
>  	rcu_move_batch(this_rdp, rdp->nxtlist, rdp->nxttail);
> +
> +	local_irq_disable();
> +	this_rdp->qlen += rdp->qlen;
> +	local_irq_enable();
>  }
> 
>  static void rcu_offline_cpu(int cpu)
> 
> 

  reply	other threads:[~2008-06-26 14:13 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2008-06-26  2:06 [PATCH]rcu classic: update qlen when cpu offline Lai Jiangshan
2008-06-26 14:13 ` Paul E. McKenney [this message]
2008-07-13 21:13   ` Ingo Molnar

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20080626141324.GA10859@linux.vnet.ibm.com \
    --to=paulmck@linux.vnet.ibm.com \
    --cc=akpm@linux-foundation.org \
    --cc=laijs@cn.fujitsu.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mingo@elte.hu \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox