All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Paul E. McKenney" <paulmck@linux.vnet.ibm.com>
To: Pranith Kumar <bobby.prani@gmail.com>
Cc: Josh Triplett <josh@joshtriplett.org>,
	Steven Rostedt <rostedt@goodmis.org>,
	Mathieu Desnoyers <mathieu.desnoyers@efficios.com>,
	Lai Jiangshan <laijs@cn.fujitsu.com>,
	"open list:READ-COPY UPDATE..." <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH v2 2/2] rcu: Check the return value of rcu_nocb_mask cpumask allocation
Date: Fri, 25 Jul 2014 11:06:32 -0700	[thread overview]
Message-ID: <20140725180632.GA6230@linux.vnet.ibm.com> (raw)
In-Reply-To: <20140725152551.GC11241@linux.vnet.ibm.com>

On Fri, Jul 25, 2014 at 08:25:51AM -0700, Paul E. McKenney wrote:
> On Thu, Jul 24, 2014 at 08:37:32PM -0400, Pranith Kumar wrote:
> > This commit checks the return value of the zalloc_cpumask_var() used for
> > allocating cpumask for rcu_nocb_mask.
> > 
> > Signed-off-by: Pranith Kumar <bobby.prani@gmail.com>
> 
> Hmmm...  I saw the check in the previous patch, but didn't see removal
> of the later have_rcu_nocb_mask check.  Please see below.

And events overtook this one.  Turns out that commit b58cc46c5f6b
(rcu: Don't offload callbacks unless specifically requested) was not
one of my best efforts.

I will adapt your patch 2/2 in this series and apply it with your
Signed-off-by.  Patch 1/2 is obsoleted by the current fix for
commit b58cc46c5f6b.

							Thanx, Paul

> > ---
> > v2: no change from v1
> > 
> >  kernel/rcu/tree_plugin.h | 23 ++++++++++++-----------
> >  1 file changed, 12 insertions(+), 11 deletions(-)
> > 
> > diff --git a/kernel/rcu/tree_plugin.h b/kernel/rcu/tree_plugin.h
> > index 520538a..9c9a01c 100644
> > --- a/kernel/rcu/tree_plugin.h
> > +++ b/kernel/rcu/tree_plugin.h
> > @@ -54,7 +54,10 @@ static void __init rcu_bootup_announce_oddness_nocb(void)
> >  {
> >  #ifndef CONFIG_RCU_NOCB_CPU_NONE
> >  	if (!have_rcu_nocb_mask) {
> > -		zalloc_cpumask_var(&rcu_nocb_mask, GFP_KERNEL);
> > +		if (!zalloc_cpumask_var(&rcu_nocb_mask, GFP_KERNEL)) {
> > +			pr_info("rcu_nocb_mask allocation failed\n");
> > +			return;
> > +		}
> >  		have_rcu_nocb_mask = true;
> >  	}
> >  #ifdef CONFIG_RCU_NOCB_CPU_ZERO
> > @@ -66,17 +69,15 @@ static void __init rcu_bootup_announce_oddness_nocb(void)
> >  	cpumask_copy(rcu_nocb_mask, cpu_possible_mask);
> >  #endif /* #ifdef CONFIG_RCU_NOCB_CPU_ALL */
> >  #endif /* #ifndef CONFIG_RCU_NOCB_CPU_NONE */
> > -	if (have_rcu_nocb_mask) {
> > -		if (!cpumask_subset(rcu_nocb_mask, cpu_possible_mask)) {
> > -			pr_info("\tNote: kernel parameter 'rcu_nocbs=' contains nonexistent CPUs.\n");
> > -			cpumask_and(rcu_nocb_mask, cpu_possible_mask,
> > -				    rcu_nocb_mask);
> > -		}
> > -		cpulist_scnprintf(nocb_buf, sizeof(nocb_buf), rcu_nocb_mask);
> > -		pr_info("\tOffload RCU callbacks from CPUs: %s.\n", nocb_buf);
> > -		if (rcu_nocb_poll)
> > -			pr_info("\tPoll for callbacks from no-CBs CPUs.\n");
> > +	if (!cpumask_subset(rcu_nocb_mask, cpu_possible_mask)) {
> 
> What happens if CONFIG_RCU_NOCB_CPU_NONE=y and the rcu_nocbs= boot
> parameter is not specified and we get here?
> 
> In order to get visible failures when testing, build with
> CONFIG_CPUMASK_OFFSTACK=y.
> 
> > +		pr_info("\tNote: kernel parameter 'rcu_nocbs=' contains nonexistent CPUs.\n");
> > +		cpumask_and(rcu_nocb_mask, cpu_possible_mask,
> > +				rcu_nocb_mask);
> >  	}
> > +	cpulist_scnprintf(nocb_buf, sizeof(nocb_buf), rcu_nocb_mask);
> > +	pr_info("\tOffload RCU callbacks from CPUs: %s.\n", nocb_buf);
> > +	if (rcu_nocb_poll)
> > +		pr_info("\tPoll for callbacks from no-CBs CPUs.\n");
> >  }
> > 
> >  /*
> > -- 
> > 2.0.1
> > 


  reply	other threads:[~2014-07-25 18:06 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-07-25  0:37 [PATCH v2 1/2] rcu: Create a function for rcu_nocb_mask boot time setup Pranith Kumar
2014-07-25  0:37 ` [PATCH v2 2/2] rcu: Check the return value of rcu_nocb_mask cpumask allocation Pranith Kumar
2014-07-25 15:25   ` Paul E. McKenney
2014-07-25 18:06     ` Paul E. McKenney [this message]
2014-07-25 19:33       ` Pranith Kumar
2014-07-25 15:27 ` [PATCH v2 1/2] rcu: Create a function for rcu_nocb_mask boot time setup Paul E. McKenney

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=20140725180632.GA6230@linux.vnet.ibm.com \
    --to=paulmck@linux.vnet.ibm.com \
    --cc=bobby.prani@gmail.com \
    --cc=josh@joshtriplett.org \
    --cc=laijs@cn.fujitsu.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mathieu.desnoyers@efficios.com \
    --cc=rostedt@goodmis.org \
    /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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.