patch "devres: Align data[] to ARCH_KMALLOC_MINALIGN" added to driver-core-linus
Subsystems (3)
This is a note to let you know that I've just added the patch titled
devres: Align data[] to ARCH_KMALLOC_MINALIGN
to my driver-core git tree which can be found at
git://git.kernel.org/pub/scm/linux/kernel/git/gregkh/driver-core.git
in the driver-core-linus branch.
The patch will show up in the next release of the linux-next tree (usually sometime within the next 24 hours during the week.)
The patch will hopefully also be merged in Linus's tree for the next -rc kernel release.
If you have any questions about this process, please let me know.
Quoted 1 line
From
a66d972465d15b1d89281258805eb8b47d66bd36Mon Sep 17 00:00:00 2001
From: Alexey Brodkin <[email protected]>
Date: Wed, 31 Oct 2018 18:25:47 +0300
Subject: devres: Align data[] to ARCH_KMALLOC_MINALIGN
Initially we bumped into problem with 32-bit aligned atomic64_t on ARC, see [1]. And then during quite lengthly discussion Peter Z. mentioned ARCH_KMALLOC_MINALIGN which IMHO makes perfect sense. If allocation is done by plain kmalloc() obtained buffer will be ARCH_KMALLOC_MINALIGN aligned and then why buffer obtained via devm_kmalloc() should have any other alignment?
This way we at least get the same behavior for both types of allocation.
[1] http://lists.infradead.org/pipermail/linux-snps-arc/2018-July/004009.html
[2] http://lists.infradead.org/pipermail/linux-snps-arc/2018-July/004036.html
--- drivers/base/devres.c | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-)
drivers/base/devres.c +8 -2
@@ -26,8 +26,14 @@ struct devres_node {struct devres {struct devres_node node; -/* -- 3 pointers */ -unsigned long long data[];/* guarantee ull alignment */ +/* +* Some archs want to perform DMA into kmalloc caches +* and need a guaranteed alignment larger than +* the alignment of a 64-bit integer. +* Thus we use ARCH_KMALLOC_MINALIGN here and get exactly the same +* buffer alignment as if it was allocated by plain kmalloc(). +*/ +u8 __aligned(ARCH_KMALLOC_MINALIGN) data[]; };struct devres_group {
Signature · 1 line
2.19.1
Hi Greg,
Quoted 37 lines
-----Original Message-----
From: gregkh at linuxfoundation.org <gregkh at linuxfoundation.org>
Sent: Sunday, November 11, 2018 10:41 PM
To: alexey.brodkin at synopsys.com; David.Laight at ACULAB.COM; alexey.brodkin at synopsys.com; geert at linux- m68k.org; greg at kroah.com; gregkh at linuxfoundation.org; peterz at infradead.org; stable at vger.kernel.org; tglx at linutronix.de; vineet.gupta1 at synopsys.com; will.deacon at arm.com
Subject: patch "devres: Align data[] toARCH_KMALLOC_MINALIGN" added to driver-core-linusThis is a note to let you know that I've just added the patch titled
devres: Align data[] to ARCH_KMALLOC_MINALIGNto my driver-core git tree which can be found at
git://git.kernel.org/pub/scm/linux/kernel/git/gregkh/driver-core.git
in the driver-core-linus branch.The patch will show up in the next release of the linux-next tree (usually sometime within the next 24 hours during the week.)
The patch will hopefully also be merged in Linus's tree for the next -rc kernel release.
If you have any questions about this process, please let me know.
From
a66d972465d15b1d89281258805eb8b47d66bd36Mon Sep 17 00:00:00 2001
From: Alexey Brodkin <alexey.brodkin at synopsys.com>
Date: Wed, 31 Oct 2018 18:25:47 +0300
Subject: devres: Align data[] toARCH_KMALLOC_MINALIGNInitially we bumped into problem with 32-bit aligned
atomic64_ton ARC, see [1]. And then during quite lengthly discussion Peter Z. mentionedARCH_KMALLOC_MINALIGNwhich IMHO makes perfect sense. If allocation is done by plainkmalloc()obtained buffer will beARCH_KMALLOC_MINALIGNaligned and then why buffer obtained viadevm_kmalloc()should have any other alignment?
[snip]
Quoted 2 lines
Cc <stable at vger.kernel.org> # 4.8+Signed-off-by Greg Kroah-Hartman <gregkh at linuxfoundation.org>
I noticed this patch was not only merged in Linus' tree quite some time ago but it was as well back-ported to v4.20, see [1] but for some reason there's no backport to either LTS kernel (v4.19, v4.14 and v4.9).
Is there any problem with this patch and LTS kernels or may we have this one applied?
-Alexey
[email protected]
maintainerQuoted 45 lines · Alexey Brodkin
Hi Greg,
Quoted 37 lines
-----Original Message-----
From: gregkh at linuxfoundation.org <gregkh at linuxfoundation.org>
Sent: Sunday, November 11, 2018 10:41 PM
To: alexey.brodkin at synopsys.com; David.Laight at ACULAB.COM; alexey.brodkin at synopsys.com; geert at linux- m68k.org; greg at kroah.com; gregkh at linuxfoundation.org; peterz at infradead.org; stable at vger.kernel.org; tglx at linutronix.de; vineet.gupta1 at synopsys.com; will.deacon at arm.com
Subject: patch "devres: Align data[] toARCH_KMALLOC_MINALIGN" added to driver-core-linusThis is a note to let you know that I've just added the patch titled
devres: Align data[] to ARCH_KMALLOC_MINALIGNto my driver-core git tree which can be found at
git://git.kernel.org/pub/scm/linux/kernel/git/gregkh/driver-core.git
in the driver-core-linus branch.The patch will show up in the next release of the linux-next tree (usually sometime within the next 24 hours during the week.)
The patch will hopefully also be merged in Linus's tree for the next -rc kernel release.
If you have any questions about this process, please let me know.
From
a66d972465d15b1d89281258805eb8b47d66bd36Mon Sep 17 00:00:00 2001
From: Alexey Brodkin <alexey.brodkin at synopsys.com>
Date: Wed, 31 Oct 2018 18:25:47 +0300
Subject: devres: Align data[] toARCH_KMALLOC_MINALIGNInitially we bumped into problem with 32-bit aligned
atomic64_ton ARC, see [1]. And then during quite lengthly discussion Peter Z. mentionedARCH_KMALLOC_MINALIGNwhich IMHO makes perfect sense. If allocation is done by plainkmalloc()obtained buffer will beARCH_KMALLOC_MINALIGNaligned and then why buffer obtained viadevm_kmalloc()should have any other alignment?[snip]
Quoted 2 lines
Cc <stable at vger.kernel.org> # 4.8+Signed-off-by Greg Kroah-Hartman <gregkh at linuxfoundation.org>
Quoted 8 lines
I noticed this patch was not only merged in Linus' tree quite some time ago but it was as well back-ported to v4.20, see [1] but for some reason there's no backport to either LTS kernel (v4.19, v4.14 and v4.9).
Is there any problem with this patch and LTS kernels or may we have this one applied?
Ah, I was waiting to see if you would notice :)
See this question from Linus about this patch:
https://lore.kernel.org/lkml/CAHk-=wj3Q7CkMQYwfZSsqUTqkEhNwVGrRbCwe7AVJ70S8i5sWw at mail.gmail.com/
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.
Anyway, is this really needed to be backported?
thanks,
greg k-h
Hi Greg,
Quoted 8 lines
-----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[] toARCH_KMALLOC_MINALIGN" added to driver-core-linus
[snip]
Quoted 1 line
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 :)
Quoted 2 lines
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.
Quoted 3 lines
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.
Quoted 1 line
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.
-Alexey
[email protected]
maintainerQuoted 47 lines · Alexey Brodkin
Hi Greg,
Quoted 8 lines
-----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[] toARCH_KMALLOC_MINALIGN" added to driver-core-linus[snip]
Quoted 1 line
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 :)
Quoted 2 lines
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.Quoted 3 lines
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.
Quoted 1 line
Anyway, is this really needed to be backported?
Quoted 7 lines
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