Re: [patch 3/4] stop_machine: implement stop_machine_from_offline_cpu()

[Peter Zijlstra <a.p.zijlstra@chello.nl>]

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.