Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:18 as permitted sender) client-ip=2620:137:e000::1:18;
Date:   Tue, 15 Feb 2022 18:08:20 +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 4/4] EDAC: Refactor edac_align_ptr() flow
Message-ID: <Ygvd6ATqmb537dEC@zn.tnic>
References: <20220113100622.12783-1-farbere@amazon.com>
 <20220113100622.12783-5-farbere@amazon.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Disposition: inline
In-Reply-To: <20220113100622.12783-5-farbere@amazon.com>
Precedence: bulk

On Thu, Jan 13, 2022 at 10:06:22AM +0000, Eliav Farber wrote:
> Modify flow to be more clear:
>  - Calculate required alignment based on size.
>  - Check if *p is aligned and fix if not.
>  - Set return ptr to to be *p.
>  - Increase *p by new size for the next call.

Right, as I said earlier, piling more on this crap design is not the
right thing to do. Looking at the call sites, what I think you should do
instead is simply compute the whole allocation size by making sure the
alignment of those substruct element pointers we're assigning to, is 8
or so, for simplicity.

You can store the offsets in those helper variables, like it is done
now.

Then do the allocation and add the offsets to the pointer kzalloc has
returned.

And then get rid of that edac_align_ptr() crap. This thing is silly
beyond repair and passing in that **p offset back'n'forth is making
stuff more complicated than it is - one can simply do that computation
before calling kzalloc and doesn't need a "helper" which ain't helping.

I'd say.

Thx.

-- 
Regards/Gruss,
    Boris.

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