linux-kernel - Re: [PATCH] x86/mce: Avoid reading every machine check bank register twice.

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: <CA+8MBbK7=PkgfMp3qL4i+Akfg45LaQcEfZa5miYDmk79OCb3jw@mail.gmail.com>
Date:	Tue, 17 Apr 2012 14:40:43 -0700
From:	Tony Luck <tony.luck@...el.com>
To:	Borislav Petkov <bp@...64.org>
Cc:	linux-kernel@...r.kernel.org, Ingo Molnar <mingo@...e.hu>,
	Chen Gong <gong.chen@...ux.intel.com>,
	"Huang, Ying" <ying.huang@...el.com>,
	Hidetoshi Seto <seto.hidetoshi@...fujitsu.com>
Subject: Re: [PATCH] x86/mce: Avoid reading every machine check bank register twice.

On Tue, Apr 17, 2012 at 11:37 AM, Borislav Petkov <bp@...64.org> wrote:
> Why, is Waldo hidden somewhere in this patch for us to find?
I leave a couple of easy things for people to nit-pick ... it makes feel
useful when they find stuff to complain about :-)

>> +                     while (++i < banks)
>> +                             __set_bit(i, hintp);
>
> I'm guessing this is there so that whenever we have a serious MCE
> worth of panic, we want to look at the remaining banks no matter their
> validity?
Correct. I didn't want to change the functionality of do_machine_fault()
[even though it doesn't currently handle finding multiple errors very
well - we ought to allow it to see everything]
>
> Why not continue through the banks, filling up the hint bitmap but make
> a note to yourself here that we'll be returning 1 when finished?

That's probably a better idea. We don't gain much by breaking
the loop early - might as well let all the cpus do the full scan of
all the banks in parallel.

>>       DECLARE_BITMAP(toclear, MAX_NR_BANKS);
>> +     DECLARE_BITMAP(hint, MAX_NR_BANKS);
>
> Maybe call it 'toread' like the toclear above so that we know what it
> means? Or even better, valid_banks or something...?

See - you found Waldo (I suck at choosing good names). valid_banks
has a good sound to it.

Thanks - I will respin a v2 soon.

-Tony
--
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/