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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <20111218082708.GC4144@elte.hu>
Date:	Sun, 18 Dec 2011 09:27:08 +0100
From:	Ingo Molnar <mingo@...e.hu>
To:	"Luck, Tony" <tony.luck@...el.com>
Cc:	linux-kernel@...r.kernel.org,
	Chen Gong <gong.chen@...ux.intel.com>,
	Borislav Petkov <bp@...en8.de>
Subject: Re: [git pull] RAS changes (queue for 3.3)


* Luck, Tony <tony.luck@...el.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
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/