Re: *PING* [PATCH]: x86: mce: fix mce warning with disabled lapic

[Ingo Molnar <mingo@elte.hu>]

* Andi Kleen <andi@firstfloor.org> wrote:

> *PING* can someone please take the patch in there and process the 
> other suggestions?

Firstly, a basic patch submission technical matter: could you please 
stop spamming maintainers of the x86/MCE code with such '*PING*' 
private mails, for patches you never properly submitted to begin 
with?

The proper way to submit an upstream kernel fix, as you should well 
be aware of, is to send a patch with a proper title and to Cc: it to 
lkml and the maintainers affected. You never did that, you only 
posted a for-testing patch into a discussion. Please stop this 
self-important posturing, it's somewhat annoying.

Also, another, patch log quality issue, please credit Johannes 
properly. You put this into the changelog:

> Originally reported by Johannes Stezenbach 
> 
> This is a 2.6.31 candidate because it fixes a regression.
> 
> Signed-off-by: Andi Kleen <ak@linux.intel.com>

The proper way is to put this into the changelog:

  Reported-by: Johannes Stezenbach <js@sig21.net>

And given that Johannes also tested the patch, another line of:

  Tested-by: Johannes Stezenbach <js@sig21.net>

Would be appropriate as well.

Thirdly, we can do better with the fix itself too:

> +++ linux/arch/x86/kernel/cpu/mcheck/therm_throt.c
> @@ -236,6 +236,9 @@ void intel_init_thermal(struct cpuinfo_x
>  	int tm2 = 0;
>  	u32 l, h;
>  
> +	if (!cpu_has_apic || disable_apic)
> +		return;
> +
>  	/* Thermal monitoring depends on ACPI and clock modulation*/
>  	if (!cpu_has(c, X86_FEATURE_ACPI) || !cpu_has(c, X86_FEATURE_ACC))
>  		return;

we already have that X86_FEATURE_ACPI and X86_FEATURE_ACC check and 
a return statement. Would be better to expand that with the APIC 
checks. Plus update the comment to also mention APIC as a 
requirement plus fix the small error in the comment too while at it.

If these problems are fixed i'll apply the fix to tip:x86/urgent.

Thanks,

	Ingo