Re: [PATCH v2 03/10] EDAC/ghes: Remove unused members of struct ghes_edac_pvt, rename it to ghes_mci

From: Borislav Petkov <bp@alien8.de>
To: Robert Richter <rrichter@marvell.com>
Cc: Mauro Carvalho Chehab <mchehab@kernel.org>,
	Tony Luck <tony.luck@intel.com>,
	James Morse <james.morse@arm.com>,
	Aristeu Rozanski <aris@redhat.com>,
	Matthias Brugger <mbrugger@suse.com>,
	linux-edac@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH v2 03/10] EDAC/ghes: Remove unused members of struct ghes_edac_pvt, rename it to ghes_mci
Date: Thu, 23 Apr 2020 19:55:17 +0200	[thread overview]
Message-ID: <20200423175517.GG26021@zn.tnic> (raw)
In-Reply-To: <20200422115814.22205-4-rrichter@marvell.com>

On Wed, Apr 22, 2020 at 01:58:07PM +0200, Robert Richter wrote:
> The struct members list and ghes of struct ghes_edac_pvt are unused,
> remove them. On that occasion, rename it to struct ghes_mci. This is
> shorter and aligns better with the current naming scheme.
> 
> Signed-off-by: Robert Richter <rrichter@marvell.com>
> ---
>  drivers/edac/ghes_edac.c | 13 +++++--------
>  1 file changed, 5 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/edac/ghes_edac.c b/drivers/edac/ghes_edac.c
> index cb3dab56a875..39efce0df881 100644
> --- a/drivers/edac/ghes_edac.c
> +++ b/drivers/edac/ghes_edac.c
> @@ -15,9 +15,7 @@
>  #include "edac_module.h"
>  #include <ras/ras_event.h>
>  
> -struct ghes_edac_pvt {
> -	struct list_head list;
> -	struct ghes *ghes;
> +struct ghes_mci {

No, that should be "ghes_pvt" because it *is* ghes_edac's private
structure and there's also an mci pointer in it.

>  	struct mem_ctl_info *mci;
>  
>  	/* Buffers for the error handling routine */
> @@ -32,7 +30,7 @@ static refcount_t ghes_refcount = REFCOUNT_INIT(0);
>   * also provides the necessary (implicit) memory barrier for the SMP
>   * case to make the pointer visible on another CPU.
>   */
> -static struct ghes_edac_pvt *ghes_pvt;
> +static struct ghes_mci *ghes_pvt;
>  
>  /* GHES registration mutex */
>  static DEFINE_MUTEX(ghes_reg_mutex);
> @@ -203,7 +201,7 @@ void ghes_edac_report_mem_error(int sev, struct cper_sec_mem_err *mem_err)
>  {
>  	struct edac_raw_error_desc *e;
>  	struct mem_ctl_info *mci;
> -	struct ghes_edac_pvt *pvt;
> +	struct ghes_mci *pvt;
>  	unsigned long flags;
>  	char *p;
>  
> @@ -457,7 +455,7 @@ int ghes_edac_register(struct ghes *ghes, struct device *dev)
>  	bool fake = false;
>  	int rc = 0, num_dimm = 0;
>  	struct mem_ctl_info *mci;
> -	struct ghes_edac_pvt *pvt;
> +	struct ghes_mci *pvt;
>  	struct edac_mc_layer layers[1];
>  	struct ghes_edac_dimm_fill dimm_fill;
>  	unsigned long flags;
> @@ -494,7 +492,7 @@ int ghes_edac_register(struct ghes *ghes, struct device *dev)
>  	layers[0].size = num_dimm;
>  	layers[0].is_virt_csrow = true;
>  
> -	mci = edac_mc_alloc(0, ARRAY_SIZE(layers), layers, sizeof(struct ghes_edac_pvt));
> +	mci = edac_mc_alloc(0, ARRAY_SIZE(layers), layers, sizeof(*pvt));

The sizeof() change doesn't make it better because now I have to go look
up what pvt is.

sizeof(struct ghes_pvt) tells you what size you're getting here.

Thx.

-- 
Regards/Gruss,
    Boris.

https://people.kernel.org/tglx/notes-about-netiquette