diff mbox series

[08/19] EDAC, mc: Split edac_mc_alloc() into smaller functions

Message ID 20191010202418.25098-9-rrichter@marvell.com
State Superseded
Headers show
Series EDAC: Rework edac_mc and ghes drivers | expand

Commit Message

Robert Richter Oct. 10, 2019, 8:25 p.m. UTC 
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@marvell.com>

---
 drivers/edac/edac_mc.c | 104 +++++++++++++++++++++++++++--------------
 1 file changed, 69 insertions(+), 35 deletions(-)

-- 
2.20.1

Comments

Mauro Carvalho Chehab Oct. 11, 2019, 10:43 a.m. UTC | #1 
Em Thu, 10 Oct 2019 20:25:20 +0000
Robert Richter <rrichter@marvell.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@marvell.com>


Reviewed-by: Mauro Carvalho Chehab <mchehab+samsung@kernel.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
diff mbox series

Patch

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