Re: [git pull] RAS changes (queue for 3.3)

[Ingo Molnar <mingo@elte.hu>]

* Luck, Tony <tony.luck@intel.com> wrote:

> Hi Ingo,
> 
> Kicking the tires on the new version of the RAS tree at kernel.org 2.0 (so
> please say if you'd like to see my "please pull" script to format this e-mail
> differently).
> 
> Only one patch for this test - you said you liked it in https://lkml.org/lkml/2011/12/9/24
> 
> please pull from:
> 
> 	git://git.kernel.org/pub/scm/linux/kernel/git/ras/ras.git mce-inject
> 	HEAD=2c29d9dd577b74b44e580f957ea44d1df73af23a
> 
> This will update the files shown below.
> 
> Thanks!
> 
> -Tony
> 
>  arch/x86/include/asm/mce.h              |    9 ++++---
>  arch/x86/kernel/cpu/mcheck/mce-inject.c |   34 +++++++++++++++++++++++++++---
>  2 files changed, 35 insertions(+), 8 deletions(-)

Pulled, thanks Tony!

Small nit for future pulls:

> Chen Gong (1):
>       x86: add IRQ context simulation in module mce-inject

In -tip we standardized on one particular convention of 
capitalizing commit tites, i.e. the above should have been:

>       x86: Add IRQ context simulation in module mce-inject

And we also try to put (sub-)subsystem qualifiers into the 
title, i.e.:

>       x86/mce: Add IRQ context simulation in module mce-inject

Another capitalization detail is comment blocks:


+                               /*
+                                * don't wait because mce_irq_ipi is necessary
+                                * to be sync with following raise_local
+                                */

We try to capitalize these consistently as well - i.e. "Don't".

This bit also shows an ugly looking line wrapping symptom:

+                               smp_call_function_many(mce_inject_cpumask,
+                                       mce_irq_ipi, NULL, 0);

which suggests that raise_mce() would be grateful for some 
cleanup and refactoring love. The whole injection logic is now 
large and complex enough to move into its own helper function.

Btw., the printk()s could be standardized as well:

                                printk(KERN_ERR
-                               "Timeout waiting for mce inject NMI %lx\n",
+                               "Timeout waiting for mce inject %lx\n",

You could use a standard prefix like "x86 RAS: Timeout waiting 
for ..." for all RAS related messages.

These are small details, not worth rebasing your tree for, but 
would be nice to fix these things in followup cleanup commit(s).

Thanks,

	Ingo