From: Peter Zijlstra <a.p.zijlstra@chello.nl>
To: Thomas Gleixner <tglx@linutronix.de>
Cc: Suresh Siddha <suresh.b.siddha@intel.com>,
mingo@elte.hu, hpa@zytor.com, trenn@novell.com,
prarit@redhat.com, tj@kernel.org, rusty@rustcorp.com.au,
akpm@linux-foundation.org, torvalds@linux-foundation.org,
linux-kernel@vger.kernel.org, youquan.song@intel.com,
stable@kernel.org
Subject: Re: [patch 1/4] x86, mtrr: lock stop machine during MTRR rendezvous sequence
Date: Thu, 23 Jun 2011 11:41:23 +0200 [thread overview]
Message-ID: <1308822083.1022.93.camel@twins> (raw)
In-Reply-To: <alpine.LFD.2.02.1106231131300.11814@ionos>
On Thu, 2011-06-23 at 11:33 +0200, Thomas Gleixner wrote:
> On Thu, 23 Jun 2011, Peter Zijlstra wrote:
>
> > On Wed, 2011-06-22 at 15:20 -0700, Suresh Siddha wrote:
> > > +#ifdef CONFIG_SMP
> > > + /*
> > > + * If we are not yet online, then there can be no stop_machine() in
> > > + * parallel. Stop machine ensures this by using get_online_cpus().
> > > + *
> > > + * If we are online, then we need to prevent a stop_machine() happening
> > > + * in parallel by taking the stop cpus mutex.
> > > + */
> > > + if (cpu_online(raw_smp_processor_id()))
> > > + mutex_lock(&stop_cpus_mutex);
> > > +#endif
> >
> > This reads like an optimization, is it really worth-while to not take
> > the mutex in the rare offline case?
>
> You cannot block on a mutex when you are not online, in fact you
> cannot block on it when not active, so the check is wrong anyway.
Duh, yeah. Comment totally mislead me.
On that whole active thing, so cpu_active() is brought into life to sort
an cpu-down problem, where we want the lb to stop using a cpu before we
can re-build the sched_domains.
But now we're having trouble because of that on the cpu-up part, where
we update the sched_domains too late (CPU_ONLINE) and hence also set
cpu_active() too late (again CPU_ONLINE).
Couldn't we update the sched_domain tree on CPU_PREPARE_UP to include
the new cpu and then set cpu_active() right along with cpu_online()?
That would also sort your other wait for active while bringup issue..
Note, I'll now go and have my morning juice, so the above might be total
crap.
next prev parent reply other threads:[~2011-06-23 9:42 UTC|newest]
Thread overview: 15+ messages / expand[flat|nested] mbox.gz Atom feed top
2011-06-22 22:20 [patch 0/4] MTRR rendezvous deadlock fix and cleanups using stop_machine() Suresh Siddha
2011-06-22 22:20 ` [patch 1/4] x86, mtrr: lock stop machine during MTRR rendezvous sequence Suresh Siddha
2011-06-23 9:05 ` Peter Zijlstra
2011-06-23 9:33 ` Thomas Gleixner
2011-06-23 9:41 ` Peter Zijlstra [this message]
2011-06-23 18:16 ` Suresh Siddha
2011-06-22 22:20 ` [patch 2/4] stop_machine: reorganize stop_cpus() implementation Suresh Siddha
2011-06-22 22:20 ` [patch 3/4] stop_machine: implement stop_machine_from_offline_cpu() Suresh Siddha
2011-06-23 9:25 ` Peter Zijlstra
2011-06-23 9:28 ` Tejun Heo
2011-06-23 9:31 ` Peter Zijlstra
2011-06-23 18:19 ` Suresh Siddha
2011-06-24 7:45 ` Peter Zijlstra
2011-06-24 17:55 ` Suresh Siddha
2011-06-22 22:20 ` [patch 4/4] x86, mtrr: use stop_machine() for doing MTRR rendezvous Suresh Siddha
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=1308822083.1022.93.camel@twins \
--to=a.p.zijlstra@chello.nl \
--cc=akpm@linux-foundation.org \
--cc=hpa@zytor.com \
--cc=linux-kernel@vger.kernel.org \
--cc=mingo@elte.hu \
--cc=prarit@redhat.com \
--cc=rusty@rustcorp.com.au \
--cc=stable@kernel.org \
--cc=suresh.b.siddha@intel.com \
--cc=tglx@linutronix.de \
--cc=tj@kernel.org \
--cc=torvalds@linux-foundation.org \
--cc=trenn@novell.com \
--cc=youquan.song@intel.com \
/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