Em 30-04-2012 10:00, Mauro Carvalho Chehab escreveu:
> Em 30-04-2012 09:38, Borislav Petkov escreveu:
>> On Mon, Apr 30, 2012 at 08:45:09AM -0300, Mauro Carvalho Chehab wrote:
>>> Em 30-04-2012 08:11, Borislav Petkov escreveu:
>>>> On Mon, Apr 30, 2012 at 07:58:33AM -0300, Mauro Carvalho Chehab wrote:
>
>>>> This way it says "initializing 12 dimms" and the user thinks there are
>>>> 12 DIMMs on his system where this might not be true.
>>>
>>>
>>> I'm OK to remove the "initializing 12 dimms" message. It doesn't add
>>> anything
>>> new.
>>>
>>> With regards do the other messages, if the debug messages are not clear,
>>> then let's fix them, instead of removing. What if we print, instead,
>>> on a message like:
>>>
>>> "row 1, chan 1 will represent dimm5 (1:2:0) if not empty"
>>
>> How about the following instead: the specific driver calls
>> edac_mc_alloc(), it gets the allocated dimm array in mci->dimms
>> _without_ dumping each dimm%d line. Then, each driver figures out which
>> subset of that dimms array actually has populated slots and prints only
>> the populated rank/slot/...
>>
>> This information is much more valuable than saying how many _possible_
>> slots the edac core has allocated.
>>
>> Then, each driver can decide whether it makes sense to dump that info or
>> not.
>
> No, that would add extra complexity at the drivers level just due to debug
> messages. I think that the better is to move this printk to the debug-specific
> routine that is called only when the dimm is filled (edac_mc_dump_dimm).
>
> With this cange, the message will be printed only for the filled dimms.
>
> This is a cleanup patch, so I'll write it, together with the change that
> will get rid of the loop that uses KERN_CONT. It will use a function added
> by a latter patch at edac_mc_sysfs so it can't be merged on this patch
> anyway.
The following patch dos the debug cleanup. I'll add at the end of my tree.
Regards,
Mauro.
From: Mauro Carvalho Chehab <mche...@redhat.com>
Date: Mon, 30 Apr 2012 10:24:43 -0300
Subject: [PATCH] edac_mc: Cleanup per-dimm_info debug messages
The edac_mc_alloc() routine allocates one dimm_info device for all
possible memories, including the non-filled ones. The debug messages
there are somewhat confusing. So, cleans them, by moving the code
that prints the memory location to edac_mc, and using it on both
edac_mc_sysfs and edac_mc.
After this patch, a dimm-based memory controller will print the debug
info as:
[ 728.430828] EDAC DEBUG: edac_mc_dump_dimm: dimm2: channel 0 slot 2 mapped
as virtual row 0, chan 2
[ 728.430834] EDAC DEBUG: edac_mc_dump_dimm: dimm->label =
'mc#0channel#0slot#2'
[ 728.430839] EDAC DEBUG: edac_mc_dump_dimm: dimm->nr_pages = 0x0
[ 728.430846] EDAC DEBUG: edac_mc_dump_dimm: dimm->grain = 0
[ 728.430850] EDAC DEBUG: edac_mc_dump_dimm: dimm->nr_pages = 0x0
(a rank-based memory controller would print, instead, "rank2"
on the above debug info)
Signed-off-by: Mauro Carvalho Chehab <mche...@redhat.com>
diff --git a/drivers/edac/edac_mc.c b/drivers/edac/edac_mc.c
index d8278b3..1bc2843 100644
--- a/drivers/edac/edac_mc.c
+++ b/drivers/edac/edac_mc.c
@@ -40,6 +40,25 @@
static DEFINE_MUTEX(mem_ctls_mutex);
static LIST_HEAD(mc_devices);
+unsigned edac_dimm_info_location(struct dimm_info *dimm, char *buf,
+ int len)
+{
+ struct mem_ctl_info *mci = dimm->mci;
+ int i, n, count = 0;
+ char *p = buf;
+
+ for (i = 0; i < mci->n_layers; i++) {
+ n = snprintf(p, len, "%s %d ",
+ edac_layer_name[mci->layers[i].type],
+ dimm->location[i]);
+ p += n;
+ len -= n;
+ count += n;
+ }
+
+ return count;
+}
+
#ifdef CONFIG_EDAC_DEBUG
static void edac_mc_dump_channel(struct rank_info *chan)
@@ -50,20 +69,18 @@ static void edac_mc_dump_channel(struct rank_info *chan)
debugf4("\tchannel->dimm = %p\n", chan->dimm);
}
-static void edac_mc_dump_dimm(struct dimm_info *dimm)
+static void edac_mc_dump_dimm(struct dimm_info *dimm, int number)
{
- int i;
+ char location[80];
+
+ edac_dimm_info_location(dimm, location, sizeof(location));
debugf4("\tdimm = %p\n", dimm);
+ debugf4("\t%s%i: %smapped as virtual row %d, chan %d\n",
+ dimm->mci->mem_is_per_rank ? "rank" : "dimm",
+ number, location, dimm->csrow, dimm->cschannel);
debugf4("\tdimm->label = '%s'\n", dimm->label);
debugf4("\tdimm->nr_pages = 0x%x\n", dimm->nr_pages);
- debugf4("\tdimm location ");
- for (i = 0; i < dimm->mci->n_layers; i++) {
- printk(KERN_CONT "%d", dimm->location[i]);
- if (i < dimm->mci->n_layers - 1)
- printk(KERN_CONT ".");
- }
- printk(KERN_CONT "\n");
debugf4("\tdimm->grain = %d\n", dimm->grain);
debugf4("\tdimm->nr_pages = 0x%x\n", dimm->nr_pages);
}
@@ -337,8 +354,6 @@ struct mem_ctl_info *edac_mc_alloc(unsigned edac_index,
memset(&pos, 0, sizeof(pos));
row = 0;
chn = 0;
- debugf4("initializing %d %s\n", tot_dimms,
- per_rank ? "ranks" : "dimms");
for (i = 0; i < tot_dimms; i++) {
chan = mci->csrows[row]->channels[chn];
off = EDAC_DIMM_OFF(layer, n_layers, pos[0], pos[1], pos[2]);
@@ -351,10 +366,6 @@ struct mem_ctl_info *edac_mc_alloc(unsigned edac_index,
mci->dimms[off] = dimm;
dimm->mci = mci;
- debugf2("%d: %s%i (%d:%d:%d): row %d, chan %d\n", i,
- per_rank ? "rank" : "dimm", off,
- pos[0], pos[1], pos[2], row, chn);
-
/*
* Copy DIMM location and initialize the memory location
*/
@@ -730,7 +741,7 @@ int edac_mc_add_mc(struct mem_ctl_info *mci)
edac_mc_dump_channel(mci->csrows[i]->channels[j]);
}
for (i = 0; i < mci->tot_dimms; i++)
- edac_mc_dump_dimm(mci->dimms[i]);
+ edac_mc_dump_dimm(mci->dimms[i], i);
}
#endif
mutex_lock(&mem_ctls_mutex);
diff --git a/drivers/edac/edac_mc_sysfs.c b/drivers/edac/edac_mc_sysfs.c
index 8f96c49..e3e9e75 100644
--- a/drivers/edac/edac_mc_sysfs.c
+++ b/drivers/edac/edac_mc_sysfs.c
@@ -488,13 +488,7 @@ static ssize_t dimmdev_location_show(struct device *dev,
int i;
char *p = data;
- for (i = 0; i < mci->n_layers; i++) {
- p += sprintf(p, "%s %d ",
- edac_layer_name[mci->layers[i].type],
- dimm->location[i]);
- }
-
- return p - data;
+ return edac_dimm_info_location(dimm, data, PAGE_SIZE);
}
static ssize_t dimmdev_label_show(struct device *dev,
diff --git a/drivers/edac/edac_module.h b/drivers/edac/edac_module.h
index 1af1367..de92756 100644
--- a/drivers/edac/edac_module.h
+++ b/drivers/edac/edac_module.h
@@ -34,6 +34,9 @@ extern int edac_mc_get_panic_on_ue(void);
extern int edac_get_poll_msec(void);
extern int edac_mc_get_poll_msec(void);
+unsigned edac_dimm_info_location(struct dimm_info *dimm, char *buf,
+ int len);
+
/* on edac_device.c */
extern int edac_device_register_sysfs_main_kobj(
struct edac_device_ctl_info *edac_dev);
_______________________________________________
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
