Messages in this thread | | | Date | Wed, 12 Aug 2009 13:59:20 +0200 | From | Ingo Molnar <> | Subject | Re: *PING* [PATCH]: x86: mce: fix mce warning with disabled lapic |
| |
* 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
| |