| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <20191011074342.71e443a9@coco.lan>
Date: Fri, 11 Oct 2019 07:43:42 -0300
From: Mauro Carvalho Chehab <mchehab+samsung@...nel.org>
To: Robert Richter <rrichter@...vell.com>
Cc: Borislav Petkov <bp@...en8.de>, Tony Luck <tony.luck@...el.com>,
James Morse <james.morse@....com>,
"linux-edac@...r.kernel.org" <linux-edac@...r.kernel.org>,
"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH 08/19] EDAC, mc: Split edac_mc_alloc() into smaller
functions
Em Thu, 10 Oct 2019 20:25:20 +0000
Robert Richter <rrichter@...vell.com> escreveu:
> edac_mc_alloc() is huge. Factor out code by moving it to the two new
> functions edac_mc_alloc_csrows() and edac_mc_alloc_dimms(). Do not
> move code yet for better review.
>
> Signed-off-by: Robert Richter <rrichter@...vell.com>
Reviewed-by: Mauro Carvalho Chehab <mchehab+samsung@...nel.org>
> ---
> drivers/edac/edac_mc.c | 104 +++++++++++++++++++++++++++--------------
> 1 file changed, 69 insertions(+), 35 deletions(-)
>
> diff --git a/drivers/edac/edac_mc.c b/drivers/edac/edac_mc.c
> index a893f793be8a..0db504cb3419 100644
> --- a/drivers/edac/edac_mc.c
> +++ b/drivers/edac/edac_mc.c
> @@ -305,6 +305,9 @@ static void _edac_mc_free(struct mem_ctl_info *mci)
> kfree(mci);
> }
>
> +static int edac_mc_alloc_csrows(struct mem_ctl_info *mci);
> +static int edac_mc_alloc_dimms(struct mem_ctl_info *mci);
> +
> struct mem_ctl_info *edac_mc_alloc(unsigned int mc_num,
> unsigned int n_layers,
> struct edac_mc_layer *layers,
> @@ -312,14 +315,9 @@ struct mem_ctl_info *edac_mc_alloc(unsigned int mc_num,
> {
> struct mem_ctl_info *mci;
> struct edac_mc_layer *layer;
> - struct csrow_info *csr;
> - struct rank_info *chan;
> - struct dimm_info *dimm;
> - unsigned int pos[EDAC_MAX_LAYERS];
> unsigned int idx, size, tot_dimms = 1;
> unsigned int tot_csrows = 1, tot_channels = 1;
> - void *pvt, *p, *ptr = NULL;
> - int j, row, chn, n, len;
> + void *pvt, *ptr = NULL;
> bool per_rank = false;
>
> if (WARN_ON(n_layers > EDAC_MAX_LAYERS || n_layers == 0))
> @@ -377,16 +375,43 @@ struct mem_ctl_info *edac_mc_alloc(unsigned int mc_num,
> mci->num_cschannel = tot_channels;
> mci->csbased = per_rank;
>
> + if (edac_mc_alloc_csrows(mci))
> + goto error;
> +
> + if (edac_mc_alloc_dimms(mci))
> + goto error;
> +
> + mci->op_state = OP_ALLOC;
> +
> + return mci;
> +
> +error:
> + _edac_mc_free(mci);
> +
> + return NULL;
> +}
> +EXPORT_SYMBOL_GPL(edac_mc_alloc);
> +
> +static int edac_mc_alloc_csrows(struct mem_ctl_info *mci)
> +{
> + unsigned int tot_csrows = mci->nr_csrows;
> + unsigned int tot_channels = mci->num_cschannel;
> + unsigned int row, chn;
> +
> /*
> * Alocate and fill the csrow/channels structs
> */
> mci->csrows = kcalloc(tot_csrows, sizeof(*mci->csrows), GFP_KERNEL);
> if (!mci->csrows)
> - goto error;
> + return -ENOMEM;
> +
> for (row = 0; row < tot_csrows; row++) {
> + struct csrow_info *csr;
> +
> csr = kzalloc(sizeof(**mci->csrows), GFP_KERNEL);
> if (!csr)
> - goto error;
> + return -ENOMEM;
> +
> mci->csrows[row] = csr;
> csr->csrow_idx = row;
> csr->mci = mci;
> @@ -394,34 +419,51 @@ struct mem_ctl_info *edac_mc_alloc(unsigned int mc_num,
> csr->channels = kcalloc(tot_channels, sizeof(*csr->channels),
> GFP_KERNEL);
> if (!csr->channels)
> - goto error;
> + return -ENOMEM;
>
> for (chn = 0; chn < tot_channels; chn++) {
> + struct rank_info *chan;
> +
> chan = kzalloc(sizeof(**csr->channels), GFP_KERNEL);
> if (!chan)
> - goto error;
> + return -ENOMEM;
> +
> csr->channels[chn] = chan;
> chan->chan_idx = chn;
> chan->csrow = csr;
> }
> }
>
> + return 0;
> +}
> +
> +static int edac_mc_alloc_dimms(struct mem_ctl_info *mci)
> +{
> + void *p;
> + unsigned int pos[EDAC_MAX_LAYERS];
> + unsigned int row, chn, idx;
> + int layer;
> +
> /*
> * Allocate and fill the dimm structs
> */
> - mci->dimms = kcalloc(tot_dimms, sizeof(*mci->dimms), GFP_KERNEL);
> + mci->dimms = kcalloc(mci->tot_dimms, sizeof(*mci->dimms), GFP_KERNEL);
> if (!mci->dimms)
> - goto error;
> + return -ENOMEM;
>
> memset(&pos, 0, sizeof(pos));
> row = 0;
> chn = 0;
> - for (idx = 0; idx < tot_dimms; idx++) {
> + for (idx = 0; idx < mci->tot_dimms; idx++) {
> + struct dimm_info *dimm;
> + struct rank_info *chan;
> + int n, len;
> +
> chan = mci->csrows[row]->channels[chn];
>
> dimm = kzalloc(sizeof(**mci->dimms), GFP_KERNEL);
> if (!dimm)
> - goto error;
> + return -ENOMEM;
> mci->dimms[idx] = dimm;
> dimm->mci = mci;
> dimm->idx = idx;
> @@ -431,16 +473,16 @@ struct mem_ctl_info *edac_mc_alloc(unsigned int mc_num,
> */
> len = sizeof(dimm->label);
> p = dimm->label;
> - n = snprintf(p, len, "mc#%u", mc_num);
> + n = snprintf(p, len, "mc#%u", mci->mc_idx);
> p += n;
> len -= n;
> - for (j = 0; j < n_layers; j++) {
> + for (layer = 0; layer < mci->n_layers; layer++) {
> n = snprintf(p, len, "%s#%u",
> - edac_layer_name[layers[j].type],
> - pos[j]);
> + edac_layer_name[mci->layers[layer].type],
> + pos[layer]);
> p += n;
> len -= n;
> - dimm->location[j] = pos[j];
> + dimm->location[layer] = pos[layer];
>
> if (len <= 0)
> break;
> @@ -452,39 +494,31 @@ struct mem_ctl_info *edac_mc_alloc(unsigned int mc_num,
> dimm->cschannel = chn;
>
> /* Increment csrow location */
> - if (layers[0].is_virt_csrow) {
> + if (mci->layers[0].is_virt_csrow) {
> chn++;
> - if (chn == tot_channels) {
> + if (chn == mci->num_cschannel) {
> chn = 0;
> row++;
> }
> } else {
> row++;
> - if (row == tot_csrows) {
> + if (row == mci->nr_csrows) {
> row = 0;
> chn++;
> }
> }
>
> /* Increment dimm location */
> - for (j = n_layers - 1; j >= 0; j--) {
> - pos[j]++;
> - if (pos[j] < layers[j].size)
> + for (layer = mci->n_layers - 1; layer >= 0; layer--) {
> + pos[layer]++;
> + if (pos[layer] < mci->layers[layer].size)
> break;
> - pos[j] = 0;
> + pos[layer] = 0;
> }
> }
>
> - mci->op_state = OP_ALLOC;
> -
> - return mci;
> -
> -error:
> - _edac_mc_free(mci);
> -
> - return NULL;
> + return 0;
> }
> -EXPORT_SYMBOL_GPL(edac_mc_alloc);
>
> void edac_mc_free(struct mem_ctl_info *mci)
> {
Thanks,
Mauro
Powered by blists - more mailing lists