linux-kernel - Re: [PATCH] stopmachine: add stopmachine

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: <200807151750.12131.rusty@rustcorp.com.au>
Date:	Tue, 15 Jul 2008 17:50:11 +1000
From:	Rusty Russell <rusty@...tcorp.com.au>
To:	Hidetoshi Seto <seto.hidetoshi@...fujitsu.com>
Cc:	linux-kernel@...r.kernel.org
Subject: Re: [PATCH] stopmachine: add stopmachine_timeout

On Tuesday 15 July 2008 11:11:34 Hidetoshi Seto wrote:
> Hi Rusty,

Hi Hidetoshi,

> However we need to be careful that the stuck CPU can restart unexpectedly.

OK, if you are worried about that race, I think we can still fix it...

> > +	for_each_online_cpu(i) {
> > +		if (!cpu_isset(i, prepared_cpus) && i != smp_processor_id()) {
> > +			bool ignore;
> > +
>
> Note that here is a window that a not-prepared frozen cpu can be thawed and
> become be prepared.
>
> > +			/* If we wanted to run on a particular CPU, and that's
> > +			 * the one which is stuck, it's a real failure. */
> > +			ignore = !cpus || !cpu_isset(i, *cpus);
> > +			printk(KERN_CRIT "stopmachine: cpu#%d seems to be "
> > +			       "stuck, %s.\n",
> > +			       i, ignore ? "ignoring" : "FAILING");
> > +			/* Unbind thread: it will exit when it sees
> > +			 * that prepared_cpus bit set. */
> > +			set_cpus_allowed(threads[i], cpu_online_map);
>
> Unbinded threads still can wake up on a cpu where they originally targeted.

What if we use:
	if (i != smp_processor_id() && !cpu_test_and_set(i, prepared_cpus)) {

instead of cpu_isset?  That means that if a CPU restarts during that window, 
either the thread will exit (because we set the bit here), or this will 
detect it.

Hmm, there's still the vague possibility that the thread doesn't schedule 
until we start a new stop_machine (and clear prepared_cpus).  We could simply 
loop in the main thread if any threads are alive, before freeing them (inside 
the lock).  A counter and notifier is the other way, but it seems like 
overkill for a very unlikely event.

Thanks for the analysis!
Rusty.
--
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/