Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Tue, 15 Feb 2022 17:55:04 +0100
From:   Borislav Petkov <bp@alien8.de>
To:     Eliav Farber <farbere@amazon.com>
Cc:     mchehab@kernel.org, linux-edac@vger.kernel.org,
        linux-kernel@vger.kernel.org, ronenk@amazon.com, talel@amazon.com,
        hhhawa@amazon.com, jonnyc@amazon.com, hanochu@amazon.com
Subject: Re: [PATCH 3/4] EDAC: Refactor edac_align_ptr() to use
 u8/u16/u32/u64 data types
Message-ID: <Ygva6E2Xrurr8hkE@zn.tnic>
References: <20220113100622.12783-1-farbere@amazon.com>
 <20220113100622.12783-4-farbere@amazon.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Disposition: inline
In-Reply-To: <20220113100622.12783-4-farbere@amazon.com>
Precedence: bulk

On Thu, Jan 13, 2022 at 10:06:21AM +0000, Eliav Farber wrote:
> Prefer well defined size variables, that are same in size across all
> systems.
> 
> Signed-off-by: Eliav Farber <farbere@amazon.com>
> ---
>  drivers/edac/edac_mc.c | 17 ++++++-----------
>  1 file changed, 6 insertions(+), 11 deletions(-)
> 
> diff --git a/drivers/edac/edac_mc.c b/drivers/edac/edac_mc.c
> index 8b9b86a7720a..3367bf997b73 100644
> --- a/drivers/edac/edac_mc.c
> +++ b/drivers/edac/edac_mc.c
> @@ -250,18 +250,13 @@ void *edac_align_ptr(void **p, unsigned size, int n_elems)
>  	 * 'size'.  Adjust 'p' so that its alignment is at least as
>  	 * stringent as what the compiler would provide for X and return
>  	 * the aligned result.
> -	 * Here we assume that the alignment of a "long long" is the most
> -	 * stringent alignment that the compiler will ever provide by default.
> -	 * As far as I know, this is a reasonable assumption.
>  	 */
> -	if (size > sizeof(long))
> -		align = sizeof(long long);
> -	else if (size > sizeof(int))
> -		align = sizeof(long);
> -	else if (size > sizeof(short))
> -		align = sizeof(int);
> -	else if (size > sizeof(char))
> -		align = sizeof(short);
> +	if (size > sizeof(u32))
> +		align = sizeof(u64);
> +	else if (size > sizeof(u16))
> +		align = sizeof(u32);
> +	else if (size > sizeof(u8))
> +		align = sizeof(u16);
>  	else
>  		return ptr;

This is just silly. I think you should simply align on 8 and kill all
that bla.

This whole pointer alignment, then picking out the actual pointers of
the embedded struct members is just a bunch of unneeded complexity. I'd
like to get rid of it completely one day...

-- 
Regards/Gruss,
    Boris.

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