Messages in this thread | | | Date | Thu, 15 Dec 2011 16:51:43 -0800 | Subject | Re: [PATCH 5/6] x86, mce: handle "action required" errors | From | Tony Luck <> |
| |
2011/12/15 Hidetoshi Seto <seto.hidetoshi@jp.fujitsu.com>: >> + m = *final; >> + >> if (!no_way_out) >> mce_clear_state(toclear); >> > > Small change, but again, you should describe reason why...
Yes - this is subtle (mce_clear_state() will clear what *final points to, so make a copy in the local variable "m"). It deserves a comment, so I'll add one.
> I know tolerant==3 is an insane option, but it is better to care about > it here too (or it would be happy if we can remove tolerant completely). > > e.g. > if (tolerant < 3) { > if (no_way_out) > mce_panic(...); > if (worst == MCE_AR_SEVERITY) { > /* schedule action before return to userland */ > mce_save_info(m.addr); > set_thread_flag(TIF_MCE_NOTIFY); > } else if (kill_it) { > force_sig(SIGBUS, current); > } > }
Good point. But I don't see how "tolerant==3" and "AR" errors ever make sense together. If we don't do something to fix the problem and just ignore it, then we will take a new machine check when we re-execute the instruction (unless the problem magically went away ... but I don't think that is likely). So the a user with tolerant=3 will loop taking the same machine check over and over. Which isn't likely to be what was wanted.
>> + * TIF_MCE_NOTFY, just before returning to errorneous userland. > > Spell checker suggests: erroneous
Will fix.
>> + if (!mi) >> + mce_panic("Lost address", NULL, NULL); > > The message is too short, isn't it?
Yes - it's a "Can't happen" error case (if we are here, then we must have saved the address when we set TIF_MCE_NOTIFY - so the only way to not find the address is for someone else to have corrupted out mce_info[] array). Perhaps I should change to BUG_ON()?
> And if this case is an another version of "Memory error not recovered" > located below then why not force_sig() but mce_panic()?
The more I look at that "Memory error not recovered" code, the more I think that it should be a panic (almost the same logic as for tolerant=3, in this case force_sig would prevent us from running right back into the machine check - but we did nothing to poison the page
Thanks for looking at this.
-Tony -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
| |