[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Date: Thu, 3 Mar 2016 16:53:16 -0500
From: Josef Bacik <jbacik@...com>
To: "Luck, Tony" <tony.luck@...el.com>
CC: <bp@...en8.de>, <tglx@...utronix.de>, <mingo@...hat.com>,
<hpa@...or.com>, <x86@...nel.org>, <linux-edac@...r.kernel.org>,
<linux-kernel@...r.kernel.org>
Subject: Re: [PATCH] mce-apei: do not rely on ACPI_ERST_GET_RECORD_ID for
record id
On 03/03/2016 04:49 PM, Luck, Tony wrote:
>> retry:
>> - rc = erst_get_record_id_next(&pos, record_id);
>> - if (rc)
>> - goto out;
>> + /*
>> + * Some hardware is broken and doesn't actually advance the record id
>
> I'd blame this on firmware rather than hardware.
Yup sorry misspoke.
>
>> + * returned by ACPI_ERST_GET_RECORD_ID when we read a record like the
>> + * spec says it is supposed to. So instead use record_id == 0 to just
>> + * grab the first record in the erst, and fall back only if we trip over
>> + * a record that isn't a MCE record.
>> + */
>> + if (lookup_record) {
>> + rc = erst_get_record_id_next(&pos, record_id);
>> + if (rc)
>> + goto out;
>> + } else {
>> + *record_id = 0;
>> + }
>> /* no more record */
>> if (*record_id == APEI_ERST_INVALID_RECORD_ID)
>> goto out;
>> rc = erst_read(*record_id, &rcd.hdr, sizeof(rcd));
>> - /* someone else has cleared the record, try next one */
>> - if (rc == -ENOENT)
>> - goto retry;
>> - else if (rc < 0)
>> + /*
>> + * someone else has cleared the record, try next one if we are looking
>> + * up records. If we aren't looking up the record id's then just bail
>> + * since this means we have an empty table.
>> + */
>> + if (rc == -ENOENT) {
>> + if (lookup_record)
>> + goto retry;
>> + rc = 0;
>> + goto out;
>> + } else if (rc < 0) {
>> goto out;
>> - /* try to skip other type records in storage */
>> - else if (rc != sizeof(rcd) ||
>> - uuid_le_cmp(rcd.hdr.creator_id, CPER_CREATOR_MCE))
>> + } else if (rc != sizeof(rcd) ||
>> + uuid_le_cmp(rcd.hdr.creator_id, CPER_CREATOR_MCE)) {
>> + /* try to skip other type records in storage */
>> + lookup_record = true;
>> goto retry;
>
> Are you still doomed by the buggy firmware if we take this "goto"?
> You be back at the top of the loop excpecting erst_get_record_id_next()
> to move on. Does this just not happen in practice (finding non MCE
> records in amognst the MCE ones)?
>
So one of the boxes had a non MCE record with MCE records, but yeah it
was on a box that was also broken in this way. I'm not super worried
about this case for us, I just want to have a fallback for any firmware
that may not be broken in this way to be able to skip things. Thanks,
Josef
Powered by blists - more mailing lists