On Tue, May 07, 2019 at 07:04:13AM +0000, Alexey Brodkin wrote: > Hi Greg, > > > -----Original Message----- > > From: Greg Kroah-Hartman <gregkh@xxxxxxxxxxxxxxxxxxx> > > Sent: Tuesday, May 7, 2019 8:52 AM > > To: Sasha Levin <sashal@xxxxxxxxxx> > > Cc: linux-kernel@xxxxxxxxxxxxxxx; stable@xxxxxxxxxxxxxxx; Alexey Brodkin <abrodkin@xxxxxxxxxxxx>; > > Alexey Brodkin <abrodkin@xxxxxxxxxxxx>; Geert Uytterhoeven <geert@xxxxxxxxxxxxxx>; David Laight > > <David.Laight@xxxxxxxxxx>; Peter Zijlstra <peterz@xxxxxxxxxxxxx>; Thomas Gleixner > > <tglx@xxxxxxxxxxxxx>; Vineet Gupta <vgupta@xxxxxxxxxxxx>; Will Deacon <will.deacon@xxxxxxx>; Sasha > > Levin <alexander.levin@xxxxxxxxxxxxx> > > Subject: Re: [PATCH AUTOSEL 4.14 72/95] devres: Align data[] to ARCH_KMALLOC_MINALIGN > > > > On Tue, May 07, 2019 at 01:38:01AM -0400, Sasha Levin wrote: > > > From: Alexey Brodkin <alexey.brodkin@xxxxxxxxxxxx> > > > > > > [ Upstream commit a66d972465d15b1d89281258805eb8b47d66bd36 ] > > > > > > 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] https://urldefense.proofpoint.com/v2/url?u=http-3A__lists.infradead.org_pipermail_linux-2Dsnps- > > 2Darc_2018- > > 2DJuly_004009.html&d=DwIBAg&c=DPL6_X_6JkXFx7AXWqB0tg&r=lqdeeSSEes0GFDDl656eViXO7breS55ytWkhpk5R81I&m=A > > YtkWKU38pzVfJMBuK0lUwxRyKT6dDfHoD3yO6OIB5k&s=e7e2sXKcjHDQdGSrKWM0jmpSOfhe0MFk4-nMZJe9En8&e= > > > [2] https://urldefense.proofpoint.com/v2/url?u=http-3A__lists.infradead.org_pipermail_linux-2Dsnps- > > 2Darc_2018- > > 2DJuly_004036.html&d=DwIBAg&c=DPL6_X_6JkXFx7AXWqB0tg&r=lqdeeSSEes0GFDDl656eViXO7breS55ytWkhpk5R81I&m=A > > YtkWKU38pzVfJMBuK0lUwxRyKT6dDfHoD3yO6OIB5k&s=L23zrl8rf2MmReUI8rT3FQpMiZU9H3Xjh9uVxJQe8dw&e= > > > > > > Signed-off-by: Alexey Brodkin <abrodkin@xxxxxxxxxxxx> > > > Cc: Greg Kroah-Hartman <gregkh@xxxxxxxxxxxxxxxxxxx> > > > Cc: Geert Uytterhoeven <geert@xxxxxxxxxxxxxx> > > > Cc: David Laight <David.Laight@xxxxxxxxxx> > > > Cc: Peter Zijlstra <peterz@xxxxxxxxxxxxx> > > > Cc: Thomas Gleixner <tglx@xxxxxxxxxxxxx> > > > Cc: Vineet Gupta <vgupta@xxxxxxxxxxxx> > > > Cc: Will Deacon <will.deacon@xxxxxxx> > > > Cc: Greg KH <greg@xxxxxxxxx> > > > Cc: <stable@xxxxxxxxxxxxxxx> # 4.8+ > > > Signed-off-by: Greg Kroah-Hartman <gregkh@xxxxxxxxxxxxxxxxxxx> > > > Signed-off-by: Sasha Levin <alexander.levin@xxxxxxxxxxxxx> > > > --- > > > 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 71d577025285..e43a04a495a3 100644 > > > --- a/drivers/base/devres.c > > > +++ b/drivers/base/devres.c > > > @@ -25,8 +25,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 { > > > > This is not needed in any of the older kernels, despite what the stable@ > > line said, as it ends up taking a lot of memory up for all other arches. > > That's why I only applied it to the one kernel version. I'm betting > > that it will be eventually reverted when people notice it as well :) > > That very well might become the case but then we're back to the initial problem, > right? So maybe some other more future-proof solution should be implemented? Possibly yes. > See initially we discussed simple explicit 8-byte alignment which won't change > data layout for most of arches while fixing our issue on ARC but for some reason > people were not happy with that proposal and that's how we ended-up with what we > discuss here now. I'm not disagreeing that this is a valid solution for you, I wasn't part of the original discussion, sorry. Just that this probably isn't something that should be backported to older kernels at this point in time. thanks, greg k-h
