On Wed, Mar 04, 2015 at 11:12:33PM +0000, Luck, Tony wrote:
> > - fixed AR and UC order in enum severity_level because UC is severer than AR
> > by definition. Current code is not affected by this wrong order by chance.
>
> AR and AO are both UC errors - that happen also to be recoverable.
Maybe just saying "UC" might be confusing, meaning "UC bit is set" or "type of
error (defined in Table SDM's Table 15-6 in vol.3B) is 'Uncorrected Error'
(clearly separate from SRAR/SRAO)". You seem to mean the former, and I meant
the latter. So I should write the description more accurately like "UC=1,PCC=0"
error and "UC=1,PCC=1" error.
> Are you really sure
> about this re-order not affecting existing code?
Sorry, I thought I checked that but I missed one, so let me check again now.
I checked all referencing site of MCE_*_SEVERITY. Most of them are using '=='
to compare the severity, where the re-order doesn't affect them.
Some are using inequalities:
- around ./arch/x86/kernel/cpu/mcheck/mce.c:720,
if (mce_severity(m, mca_cfg.tolerant, msg, true) >=
MCE_PANIC_SEVERITY)
, the re-order doesn't affect.
- ./arch/x86/kernel/cpu/mcheck/mce.c:819:
if (m && global_worst >= MCE_PANIC_SEVERITY && mca_cfg.tolerant < 3)
, the re-order doesn't affect.
- ./arch/x86/kernel/cpu/mcheck/mce.c:832:
if (global_worst <= MCE_KEEP_SEVERITY && mca_cfg.tolerant < 3)
, ditto.
- ./arch/x86/kernel/cpu/mcheck/mce.c:1196:
no_way_out = worst >= MCE_PANIC_SEVERITY;
, ditto.
- ./arch/x86/kernel/cpu/mcheck/mce-severity.c:211:
if (s->sev >= MCE_UC_SEVERITY && ctx == IN_KERNEL) {
, the re-order should change to s->sev >= MCE_AR_SEVERITY to keep the
kernel behavior.
So I fixed the last part to be included in the re-order patch.
> You might well be right, but as every one
> else has pointed out mce_severity() is full of odd subtleties that catch
> people out.
I agree that this one big table is hard to maintain and bug-prone. One problem
is that it has too many fields to check so the parameter space is huge. I think
some field are checked only once, so separating it out makes table more simple
and readable.
> Is the "UC" entry at the end of the severities[] table just a catch-all for
> things that made it
> past all the other entries? Does it ever really get used?
I read through the severity check table and it seems that all UC=1 case
are already considered by the above entries, so it seems not used.
> What was the test case that made you promote UC above AR?
I thought of "Action required but unaffected thread is continuable" case
on kexec kernel, but I'm not sure that such a case really happens.
My motivation on the promotion was mainly from what SDM says rather than
the real testcase.
> This absolutely should not be buried in the middle of your other patch - it
> needs to
> be separate with a much more than two lines of commit description.
OK, I might not include this part in this series in later post, but if I do,
I'll separete it out.
Thanks,
Naoya Horiguchi--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
