| Message ID | 20220113100622.12783-5-farbere@amazon.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | edac_align_ptr() bug fix and refactoring | expand |
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.
diff --git a/drivers/edac/edac_mc.c b/drivers/edac/edac_mc.c index 3367bf997b73..a3ff5a019fc7 100644 --- a/drivers/edac/edac_mc.c +++ b/drivers/edac/edac_mc.c @@ -241,9 +241,7 @@ EXPORT_SYMBOL_GPL(edac_mem_types); void *edac_align_ptr(void **p, unsigned size, int n_elems) { unsigned align, r; - void *ptr = *p; - - *p += size * n_elems; + void *ptr; /* * 'p' can possibly be an unaligned item X such that sizeof(X) is @@ -258,16 +256,22 @@ void *edac_align_ptr(void **p, unsigned size, int n_elems) else if (size > sizeof(u8)) align = sizeof(u16); else - return ptr; - - r = (unsigned long)ptr % align; + goto out; - if (r == 0) - return ptr; + /* Calculate alignment, and fix *p if not aligned. */ + r = (unsigned long)*p % align; + if (r) + *p += align - r; - *p += align - r; +out: + /* + * Set return ptr to to be *p (after alignment if it was needed), + * and increase *p by new size for the next call. + */ + ptr = *p; + *p += size * n_elems; - return (void *)(((unsigned long)ptr) + align - r); + return ptr; } static void _edac_mc_free(struct mem_ctl_info *mci)
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. Signed-off-by: Eliav Farber <farbere@amazon.com> --- drivers/edac/edac_mc.c | 24 ++++++++++++++---------- 1 file changed, 14 insertions(+), 10 deletions(-)