RE: [PATCHV3 1/3] x86, ras: Add new infrastructure for machine check fixup tables

From: "Luck, Tony" <tony.luck@intel.com>
To: Andy Lutomirski <luto@amacapital.net>
Cc: Ingo Molnar <mingo@kernel.org>, Borislav Petkov <bp@alien8.de>,
	"Andrew Morton" <akpm@linux-foundation.org>,
	Andy Lutomirski <luto@kernel.org>,
	"Williams, Dan J" <dan.j.williams@intel.com>,
	Robert <elliott@hpe.com>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	"linux-mm@kvack.org" <linux-mm@kvack.org>,
	linux-nvdimm <linux-nvdimm@ml01.01.org>, X86 ML <x86@kernel.org>
Subject: RE: [PATCHV3 1/3] x86, ras: Add new infrastructure for machine check fixup tables
Date: Wed, 16 Dec 2015 22:51:04 +0000	[thread overview]
Message-ID: <3908561D78D1C84285E8C5FCA982C28F39F87180@ORSMSX114.amr.corp.intel.com> (raw)
In-Reply-To: <CALCETrVHqi9ixUQbeN82T14CVom1N6QegSNR+r=jtjRgcfC0kg@mail.gmail.com>

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset="utf-8", Size: 912 bytes --]

> Looks generally good.
>
> Reviewed-by: Andy Lutomirski <luto@kernel.org>

You say that to part 1/3 ... what happens when you get to part 3/3 and you
read my attempts at writing x86 assembly code?

>> +#ifdef CONFIG_MCE_KERNEL_RECOVERY
>> +int fixup_mcexception(struct pt_regs *regs)
>> +{
>> +       const struct exception_table_entry *fixup;
>> +       unsigned long new_ip;
>> +
>> +       fixup = search_mcexception_tables(regs->ip);
>> +       if (fixup) {
>> +               new_ip = ex_fixup_addr(fixup);
>> +
>> +               regs->ip = new_ip;
>
>  You could very easily save a line of code here :)

Two lines (the declaration of the variable can go away as well).
Will include if we need a V4 when everyone else gets to commenting.

-Tony
ÿôèº{.nÇ+‰·Ÿ®‰†+%ŠËÿ±éÝ¶\x17¥Šwÿº{.nÇ+‰·¥Š{±þG«éÿŠ{ayº\x1dÊ‡Ú™ë,j\a¢f£¢·hšïêÿ‘êçz_è®\x03(éšŽŠÝ¢j"ú\x1a¶^[m§ÿÿ¾\a«þG«éÿ¢¸?™¨èÚ&£ø§~á¶iO•æ¬z·švØ^\x14\x04\x1a¶^[m§ÿÿÃ\fÿ¶ìÿ¢¸?–I¥