linux-kernel - Re: [patch 3/4] stop_machine: implement stop_machine_from_offline

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <1308821504.1022.86.camel@twins>
Date:	Thu, 23 Jun 2011 11:31:44 +0200
From:	Peter Zijlstra <a.p.zijlstra@...llo.nl>
To:	Tejun Heo <tj@...nel.org>
Cc:	Suresh Siddha <suresh.b.siddha@...el.com>, mingo@...e.hu,
	tglx@...utronix.de, hpa@...or.com, trenn@...ell.com,
	prarit@...hat.com, rusty@...tcorp.com.au,
	akpm@...ux-foundation.org, torvalds@...ux-foundation.org,
	linux-kernel@...r.kernel.org, youquan.song@...el.com
Subject: Re: [patch 3/4] stop_machine: implement
 stop_machine_from_offline_cpu()

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@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/