* [PATCH 2/3] i7core_edac: Use a lockless ringbuffer
[not found] <20090924192821.7fcffc80@pedra.chehab.org>
2009-10-04 19:16 ` [PATCH 0/3] i7core_edac and edac_mce Mauro Carvalho Chehab
2009-10-04 19:43 ` [PATCH 3/3] i7core_edac: Better parse "any" addrmask Mauro Carvalho Chehab
@ 2009-10-04 19:43 ` Mauro Carvalho Chehab
2009-10-04 19:43 ` [PATCH 1/3] edac: Create an unique instance for each kobj Mauro Carvalho Chehab
3 siblings, 0 replies; 8+ messages in thread
From: Mauro Carvalho Chehab @ 2009-10-04 19:43 UTC (permalink / raw)
To: Mauro Carvalho Chehab
Cc: Ingo Molnar, bluesmoke-devel, Andrew Morton, linux-kernel
>From ece799f1fbe9af74c518c00ab5dd271ba9eb8855 Mon Sep 17 00:00:00 2001
From: Mauro Carvalho Chehab <mchehab@redhat.com>
Date: Sun, 4 Oct 2009 10:15:40 -0300
Subject: [PATCH] i7core_edac: Use a lockless ringbuffer
Signed-off-by: Mauro Carvalho Chehab <mchehab@redhat.com>
---
drivers/edac/i7core_edac.c | 83 +++++++++++++++++++++++++++++---------------
1 files changed, 55 insertions(+), 28 deletions(-)
diff --git a/drivers/edac/i7core_edac.c b/drivers/edac/i7core_edac.c
index 97f6d17..94aeca0 100644
--- a/drivers/edac/i7core_edac.c
+++ b/drivers/edac/i7core_edac.c
@@ -28,7 +28,6 @@
#include <linux/edac.h>
#include <linux/mmzone.h>
#include <linux/edac_mce.h>
-#include <linux/spinlock.h>
#include <linux/smp.h>
#include <asm/processor.h>
@@ -239,9 +238,16 @@ struct i7core_pvt {
/* mcelog glue */
struct edac_mce edac_mce;
+
+ /* Fifo double buffers */
struct mce mce_entry[MCE_LOG_LEN];
- unsigned mce_count;
- spinlock_t mce_lock;
+ struct mce mce_outentry[MCE_LOG_LEN];
+
+ /* Fifo in/out counters */
+ unsigned mce_in, mce_out;
+
+ /* Count indicator to show errors not got */
+ unsigned mce_overrun;
};
/* Static vars */
@@ -1617,30 +1623,50 @@ static void i7core_check_error(struct mem_ctl_info *mci)
struct i7core_pvt *pvt = mci->pvt_info;
int i;
unsigned count = 0;
- struct mce *m = NULL;
- unsigned long flags;
+ struct mce *m;
- /* Copy all mce errors into a temporary buffer */
- spin_lock_irqsave(&pvt->mce_lock, flags);
- if (pvt->mce_count) {
- m = kmalloc(sizeof(*m) * pvt->mce_count, GFP_ATOMIC);
+ /*
+ * MCE first step: Copy all mce errors into a temporary buffer
+ * We use a double buffering here, to reduce the risk of
+ * loosing an error.
+ */
+ smp_rmb();
+ count = (pvt->mce_out + sizeof(mce_entry) - pvt->mce_in)
+ % sizeof(mce_entry);
+ if (!count)
+ return;
- if (m) {
- count = pvt->mce_count;
- memcpy(m, &pvt->mce_entry, sizeof(*m) * count);
- }
- pvt->mce_count = 0;
- }
+ m = pvt->mce_outentry;
+ if (pvt->mce_in + count > sizeof(mce_entry)) {
+ unsigned l = sizeof(mce_entry) - pvt->mce_in;
- spin_unlock_irqrestore(&pvt->mce_lock, flags);
+ memcpy(m, &pvt->mce_entry[pvt->mce_in], sizeof(*m) * l);
+ smp_wmb();
+ pvt->mce_in = 0;
+ count -= l;
+ m += l;
+ }
+ memcpy(m, &pvt->mce_entry[pvt->mce_in], sizeof(*m) * count);
+ smp_wmb();
+ pvt->mce_in += count;
+
+ smp_rmb();
+ if (pvt->mce_overrun) {
+ i7core_printk(KERN_ERR, "Lost %d memory errors\n",
+ pvt->mce_overrun);
+ smp_wmb();
+ pvt->mce_overrun = 0;
+ }
- /* proccess mcelog errors */
+ /*
+ * MCE second step: parse errors and display
+ */
for (i = 0; i < count; i++)
- i7core_mce_output_error(mci, &m[i]);
+ i7core_mce_output_error(mci, &pvt->mce_outentry[i]);
- kfree(m);
-
- /* check memory count errors */
+ /*
+ * Now, let's increment CE error counts
+ */
if (!pvt->is_registered)
i7core_udimm_check_mc_ecc_err(mci);
else
@@ -1657,7 +1683,6 @@ static int i7core_mce_check_error(void *priv, struct mce *mce)
{
struct mem_ctl_info *mci = priv;
struct i7core_pvt *pvt = mci->pvt_info;
- unsigned long flags;
/*
* Just let mcelog handle it if the error is
@@ -1679,12 +1704,15 @@ static int i7core_mce_check_error(void *priv, struct mce *mce)
return 0;
}
- spin_lock_irqsave(&pvt->mce_lock, flags);
- if (pvt->mce_count < MCE_LOG_LEN) {
- memcpy(&pvt->mce_entry[pvt->mce_count], mce, sizeof(*mce));
- pvt->mce_count++;
+ smp_rmb();
+ if ((pvt->mce_out + 1) % sizeof(mce_entry) == pvt->mce_in) {
+ smp_wmb();
+ pvt->mce_overrun++;
+ return 0;
}
- spin_unlock_irqrestore(&pvt->mce_lock, flags);
+ smp_wmb();
+ pvt->mce_out = (pvt->mce_out + 1) % sizeof(mce_entry);
+ memcpy(&pvt->mce_entry[pvt->mce_out], mce, sizeof(*mce));
/* Handle fatal errors immediately */
if (mce->mcgstatus & 1)
@@ -1777,7 +1805,6 @@ static int i7core_register_mci(struct i7core_dev *i7core_dev,
/* Registers on edac_mce in order to receive memory errors */
pvt->edac_mce.priv = mci;
pvt->edac_mce.check_error = i7core_mce_check_error;
- spin_lock_init(&pvt->mce_lock);
rc = edac_mce_register(&pvt->edac_mce);
if (unlikely(rc < 0))
--
Cheers,
Mauro
------------------------------------------------------------------------------
Come build with us! The BlackBerry® Developer Conference in SF, CA
is the only developer event you need to attend this year. Jumpstart your
developing skills, take BlackBerry mobile applications to market and stay
ahead of the curve. Join us from November 9-12, 2009. Register now!
http://p.sf.net/sfu/devconf
^ permalink raw reply related [flat|nested] 8+ messages in thread* [PATCH 1/3] edac: Create an unique instance for each kobj
[not found] <20090924192821.7fcffc80@pedra.chehab.org>
` (2 preceding siblings ...)
2009-10-04 19:43 ` [PATCH 2/3] i7core_edac: Use a lockless ringbuffer Mauro Carvalho Chehab
@ 2009-10-04 19:43 ` Mauro Carvalho Chehab
2009-10-14 3:01 ` Andrew Morton
3 siblings, 1 reply; 8+ messages in thread
From: Mauro Carvalho Chehab @ 2009-10-04 19:43 UTC (permalink / raw)
To: Mauro Carvalho Chehab
Cc: Ingo Molnar, bluesmoke-devel, Andrew Morton, linux-kernel
Current code only works when there's just one memory
controller, since we need one kobj for each instance.
Signed-off-by: Mauro Carvalho Chehab <mchehab@redhat.com>
---
drivers/edac/edac_core.h | 21 ++++++++---
drivers/edac/edac_mc_sysfs.c | 75 +++++++++++++++++++++++++++--------------
2 files changed, 64 insertions(+), 32 deletions(-)
diff --git a/drivers/edac/edac_core.h b/drivers/edac/edac_core.h
index d035ee9..f84da6e 100644
--- a/drivers/edac/edac_core.h
+++ b/drivers/edac/edac_core.h
@@ -341,23 +341,29 @@ struct csrow_info {
};
struct mcidev_sysfs_group {
- const char *name;
- struct mcidev_sysfs_attribute *mcidev_attr;
- struct kobject kobj;
+ const char *name; /* group name */
+ struct mcidev_sysfs_attribute *mcidev_attr; /* group attributes */
+};
+
+struct mcidev_sysfs_group_kobj {
+ struct list_head list; /* list for all instances within a mc */
+
+ struct kobject kobj; /* kobj for the group */
+ struct mcidev_sysfs_group *grp; /* group description table */
struct mem_ctl_info *mci; /* the parent */
};
-
/* mcidev_sysfs_attribute structure
* used for driver sysfs attributes and in mem_ctl_info
* sysfs top level entries
*/
struct mcidev_sysfs_attribute {
+ /* It should use either attr or grp */
struct attribute attr;
+ struct mcidev_sysfs_group *grp; /* Points to a group of attributes */
- struct mcidev_sysfs_group *grp;
-
+ /* Ops for show/store values at the attribute - not used on group */
ssize_t (*show)(struct mem_ctl_info *,char *);
ssize_t (*store)(struct mem_ctl_info *, const char *,size_t);
};
@@ -435,6 +441,9 @@ struct mem_ctl_info {
/* edac sysfs device control */
struct kobject edac_mci_kobj;
+ /* list for all grp instances within a mc */
+ struct list_head grp_kobj_list;
+
/* Additional top controller level attributes, but specified
* by the low level driver.
*
diff --git a/drivers/edac/edac_mc_sysfs.c b/drivers/edac/edac_mc_sysfs.c
index 473246b..299547c 100644
--- a/drivers/edac/edac_mc_sysfs.c
+++ b/drivers/edac/edac_mc_sysfs.c
@@ -729,7 +729,7 @@ void edac_mc_unregister_sysfs_main_kobj(struct mem_ctl_info *mci)
#define EDAC_DEVICE_SYMLINK "device"
-#define grp_to_mci(k) (container_of(k, struct mcidev_sysfs_group, kobj)->mci)
+#define grp_to_mci(k) (container_of(k, struct mcidev_sysfs_group_kobj, kobj)->mci)
/* MCI show/store functions for top most object */
static ssize_t inst_grp_show(struct kobject *kobj, struct attribute *attr,
@@ -763,12 +763,12 @@ static ssize_t inst_grp_store(struct kobject *kobj, struct attribute *attr,
/* No memory to release for this kobj */
static void edac_inst_grp_release(struct kobject *kobj)
{
- struct mcidev_sysfs_group *grp;
+ struct mcidev_sysfs_group_kobj *grp;
struct mem_ctl_info *mci;
debugf1("%s()\n", __func__);
- grp = container_of(kobj, struct mcidev_sysfs_group, kobj);
+ grp = container_of(kobj, struct mcidev_sysfs_group_kobj, kobj);
mci = grp->mci;
kobject_put(&mci->edac_mci_kobj);
@@ -803,22 +803,30 @@ static int edac_create_mci_instance_attributes(struct mem_ctl_info *mci,
while (sysfs_attrib) {
if (sysfs_attrib->grp) {
- struct kobject *newkobj = &sysfs_attrib->grp->kobj;
+ struct mcidev_sysfs_group_kobj *grp_kobj;
+
+ grp_kobj = kzalloc(sizeof(*grp_kobj), GFP_KERNEL);
+ if (!grp_kobj)
+ return -ENOMEM;
+
+ list_add_tail(&grp_kobj->list, &mci->grp_kobj_list);
+
+ grp_kobj->grp = sysfs_attrib->grp;
+ grp_kobj->mci = mci;
debugf0("%s() grp %s, mci %p\n", __func__,
sysfs_attrib->grp->name, mci);
- sysfs_attrib->grp->mci = mci;
-
- err = kobject_init_and_add(newkobj, &ktype_inst_grp,
+ err = kobject_init_and_add(&grp_kobj->kobj,
+ &ktype_inst_grp,
&mci->edac_mci_kobj,
sysfs_attrib->grp->name);
if (err)
return err;
err = edac_create_mci_instance_attributes(mci,
- sysfs_attrib->grp->mcidev_attr,
- newkobj);
+ grp_kobj->grp->mcidev_attr,
+ &grp_kobj->kobj);
if (err)
return err;
@@ -844,25 +852,27 @@ static int edac_create_mci_instance_attributes(struct mem_ctl_info *mci,
* remove MC driver specific attributes at the topmost level
* directory of this mci instance.
*/
-static void edac_remove_mci_instance_attributes(
+static void edac_remove_mci_instance_attributes(struct mem_ctl_info *mci,
struct mcidev_sysfs_attribute *sysfs_attrib,
- struct kobject *kobj)
+ struct kobject *kobj, int count)
{
+ struct mcidev_sysfs_group_kobj *grp_kobj, *tmp;
+
debugf1("%s()\n", __func__);
- /* loop if there are attributes and until we hit a NULL entry */
+ /*
+ * loop if there are attributes and until we hit a NULL entry
+ * Remove first all the atributes
+ */
while (sysfs_attrib) {
if (sysfs_attrib->grp) {
- struct kobject *newkobj = &sysfs_attrib->grp->kobj;
-
- debugf0("%s() grp %s\n", __func__,
- sysfs_attrib->grp->name);
-
- edac_remove_mci_instance_attributes(
- sysfs_attrib->grp->mcidev_attr, newkobj);
-
- kobject_put(newkobj);
- } else if (sysfs_attrib->attr.name) {
+ list_for_each_entry(grp_kobj, &mci->grp_kobj_list,
+ list)
+ if (grp_kobj->grp == sysfs_attrib->grp)
+ edac_remove_mci_instance_attributes(mci,
+ grp_kobj->grp->mcidev_attr,
+ &grp_kobj->kobj, count + 1);
+ } else if (sysfs_attrib->attr.name) {
debugf0("%s() file %s\n", __func__,
sysfs_attrib->attr.name);
sysfs_remove_file(kobj, &sysfs_attrib->attr);
@@ -870,6 +880,16 @@ static void edac_remove_mci_instance_attributes(
break;
sysfs_attrib++;
}
+
+ /*
+ * Now that all attributes got removed, it is save to remove all groups
+ */
+ if (!count)
+ list_for_each_entry_safe(grp_kobj, tmp, &mci->grp_kobj_list,
+ list) {
+ debugf0("%s() grp %s\n", __func__, grp_kobj->grp->name);
+ kobject_put(&grp_kobj->kobj);
+ }
}
@@ -890,6 +910,8 @@ int edac_create_sysfs_mci_device(struct mem_ctl_info *mci)
debugf0("%s() idx=%d\n", __func__, mci->mc_idx);
+ INIT_LIST_HEAD(&mci->grp_kobj_list);
+
/* create a symlink for the device */
err = sysfs_create_link(kobj_mci, &mci->dev->kobj,
EDAC_DEVICE_SYMLINK);
@@ -939,8 +961,8 @@ fail1:
}
/* remove the mci instance's attributes, if any */
- edac_remove_mci_instance_attributes(
- mci->mc_driver_sysfs_attributes, &mci->edac_mci_kobj);
+ edac_remove_mci_instance_attributes(mci,
+ mci->mc_driver_sysfs_attributes, &mci->edac_mci_kobj, 0);
/* remove the symlink */
sysfs_remove_link(kobj_mci, EDAC_DEVICE_SYMLINK);
@@ -974,8 +996,9 @@ void edac_remove_sysfs_mci_device(struct mem_ctl_info *mci)
debugf0("%s() remove_mci_instance\n", __func__);
/* remove this mci instance's attribtes */
- edac_remove_mci_instance_attributes(mci->mc_driver_sysfs_attributes,
- &mci->edac_mci_kobj);
+ edac_remove_mci_instance_attributes(mci,
+ mci->mc_driver_sysfs_attributes,
+ &mci->edac_mci_kobj, 0);
debugf0("%s() unregister this mci kobj\n", __func__);
/* unregister this instance's kobject *
--
Cheers,
Mauro
------------------------------------------------------------------------------
Come build with us! The BlackBerry® Developer Conference in SF, CA
is the only developer event you need to attend this year. Jumpstart your
developing skills, take BlackBerry mobile applications to market and stay
ahead of the curve. Join us from November 9-12, 2009. Register now!
http://p.sf.net/sfu/devconf
^ permalink raw reply related [flat|nested] 8+ messages in thread