From: gregkh@linuxfoundation.org (gregkh@linuxfoundation.org)
To: linux-snps-arc@lists.infradead.org
Subject: patch "devres: Align data[] to ARCH_KMALLOC_MINALIGN" added to driver-core-linus
Date: Fri, 8 Feb 2019 08:30:02 +0100 [thread overview]
Message-ID: <20190208073002.GA2869@kroah.com> (raw)
In-Reply-To: <4881796E12491D4BB15146FE0209CE64681D64C3@DE02WEMBXB.internal.synopsys.com>
On Fri, Feb 08, 2019@07:13:39AM +0000, Alexey Brodkin wrote:
> Hi Greg,
>
> > -----Original Message-----
> > From: gregkh at linuxfoundation.org <gregkh at linuxfoundation.org>
> > Sent: Thursday, February 7, 2019 6:52 PM
> > To: Alexey Brodkin <alexey.brodkin at synopsys.com>
> > Cc: David.Laight at ACULAB.COM; geert at linux-m68k.org; peterz at infradead.org; stable at vger.kernel.org;
> > tglx at linutronix.de; will.deacon at arm.com; Vineet Gupta <vineet.gupta1 at synopsys.com>; linux-snps-
> > arc at lists.infradead.org
> > Subject: Re: patch "devres: Align data[] to ARCH_KMALLOC_MINALIGN" added to driver-core-linus
>
> [snip]
>
> > Ah, I was waiting to see if you would notice :)
>
> Well I was just patiently waiting as I guess there's a long queue
> of patches to deal with in your inbox :)
>
> > See this question from Linus about this patch:
> > https://lore.kernel.org/lkml/CAHk-=wj3Q7CkMQYwfZSsqUTqkEhNwVGrRbCwe7AVJ70S8i5sWw at mail.gmail.com/
>
> I didn't see that. Though I intentionally sent my patch to most if not all
> arch maintainers so they might share their concerns... but IIRC nobody ever
> replied with either concerns or acks.
>
> Also I do agree that it's a trade-off between:
> 1. Predictability
> I was completely sure devm-allocated buffer is the same as anything kmalloced
> except some meta-data stored _separately_ and so supposed alignment
> should match as well... but how wrong that feeling was.
>
> 2. Optimization
> Indeed it's so sweet when both devm "meta-data" and real small buffer fit
> into 1 cache line.
>
> > I figured that you all did this for a good reason, and wasting that much
> > space was going to be ok. But, I wanted to be sure, so if you never
> > noticed it, I figured it was not that pressing of an issue.
>
> It's not super pressing because:
> 1. Fortunately [or unfortunately] this problem happens only in pretty rare cases
> like that Etnaviv driver where I first caught it.
>
> 2. There's a solution and affected parties may apply known patch locally.
>
> > Anyway, is this really needed to be backported?
>
> For us poor ARC developers and users it's really needed as our tools ABI
> sets 32-bit alignment for 64-bit types. See that's the same optimization -
> why wasting precious bytes on useless holes - let's pack data tighter :)
>
> So having that fix at least in the most recent LTS (i.e. 4.19) would be really good.
> As for older kernels I think for now we may not touch them as indeed change is
> quite intrusive.
Ok, that sounds reasonable. I'll go add the patch there and see if the
ARM64 people even notice :)
thanks,
greg k-h
WARNING: multiple messages have this Message-ID (diff)
From: "gregkh@linuxfoundation.org" <gregkh@linuxfoundation.org>
To: Alexey Brodkin <alexey.brodkin@synopsys.com>
Cc: "David.Laight@ACULAB.COM" <David.Laight@aculab.com>,
"geert@linux-m68k.org" <geert@linux-m68k.org>,
"peterz@infradead.org" <peterz@infradead.org>,
"stable@vger.kernel.org" <stable@vger.kernel.org>,
"tglx@linutronix.de" <tglx@linutronix.de>,
"will.deacon@arm.com" <will.deacon@arm.com>,
"linux-snps-arc@lists.infradead.org"
<linux-snps-arc@lists.infradead.org>,
Vineet Gupta <vineet.gupta1@synopsys.com>
Subject: Re: patch "devres: Align data[] to ARCH_KMALLOC_MINALIGN" added to driver-core-linus
Date: Fri, 8 Feb 2019 08:30:02 +0100 [thread overview]
Message-ID: <20190208073002.GA2869@kroah.com> (raw)
In-Reply-To: <4881796E12491D4BB15146FE0209CE64681D64C3@DE02WEMBXB.internal.synopsys.com>
On Fri, Feb 08, 2019 at 07:13:39AM +0000, Alexey Brodkin wrote:
> Hi Greg,
>
> > -----Original Message-----
> > From: gregkh@linuxfoundation.org <gregkh@linuxfoundation.org>
> > Sent: Thursday, February 7, 2019 6:52 PM
> > To: Alexey Brodkin <alexey.brodkin@synopsys.com>
> > Cc: David.Laight@ACULAB.COM; geert@linux-m68k.org; peterz@infradead.org; stable@vger.kernel.org;
> > tglx@linutronix.de; will.deacon@arm.com; Vineet Gupta <vineet.gupta1@synopsys.com>; linux-snps-
> > arc@lists.infradead.org
> > Subject: Re: patch "devres: Align data[] to ARCH_KMALLOC_MINALIGN" added to driver-core-linus
>
> [snip]
>
> > Ah, I was waiting to see if you would notice :)
>
> Well I was just patiently waiting as I guess there's a long queue
> of patches to deal with in your inbox :)
>
> > See this question from Linus about this patch:
> > https://lore.kernel.org/lkml/CAHk-=wj3Q7CkMQYwfZSsqUTqkEhNwVGrRbCwe7AVJ70S8i5sWw@mail.gmail.com/
>
> I didn't see that. Though I intentionally sent my patch to most if not all
> arch maintainers so they might share their concerns... but IIRC nobody ever
> replied with either concerns or acks.
>
> Also I do agree that it's a trade-off between:
> 1. Predictability
> I was completely sure devm-allocated buffer is the same as anything kmalloced
> except some meta-data stored _separately_ and so supposed alignment
> should match as well... but how wrong that feeling was.
>
> 2. Optimization
> Indeed it's so sweet when both devm "meta-data" and real small buffer fit
> into 1 cache line.
>
> > I figured that you all did this for a good reason, and wasting that much
> > space was going to be ok. But, I wanted to be sure, so if you never
> > noticed it, I figured it was not that pressing of an issue.
>
> It's not super pressing because:
> 1. Fortunately [or unfortunately] this problem happens only in pretty rare cases
> like that Etnaviv driver where I first caught it.
>
> 2. There's a solution and affected parties may apply known patch locally.
>
> > Anyway, is this really needed to be backported?
>
> For us poor ARC developers and users it's really needed as our tools ABI
> sets 32-bit alignment for 64-bit types. See that's the same optimization -
> why wasting precious bytes on useless holes - let's pack data tighter :)
>
> So having that fix at least in the most recent LTS (i.e. 4.19) would be really good.
> As for older kernels I think for now we may not touch them as indeed change is
> quite intrusive.
Ok, that sounds reasonable. I'll go add the patch there and see if the
ARM64 people even notice :)
thanks,
greg k-h
next prev parent reply other threads:[~2019-02-08 7:30 UTC|newest]
Thread overview: 9+ messages / expand[flat|nested] mbox.gz Atom feed top
2018-11-11 19:41 patch "devres: Align data[] to ARCH_KMALLOC_MINALIGN" added to driver-core-linus gregkh
2019-02-07 13:12 ` Alexey Brodkin
2019-02-07 13:12 ` Alexey Brodkin
2019-02-07 15:52 ` gregkh
2019-02-07 15:52 ` gregkh
2019-02-08 7:13 ` Alexey Brodkin
2019-02-08 7:13 ` Alexey Brodkin
2019-02-08 7:30 ` gregkh [this message]
2019-02-08 7:30 ` gregkh
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=20190208073002.GA2869@kroah.com \
--to=gregkh@linuxfoundation.org \
--cc=linux-snps-arc@lists.infradead.org \
/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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.