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

From: Tony Luck
Date: Tue Apr 17 2012 - 17:40:49 EST


On Tue, Apr 17, 2012 at 11:37 AM, Borislav Petkov <bp@xxxxxxxxx> 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@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/