Re: [PATCH] x86: mce: Honour bios-set CMCI threshold

From: Naveen N. Rao
Date: Mon Aug 27 2012 - 05:56:00 EST


On 08/27/2012 02:42 PM, Borislav Petkov wrote:
On Thu, Aug 23, 2012 at 05:26:09PM +0530, Naveen N. Rao wrote:
Sure - sounds like a good idea. Further, a #define could eliminate
the need to change other references, but I'm not sure that's
GENERALLacceptable

#define mce_bios_cmci_threshold boot_flags.mce_bios_cmci_threshold

could eliminate the need to change other references, but I'm not sure
that's acceptable

Yeah, that's kinda obfuscating it for no reason. As I said before, we
can always add it later if it makes sense.

Ouch!
This was an old draft and I'm not sure how this ended up on the list!
Sorry for all the trouble.


But, I just had a quick look and it seems to me that these were
defined as integers since they are exposed via sysfs. For instance:

static struct dev_ext_attribute dev_attr_cmci_disabled = {
__ATTR(cmci_disabled, 0644, device_show_int, set_cmci_disabled),
&mce_cmci_disabled
};

Converting mce_cmci_disabled to a bit-field doesn't work since we
take its address above. We could ignore and not set the second field
at all (dev_ext_attribute->var) and define our own callbacks, but
that'll be more work and I'm not sure if we work fine without

Right,

but take a look at set_cmci_disabled(): it converts the newly read value
to bool anyway:

if (mce_cmci_disabled ^ !!new) {

so we can do later

flags.mce_cmci_disabled = true;

or
flags.mce_cmci_disabled = false;

instead of assigning 0 or 1 to it.

And, about showing it with device_show_int, a simple test works:

---
#include <stdio.h>
#include <stdbool.h>

int main()
{
bool a = true;
printf("%d\n", a);
return 0;
}
--

but even if there are troubles with that, we can change device_show_int
to a locally defined function.

But, anyway, long story short: I wasn't suggesting you go and change all
of them - simply start by adding your flag mce_bios_cmci_threshold to a
struct <something>_flags and I'll take care of the rest.

Unless you really want to do it, of course :-)

Sure, I'll send a patch for this soon.


Regards,
Naveen


Oh, and the more important thing is, Tony would need to review your
Intel-specific changes so pls keep him CCed on your next iteration too.

Thanks.


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/