Messages in this thread | | | Date | Thu, 7 Apr 2016 19:03:26 +0200 | From | Borislav Petkov <> | Subject | Re: [PATCHv2] x86/mce: Look in genpool instead of mcelog.entry[] for pending error records |
| |
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.
-- Regards/Gruss, Boris.
ECO tip #101: Trim your mails when you reply.
| |