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.
From a66d972465d15b1d89281258805eb8b47d66bd36 Mon Sep 17 00:00:00 2001
From: Alexey Brodkin alexey.brodkin@synopsys.com 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
Signed-off-by: Alexey Brodkin abrodkin@synopsys.com Cc: Greg Kroah-Hartman gregkh@linuxfoundation.org Cc: Geert Uytterhoeven geert@linux-m68k.org Cc: David Laight David.Laight@ACULAB.COM Cc: Peter Zijlstra peterz@infradead.org Cc: Thomas Gleixner tglx@linutronix.de Cc: Vineet Gupta vgupta@synopsys.com Cc: Will Deacon will.deacon@arm.com Cc: Greg KH greg@kroah.com Cc: stable@vger.kernel.org # 4.8+ Signed-off-by: Greg Kroah-Hartman gregkh@linuxfoundation.org --- drivers/base/devres.c | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-)
diff --git a/drivers/base/devres.c b/drivers/base/devres.c index 4aaf00d2098b..e038e2b3b7ea 100644 --- a/drivers/base/devres.c +++ b/drivers/base/devres.c @@ -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 {
Hi Greg,
-----Original Message----- From: gregkh@linuxfoundation.org gregkh@linuxfoundation.org Sent: Sunday, November 11, 2018 10:41 PM To: alexey.brodkin@synopsys.com; David.Laight@ACULAB.COM; alexey.brodkin@synopsys.com; geert@linux- m68k.org; greg@kroah.com; gregkh@linuxfoundation.org; peterz@infradead.org; stable@vger.kernel.org; tglx@linutronix.de; vineet.gupta1@synopsys.com; will.deacon@arm.com Subject: patch "devres: Align data[] to ARCH_KMALLOC_MINALIGN" added to driver-core-linus
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.
From a66d972465d15b1d89281258805eb8b47d66bd36 Mon Sep 17 00:00:00 2001 From: Alexey Brodkin alexey.brodkin@synopsys.com 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?
[snip]
Cc: stable@vger.kernel.org # 4.8+ Signed-off-by: Greg Kroah-Hartman gregkh@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?
[1] https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/commit/?h=l...
-Alexey
On Thu, Feb 07, 2019 at 01:12:43PM +0000, Alexey Brodkin wrote:
Hi Greg,
-----Original Message----- From: gregkh@linuxfoundation.org gregkh@linuxfoundation.org Sent: Sunday, November 11, 2018 10:41 PM To: alexey.brodkin@synopsys.com; David.Laight@ACULAB.COM; alexey.brodkin@synopsys.com; geert@linux- m68k.org; greg@kroah.com; gregkh@linuxfoundation.org; peterz@infradead.org; stable@vger.kernel.org; tglx@linutronix.de; vineet.gupta1@synopsys.com; will.deacon@arm.com Subject: patch "devres: Align data[] to ARCH_KMALLOC_MINALIGN" added to driver-core-linus
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.
From a66d972465d15b1d89281258805eb8b47d66bd36 Mon Sep 17 00:00:00 2001 From: Alexey Brodkin alexey.brodkin@synopsys.com 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?
[snip]
Cc: stable@vger.kernel.org # 4.8+ Signed-off-by: Greg Kroah-Hartman gregkh@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?
[1] https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/commit/?h=l...
Ah, I was waiting to see if you would notice :)
See this question from Linus about this patch: https://lore.kernel.org/lkml/CAHk-=wj3Q7CkMQYwfZSsqUTqkEhNwVGrRbCwe7AVJ70S8i...
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,
-----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-=wj3Q7CkMQYwfZSsqUTqkEhNwVGrRbCwe7AVJ70S8i...
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.
-Alexey
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-=wj3Q7CkMQYwfZSsqUTqkEhNwVGrRbCwe7AVJ70S8i...
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:
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.
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:
Fortunately [or unfortunately] this problem happens only in pretty rare cases like that Etnaviv driver where I first caught it.
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
linux-stable-mirror@lists.linaro.org