linux-wireless.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Johannes Berg <johannes@sipsolutions.net>
To: Dan Carpenter <dan.carpenter@oracle.com>
Cc: Kalle Valo <kvalo@codeaurora.org>,
	Amitkumar Karwar <amit.karwar@redpinesignals.com>,
	Prameela Rani Garnepudi <prameela.j04cs@gmail.com>,
	Karun Eagalapati <karun256@gmail.com>,
	Siva Rebbagondla <siva.rebbagondla@redpinesignals.com>,
	linux-wireless@vger.kernel.org, kernel-janitors@vger.kernel.org
Subject: Re: [PATCH] rsi: Free the unaligned pointer
Date: Thu, 05 Apr 2018 13:46:44 +0200	[thread overview]
Message-ID: <1522928804.7140.10.camel@sipsolutions.net> (raw)
In-Reply-To: <20180405114124.v2cgo36uuniirby2@mwanda>

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.

johannes

  reply	other threads:[~2018-04-05 11:46 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-04-05 11:23 [PATCH] rsi: Free the unaligned pointer Dan Carpenter
2018-04-05 11:30 ` Johannes Berg
2018-04-05 11:39   ` Dan Carpenter
2018-04-05 11:41     ` Dan Carpenter
2018-04-05 11:46       ` Johannes Berg [this message]
2018-04-05 12:23         ` Kalle Valo
2018-04-06  8:37           ` [PATCH v2] rsi: remove unecessary PTR_ALIGN()s Dan Carpenter
2018-04-06  8:45             ` Arend van Spriel
2018-04-06  9:01               ` Kalle Valo
2018-04-24 17:24             ` [v2] " Kalle Valo

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1522928804.7140.10.camel@sipsolutions.net \
    --to=johannes@sipsolutions.net \
    --cc=amit.karwar@redpinesignals.com \
    --cc=dan.carpenter@oracle.com \
    --cc=karun256@gmail.com \
    --cc=kernel-janitors@vger.kernel.org \
    --cc=kvalo@codeaurora.org \
    --cc=linux-wireless@vger.kernel.org \
    --cc=prameela.j04cs@gmail.com \
    --cc=siva.rebbagondla@redpinesignals.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).