Re: [patch] i386: cpu_relax() in crash.c and doublefault.c

[Pavel Machek <pavel@ucw.cz>]

Hi!

> > Add cpu_relax() to infinite loops in crash.c and
> > doublefault.c.  This is the safest change.
> 
> Thanks for your continued work on this!
> 
> What's the reasoning for not running halt() in doublefault_fn()?
> 
> 
> In order to consolidate those places to safely halt a CPU,
> I could think of (possibly in a header file):
> 
> /* very, very safely halt CPU:
>    - do minimal checking in case CPU might already be overheated (unreliable!)
>      (also use inlining to avoid call overhead on an unreliable CPU)
>    - try to use halt() and cpu_relax() very liberally to keep the crashed
>      CPU as cool as possible (crash might have happened due to CPU fan failure!)
>      While ACPI specifies CPU shutdown on over-temperature, we really don't
>      want to rely on this since it might be broken or we simply don't use ACPI
>      mode at all...
> */ 
> inline void safely_halt_cpu(int do_minimal_checking)
> {
> 	/* inlining will optimize the branching away */
> 	if (!do_minimal_checking) {
> 		if (cpu_data[smp_processor_id()].hlt_works_ok)
> 			for (;;) {
> 				halt();

Should not halt() check that itself?

And you probably want to disable interrupts here...

> 				/* halt failed? still make sure to cpu_relax()! */
> 				cpu_relax();
> 			}
> 		else
> 			for (;;) {
> 				cpu_relax();
> 				cpu_relax();
> 				cpu_relax();
> 			}
> 	} else {
> 		halt();
> 		/* halt didn't work, so still keep as cool as possible: */
> 		for (;;) {
> 			cpu_relax();
> 			cpu_relax();
> 			cpu_relax();
> 		}
> 	}
> }
> 
> Does my preliminary code even make any sense at all? ;)
> Might want to cleverly rearrange it to try to get rid of the cpu_relax()
> duplication while not abandoning any advantage of those different
> conditions.

									Pavel

-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html