Johannes Berg <johannes@xxxxxxxxxxxxxxxx> writes: > On Thu, 2018-04-05 at 14:41 +0300, Dan Carpenter wrote: >> On Thu, Apr 05, 2018 at 02:39:31PM +0300, Dan Carpenter wrote: >> > On Thu, Apr 05, 2018 at 01:30:35PM +0200, Johannes Berg wrote: >> > > On Thu, 2018-04-05 at 14:23 +0300, Dan Carpenter wrote: >> > > > The problem here is that we allocate "data". Then we do >> > > > "data = PTR_ALIGN(data, 8);" and then we free the aligned pointer and >> > > > not the one we allocated. >> > > >> > > That seems pretty pointless, since kmalloc guarantees such alignment for >> > > sure. Better to just remove PTR_ALIGN()? >> > >> > Yeah. You're probably right. I was thinking that maybe >> > ARCH_SLAB_MINALIGN was smaller than 8 somewhere but look it it now, I >> > think it's always 8 or more. >> > >> >> Perhaps on certain xtensa variants? >> >> arch/xtensa/include/asm/processor.h:#define ARCH_SLAB_MINALIGN XCHAL_DATA_WIDTH >> arch/xtensa/variants/fsf/include/variant/core.h:#define >> XCHAL_DATA_WIDTH 4 /* data width in bytes */ > > That's ... interesting. The comment on the original of this says it's > supposed to be used for "better alignment" (more zero bits), and I'd > think that there's lots of code making such assumptions... > > I'd argue it's an xtensa bug, if we need to deal with this everywhere > then it might get messy. Mostly we don't have to care, since pointer > alignment is sufficient in many cases, but still... > > Hmm. Dunno what to do here then. IMHO let's just get rid of the ugly PTR_ALIGN(), I strongly doubt it was added because of this xtensa "feature" :) If we ever get a bug report about this we can then talk with the xtensa folks. -- Kalle Valo
