Re: [PATCH v3 1/5] x86: initialize secondary CPU only if master CPU will wait for it

[Ingo Molnar]

* Igor Mammedov <imammedo@xxxxxxxxxx> wrote:

> On Mon, 14 Apr 2014 11:16:00 +0200
> Ingo Molnar <mingo@xxxxxxxxxx> wrote:
> 
> > 
> > * Igor Mammedov <imammedo@xxxxxxxxxx> wrote:
> > 
> > >  	/*
> > > +	 * wait for ACK from master CPU before continuing
> > > +	 * with AP initialization
> > > +	 */
> > > +	cpumask_set_cpu(cpu, cpu_initialized_mask);
> > > +	while (!cpumask_test_cpu(cpu, cpu_callout_mask))
> > > +		cpu_relax();
> > 
> > > +	/*
> > > +	 * wait for ACK from master CPU before continuing
> > > +	 * with AP initialization
> > > +	 */
> > > +	cpumask_set_cpu(cpu, cpu_initialized_mask);
> > > +	while (!cpumask_test_cpu(cpu, cpu_callout_mask))
> > > +		cpu_relax();
> > 
> > That repetitive pattern could be stuck into a properly named helper 
> > inline function.
> sure
> 
> > (Also, before the cpumask_set_cpu() we should probably do a WARN_ON() 
> > if the bit is already set.)
> The reason why there is no any WARN_ON or likes is that printk is quite
> complicated, takes looks and so on. [...]

[ Yeah, I too heard that printk(), like a pretty girl, is complicated
  and makes people look twice. ]

> [...] So it's not safe at this point since
> CPU could be shot down by any time by INIT/SIPI until it's out of 
> cpu_callout_mask loop.

Not sure where you got that from, but it's not a valid concern really: 
the only place where we don't want to do a printk() is in printk code 
itself.

Debug warnings, by definition, should never trigger. If they trigger 
then they will very likely not cause lockups, but will cause the bug 
to be fixed.

Thanks,

	Ingo
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/