Subject: Re: [patch 3/4] stop_machine: implement
 stop_machine_from_offline_cpu()
From: Peter Zijlstra <a.p.zijlstra@chello.nl>
To: Tejun Heo <tj@kernel.org>
Cc: Suresh Siddha <suresh.b.siddha@intel.com>, mingo@elte.hu,
        tglx@linutronix.de, hpa@zytor.com, trenn@novell.com, prarit@redhat.com,
        rusty@rustcorp.com.au, akpm@linux-foundation.org,
        torvalds@linux-foundation.org, linux-kernel@vger.kernel.org,
        youquan.song@intel.com
In-Reply-To: <20110623092845.GJ30101@htj.dyndns.org>
References: <20110622222021.904952469@sbsiddha-MOBL3.sc.intel.com>
	 <20110622222044.038298780@sbsiddha-MOBL3.sc.intel.com>
	 <1308821119.1022.84.camel@twins>  <20110623092845.GJ30101@htj.dyndns.org>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: 8BIT
Date: Thu, 23 Jun 2011 11:31:44 +0200
Message-ID: <1308821504.1022.86.camel@twins>
Mime-Version: 1.0
Sender: linux-kernel-owner@vger.kernel.org
Content-Length: 2427
Lines: 54

On Thu, 2011-06-23 at 11:28 +0200, Tejun Heo wrote:
> Hello, Peter.
> 
> On Thu, Jun 23, 2011 at 11:25:19AM +0200, Peter Zijlstra wrote:
> > On Wed, 2011-06-22 at 15:20 -0700, Suresh Siddha wrote:
> > > +int stop_machine_from_offline_cpu(int (*fn)(void *), void *data,
> > > +                                 const struct cpumask *cpus)
> > > +{
> > > +       struct stop_machine_data smdata = { .fn = fn, .data = data,
> > > +                                           .active_cpus = cpus };
> > > +       struct cpu_stop_done done;
> > > +       int ret;
> > > +
> > > +       /* Local CPU must be offline and CPU hotplug in progress. */
> > > +       BUG_ON(cpu_online(raw_smp_processor_id()));
> > > +       smdata.num_threads = num_online_cpus() + 1;     /* +1 for local */
> > > +
> > > +       /* No proper task established and can't sleep - busy wait for lock. */
> > > +       while (!mutex_trylock(&stop_cpus_mutex))
> > > +               cpu_relax();
> > > +
> > > +       /* Schedule work on other CPUs and execute directly for local CPU */
> > > +       set_state(&smdata, STOPMACHINE_PREPARE);
> > > +       cpu_stop_init_done(&done, num_online_cpus());
> > > +       queue_stop_cpus_work(cpu_online_mask, stop_machine_cpu_stop, &smdata,
> > > +                            &done);
> > > +       ret = stop_machine_cpu_stop(&smdata);
> > > +
> > > +       /* Busy wait for completion. */
> > > +       while (!completion_done(&done.completion))
> > > +               cpu_relax();
> > > +
> > > +       mutex_unlock(&stop_cpus_mutex);
> > > +       return ret ?: done.ret;
> > > +} 
> > 
> > Damn thats ugly, I sure hope you're going to make those hardware folks
> > pay for this :-)
> 
> Oh, I agree it's fugly.  It's trying to orchestrate stop_machine from
> a CPU which doesn't have proper scheduler/task set up.  At least it's
> contained in a single relatively short function.

Yeah, no complaints on that grounds, its just sad we need to actually do
it, but given its a hardware constraint we're working with we don't have
much choice.

Just wanted to make sure Suresh (and possibly others) are going to tell
the hardware folks that such constraints are not cool.
--
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/