Re: [PATCHv2] x86/mce: Look in genpool instead of mcelog.entry[] for pending error records

Borislav Petkov
2016-04-07T17:03:31Z

On Thu, Apr 07, 2016 at 09:34:06AM -0700, Tony Luck wrote:
> Couple of issues here:
> 1) MCE_LOG_LEN is only 32 - so we may have more pending records than will
> fit in the buffer on high core count cpus
> 2) During a panic we may have a lot of duplicate records because multiple
> logical cpus may have seen and logged the same error because some
> banks are shared.
>
> Switch to using the genpool to look for the pending records. Squeeze
> out duplicated records.
>
> Signed-off-by: Tony Luck <tony.luck@intel.com>
> ---
> v2: Better names and code layout (Boris)
> Revised commments on mce record comparisons (Ashok)
>
> arch/x86/kernel/cpu/mcheck/mce-genpool.c | 46 +++++++++++++++++++++++++++++++
> arch/x86/kernel/cpu/mcheck/mce-internal.h | 15 ++++++++++
> arch/x86/kernel/cpu/mcheck/mce.c | 21 ++++++--------
> 3 files changed, 70 insertions(+), 12 deletions(-)
>
> diff --git a/arch/x86/kernel/cpu/mcheck/mce-genpool.c b/arch/x86/kernel/cpu/mcheck/mce-genpool.c
> index 0a850100c594..c43050b91d6d 100644
> --- a/arch/x86/kernel/cpu/mcheck/mce-genpool.c
> +++ b/arch/x86/kernel/cpu/mcheck/mce-genpool.c
> @@ -26,6 +26,52 @@ static struct gen_pool *mce_evt_pool;
> static LLIST_HEAD(mce_event_llist);
> static char gen_pool_buf[MCE_POOLSZ];
>
> +/*
> + * Compare the record "t" with each of the records on list "l" to see if
> + * a functionally equivalent one is present in the list.

functionally?

> + */
> +static bool is_duplicate_mce_record(struct mce_evt_llist *t, struct mce_evt_llist *l)
> +{
> + struct mce_evt_llist *node;
> + struct mce *m1, *m2;
> +
> + m1 = &t->mce;
> +
> + llist_for_each_entry(node, &l->llnode, llnode) {
> + m2 = &node->mce;
> +
> + if (mce_cmp(m1, m2))

Sorry for nitpicking but isn't it usually the case that a
_cmp()-something function should return 0 when both things are equal?

I.e., you have:

if (!strcmp(s1, s2))
...

I think if we do it this way here too, it'll be very natural. mce_cmp()
would then have to do:

return !(m1->bank == m2->bank &&
m1->status == m2->status &&
m1->addr == m2->addr &&
m1->misc == m2->misc);

simply.

Hmmm?

Rest looks ok.