linux-kernel - Re: [Patch -v4 1/4] Migrate shutdown/reboot to boot cpu.

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]

Date:	Wed, 17 Apr 2013 09:46:53 +0200
From:	Ingo Molnar <mingo@...nel.org>
To:	Robin Holt <holt@....com>
Cc:	Ingo Molnar <mingo@...hat.com>, Russ Anderson <rja@....com>,
	Shawn Guo <shawn.guo@...aro.org>,
	Oleg Nesterov <oleg@...hat.com>,
	Andrew Morton <akpm@...ux-foundation.org>,
	"H. Peter Anvin" <hpa@...or.com>, Joe Perches <joe@...ches.com>,
	Lai Jiangshan <laijs@...fujitsu.com>,
	Linus Torvalds <torvalds@...ux-foundation.org>,
	Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
	Michel Lespinasse <walken@...gle.com>,
	"Paul E. McKenney" <paulmck@...ux.vnet.ibm.com>,
	Paul Mackerras <paulus@...ba.org>,
	Peter Zijlstra <peterz@...radead.org>,
	"rusty@...tcorp.com.au" <rusty@...tcorp.com.au>,
	Tejun Heo <tj@...nel.org>,
	the arch/x86 maintainers <x86@...nel.org>,
	Thomas Gleixner <tglx@...utronix.de>
Subject: Re: [Patch -v4 1/4] Migrate shutdown/reboot to boot cpu.

* Robin Holt <holt@....com> wrote:

> > 		reboot_cpu_id = cpumask_first(cpu_online_mask);
> > 
> > > Also, does this codepath prevent hotplug from going on in parallel?
> > 
> > Not sure.  I have not considered hotplug.  I will look that over when I
> > am in the office.
> 
> OK.  I have been mulling this over for a bit and I don't think I
> understand what you are asking.

Well, I just saw the apparently naked use of cpu_online_mask, and asked myself 
whether that's safe against hotplug.

Upstream we had two methods:

 - historical: just reboot on any random CPU we happen to run on
 - current: offline all nonboot CPUs then reboot on the boot CPU

Both methods were implicitly "CPU hotplug safe", no locking needed, because either 
they didn't need any, or because it used disable_nonboot_cpus() which is a hotplug 
safe method.

Now your patches change this to:

 - migrate to CPU#0 [if possible] and reboot there

Given that on a system CPU-hotplugging might be executing on any given CPU, if 
reboot is running on another you have to consider the interactions. The previous 
historic and current upstream method was reasonably hotplug safe - yours I'm not 
sure about, there's just no hotplug locking in it, etc?

> I would expect that if an architecture depends upon a certain cpu for 
> shutdown/reboot/halt/suspend/hibernate and that support has been compiled in, 
> then the arch should be preventing that cpu from being removed. I do not know 
> how that would work and think that is far beyond the scope of the initial 
> problem I have been trying to solve.  If that is your question, I certainly do 
> not know how to address it.  I get the feeling this is off your mark due to the 
> "parallel" wording above.

What you mention here should indeed already be handled by the architecture hotplug 
code (for example on x86 the boot CPU cannot be hot-removed).

But beyond that, your use of cpu_online_map is AFAICS not hotplug-safe. For 
example a possible race would be that another CPU might be not-unplugging a CPU 
and you try to reboot-migrate to it.

Thanks,

	Ingo
--
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/