From mboxrd@z Thu Jan 1 00:00:00 1970 From: Kalle Valo Date: Thu, 05 Apr 2018 12:23:06 +0000 Subject: Re: [PATCH] rsi: Free the unaligned pointer Message-Id: <87woxletat.fsf@kamboji.qca.qualcomm.com> List-Id: References: <20180405112311.GD4218@mwanda> <1522927835.7140.8.camel@sipsolutions.net> <20180405113931.n6q6ix47quuvklyp@mwanda> <20180405114124.v2cgo36uuniirby2@mwanda> <1522928804.7140.10.camel@sipsolutions.net> In-Reply-To: <1522928804.7140.10.camel@sipsolutions.net> (Johannes Berg's message of "Thu, 05 Apr 2018 13:46:44 +0200") MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: Johannes Berg Cc: Dan Carpenter , Amitkumar Karwar , Prameela Rani Garnepudi , Karun Eagalapati , Siva Rebbagondla , linux-wireless@vger.kernel.org, kernel-janitors@vger.kernel.org Johannes Berg 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