* [PATCH 0/3] x86:mce: Some cleanups and bios-set CMCI thresholds
@ 2012-09-05 10:21 Naveen N. Rao
2012-09-05 10:21 ` [PATCH 1/3] x86/mce: Make sysfs tunables available globally across all cpus Naveen N. Rao
` (2 more replies)
0 siblings, 3 replies; 16+ messages in thread
From: Naveen N. Rao @ 2012-09-05 10:21 UTC (permalink / raw)
To: tony.luck, andi, bp
Cc: gong.chen, ananth, x86, linux-kernel, mingo, hpa, tglx, gregkh,
linux-edac
Hello,
I'm putting together my previous set of patches for MCE.
The first two patches are cleanups, as per previous discussions. The first
patch makes mce sysfs tunables available under global machinecheck sysfs
directory. We no longer remove the existing entries in the per-cpu location.
The second patch consolidates some of the mce boot options into a single
bitfield. The structure name has been changed and is now in mce-internal.h
The third patch adds a new boot option to honour bios-set CMCI thresholds.
This has been rebased on the above patches.
This patch series applies on top of -tip.
Thanks,
Naveen
---
Naveen N. Rao (3):
x86/mce: Make sysfs tunables available globally across all cpus
x86/mce: Pack boolean MCE flags into a structure
x86/mce: Honour bios-set CMCI threshold
Documentation/x86/x86_64/boot-options.txt | 5 +
Documentation/x86/x86_64/machinecheck | 4 +
arch/x86/include/asm/mce.h | 2 -
arch/x86/kernel/cpu/mcheck/mce-internal.h | 10 +++
arch/x86/kernel/cpu/mcheck/mce.c | 112 ++++++++++++++++++++++-------
arch/x86/kernel/cpu/mcheck/mce_intel.c | 41 ++++++++++-
6 files changed, 140 insertions(+), 34 deletions(-)
--
^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH 1/3] x86/mce: Make sysfs tunables available globally across all cpus
2012-09-05 10:21 [PATCH 0/3] x86:mce: Some cleanups and bios-set CMCI thresholds Naveen N. Rao
@ 2012-09-05 10:21 ` Naveen N. Rao
2012-09-05 10:32 ` [PATCH] [mcelog] Start using the new sysfs tunables location Naveen N. Rao
2012-09-05 10:22 ` [PATCH 2/3] x86/mce: Pack boolean MCE flags into a structure Naveen N. Rao
2012-09-05 10:22 ` [PATCH 3/3] x86/mce: Honour bios-set CMCI threshold Naveen N. Rao
2 siblings, 1 reply; 16+ messages in thread
From: Naveen N. Rao @ 2012-09-05 10:21 UTC (permalink / raw)
To: tony.luck, andi, bp
Cc: gong.chen, ananth, x86, linux-kernel, mingo, hpa, tglx, gregkh,
linux-edac
All the MCE attributes currently exported via sysfs appear under
/sys/devices/system/machinecheck/machinecheck<n>/. Pretty much all of
these are global in nature and not specific to a processor. So, make these
available under /sys/devices/system/machinecheck/ where they rightly belong.
Update documentation to also point to the new location so that user-space
tools can pick up on the new location. We would eventually want to remove
these from the per-cpu location.
Signed-off-by: Naveen N. Rao <naveen.n.rao@linux.vnet.ibm.com>
---
Documentation/x86/x86_64/machinecheck | 4 ++--
arch/x86/kernel/cpu/mcheck/mce.c | 24 +++++++++++++++++++++++-
2 files changed, 25 insertions(+), 3 deletions(-)
diff --git a/Documentation/x86/x86_64/machinecheck b/Documentation/x86/x86_64/machinecheck
index b1fb302..02b84a6 100644
--- a/Documentation/x86/x86_64/machinecheck
+++ b/Documentation/x86/x86_64/machinecheck
@@ -31,8 +31,8 @@ bankNctl
Note that BIOS maintain another mask to disable specific events
per bank. This is not visible here
-The following entries appear for each CPU, but they are truly shared
-between all CPUs.
+The following entries are shared between all CPUs and appear under
+/sys/devices/system/machinecheck:
check_interval
How often to poll for corrected machine check errors, in seconds
diff --git a/arch/x86/kernel/cpu/mcheck/mce.c b/arch/x86/kernel/cpu/mcheck/mce.c
index c311122..bf276eb 100644
--- a/arch/x86/kernel/cpu/mcheck/mce.c
+++ b/arch/x86/kernel/cpu/mcheck/mce.c
@@ -2205,6 +2205,7 @@ static struct dev_ext_attribute dev_attr_cmci_disabled = {
&mce_cmci_disabled
};
+/* Use this _only_ for per-cpu attributes */
static struct device_attribute *mce_device_attrs[] = {
&dev_attr_tolerant.attr,
&dev_attr_check_interval.attr,
@@ -2216,6 +2217,27 @@ static struct device_attribute *mce_device_attrs[] = {
NULL
};
+/* All new global attributes go here */
+static struct attribute *mce_device_global_attrs[] = {
+ &dev_attr_tolerant.attr.attr,
+ &dev_attr_check_interval.attr.attr,
+ &dev_attr_trigger.attr,
+ &dev_attr_monarch_timeout.attr.attr,
+ &dev_attr_dont_log_ce.attr.attr,
+ &dev_attr_ignore_ce.attr.attr,
+ &dev_attr_cmci_disabled.attr.attr,
+ NULL
+};
+
+static struct attribute_group mce_device_attr_group = {
+ .attrs = mce_device_global_attrs,
+};
+
+static const struct attribute_group *mce_device_attr_groups[] = {
+ &mce_device_attr_group,
+ NULL,
+};
+
static cpumask_var_t mce_device_initialized;
static void mce_device_release(struct device *dev)
@@ -2397,7 +2419,7 @@ static __init int mcheck_init_device(void)
mce_init_banks();
- err = subsys_system_register(&mce_subsys, NULL);
+ err = subsys_system_register(&mce_subsys, mce_device_attr_groups);
if (err)
return err;
^ permalink raw reply related [flat|nested] 16+ messages in thread
* [PATCH 2/3] x86/mce: Pack boolean MCE flags into a structure
2012-09-05 10:21 [PATCH 0/3] x86:mce: Some cleanups and bios-set CMCI thresholds Naveen N. Rao
2012-09-05 10:21 ` [PATCH 1/3] x86/mce: Make sysfs tunables available globally across all cpus Naveen N. Rao
@ 2012-09-05 10:22 ` Naveen N. Rao
2012-09-05 17:15 ` Joe Perches
2012-09-05 18:56 ` Tony Luck
2012-09-05 10:22 ` [PATCH 3/3] x86/mce: Honour bios-set CMCI threshold Naveen N. Rao
2 siblings, 2 replies; 16+ messages in thread
From: Naveen N. Rao @ 2012-09-05 10:22 UTC (permalink / raw)
To: tony.luck, andi, bp
Cc: gong.chen, ananth, x86, linux-kernel, mingo, hpa, tglx, gregkh,
linux-edac
Many MCE flags are boolean in nature, but are declared as integers
currently. We can pack these into a bitfield to save some space.
Signed-off-by: Naveen N. Rao <naveen.n.rao@linux.vnet.ibm.com>
---
arch/x86/include/asm/mce.h | 2 -
arch/x86/kernel/cpu/mcheck/mce-internal.h | 9 +++
arch/x86/kernel/cpu/mcheck/mce.c | 88 +++++++++++++++++++----------
arch/x86/kernel/cpu/mcheck/mce_intel.c | 2 -
4 files changed, 67 insertions(+), 34 deletions(-)
diff --git a/arch/x86/include/asm/mce.h b/arch/x86/include/asm/mce.h
index a3ac52b..4576930 100644
--- a/arch/x86/include/asm/mce.h
+++ b/arch/x86/include/asm/mce.h
@@ -169,8 +169,6 @@ DECLARE_PER_CPU(struct device *, mce_device);
#define MAX_NR_BANKS 32
#ifdef CONFIG_X86_MCE_INTEL
-extern int mce_cmci_disabled;
-extern int mce_ignore_ce;
void mce_intel_feature_init(struct cpuinfo_x86 *c);
void cmci_clear(void);
void cmci_reenable(void);
diff --git a/arch/x86/kernel/cpu/mcheck/mce-internal.h b/arch/x86/kernel/cpu/mcheck/mce-internal.h
index 6a05c1d..9a165a2 100644
--- a/arch/x86/kernel/cpu/mcheck/mce-internal.h
+++ b/arch/x86/kernel/cpu/mcheck/mce-internal.h
@@ -11,6 +11,15 @@ enum severity_level {
MCE_PANIC_SEVERITY,
};
+struct mce_config {
+ __u32 cmci_disabled : 1,
+ ignore_ce : 1,
+ dont_log_ce : 1,
+ __pad : 29;
+};
+
+extern struct mce_config mce_cfg;
+
#define ATTR_LEN 16
/* One object for each MCE bank, shared by all CPUs */
diff --git a/arch/x86/kernel/cpu/mcheck/mce.c b/arch/x86/kernel/cpu/mcheck/mce.c
index bf276eb..cbdef73 100644
--- a/arch/x86/kernel/cpu/mcheck/mce.c
+++ b/arch/x86/kernel/cpu/mcheck/mce.c
@@ -79,11 +79,10 @@ static int rip_msr __read_mostly;
static int mce_bootlog __read_mostly = -1;
static int monarch_timeout __read_mostly = -1;
static int mce_panic_timeout __read_mostly;
-static int mce_dont_log_ce __read_mostly;
-int mce_cmci_disabled __read_mostly;
-int mce_ignore_ce __read_mostly;
int mce_ser __read_mostly;
+struct mce_config mce_cfg __read_mostly;
+
struct mce_bank *mce_banks __read_mostly;
/* User mode helper program triggered by machine check event */
@@ -630,7 +629,7 @@ void machine_check_poll(enum mcp_flags flags, mce_banks_t *b)
* Don't get the IP here because it's unlikely to
* have anything to do with the actual error location.
*/
- if (!(flags & MCP_DONTLOG) && !mce_dont_log_ce)
+ if (!(flags & MCP_DONTLOG) && !mce_cfg.dont_log_ce)
mce_log(&m);
/*
@@ -1634,7 +1633,7 @@ static void mce_start_timer(unsigned int cpu, struct timer_list *t)
__this_cpu_write(mce_next_interval, iv);
- if (mce_ignore_ce || !iv)
+ if (mce_cfg.ignore_ce || !iv)
return;
t->expires = round_jiffies(jiffies + iv);
@@ -1958,11 +1957,11 @@ static int __init mcheck_enable(char *str)
if (!strcmp(str, "off"))
mce_disabled = 1;
else if (!strcmp(str, "no_cmci"))
- mce_cmci_disabled = 1;
+ mce_cfg.cmci_disabled = 1;
else if (!strcmp(str, "dont_log_ce"))
- mce_dont_log_ce = 1;
+ mce_cfg.dont_log_ce = 1;
else if (!strcmp(str, "ignore_ce"))
- mce_ignore_ce = 1;
+ mce_cfg.ignore_ce = 1;
else if (!strcmp(str, "bootlog") || !strcmp(str, "nobootlog"))
mce_bootlog = (str[0] == 'b');
else if (isdigit(str[0])) {
@@ -2129,6 +2128,34 @@ static ssize_t set_trigger(struct device *s, struct device_attribute *attr,
return strlen(mce_helper) + !!p;
}
+static ssize_t get_dont_log_ce(struct device *dev,
+ struct device_attribute *attr,
+ char *buf)
+{
+ return snprintf(buf, PAGE_SIZE, "%d\n", mce_cfg.dont_log_ce);
+}
+
+static ssize_t set_dont_log_ce(struct device *s,
+ struct device_attribute *attr,
+ const char *buf, size_t size)
+{
+ u64 new;
+
+ if (strict_strtoull(buf, 0, &new) < 0)
+ return -EINVAL;
+
+ mce_cfg.dont_log_ce = !!new;
+
+ return size;
+}
+
+static ssize_t get_ignore_ce(struct device *dev,
+ struct device_attribute *attr,
+ char *buf)
+{
+ return snprintf(buf, PAGE_SIZE, "%d\n", mce_cfg.ignore_ce);
+}
+
static ssize_t set_ignore_ce(struct device *s,
struct device_attribute *attr,
const char *buf, size_t size)
@@ -2138,21 +2165,28 @@ static ssize_t set_ignore_ce(struct device *s,
if (strict_strtoull(buf, 0, &new) < 0)
return -EINVAL;
- if (mce_ignore_ce ^ !!new) {
+ if (mce_cfg.ignore_ce ^ !!new) {
if (new) {
/* disable ce features */
mce_timer_delete_all();
on_each_cpu(mce_disable_cmci, NULL, 1);
- mce_ignore_ce = 1;
+ mce_cfg.ignore_ce = 1;
} else {
/* enable ce features */
- mce_ignore_ce = 0;
+ mce_cfg.ignore_ce = 0;
on_each_cpu(mce_enable_ce, (void *)1, 1);
}
}
return size;
}
+static ssize_t get_cmci_disabled(struct device *dev,
+ struct device_attribute *attr,
+ char *buf)
+{
+ return snprintf(buf, PAGE_SIZE, "%d\n", mce_cfg.cmci_disabled);
+}
+
static ssize_t set_cmci_disabled(struct device *s,
struct device_attribute *attr,
const char *buf, size_t size)
@@ -2162,14 +2196,14 @@ static ssize_t set_cmci_disabled(struct device *s,
if (strict_strtoull(buf, 0, &new) < 0)
return -EINVAL;
- if (mce_cmci_disabled ^ !!new) {
+ if (mce_cfg.cmci_disabled ^ !!new) {
if (new) {
/* disable cmci */
on_each_cpu(mce_disable_cmci, NULL, 1);
- mce_cmci_disabled = 1;
+ mce_cfg.cmci_disabled = 1;
} else {
/* enable cmci */
- mce_cmci_disabled = 0;
+ mce_cfg.cmci_disabled = 0;
on_each_cpu(mce_enable_ce, NULL, 1);
}
}
@@ -2188,32 +2222,24 @@ static ssize_t store_int_with_restart(struct device *s,
static DEVICE_ATTR(trigger, 0644, show_trigger, set_trigger);
static DEVICE_INT_ATTR(tolerant, 0644, tolerant);
static DEVICE_INT_ATTR(monarch_timeout, 0644, monarch_timeout);
-static DEVICE_INT_ATTR(dont_log_ce, 0644, mce_dont_log_ce);
+static DEVICE_ATTR(dont_log_ce, 0644, get_dont_log_ce, set_dont_log_ce);
+static DEVICE_ATTR(ignore_ce, 0644, get_ignore_ce, set_ignore_ce);
+static DEVICE_ATTR(cmci_disabled, 0644, get_cmci_disabled, set_cmci_disabled);
static struct dev_ext_attribute dev_attr_check_interval = {
__ATTR(check_interval, 0644, device_show_int, store_int_with_restart),
&check_interval
};
-static struct dev_ext_attribute dev_attr_ignore_ce = {
- __ATTR(ignore_ce, 0644, device_show_int, set_ignore_ce),
- &mce_ignore_ce
-};
-
-static struct dev_ext_attribute dev_attr_cmci_disabled = {
- __ATTR(cmci_disabled, 0644, device_show_int, set_cmci_disabled),
- &mce_cmci_disabled
-};
-
/* Use this _only_ for per-cpu attributes */
static struct device_attribute *mce_device_attrs[] = {
&dev_attr_tolerant.attr,
&dev_attr_check_interval.attr,
&dev_attr_trigger,
&dev_attr_monarch_timeout.attr,
- &dev_attr_dont_log_ce.attr,
- &dev_attr_ignore_ce.attr,
- &dev_attr_cmci_disabled.attr,
+ &dev_attr_dont_log_ce,
+ &dev_attr_ignore_ce,
+ &dev_attr_cmci_disabled,
NULL
};
@@ -2223,9 +2249,9 @@ static struct attribute *mce_device_global_attrs[] = {
&dev_attr_check_interval.attr.attr,
&dev_attr_trigger.attr,
&dev_attr_monarch_timeout.attr.attr,
- &dev_attr_dont_log_ce.attr.attr,
- &dev_attr_ignore_ce.attr.attr,
- &dev_attr_cmci_disabled.attr.attr,
+ &dev_attr_dont_log_ce.attr,
+ &dev_attr_ignore_ce.attr,
+ &dev_attr_cmci_disabled.attr,
NULL
};
diff --git a/arch/x86/kernel/cpu/mcheck/mce_intel.c b/arch/x86/kernel/cpu/mcheck/mce_intel.c
index 098386f..a6c028d 100644
--- a/arch/x86/kernel/cpu/mcheck/mce_intel.c
+++ b/arch/x86/kernel/cpu/mcheck/mce_intel.c
@@ -53,7 +53,7 @@ static int cmci_supported(int *banks)
{
u64 cap;
- if (mce_cmci_disabled || mce_ignore_ce)
+ if (mce_cfg.cmci_disabled || mce_cfg.ignore_ce)
return 0;
/*
^ permalink raw reply related [flat|nested] 16+ messages in thread
* [PATCH 3/3] x86/mce: Honour bios-set CMCI threshold
2012-09-05 10:21 [PATCH 0/3] x86:mce: Some cleanups and bios-set CMCI thresholds Naveen N. Rao
2012-09-05 10:21 ` [PATCH 1/3] x86/mce: Make sysfs tunables available globally across all cpus Naveen N. Rao
2012-09-05 10:22 ` [PATCH 2/3] x86/mce: Pack boolean MCE flags into a structure Naveen N. Rao
@ 2012-09-05 10:22 ` Naveen N. Rao
2 siblings, 0 replies; 16+ messages in thread
From: Naveen N. Rao @ 2012-09-05 10:22 UTC (permalink / raw)
To: tony.luck, andi, bp
Cc: gong.chen, ananth, x86, linux-kernel, mingo, hpa, tglx, gregkh,
linux-edac
The ACPI spec doesn't provide for a way for the bios to pass down
recommended thresholds to the OS on a _per-bank_ basis. This patch adds
a new boot option, which if passed, allows bios to initialize the CMCI
threshold. In such a case, we simply skip programming any threshold
value.
As fail-safe, we initialize threshold to 1 if some banks have not been
initialized by the bios and warn the user.
Signed-off-by: Naveen N. Rao <naveen.n.rao@linux.vnet.ibm.com>
---
Documentation/x86/x86_64/boot-options.txt | 5 ++++
arch/x86/kernel/cpu/mcheck/mce-internal.h | 3 +-
arch/x86/kernel/cpu/mcheck/mce.c | 12 +++++++++
arch/x86/kernel/cpu/mcheck/mce_intel.c | 39 +++++++++++++++++++++++++++--
4 files changed, 55 insertions(+), 4 deletions(-)
diff --git a/Documentation/x86/x86_64/boot-options.txt b/Documentation/x86/x86_64/boot-options.txt
index c54b4f5..ec92540 100644
--- a/Documentation/x86/x86_64/boot-options.txt
+++ b/Documentation/x86/x86_64/boot-options.txt
@@ -50,6 +50,11 @@ Machine check
monarchtimeout:
Sets the time in us to wait for other CPUs on machine checks. 0
to disable.
+ mce=bios_cmci_threshold
+ Don't overwrite the bios-set CMCI threshold. This boot option
+ prevents Linux from overwriting the CMCI threshold set by the
+ bios. Without this option, Linux always sets the CMCI
+ threshold to 1.
nomce (for compatibility with i386): same as mce=off
diff --git a/arch/x86/kernel/cpu/mcheck/mce-internal.h b/arch/x86/kernel/cpu/mcheck/mce-internal.h
index 9a165a2..cfc2852 100644
--- a/arch/x86/kernel/cpu/mcheck/mce-internal.h
+++ b/arch/x86/kernel/cpu/mcheck/mce-internal.h
@@ -15,7 +15,8 @@ struct mce_config {
__u32 cmci_disabled : 1,
ignore_ce : 1,
dont_log_ce : 1,
- __pad : 29;
+ bios_cmci_threshold : 1,
+ __pad : 28;
};
extern struct mce_config mce_cfg;
diff --git a/arch/x86/kernel/cpu/mcheck/mce.c b/arch/x86/kernel/cpu/mcheck/mce.c
index cbdef73..e0985a7 100644
--- a/arch/x86/kernel/cpu/mcheck/mce.c
+++ b/arch/x86/kernel/cpu/mcheck/mce.c
@@ -1945,6 +1945,7 @@ static struct miscdevice mce_chrdev_device = {
* check, or 0 to not wait
* mce=bootlog Log MCEs from before booting. Disabled by default on AMD.
* mce=nobootlog Don't log MCEs from before booting.
+ * mce=bios_cmci_threshold Don't program the CMCI threshold
*/
static int __init mcheck_enable(char *str)
{
@@ -1964,6 +1965,8 @@ static int __init mcheck_enable(char *str)
mce_cfg.ignore_ce = 1;
else if (!strcmp(str, "bootlog") || !strcmp(str, "nobootlog"))
mce_bootlog = (str[0] == 'b');
+ else if (!strcmp(str, "bios_cmci_threshold"))
+ mce_cfg.bios_cmci_threshold = 1;
else if (isdigit(str[0])) {
get_option(&str, &tolerant);
if (*str == ',') {
@@ -2210,6 +2213,13 @@ static ssize_t set_cmci_disabled(struct device *s,
return size;
}
+static ssize_t get_bios_cmci_threshold(struct device *dev,
+ struct device_attribute *attr,
+ char *buf)
+{
+ return snprintf(buf, PAGE_SIZE, "%d\n", mce_cfg.bios_cmci_threshold);
+}
+
static ssize_t store_int_with_restart(struct device *s,
struct device_attribute *attr,
const char *buf, size_t size)
@@ -2225,6 +2235,7 @@ static DEVICE_INT_ATTR(monarch_timeout, 0644, monarch_timeout);
static DEVICE_ATTR(dont_log_ce, 0644, get_dont_log_ce, set_dont_log_ce);
static DEVICE_ATTR(ignore_ce, 0644, get_ignore_ce, set_ignore_ce);
static DEVICE_ATTR(cmci_disabled, 0644, get_cmci_disabled, set_cmci_disabled);
+static DEVICE_ATTR(bios_cmci_threshold, 0444, get_bios_cmci_threshold, NULL);
static struct dev_ext_attribute dev_attr_check_interval = {
__ATTR(check_interval, 0644, device_show_int, store_int_with_restart),
@@ -2252,6 +2263,7 @@ static struct attribute *mce_device_global_attrs[] = {
&dev_attr_dont_log_ce.attr,
&dev_attr_ignore_ce.attr,
&dev_attr_cmci_disabled.attr,
+ &dev_attr_bios_cmci_threshold.attr,
NULL
};
diff --git a/arch/x86/kernel/cpu/mcheck/mce_intel.c b/arch/x86/kernel/cpu/mcheck/mce_intel.c
index a6c028d..6a2de06 100644
--- a/arch/x86/kernel/cpu/mcheck/mce_intel.c
+++ b/arch/x86/kernel/cpu/mcheck/mce_intel.c
@@ -181,10 +181,16 @@ static void cmci_discover(int banks)
unsigned long *owned = (void *)&__get_cpu_var(mce_banks_owned);
unsigned long flags;
int i;
+ int bios_wrong_thresh = 0;
+
+ if (mce_cfg.bios_cmci_threshold)
+ printk_once(KERN_INFO
+ "bios_cmci_threshold: Using bios-set threshold values for CMCI");
raw_spin_lock_irqsave(&cmci_discover_lock, flags);
for (i = 0; i < banks; i++) {
u64 val;
+ int bios_zero_thresh = 0;
if (test_bit(i, owned))
continue;
@@ -198,8 +204,20 @@ static void cmci_discover(int banks)
continue;
}
- val &= ~MCI_CTL2_CMCI_THRESHOLD_MASK;
- val |= MCI_CTL2_CMCI_EN | CMCI_THRESHOLD;
+ if (!mce_cfg.bios_cmci_threshold) {
+ val &= ~MCI_CTL2_CMCI_THRESHOLD_MASK;
+ val |= CMCI_THRESHOLD;
+ } else if (!(val & MCI_CTL2_CMCI_THRESHOLD_MASK)) {
+ /*
+ * If bios_cmci_threshold boot option was specified
+ * but the threshold is zero, we'll try to initialize
+ * it to 1.
+ */
+ bios_zero_thresh = 1;
+ val |= CMCI_THRESHOLD;
+ }
+
+ val |= MCI_CTL2_CMCI_EN;
wrmsrl(MSR_IA32_MCx_CTL2(i), val);
rdmsrl(MSR_IA32_MCx_CTL2(i), val);
@@ -207,11 +225,26 @@ static void cmci_discover(int banks)
if (val & MCI_CTL2_CMCI_EN) {
set_bit(i, owned);
__clear_bit(i, __get_cpu_var(mce_poll_banks));
+ /*
+ * We are able to set thresholds for some banks that
+ * had a threshold of 0. This means the BIOS has not
+ * set the thresholds properly or does not work with
+ * this boot option. Note down now and report later.
+ */
+ if (mce_cfg.bios_cmci_threshold && bios_zero_thresh &&
+ (val & MCI_CTL2_CMCI_THRESHOLD_MASK))
+ bios_wrong_thresh = 1;
} else {
WARN_ON(!test_bit(i, __get_cpu_var(mce_poll_banks)));
}
}
raw_spin_unlock_irqrestore(&cmci_discover_lock, flags);
+ if (mce_cfg.bios_cmci_threshold && bios_wrong_thresh) {
+ printk_once(KERN_INFO
+ "bios_cmci_threshold: Some banks do not have valid thresholds set");
+ printk_once(KERN_INFO
+ "bios_cmci_threshold: Make sure your BIOS supports this boot option");
+ }
}
/*
@@ -249,7 +282,7 @@ void cmci_clear(void)
continue;
/* Disable CMCI */
rdmsrl(MSR_IA32_MCx_CTL2(i), val);
- val &= ~(MCI_CTL2_CMCI_EN|MCI_CTL2_CMCI_THRESHOLD_MASK);
+ val &= ~MCI_CTL2_CMCI_EN;
wrmsrl(MSR_IA32_MCx_CTL2(i), val);
__clear_bit(i, __get_cpu_var(mce_banks_owned));
}
^ permalink raw reply related [flat|nested] 16+ messages in thread
* [PATCH] [mcelog] Start using the new sysfs tunables location
2012-09-05 10:21 ` [PATCH 1/3] x86/mce: Make sysfs tunables available globally across all cpus Naveen N. Rao
@ 2012-09-05 10:32 ` Naveen N. Rao
2012-09-05 18:47 ` Andi Kleen
0 siblings, 1 reply; 16+ messages in thread
From: Naveen N. Rao @ 2012-09-05 10:32 UTC (permalink / raw)
To: ak, tony.luck, andi, bp; +Cc: x86, linux-kernel, linux-edac, ananth
All the current mce tunables are now available under
/sys/devices/system/machinecheck. Start using this new location, but fall back
to the older per-cpu location so that we continue working with older kernels.
Signed-off-by: Naveen N. Rao <naveen.n.rao@linux.vnet.ibm.com>
---
README | 2 +-
mcelog.init | 5 ++++-
tests/test | 7 ++++++-
3 files changed, 11 insertions(+), 3 deletions(-)
diff --git a/README b/README
index 08184ed..0426460 100644
--- a/README
+++ b/README
@@ -18,7 +18,7 @@ significantly (upto 10 minutes) and does not allow mcelog to keep extended state
trigger is a newer method where the kernel runs mcelog on a error.
This is configured with
-echo /usr/sbin/mcelog > /sys/devices/system/machinecheck/machinecheck0/trigger
+echo /usr/sbin/mcelog > /sys/devices/system/machinecheck/trigger
This is faster, but still doesn't allow mcelog to keep state,
and has relatively high overhead for each error because a program has
to be initialized from scratch.
diff --git a/mcelog.init b/mcelog.init
index 0abe786..5f32ba7 100755
--- a/mcelog.init
+++ b/mcelog.init
@@ -31,7 +31,10 @@ MCELOG_OPTIONS=""
# private settings
MCELOG=${MCELOG:-/usr/sbin/mcelog}
-TRIGGER=/sys/devices/system/machinecheck/machinecheck0/trigger
+TRIGGER=/sys/devices/system/machinecheck/trigger
+if [ ! -e $TRIGGER ] ; then
+ TRIGGER=/sys/devices/system/machinecheck/machinecheck0/trigger
+fi
[ ! -x $MCELOG ] && ( echo "mcelog not found" ; exit 1 )
[ ! -r /dev/mcelog ] && ( echo "/dev/mcelog not active" ; exit 0 )
diff --git a/tests/test b/tests/test
index c673eb2..52daf01 100755
--- a/tests/test
+++ b/tests/test
@@ -17,10 +17,15 @@ if [ "$(whoami)" != "root" ] ; then
exit 1
fi
+TRIGGER=/sys/devices/system/machinecheck/trigger
+if [ ! -e $TRIGGER ] ; then
+ TRIGGER=/sys/devices/system/machinecheck/machinecheck0/trigger
+fi
+
echo "++++++++++++ running $1 test +++++++++++++++++++"
# disable trigger
-echo -n "" > /sys/devices/system/machinecheck/machinecheck0/trigger
+echo -n "" > $TRIGGER
killall mcelog || true
#killwatchdog() {
^ permalink raw reply related [flat|nested] 16+ messages in thread
* Re: [PATCH 2/3] x86/mce: Pack boolean MCE flags into a structure
2012-09-05 10:22 ` [PATCH 2/3] x86/mce: Pack boolean MCE flags into a structure Naveen N. Rao
@ 2012-09-05 17:15 ` Joe Perches
2012-09-05 18:56 ` Tony Luck
1 sibling, 0 replies; 16+ messages in thread
From: Joe Perches @ 2012-09-05 17:15 UTC (permalink / raw)
To: Naveen N. Rao
Cc: tony.luck, andi, bp, gong.chen, ananth, x86, linux-kernel, mingo,
hpa, tglx, gregkh, linux-edac
On Wed, 2012-09-05 at 15:52 +0530, Naveen N. Rao wrote:
> Many MCE flags are boolean in nature, but are declared as integers
> currently. We can pack these into a bitfield to save some space.
[]
> diff --git a/arch/x86/kernel/cpu/mcheck/mce-internal.h b/arch/x86/kernel/cpu/mcheck/mce-internal.h
[]
> @@ -11,6 +11,15 @@ enum severity_level {
> MCE_PANIC_SEVERITY,
> };
>
> +struct mce_config {
> + __u32 cmci_disabled : 1,
> + ignore_ce : 1,
> + dont_log_ce : 1,
> + __pad : 29;
You don't need to declare __pad
Perhaps bool:1 instead.
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH] [mcelog] Start using the new sysfs tunables location
2012-09-05 10:32 ` [PATCH] [mcelog] Start using the new sysfs tunables location Naveen N. Rao
@ 2012-09-05 18:47 ` Andi Kleen
2012-09-05 19:09 ` Tony Luck
0 siblings, 1 reply; 16+ messages in thread
From: Andi Kleen @ 2012-09-05 18:47 UTC (permalink / raw)
To: Naveen N. Rao
Cc: ak, tony.luck, andi, bp, x86, linux-kernel, linux-edac, ananth
On Wed, Sep 05, 2012 at 04:02:37PM +0530, Naveen N. Rao wrote:
> All the current mce tunables are now available under
> /sys/devices/system/machinecheck. Start using this new location, but fall back
> to the older per-cpu location so that we continue working with older kernels.
Who did that change in the kernel?
That breaks Linus rule that the kernel should not break userland.
Kernel needs to fix that.
-Andi
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 2/3] x86/mce: Pack boolean MCE flags into a structure
2012-09-05 10:22 ` [PATCH 2/3] x86/mce: Pack boolean MCE flags into a structure Naveen N. Rao
2012-09-05 17:15 ` Joe Perches
@ 2012-09-05 18:56 ` Tony Luck
2012-09-06 6:48 ` Naveen N. Rao
1 sibling, 1 reply; 16+ messages in thread
From: Tony Luck @ 2012-09-05 18:56 UTC (permalink / raw)
To: Naveen N. Rao
Cc: andi, bp, gong.chen, ananth, x86, linux-kernel, mingo, hpa, tglx,
gregkh, linux-edac
On Wed, Sep 5, 2012 at 3:22 AM, Naveen N. Rao
<naveen.n.rao@linux.vnet.ibm.com> wrote:
> Many MCE flags are boolean in nature, but are declared as integers
> currently. We can pack these into a bitfield to save some space.
Before this patch:
size arch/x86/kernel/cpu/mcheck/mce.o
text data bss dec hex filename
18946 4930 776 24652 604c arch/x86/kernel/cpu/mcheck/mce.o
After:
size arch/x86/kernel/cpu/mcheck/mce.o
text data bss dec hex filename
19335 4890 776 25001 61a9 arch/x86/kernel/cpu/mcheck/mce.o
So we do indeed see "data" reduced by 40 bytes. But
"text" is up by 389. This seems to be because you have
another change, not described in the commit log, buried
in part 2 to add get_dont_log_ce(), set_dont_log_ce() etc.
Compiler version: gcc version 4.4.6 20120305 (Red Hat 4.4.6-4) (GCC)
I know I'm contradicting the feedback you got from Borislav here, but
is this code churn really worth it to save 40 bytes? I don't think so.
-Tony
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH] [mcelog] Start using the new sysfs tunables location
2012-09-05 18:47 ` Andi Kleen
@ 2012-09-05 19:09 ` Tony Luck
2012-09-06 6:40 ` Naveen N. Rao
2012-09-06 12:28 ` Andi Kleen
0 siblings, 2 replies; 16+ messages in thread
From: Tony Luck @ 2012-09-05 19:09 UTC (permalink / raw)
To: Andi Kleen; +Cc: Naveen N. Rao, ak, bp, x86, linux-kernel, linux-edac, ananth
On Wed, Sep 5, 2012 at 11:47 AM, Andi Kleen <andi@firstfloor.org> wrote:
> On Wed, Sep 05, 2012 at 04:02:37PM +0530, Naveen N. Rao wrote:
>> All the current mce tunables are now available under
>> /sys/devices/system/machinecheck. Start using this new location, but fall back
>> to the older per-cpu location so that we continue working with older kernels.
>
> Who did that change in the kernel?
>
> That breaks Linus rule that the kernel should not break userland.
> Kernel needs to fix that.
The change is still under discussion. Stage one is to add the new global
pathnames in addition to keeping the old per-cpu ones. Also fix all utilities
(just mcelog(8) as far as we know) to prefer the new paths.
After some time[1] ... delete the old paths. This is allowable under Linus'
modified edict that you can change ABI "if nobody complains". If we wait
long enough that the new mcelog is widely deployed, then nobody should
complain.
-Tony
[1] several years - not just a kernel release or two.
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH] [mcelog] Start using the new sysfs tunables location
2012-09-05 19:09 ` Tony Luck
@ 2012-09-06 6:40 ` Naveen N. Rao
2012-09-06 12:28 ` Andi Kleen
1 sibling, 0 replies; 16+ messages in thread
From: Naveen N. Rao @ 2012-09-06 6:40 UTC (permalink / raw)
To: Tony Luck; +Cc: Andi Kleen, ak, bp, x86, linux-kernel, linux-edac, ananth
On 09/06/2012 12:39 AM, Tony Luck wrote:
> On Wed, Sep 5, 2012 at 11:47 AM, Andi Kleen <andi@firstfloor.org> wrote:
>> On Wed, Sep 05, 2012 at 04:02:37PM +0530, Naveen N. Rao wrote:
>>> All the current mce tunables are now available under
>>> /sys/devices/system/machinecheck. Start using this new location, but fall back
>>> to the older per-cpu location so that we continue working with older kernels.
>>
>> Who did that change in the kernel?
>>
>> That breaks Linus rule that the kernel should not break userland.
>> Kernel needs to fix that.
>
> The change is still under discussion. Stage one is to add the new global
> pathnames in addition to keeping the old per-cpu ones. Also fix all utilities
> (just mcelog(8) as far as we know) to prefer the new paths.
>
> After some time[1] ... delete the old paths. This is allowable under Linus'
> modified edict that you can change ABI "if nobody complains". If we wait
> long enough that the new mcelog is widely deployed, then nobody should
> complain.
>
> -Tony
>
> [1] several years - not just a kernel release or two.
>
Tony,
Thanks for clarifying. I should have mentioned in the patch description
that this is indeed subject to the original patch making it into the kernel.
On a related topic. I recently noticed that we don't have an entry for
machinecheck in Documentation/ABI/. Should we add an entry in there? We
could perhaps add the existing entries under obsolete/ and the new
location under testing/?
- Naveen
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 2/3] x86/mce: Pack boolean MCE flags into a structure
2012-09-05 18:56 ` Tony Luck
@ 2012-09-06 6:48 ` Naveen N. Rao
2012-09-06 12:15 ` Borislav Petkov
0 siblings, 1 reply; 16+ messages in thread
From: Naveen N. Rao @ 2012-09-06 6:48 UTC (permalink / raw)
To: Tony Luck, bp
Cc: andi, gong.chen, ananth, x86, linux-kernel, mingo, hpa, tglx,
gregkh, linux-edac
On 09/06/2012 12:26 AM, Tony Luck wrote:
> On Wed, Sep 5, 2012 at 3:22 AM, Naveen N. Rao
> <naveen.n.rao@linux.vnet.ibm.com> wrote:
>> Many MCE flags are boolean in nature, but are declared as integers
>> currently. We can pack these into a bitfield to save some space.
>
> Before this patch:
> size arch/x86/kernel/cpu/mcheck/mce.o
> text data bss dec hex filename
> 18946 4930 776 24652 604c arch/x86/kernel/cpu/mcheck/mce.o
>
> After:
> size arch/x86/kernel/cpu/mcheck/mce.o
> text data bss dec hex filename
> 19335 4890 776 25001 61a9 arch/x86/kernel/cpu/mcheck/mce.o
>
> So we do indeed see "data" reduced by 40 bytes. But
> "text" is up by 389. This seems to be because you have
> another change, not described in the commit log, buried
> in part 2 to add get_dont_log_ce(), set_dont_log_ce() etc.
>
> Compiler version: gcc version 4.4.6 20120305 (Red Hat 4.4.6-4) (GCC)
>
> I know I'm contradicting the feedback you got from Borislav here, but
> is this code churn really worth it to save 40 bytes? I don't think so.
Hmm.. I think I agree. I don't see a good way to get rid of the
individual getters and setters without adding some more code churn. I
guess using boolean would be better. Boris?
Thanks,
Naveen
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 2/3] x86/mce: Pack boolean MCE flags into a structure
2012-09-06 6:48 ` Naveen N. Rao
@ 2012-09-06 12:15 ` Borislav Petkov
0 siblings, 0 replies; 16+ messages in thread
From: Borislav Petkov @ 2012-09-06 12:15 UTC (permalink / raw)
To: Naveen N. Rao
Cc: Tony Luck, andi, gong.chen, ananth, x86, linux-kernel, mingo,
hpa, tglx, gregkh, linux-edac
On Thu, Sep 06, 2012 at 12:18:31PM +0530, Naveen N. Rao wrote:
> >I know I'm contradicting the feedback you got from Borislav here, but
> >is this code churn really worth it to save 40 bytes? I don't think
> >so.
Well, to answer Tony's question, I wanted to have all those config
booleans at the beginning of mce.c packed tightly together as a
single-bit bool values in a config-like struct so that they don't
scatter all over the place and grow out of proportion with more features
being added. So actually I'm willing to swallow the slight increase in
code size for better/more clear code.
> Hmm.. I think I agree. I don't see a good way to get rid of the
> individual getters and setters without adding some more code churn.
> I guess using boolean would be better. Boris?
Yes, it would be worth to try to add a DEVICE_BOOL_ATTR which introduce
a setter/getter flavour for bools and then use that for all.
Thanks.
--
Regards/Gruss,
Boris.
Advanced Micro Devices GmbH
Einsteinring 24, 85609 Dornach
GM: Alberto Bozzo
Reg: Dornach, Landkreis Muenchen
HRB Nr. 43632 WEEE Registernr: 129 19551
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH] [mcelog] Start using the new sysfs tunables location
2012-09-05 19:09 ` Tony Luck
2012-09-06 6:40 ` Naveen N. Rao
@ 2012-09-06 12:28 ` Andi Kleen
2012-09-06 12:34 ` Naveen N. Rao
1 sibling, 1 reply; 16+ messages in thread
From: Andi Kleen @ 2012-09-06 12:28 UTC (permalink / raw)
To: Tony Luck
Cc: Andi Kleen, Naveen N. Rao, bp, x86, linux-kernel, linux-edac, ananth
> The change is still under discussion. Stage one is to add the new global
> pathnames in addition to keeping the old per-cpu ones. Also fix all utilities
> (just mcelog(8) as far as we know) to prefer the new paths.
But why do you even want to change it? Does it fix anything?
AFAIK the old setup -- while not being pretty -- works just fine.
-Andi
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH] [mcelog] Start using the new sysfs tunables location
2012-09-06 12:28 ` Andi Kleen
@ 2012-09-06 12:34 ` Naveen N. Rao
2012-09-06 12:51 ` Alan Cox
2012-09-06 13:21 ` Andi Kleen
0 siblings, 2 replies; 16+ messages in thread
From: Naveen N. Rao @ 2012-09-06 12:34 UTC (permalink / raw)
To: Andi Kleen
Cc: Tony Luck, Andi Kleen, bp, x86, linux-kernel, linux-edac, ananth
On 09/06/2012 05:58 PM, Andi Kleen wrote:
>> The change is still under discussion. Stage one is to add the new global
>> pathnames in addition to keeping the old per-cpu ones. Also fix all utilities
>> (just mcelog(8) as far as we know) to prefer the new paths.
>
> But why do you even want to change it? Does it fix anything?
> AFAIK the old setup -- while not being pretty -- works just fine.
The reason for this was explained in this thread:
http://www.mail-archive.com/linux-kernel@vger.kernel.org/msg298302.html
Even if we decide not to remove these tunables from under their current
per-cpu location, I still think it is much cleaner to have them
available under /sys/devices/system/machinecheck.
- Naveen
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH] [mcelog] Start using the new sysfs tunables location
2012-09-06 12:34 ` Naveen N. Rao
@ 2012-09-06 12:51 ` Alan Cox
2012-09-06 13:21 ` Andi Kleen
1 sibling, 0 replies; 16+ messages in thread
From: Alan Cox @ 2012-09-06 12:51 UTC (permalink / raw)
To: Naveen N. Rao
Cc: Andi Kleen, Tony Luck, Andi Kleen, bp, x86, linux-kernel,
linux-edac, ananth
On Thu, 06 Sep 2012 18:04:27 +0530
"Naveen N. Rao" <naveen.n.rao@linux.vnet.ibm.com> wrote:
> On 09/06/2012 05:58 PM, Andi Kleen wrote:
> >> The change is still under discussion. Stage one is to add the new global
> >> pathnames in addition to keeping the old per-cpu ones. Also fix all utilities
> >> (just mcelog(8) as far as we know) to prefer the new paths.
> >
> > But why do you even want to change it? Does it fix anything?
> > AFAIK the old setup -- while not being pretty -- works just fine.
>
> The reason for this was explained in this thread:
> http://www.mail-archive.com/linux-kernel@vger.kernel.org/msg298302.html
>
> Even if we decide not to remove these tunables from under their current
> per-cpu location, I still think it is much cleaner to have them
> available under /sys/devices/system/machinecheck.
That to me seems a ridiculous proposal. What are you going to do if in
future they ceased to be system wide ? Move them back ?
The threshold for playing musical chairs with sysfs nodes is a lot higher
than "I think it's much cleaner"
The current approach is a lot more futureproof even if a spot more ugly.
Alan
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH] [mcelog] Start using the new sysfs tunables location
2012-09-06 12:34 ` Naveen N. Rao
2012-09-06 12:51 ` Alan Cox
@ 2012-09-06 13:21 ` Andi Kleen
1 sibling, 0 replies; 16+ messages in thread
From: Andi Kleen @ 2012-09-06 13:21 UTC (permalink / raw)
To: Naveen N. Rao
Cc: Andi Kleen, Tony Luck, Andi Kleen, bp, x86, linux-kernel,
linux-edac, ananth
> Even if we decide not to remove these tunables from under their current
> per-cpu location, I still think it is much cleaner to have them
> available under /sys/devices/system/machinecheck.
"much cleaner" is not sufficient justification to break an ABI.
-Andi
--
ak@linux.intel.com -- Speaking for myself only.
^ permalink raw reply [flat|nested] 16+ messages in thread
end of thread, other threads:[~2012-09-06 13:21 UTC | newest]
Thread overview: 16+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2012-09-05 10:21 [PATCH 0/3] x86:mce: Some cleanups and bios-set CMCI thresholds Naveen N. Rao
2012-09-05 10:21 ` [PATCH 1/3] x86/mce: Make sysfs tunables available globally across all cpus Naveen N. Rao
2012-09-05 10:32 ` [PATCH] [mcelog] Start using the new sysfs tunables location Naveen N. Rao
2012-09-05 18:47 ` Andi Kleen
2012-09-05 19:09 ` Tony Luck
2012-09-06 6:40 ` Naveen N. Rao
2012-09-06 12:28 ` Andi Kleen
2012-09-06 12:34 ` Naveen N. Rao
2012-09-06 12:51 ` Alan Cox
2012-09-06 13:21 ` Andi Kleen
2012-09-05 10:22 ` [PATCH 2/3] x86/mce: Pack boolean MCE flags into a structure Naveen N. Rao
2012-09-05 17:15 ` Joe Perches
2012-09-05 18:56 ` Tony Luck
2012-09-06 6:48 ` Naveen N. Rao
2012-09-06 12:15 ` Borislav Petkov
2012-09-05 10:22 ` [PATCH 3/3] x86/mce: Honour bios-set CMCI threshold Naveen N. Rao
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).