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.
> 
> Signed-off-by: Chuck Ebbert <76306.1226@compuserve.com>

Have you actually tested this? Well, probably not, it is hard to test
this. I assume you want cpu not to overheat during panics...

> --- 2.6.17-32.orig/arch/i386/kernel/crash.c
> +++ 2.6.17-32/arch/i386/kernel/crash.c
> @@ -114,7 +114,8 @@ static int crash_nmi_callback(struct pt_
>  	atomic_dec(&waiting_for_crash_ipi);
>  	/* Assume hlt works */
>  	halt();
> -	for(;;);
> +	for (;;)
> +		cpu_relax();
>  
>  	return 1;
>  }

This is useless... cpu_relax is rep nop, that only helps on
hyperthreading-enabled CPUS. Anything new enough to support
hyperthreading already has good thermal protection.

> --- 2.6.17-32.orig/arch/i386/kernel/doublefault.c
> +++ 2.6.17-32/arch/i386/kernel/doublefault.c
> @@ -44,7 +44,8 @@ static void doublefault_fn(void)
>  		}
>  	}
>  
> -	for (;;) /* nothing */;
> +	for (;;)
> +		cpu_relax();
>  }
>  
>  struct tss_struct doublefault_tss __cacheline_aligned = {

Same here. halt() would make sense here.

But this probably needs documentation, and centralizing into
kill_current_cpu() function, or something.
							Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html