Re: [LEDE-DEV] [PATCH] libubox: Fix calloc_a() to return mem aligned pointers
-Original Message- From: Felix Fietkau Sent: Friday, February 24, 2017 3:39 AM To: Ted Hess ; Yousong Zhou ; lede-dev Subject: Re: [LEDE-DEV] [PATCH] libubox: Fix calloc_a() to return mem aligned pointers On 2017-02-24 03:37, Ted Hess wrote: Yousong - As a side note to your side note - If you examine the actual mechanics of the allocation, the memory block is indeed size aligned to (4*sizeof(size_t)), but the actual pointer returned is offset of (2*sizeof(size_t)) within the block. As in CHUNK_TO_MEM... I think that for calloc_a, using 2*sizeof(size_t) is probably overkill; it would be 64 bit on 32 bit architectures and 128 bit on 64 bit architectures. Using sizeof(size_t) should be enough: Agree - If no other objections, I'll push the fix directly. /ted ___ Lede-dev mailing list Lede-dev@lists.infradead.org http://lists.infradead.org/mailman/listinfo/lede-dev
Re: [LEDE-DEV] [PATCH] libubox: Fix calloc_a() to return mem aligned pointers
On 2017-02-24 03:37, Ted Hess wrote: > Yousong - > > As a side note to your side note - If you examine the actual > mechanics of the allocation, the memory block is indeed size aligned to > (4*sizeof(size_t)), but the actual pointer returned is offset of > (2*sizeof(size_t)) within the block. As in CHUNK_TO_MEM... I think that for calloc_a, using 2*sizeof(size_t) is probably overkill; it would be 64 bit on 32 bit architectures and 128 bit on 64 bit architectures. Using sizeof(size_t) should be enough: On AArch64, misalignment could only happen if something uses 128-bit types natively. I'm not aware of any code doing that. On ARMv5 there's a set of 64-bit load/store ops which can't handle unaligned access, but that has been causing so many issues (including in kernel code) that I've patched the toolchain to change the minimum architecture level of it to ARMv6 (where unaligned load/store is explicitly allowed). On MIPS and PPC I'm not aware of any similar issues with 64 bit load/store. On x86 we wouldn't even need to do any alignment, it's handled efficiently in the CPU. - Felix ___ Lede-dev mailing list Lede-dev@lists.infradead.org http://lists.infradead.org/mailman/listinfo/lede-dev
Re: [LEDE-DEV] [PATCH] libubox: Fix calloc_a() to return mem aligned pointers
On 24 February 2017 at 08:30, Ted Hess <th...@kitschensync.net> wrote: > > > -Original Message- From: Yousong Zhou > Sent: Thursday, February 23, 2017 7:15 PM > To: Ted Hess > Cc: lede-dev > Subject: Re: [LEDE-DEV] [PATCH] libubox: Fix calloc_a() to return mem > aligned pointers > > > On 24 February 2017 at 05:20, Ted Hess <th...@kitschensync.net> wrote: >> >> The current implementation of calloc_a() returns packed pointers for the >> extra >> arguments. These packed, unaligned, pointers are OK for a lot of >> architectures, >> but not all. This patch will aligned the pointers returned in a manner >> congruent >> with malloc(). I do not believe the extra padding overhead is all the >> burdensome >> considering the overhead of separate malloc/calloc/free call to accomplish >> the >> same thing. >> >> >> Signed-off-by: Ted Hess <th...@kitschensync.net> >> --- >> utils.c | 7 +-- >> 1 file changed, 5 insertions(+), 2 deletions(-) >> >> diff --git a/utils.c b/utils.c >> index 5d9d5aa..314f716 100644 >> --- a/utils.c >> +++ b/utils.c >> @@ -27,6 +27,9 @@ >> _addr; \ >> _addr = va_arg(_arg, void **), _len = _addr ? va_arg(_arg, >> size_t) : 0) >> >> +#define C_PTR_ALIGN(2*sizeof(size_t)) >> +#define C_PTR_MASK (-C_PTR_ALIGN) >> + > > > sizeof(long) should be used for C_PTR_ALIGN, though I cannot find the > quote at the moment... > >yousong > > I picked the expression from malloc in the musl sources. No hard > preferences, but it does do proper alignment for 64-bit systems and other > sensitive data-types AFAICT. > > /ted > Okay, according to the c99, size_t is supposed be able to express sizeof(char[LONG_MAX]). So I guess your code is safer than sizeof(long) actually ;) But as a side note, at the moment, musl malloc uses (4*sizeof(size_t)) as SIZE_ALIGN, uClibc uses (2 * (sizeof(size_t))) as MALLOC_ALIGNMENT yousong ___ Lede-dev mailing list Lede-dev@lists.infradead.org http://lists.infradead.org/mailman/listinfo/lede-dev
Re: [LEDE-DEV] [PATCH] libubox: Fix calloc_a() to return mem aligned pointers
-Original Message- From: Yousong Zhou Sent: Thursday, February 23, 2017 7:15 PM To: Ted Hess Cc: lede-dev Subject: Re: [LEDE-DEV] [PATCH] libubox: Fix calloc_a() to return mem aligned pointers On 24 February 2017 at 05:20, Ted Hess <th...@kitschensync.net> wrote: The current implementation of calloc_a() returns packed pointers for the extra arguments. These packed, unaligned, pointers are OK for a lot of architectures, but not all. This patch will aligned the pointers returned in a manner congruent with malloc(). I do not believe the extra padding overhead is all the burdensome considering the overhead of separate malloc/calloc/free call to accomplish the same thing. Signed-off-by: Ted Hess <th...@kitschensync.net> --- utils.c | 7 +-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/utils.c b/utils.c index 5d9d5aa..314f716 100644 --- a/utils.c +++ b/utils.c @@ -27,6 +27,9 @@ _addr; \ _addr = va_arg(_arg, void **), _len = _addr ? va_arg(_arg, size_t) : 0) +#define C_PTR_ALIGN(2*sizeof(size_t)) +#define C_PTR_MASK (-C_PTR_ALIGN) + sizeof(long) should be used for C_PTR_ALIGN, though I cannot find the quote at the moment... yousong I picked the expression from malloc in the musl sources. No hard preferences, but it does do proper alignment for 64-bit systems and other sensitive data-types AFAICT. /ted ___ Lede-dev mailing list Lede-dev@lists.infradead.org http://lists.infradead.org/mailman/listinfo/lede-dev
