On Wed, Apr 05, 2017 at 05:53:57PM +0000, Ghannam, Yazen wrote: > Correct, but only on SMCA systems.
On !SMCA systems you log only once anyway - you don't have DE* MSRs. > This works so I don't know why it's not okay. So I should take the code just because you have found one way that it works? Screw readability or future proof design - people should dump the code on maintainers and maintainers should deal with the stinking pile. Who cares, it works for you. > Your suggestion also does an SMCA check. Why TF does that even matter? Enough with the dumb checks argument already. > So code that does a check-and-return is preferable to code using > if/else-if statements? If that's the case then I can try to rework it. No, readable code matters. Your suggestion is confusing. I see if (is_deferred_error(status)) { __log_error } else if (mce_flags.smca) { ... if (is_deferred_error(status)) But then here ---> we still deal with deferred errors. Even though we did the deferred check in the if-clause. And that is confusing and makes the code hard to follow. My suggestion does *exactly* what the commit message says: if (__log_error(..)) goto out; when we go to the out label, we know we've succeeded logging the deferred error and we're done. if (!mce_flags.smca) return; When we return here, we know, we've taken care of the !SMCA systems. Now here starts the second attempt to read the DE* registers and we *know* we're on SMCA systems. > How does log_error() know if we can't use the normal MSRs? MCI_STATUS_VAL. > We check for MCI_STATUS_VAL in log_error(). Yes. > We also need to check for MCI_STATUS_DEFERRED but only if we're coming > from the deferred error handler. Why? We *are* coming from the #DF handler so are you expecting a different type of error in the MSRs? -- Regards/Gruss, Boris. Good mailing practices for 400: avoid top-posting and trim the reply.