lkml.org 
[lkml]   [2012]   [Apr]   [2]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    Patch in this message
    /
    Date
    From
    SubjectRe: [PATCH 04/13] edac: move nr_pages to dimm struct
    On Thu, Mar 29, 2012 at 01:45:37PM -0300, Mauro Carvalho Chehab wrote:
    > The number of pages is a dimm property. Move it to the dimm struct.
    >
    > After this change, it is possible to add sysfs nodes for the DIMM's that

    DIMMs

    > will properly represent the DIMM stick properties, including its size.
    >
    > A TODO fix here is to properly represent dual-rank/quad-rank DIMMs when
    > the memory controller represents the memory via chip select rows.
    >
    > Signed-off-by: Mauro Carvalho Chehab <mchehab@redhat.com>
    > ---
    > drivers/edac/amd64_edac.c | 13 ++++------
    > drivers/edac/amd76x_edac.c | 6 ++--
    > drivers/edac/cell_edac.c | 8 ++++--
    > drivers/edac/cpc925_edac.c | 8 ++++--
    > drivers/edac/e752x_edac.c | 6 +++-
    > drivers/edac/e7xxx_edac.c | 5 ++-
    > drivers/edac/edac_mc.c | 28 ++++++++++++---------
    > drivers/edac/edac_mc_sysfs.c | 52 ++++++++++++++++++++++++++++++----------
    > drivers/edac/i3000_edac.c | 6 +++-
    > drivers/edac/i3200_edac.c | 3 +-
    > drivers/edac/i5000_edac.c | 14 ++++++----
    > drivers/edac/i5100_edac.c | 22 ++++++++++-------
    > drivers/edac/i5400_edac.c | 9 ++----
    > drivers/edac/i7300_edac.c | 22 ++++------------
    > drivers/edac/i7core_edac.c | 10 ++-----
    > drivers/edac/i82443bxgx_edac.c | 2 +-
    > drivers/edac/i82860_edac.c | 2 +-
    > drivers/edac/i82875p_edac.c | 5 ++-
    > drivers/edac/i82975x_edac.c | 11 ++++++--
    > drivers/edac/mpc85xx_edac.c | 3 +-
    > drivers/edac/mv64x60_edac.c | 3 +-
    > drivers/edac/pasemi_edac.c | 14 +++++-----
    > drivers/edac/ppc4xx_edac.c | 5 ++-
    > drivers/edac/r82600_edac.c | 3 +-
    > drivers/edac/sb_edac.c | 8 +----
    > drivers/edac/tile_edac.c | 2 +-
    > drivers/edac/x38_edac.c | 4 +-
    > include/linux/edac.h | 10 ++++---
    > 28 files changed, 158 insertions(+), 126 deletions(-)
    >
    > diff --git a/drivers/edac/amd64_edac.c b/drivers/edac/amd64_edac.c
    > index b1b1551..ad0376e 100644
    > --- a/drivers/edac/amd64_edac.c
    > +++ b/drivers/edac/amd64_edac.c
    > @@ -2126,12 +2126,6 @@ static u32 amd64_csrow_nr_pages(struct amd64_pvt *pvt, u8 dct, int csrow_nr)
    >
    > nr_pages = pvt->ops->dbam_to_cs(pvt, dct, cs_mode) << (20 - PAGE_SHIFT);
    >
    > - /*
    > - * If dual channel then double the memory size of single channel.
    > - * Channel count is 1 or 2
    > - */
    > - nr_pages <<= (pvt->channel_count - 1);
    > -
    > debugf0(" (csrow=%d) DBAM map index= %d\n", csrow_nr, cs_mode);
    > debugf0(" nr_pages= %u channel-count = %d\n",
    > nr_pages, pvt->channel_count);
    > @@ -2152,6 +2146,7 @@ static int init_csrows(struct mem_ctl_info *mci)
    > int i, j, empty = 1;
    > enum mem_type mtype;
    > enum edac_type edac_mode;
    > + int nr_pages;
    >
    > amd64_read_pci_cfg(pvt->F3, NBCFG, &val);
    >
    > @@ -2174,7 +2169,7 @@ static int init_csrows(struct mem_ctl_info *mci)
    > i, pvt->mc_node_id);
    >
    > empty = 0;
    > - csrow->nr_pages = amd64_csrow_nr_pages(pvt, 0, i);
    > + nr_pages = amd64_csrow_nr_pages(pvt, 0, i);
    > get_cs_base_and_mask(pvt, i, 0, &base, &mask);
    > /* 8 bytes of resolution */
    >
    > @@ -2192,10 +2187,12 @@ static int init_csrows(struct mem_ctl_info *mci)
    > for (j = 0; j < pvt->channel_count; j++) {
    > csrow->channels[j].dimm->mtype = mtype;
    > csrow->channels[j].dimm->edac_mode = edac_mode;
    > + csrow->channels[j].dimm->nr_pages = nr_pages;
    > +

    One newline too many.

    > }
    >
    > debugf1(" for MC node %d csrow %d:\n", pvt->mc_node_id, i);
    > - debugf1(" nr_pages: %u\n", csrow->nr_pages);
    > + debugf1(" nr_pages: %u\n", nr_pages);
    > }
    >
    > return empty;


    <snip>

    > diff --git a/drivers/edac/edac_mc.c b/drivers/edac/edac_mc.c
    > index 2430ddb..02263c3 100644
    > --- a/drivers/edac/edac_mc.c
    > +++ b/drivers/edac/edac_mc.c
    > @@ -43,22 +43,22 @@ static void edac_mc_dump_channel(struct rank_info *chan)
    > {
    > debugf4("\tchannel = %p\n", chan);
    > debugf4("\tchannel->chan_idx = %d\n", chan->chan_idx);
    > - debugf4("\tchannel->ce_count = %d\n", chan->dimm->ce_count);
    > - debugf4("\tchannel->label = '%s'\n", chan->dimm->label);
    > debugf4("\tchannel->csrow = %p\n\n", chan->csrow);
    > + debugf4("\tdimm->ce_count = %d\n", chan->dimm->ce_count);
    > + debugf4("\tdimm->label = '%s'\n", chan->dimm->label);
    > + debugf4("\tdimm->nr_pages = 0x%x\n", chan->dimm->nr_pages);
    > }
    >
    > static void edac_mc_dump_csrow(struct csrow_info *csrow)
    > {
    > debugf4("\tcsrow = %p\n", csrow);
    > debugf4("\tcsrow->csrow_idx = %d\n", csrow->csrow_idx);
    > - debugf4("\tcsrow->first_page = 0x%lx\n", csrow->first_page);
    > - debugf4("\tcsrow->last_page = 0x%lx\n", csrow->last_page);
    > - debugf4("\tcsrow->page_mask = 0x%lx\n", csrow->page_mask);
    > - debugf4("\tcsrow->nr_pages = 0x%x\n", csrow->nr_pages);
    > debugf4("\tcsrow->nr_channels = %d\n", csrow->nr_channels);
    > debugf4("\tcsrow->channels = %p\n", csrow->channels);
    > debugf4("\tcsrow->mci = %p\n\n", csrow->mci);
    > + debugf4("\tcsrow->first_page = 0x%lx\n", csrow->first_page);
    > + debugf4("\tcsrow->last_page = 0x%lx\n", csrow->last_page);
    > + debugf4("\tcsrow->page_mask = 0x%lx\n", csrow->page_mask);

    Useless code churn: simply remove the ->nr_pages line.

    > }
    >
    > static void edac_mc_dump_mci(struct mem_ctl_info *mci)
    > @@ -655,15 +655,19 @@ static void edac_mc_scrub_block(unsigned long page, unsigned long offset,
    > int edac_mc_find_csrow_by_page(struct mem_ctl_info *mci, unsigned long page)
    > {
    > struct csrow_info *csrows = mci->csrows;
    > - int row, i;
    > + int row, i, j, n;
    >
    > debugf1("MC%d: %s(): 0x%lx\n", mci->mc_idx, __func__, page);
    > row = -1;
    >
    > for (i = 0; i < mci->nr_csrows; i++) {
    > struct csrow_info *csrow = &csrows[i];
    > -
    > - if (csrow->nr_pages == 0)
    > + n = 0;
    > + for (j = 0; j < csrow->nr_channels; j++) {
    > + struct dimm_info *dimm = csrow->channels[j].dimm;
    > + n += dimm->nr_pages;
    > + }
    > + if (n == 0)
    > continue;
    >
    > debugf3("MC%d: %s(): first(0x%lx) page(0x%lx) last(0x%lx) "
    > @@ -672,9 +676,9 @@ int edac_mc_find_csrow_by_page(struct mem_ctl_info *mci, unsigned long page)
    > csrow->page_mask);
    >
    > if ((page >= csrow->first_page) &&
    > - (page <= csrow->last_page) &&
    > - ((page & csrow->page_mask) ==
    > - (csrow->first_page & csrow->page_mask))) {
    > + (page <= csrow->last_page) &&
    > + ((page & csrow->page_mask) ==
    > + (csrow->first_page & csrow->page_mask))) {

    Useless and wrong formatting change, please retain the original formatting.

    > row = i;
    > break;
    > }
    > diff --git a/drivers/edac/edac_mc_sysfs.c b/drivers/edac/edac_mc_sysfs.c
    > index d63904e..52c56cf 100644
    > --- a/drivers/edac/edac_mc_sysfs.c
    > +++ b/drivers/edac/edac_mc_sysfs.c
    > @@ -144,7 +144,13 @@ static ssize_t csrow_ce_count_show(struct csrow_info *csrow, char *data,
    > static ssize_t csrow_size_show(struct csrow_info *csrow, char *data,
    > int private)
    > {
    > - return sprintf(data, "%u\n", PAGES_TO_MiB(csrow->nr_pages));
    > + int i;
    > + u32 nr_pages = 0;
    > +
    > + for (i = 0; i < csrow->nr_channels; i++)
    > + nr_pages += csrow->channels[i].dimm->nr_pages;
    > +
    > + return sprintf(data, "%u\n", PAGES_TO_MiB(nr_pages));
    > }
    >
    > static ssize_t csrow_mem_type_show(struct csrow_info *csrow, char *data,
    > @@ -519,16 +525,17 @@ static ssize_t mci_ctl_name_show(struct mem_ctl_info *mci, char *data)
    >
    > static ssize_t mci_size_mb_show(struct mem_ctl_info *mci, char *data)
    > {
    > - int total_pages, csrow_idx;
    > + int total_pages, csrow_idx, j;
    >
    > for (total_pages = csrow_idx = 0; csrow_idx < mci->nr_csrows;
    > - csrow_idx++) {
    > + csrow_idx++) {

    You can do the total_pages initialization in the declaration above and
    thus don't have to break the loop across two lines:

    int total_pages = 0;
    ...

    for (csrow_idx = 0; csrow_idx < mci->nr_csrows; csrow_idx++) {
    ...

    > struct csrow_info *csrow = &mci->csrows[csrow_idx];
    >
    > - if (!csrow->nr_pages)
    > - continue;
    > + for (j = 0; j < csrow->nr_channels; j++) {
    > + struct dimm_info *dimm = csrow->channels[j].dimm;
    >
    > - total_pages += csrow->nr_pages;
    > + total_pages += dimm->nr_pages;
    > + }
    > }
    >
    > return sprintf(data, "%u\n", PAGES_TO_MiB(total_pages));
    > @@ -900,7 +907,7 @@ static void edac_remove_mci_instance_attributes(struct mem_ctl_info *mci,
    > */
    > int edac_create_sysfs_mci_device(struct mem_ctl_info *mci)
    > {
    > - int i;
    > + int i, j;
    > int err;
    > struct csrow_info *csrow;
    > struct kobject *kobj_mci = &mci->edac_mci_kobj;
    > @@ -934,10 +941,15 @@ int edac_create_sysfs_mci_device(struct mem_ctl_info *mci)
    > /* Make directories for each CSROW object under the mc<id> kobject
    > */
    > for (i = 0; i < mci->nr_csrows; i++) {
    > + int n = 0;

    Let's call this variable with a bit more descriptive name like
    'nr_pages' or 'pages_total' or whatever but not simply n.

    > +
    > csrow = &mci->csrows[i];
    > + for (j = 0; j < csrow->nr_channels; j++) {
    > + struct dimm_info *dimm = csrow->channels[j].dimm;
    > + n += dimm->nr_pages;
    > + }

    Btw, you can make the loop a single-statement one and drop the curly
    braces like so:

    for (j = 0; j < csrow->nr_channels; j++)
    nr_pages += csrow->channels[j].dimm->nr_pages;

    The same applies for all the other occurrences of when you iterate over
    the channels, accumulating the pages per dimm.

    >
    > - /* Only expose populated CSROWs */
    > - if (csrow->nr_pages > 0) {
    > + if (n > 0) {
    > err = edac_create_csrow_object(mci, csrow, i);
    > if (err) {
    > debugf1("%s() failure: create csrow %d obj\n",
    > @@ -949,10 +961,16 @@ int edac_create_sysfs_mci_device(struct mem_ctl_info *mci)
    >
    > return 0;
    >
    > - /* CSROW error: backout what has already been registered, */
    > fail1:
    > for (i--; i >= 0; i--) {
    > - if (mci->csrows[i].nr_pages > 0)
    > + int n = 0;

    ditto.

    > +
    > + csrow = &mci->csrows[i];
    > + for (j = 0; j < csrow->nr_channels; j++) {
    > + struct dimm_info *dimm = csrow->channels[j].dimm;
    > + n += dimm->nr_pages;
    > + }
    > + if (n > 0)
    > kobject_put(&mci->csrows[i].kobj);
    > }
    >
    > @@ -972,14 +990,22 @@ fail0:
    > */
    > void edac_remove_sysfs_mci_device(struct mem_ctl_info *mci)
    > {
    > - int i;
    > + struct csrow_info *csrow;
    > + int i, j;
    >
    > debugf0("%s()\n", __func__);
    >
    > /* remove all csrow kobjects */
    > debugf4("%s() unregister this mci kobj\n", __func__);
    > for (i = 0; i < mci->nr_csrows; i++) {
    > - if (mci->csrows[i].nr_pages > 0) {
    > + int n = 0;

    ditto.

    > +
    > + csrow = &mci->csrows[i];
    > + for (j = 0; j < csrow->nr_channels; j++) {
    > + struct dimm_info *dimm = csrow->channels[j].dimm;
    > + n += dimm->nr_pages;
    > + }
    > + if (n > 0) {
    > debugf0("%s() unreg csrow-%d\n", __func__, i);
    > kobject_put(&mci->csrows[i].kobj);
    > }

    <snip>

    > diff --git a/include/linux/edac.h b/include/linux/edac.h
    > index 5244193..de22d4c 100644
    > --- a/include/linux/edac.h
    > +++ b/include/linux/edac.h
    > @@ -320,6 +320,8 @@ struct dimm_info {
    > enum mem_type mtype; /* memory dimm type */
    > enum edac_type edac_mode; /* EDAC mode for this dimm */
    >
    > + u32 nr_pages; /* number of pages in csrow */
    > +
    > u32 ce_count; /* Correctable Errors for this dimm */
    > };
    >
    > @@ -346,13 +348,13 @@ struct rank_info {
    > };
    >
    > struct csrow_info {
    > + int csrow_idx; /* the chip-select row */
    > +
    > + /* Used only by edac_mc_find_csrow_by_page() */
    > unsigned long first_page; /* first page number in csrow */
    > unsigned long last_page; /* last page number in csrow */
    > - u32 nr_pages; /* number of pages in csrow */
    > unsigned long page_mask; /* used for interleaving -
    > - * 0UL for non intlv
    > - */
    > - int csrow_idx; /* the chip-select row */
    > + * 0UL for non intlv */

    Useless moving of code around.

    >
    > u32 ue_count; /* Uncorrectable Errors for this csrow */
    > u32 ce_count; /* Correctable Errors for this csrow */
    > --
    > 1.7.8

    --
    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


    \
     
     \ /
      Last update: 2012-04-02 15:21    [W:0.046 / U:89.444 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site