linux-arch.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v3] devres: Explicitly align datai[] to 64-bit
@ 2018-07-09 13:45 Alexey Brodkin
  2018-07-09 13:45 ` Alexey Brodkin
                   ` (2 more replies)
  0 siblings, 3 replies; 38+ messages in thread
From: Alexey Brodkin @ 2018-07-09 13:45 UTC (permalink / raw)
  To: linux-kernel
  Cc: linux-snps-arc, linux-arch, Alexey Brodkin, Greg Kroah-Hartman,
	Geert Uytterhoeven, David Laight, Peter Zijlstra,
	Thomas Gleixner, Will Deacon, Greg KH, stable

data[] must be 64-bit aligned even on 32-bit architectures because
it might be accessed by instructions that require aligned memory arguments.

One example is "atomic64_t" type accessed by special atomic instructions
which may read/write entire 64-bit word.

Atomic instructions are a bit special compared to normal loads and stores.
Even if normal loads and stores may deal with unaligned data, atomic
instructions still require data to be aligned because it's hard to manage
atomic value that spans through multiple cache lines or even MMU pages.
And hardware just raises an alignment fault exception.

The problem with previously used approach is that depending on ABI
"long long" type of a particular 32-bit CPU might be aligned to
8-, 16-, 32- or 64-bit boundary. Which will get in the way of mentioned
above atomic instructions.

Consider the following snippet:
|        struct mystruct {
|                atomic64_t myvar;
|        }
|
|        struct mystruct *p;
|        p = devm_kzalloc(dev, sizeof(*p), GFP_KERNEL);

Here address of "myvar" will match  data[] in "struct devres",
that said if "data" is not 64-bit aligned atomic instruction will
fail on the first access to "myvar".

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: Will Deacon <will.deacon@arm.com>
Cc: Greg KH <greg@kroah.com>
Cc: <stable@vger.kernel.org> # 4.8+
---

Changes v2 -> v3:

 * Align explicitly to 8 bytes [David]
 * Rephrased in-line comment [David]
 * Added more techinical details to commit message [Greg]
 * Mention more alignment options in commit message [Geert]

Changes v1 -> v2:

 * Reworded commit message
 * Inserted comment right in source [Thomas]

 drivers/base/devres.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/drivers/base/devres.c b/drivers/base/devres.c
index f98a097e73f2..d65327cb83c9 100644
--- a/drivers/base/devres.c
+++ b/drivers/base/devres.c
@@ -24,8 +24,12 @@ struct devres_node {
 
 struct devres {
 	struct devres_node		node;
-	/* -- 3 pointers */
-	unsigned long long		data[];	/* guarantee ull alignment */
+	/*
+	 * data[] must be 64 bit aligned even on 32 bit architectures
+	 * because it might be accessed by instructions that require
+	 * aligned memory arguments such as atomic64_t.
+	 */
+	u8 __aligned(8)			data[];
 };
 
 struct devres_group {
-- 
2.17.1

^ permalink raw reply related	[flat|nested] 38+ messages in thread

* [PATCH v3] devres: Explicitly align datai[] to 64-bit
  2018-07-09 13:45 [PATCH v3] devres: Explicitly align datai[] to 64-bit Alexey Brodkin
@ 2018-07-09 13:45 ` Alexey Brodkin
  2018-07-09 13:54 ` Peter Zijlstra
  2018-07-09 14:07 ` Peter Zijlstra
  2 siblings, 0 replies; 38+ messages in thread
From: Alexey Brodkin @ 2018-07-09 13:45 UTC (permalink / raw)
  To: linux-kernel
  Cc: linux-snps-arc, linux-arch, Alexey Brodkin, Greg Kroah-Hartman,
	Geert Uytterhoeven, David Laight, Peter Zijlstra,
	Thomas Gleixner, Will Deacon, Greg KH, stable

data[] must be 64-bit aligned even on 32-bit architectures because
it might be accessed by instructions that require aligned memory arguments.

One example is "atomic64_t" type accessed by special atomic instructions
which may read/write entire 64-bit word.

Atomic instructions are a bit special compared to normal loads and stores.
Even if normal loads and stores may deal with unaligned data, atomic
instructions still require data to be aligned because it's hard to manage
atomic value that spans through multiple cache lines or even MMU pages.
And hardware just raises an alignment fault exception.

The problem with previously used approach is that depending on ABI
"long long" type of a particular 32-bit CPU might be aligned to
8-, 16-, 32- or 64-bit boundary. Which will get in the way of mentioned
above atomic instructions.

Consider the following snippet:
|        struct mystruct {
|                atomic64_t myvar;
|        }
|
|        struct mystruct *p;
|        p = devm_kzalloc(dev, sizeof(*p), GFP_KERNEL);

Here address of "myvar" will match  data[] in "struct devres",
that said if "data" is not 64-bit aligned atomic instruction will
fail on the first access to "myvar".

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: Will Deacon <will.deacon@arm.com>
Cc: Greg KH <greg@kroah.com>
Cc: <stable@vger.kernel.org> # 4.8+
---

Changes v2 -> v3:

 * Align explicitly to 8 bytes [David]
 * Rephrased in-line comment [David]
 * Added more techinical details to commit message [Greg]
 * Mention more alignment options in commit message [Geert]

Changes v1 -> v2:

 * Reworded commit message
 * Inserted comment right in source [Thomas]

 drivers/base/devres.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/drivers/base/devres.c b/drivers/base/devres.c
index f98a097e73f2..d65327cb83c9 100644
--- a/drivers/base/devres.c
+++ b/drivers/base/devres.c
@@ -24,8 +24,12 @@ struct devres_node {
 
 struct devres {
 	struct devres_node		node;
-	/* -- 3 pointers */
-	unsigned long long		data[];	/* guarantee ull alignment */
+	/*
+	 * data[] must be 64 bit aligned even on 32 bit architectures
+	 * because it might be accessed by instructions that require
+	 * aligned memory arguments such as atomic64_t.
+	 */
+	u8 __aligned(8)			data[];
 };
 
 struct devres_group {
-- 
2.17.1

^ permalink raw reply related	[flat|nested] 38+ messages in thread

* Re: [PATCH v3] devres: Explicitly align datai[] to 64-bit
  2018-07-09 13:45 [PATCH v3] devres: Explicitly align datai[] to 64-bit Alexey Brodkin
  2018-07-09 13:45 ` Alexey Brodkin
@ 2018-07-09 13:54 ` Peter Zijlstra
  2018-07-09 13:54   ` Peter Zijlstra
                     ` (2 more replies)
  2018-07-09 14:07 ` Peter Zijlstra
  2 siblings, 3 replies; 38+ messages in thread
From: Peter Zijlstra @ 2018-07-09 13:54 UTC (permalink / raw)
  To: Alexey Brodkin
  Cc: linux-arch, Greg Kroah-Hartman, Will Deacon, linux-kernel,
	stable, David Laight, Geert Uytterhoeven, Greg KH,
	Thomas Gleixner, linux-snps-arc

On Mon, Jul 09, 2018 at 04:45:50PM +0300, Alexey Brodkin wrote:
> diff --git a/drivers/base/devres.c b/drivers/base/devres.c
> index f98a097e73f2..d65327cb83c9 100644
> --- a/drivers/base/devres.c
> +++ b/drivers/base/devres.c
> @@ -24,8 +24,12 @@ struct devres_node {
>  
>  struct devres {
>  	struct devres_node		node;
> -	/* -- 3 pointers */
> -	unsigned long long		data[];	/* guarantee ull alignment */
> +	/*
> +	 * data[] must be 64 bit aligned even on 32 bit architectures
> +	 * because it might be accessed by instructions that require
> +	 * aligned memory arguments such as atomic64_t.
> +	 */
> +	u8 __aligned(8)			data[];
>  };

From a quick reading in Documentation/driver-model/devres.txt this
devres muck is supposed to be device memory, right?

atomics (as in atomic*_t) are not defined for use on mmio.

wth kind of code is relying on this?

^ permalink raw reply	[flat|nested] 38+ messages in thread

* Re: [PATCH v3] devres: Explicitly align datai[] to 64-bit
  2018-07-09 13:54 ` Peter Zijlstra
@ 2018-07-09 13:54   ` Peter Zijlstra
  2018-07-09 14:04   ` Mark Rutland
  2018-07-09 14:07   ` Alexey Brodkin
  2 siblings, 0 replies; 38+ messages in thread
From: Peter Zijlstra @ 2018-07-09 13:54 UTC (permalink / raw)
  To: Alexey Brodkin
  Cc: linux-kernel, linux-snps-arc, linux-arch, Greg Kroah-Hartman,
	Geert Uytterhoeven, David Laight, Thomas Gleixner, Will Deacon,
	Greg KH, stable

On Mon, Jul 09, 2018 at 04:45:50PM +0300, Alexey Brodkin wrote:
> diff --git a/drivers/base/devres.c b/drivers/base/devres.c
> index f98a097e73f2..d65327cb83c9 100644
> --- a/drivers/base/devres.c
> +++ b/drivers/base/devres.c
> @@ -24,8 +24,12 @@ struct devres_node {
>  
>  struct devres {
>  	struct devres_node		node;
> -	/* -- 3 pointers */
> -	unsigned long long		data[];	/* guarantee ull alignment */
> +	/*
> +	 * data[] must be 64 bit aligned even on 32 bit architectures
> +	 * because it might be accessed by instructions that require
> +	 * aligned memory arguments such as atomic64_t.
> +	 */
> +	u8 __aligned(8)			data[];
>  };

^ permalink raw reply	[flat|nested] 38+ messages in thread

* Re: [PATCH v3] devres: Explicitly align datai[] to 64-bit
  2018-07-09 13:54 ` Peter Zijlstra
  2018-07-09 13:54   ` Peter Zijlstra
@ 2018-07-09 14:04   ` Mark Rutland
  2018-07-09 14:04     ` Mark Rutland
  2018-07-09 14:10     ` Geert Uytterhoeven
  2018-07-09 14:07   ` Alexey Brodkin
  2 siblings, 2 replies; 38+ messages in thread
From: Mark Rutland @ 2018-07-09 14:04 UTC (permalink / raw)
  To: Peter Zijlstra
  Cc: Alexey Brodkin, linux-kernel, linux-snps-arc, linux-arch,
	Greg Kroah-Hartman, Geert Uytterhoeven, David Laight,
	Thomas Gleixner, Will Deacon, Greg KH, stable

On Mon, Jul 09, 2018 at 03:54:09PM +0200, Peter Zijlstra wrote:
> On Mon, Jul 09, 2018 at 04:45:50PM +0300, Alexey Brodkin wrote:
> > diff --git a/drivers/base/devres.c b/drivers/base/devres.c
> > index f98a097e73f2..d65327cb83c9 100644
> > --- a/drivers/base/devres.c
> > +++ b/drivers/base/devres.c
> > @@ -24,8 +24,12 @@ struct devres_node {
> >  
> >  struct devres {
> >  	struct devres_node		node;
> > -	/* -- 3 pointers */
> > -	unsigned long long		data[];	/* guarantee ull alignment */
> > +	/*
> > +	 * data[] must be 64 bit aligned even on 32 bit architectures
> > +	 * because it might be accessed by instructions that require
> > +	 * aligned memory arguments such as atomic64_t.
> > +	 */
> > +	u8 __aligned(8)			data[];
> >  };
> 
> From a quick reading in Documentation/driver-model/devres.txt this
> devres muck is supposed to be device memory, right?

It's for associating resources (e.g. memory allocations) with a struct
device.

e.g. you do:

	devm_kmalloc(dev, size, GFP_KERNEL);

... and that allocates sizeof(struct devres) + size, putting some
accounting data into that devres, and returning a pointer to the
remaining size bytes.

The data[] thing is a hack to ensure that the structure is padded to
64-bit alignment, in case you'd done:

struct foo {
	atomic64_t counter;
}

struct foo *f = devm_kmalloc(dev, sizeof(*f), GFP_KERNEL);

Mark.

^ permalink raw reply	[flat|nested] 38+ messages in thread

* Re: [PATCH v3] devres: Explicitly align datai[] to 64-bit
  2018-07-09 14:04   ` Mark Rutland
@ 2018-07-09 14:04     ` Mark Rutland
  2018-07-09 14:10     ` Geert Uytterhoeven
  1 sibling, 0 replies; 38+ messages in thread
From: Mark Rutland @ 2018-07-09 14:04 UTC (permalink / raw)
  To: Peter Zijlstra
  Cc: Alexey Brodkin, linux-kernel, linux-snps-arc, linux-arch,
	Greg Kroah-Hartman, Geert Uytterhoeven, David Laight,
	Thomas Gleixner, Will Deacon, Greg KH, stable

On Mon, Jul 09, 2018 at 03:54:09PM +0200, Peter Zijlstra wrote:
> On Mon, Jul 09, 2018 at 04:45:50PM +0300, Alexey Brodkin wrote:
> > diff --git a/drivers/base/devres.c b/drivers/base/devres.c
> > index f98a097e73f2..d65327cb83c9 100644
> > --- a/drivers/base/devres.c
> > +++ b/drivers/base/devres.c
> > @@ -24,8 +24,12 @@ struct devres_node {
> >  
> >  struct devres {
> >  	struct devres_node		node;
> > -	/* -- 3 pointers */
> > -	unsigned long long		data[];	/* guarantee ull alignment */
> > +	/*
> > +	 * data[] must be 64 bit aligned even on 32 bit architectures
> > +	 * because it might be accessed by instructions that require
> > +	 * aligned memory arguments such as atomic64_t.
> > +	 */
> > +	u8 __aligned(8)			data[];
> >  };
> 
> From a quick reading in Documentation/driver-model/devres.txt this
> devres muck is supposed to be device memory, right?

It's for associating resources (e.g. memory allocations) with a struct
device.

e.g. you do:

	devm_kmalloc(dev, size, GFP_KERNEL);

... and that allocates sizeof(struct devres) + size, putting some
accounting data into that devres, and returning a pointer to the
remaining size bytes.

The data[] thing is a hack to ensure that the structure is padded to
64-bit alignment, in case you'd done:

struct foo {
	atomic64_t counter;
}

struct foo *f = devm_kmalloc(dev, sizeof(*f), GFP_KERNEL);

Mark.

^ permalink raw reply	[flat|nested] 38+ messages in thread

* Re: [PATCH v3] devres: Explicitly align datai[] to 64-bit
  2018-07-09 13:45 [PATCH v3] devres: Explicitly align datai[] to 64-bit Alexey Brodkin
  2018-07-09 13:45 ` Alexey Brodkin
  2018-07-09 13:54 ` Peter Zijlstra
@ 2018-07-09 14:07 ` Peter Zijlstra
  2018-07-09 14:07   ` Peter Zijlstra
  2018-07-09 14:10   ` Peter Zijlstra
  2 siblings, 2 replies; 38+ messages in thread
From: Peter Zijlstra @ 2018-07-09 14:07 UTC (permalink / raw)
  To: Alexey Brodkin
  Cc: linux-arch, Greg Kroah-Hartman, Will Deacon, linux-kernel,
	stable, David Laight, Geert Uytterhoeven, Greg KH,
	Thomas Gleixner, linux-snps-arc

On Mon, Jul 09, 2018 at 04:45:50PM +0300, Alexey Brodkin wrote:
> --- a/drivers/base/devres.c
> +++ b/drivers/base/devres.c
> @@ -24,8 +24,12 @@ struct devres_node {
>  
>  struct devres {
>  	struct devres_node		node;
> -	/* -- 3 pointers */
> -	unsigned long long		data[];	/* guarantee ull alignment */
> +	/*
> +	 * data[] must be 64 bit aligned even on 32 bit architectures
> +	 * because it might be accessed by instructions that require
> +	 * aligned memory arguments such as atomic64_t.
> +	 */
> +	u8 __aligned(8)			data[];
>  };

Seeing that this ends up in a semi generic allocation thing, I don't
feel this should be different from ARCH_KMALLOC_MINALIGN.

^ permalink raw reply	[flat|nested] 38+ messages in thread

* Re: [PATCH v3] devres: Explicitly align datai[] to 64-bit
  2018-07-09 14:07 ` Peter Zijlstra
@ 2018-07-09 14:07   ` Peter Zijlstra
  2018-07-09 14:10   ` Peter Zijlstra
  1 sibling, 0 replies; 38+ messages in thread
From: Peter Zijlstra @ 2018-07-09 14:07 UTC (permalink / raw)
  To: Alexey Brodkin
  Cc: linux-kernel, linux-snps-arc, linux-arch, Greg Kroah-Hartman,
	Geert Uytterhoeven, David Laight, Thomas Gleixner, Will Deacon,
	Greg KH, stable

On Mon, Jul 09, 2018 at 04:45:50PM +0300, Alexey Brodkin wrote:
> --- a/drivers/base/devres.c
> +++ b/drivers/base/devres.c
> @@ -24,8 +24,12 @@ struct devres_node {
>  
>  struct devres {
>  	struct devres_node		node;
> -	/* -- 3 pointers */
> -	unsigned long long		data[];	/* guarantee ull alignment */
> +	/*
> +	 * data[] must be 64 bit aligned even on 32 bit architectures
> +	 * because it might be accessed by instructions that require
> +	 * aligned memory arguments such as atomic64_t.
> +	 */
> +	u8 __aligned(8)			data[];
>  };

Seeing that this ends up in a semi generic allocation thing, I don't
feel this should be different from ARCH_KMALLOC_MINALIGN.

^ permalink raw reply	[flat|nested] 38+ messages in thread

* Re: [PATCH v3] devres: Explicitly align datai[] to 64-bit
  2018-07-09 13:54 ` Peter Zijlstra
  2018-07-09 13:54   ` Peter Zijlstra
  2018-07-09 14:04   ` Mark Rutland
@ 2018-07-09 14:07   ` Alexey Brodkin
  2018-07-09 14:07     ` Alexey Brodkin
  2 siblings, 1 reply; 38+ messages in thread
From: Alexey Brodkin @ 2018-07-09 14:07 UTC (permalink / raw)
  To: peterz
  Cc: linux-kernel, tglx, linux-snps-arc, stable, daniel.vetter,
	airlied, greg, will.deacon, gregkh, David.Laight, l.stach,
	linux-arch, geert

Hi Peter,

On Mon, 2018-07-09 at 15:54 +0200, Peter Zijlstra wrote:
> On Mon, Jul 09, 2018 at 04:45:50PM +0300, Alexey Brodkin wrote:
> > diff --git a/drivers/base/devres.c b/drivers/base/devres.c
> > index f98a097e73f2..d65327cb83c9 100644
> > --- a/drivers/base/devres.c
> > +++ b/drivers/base/devres.c
> > @@ -24,8 +24,12 @@ struct devres_node {
> >  
> >  struct devres {
> >  	struct devres_node		node;
> > -	/* -- 3 pointers */
> > -	unsigned long long		data[];	/* guarantee ull alignment */
> > +	/*
> > +	 * data[] must be 64 bit aligned even on 32 bit architectures
> > +	 * because it might be accessed by instructions that require
> > +	 * aligned memory arguments such as atomic64_t.
> > +	 */
> > +	u8 __aligned(8)			data[];
> >  };
> 
> From a quick reading in Documentation/driver-model/devres.txt this
> devres muck is supposed to be device memory, right?
> 
> atomics (as in atomic*_t) are not defined for use on mmio.
> 
> wth kind of code is relying on this?

It's Etnaviv (GPU/DRM) driver in etnaviv_gpu_platform_probe(),
see https://elixir.bootlin.com/linux/latest/source/drivers/gpu/drm/etnaviv/etnaviv_gpu.c#L1740:
---------------------->8---------------------
	struct drm_gpu_scheduler {
		...
		atomic64_t			job_id_count;
		...
	};

	struct etnaviv_gpu {
		...
		struct drm_gpu_scheduler	sched;
	};

	struct etnaviv_gpu *gpu;

	gpu = devm_kzalloc(dev, sizeof(*gpu), GFP_KERNEL);
---------------------->8---------------------

-Alexey

^ permalink raw reply	[flat|nested] 38+ messages in thread

* Re: [PATCH v3] devres: Explicitly align datai[] to 64-bit
  2018-07-09 14:07   ` Alexey Brodkin
@ 2018-07-09 14:07     ` Alexey Brodkin
  0 siblings, 0 replies; 38+ messages in thread
From: Alexey Brodkin @ 2018-07-09 14:07 UTC (permalink / raw)
  To: peterz
  Cc: linux-kernel, tglx, linux-snps-arc, stable, daniel.vetter,
	airlied, greg, will.deacon, gregkh, David.Laight, l.stach,
	linux-arch, geert

Hi Peter,

On Mon, 2018-07-09 at 15:54 +0200, Peter Zijlstra wrote:
> On Mon, Jul 09, 2018 at 04:45:50PM +0300, Alexey Brodkin wrote:
> > diff --git a/drivers/base/devres.c b/drivers/base/devres.c
> > index f98a097e73f2..d65327cb83c9 100644
> > --- a/drivers/base/devres.c
> > +++ b/drivers/base/devres.c
> > @@ -24,8 +24,12 @@ struct devres_node {
> >  
> >  struct devres {
> >  	struct devres_node		node;
> > -	/* -- 3 pointers */
> > -	unsigned long long		data[];	/* guarantee ull alignment */
> > +	/*
> > +	 * data[] must be 64 bit aligned even on 32 bit architectures
> > +	 * because it might be accessed by instructions that require
> > +	 * aligned memory arguments such as atomic64_t.
> > +	 */
> > +	u8 __aligned(8)			data[];
> >  };
> 
> From a quick reading in Documentation/driver-model/devres.txt this
> devres muck is supposed to be device memory, right?
> 
> atomics (as in atomic*_t) are not defined for use on mmio.
> 
> wth kind of code is relying on this?

It's Etnaviv (GPU/DRM) driver in etnaviv_gpu_platform_probe(),
see https://elixir.bootlin.com/linux/latest/source/drivers/gpu/drm/etnaviv/etnaviv_gpu.c#L1740:
---------------------->8---------------------
	struct drm_gpu_scheduler {
		...
		atomic64_t			job_id_count;
		...
	};

	struct etnaviv_gpu {
		...
		struct drm_gpu_scheduler	sched;
	};

	struct etnaviv_gpu *gpu;

	gpu = devm_kzalloc(dev, sizeof(*gpu), GFP_KERNEL);
---------------------->8---------------------

-Alexey

^ permalink raw reply	[flat|nested] 38+ messages in thread

* Re: [PATCH v3] devres: Explicitly align datai[] to 64-bit
  2018-07-09 14:04   ` Mark Rutland
  2018-07-09 14:04     ` Mark Rutland
@ 2018-07-09 14:10     ` Geert Uytterhoeven
  2018-07-09 14:10       ` Geert Uytterhoeven
  1 sibling, 1 reply; 38+ messages in thread
From: Geert Uytterhoeven @ 2018-07-09 14:10 UTC (permalink / raw)
  To: Mark Rutland
  Cc: Peter Zijlstra, Alexey Brodkin, Linux Kernel Mailing List, arcml,
	Linux-Arch, Greg KH, David Laight, Thomas Gleixner, Will Deacon,
	Greg KH, stable

On Mon, Jul 9, 2018 at 4:04 PM Mark Rutland <mark.rutland@arm.com> wrote:
> On Mon, Jul 09, 2018 at 03:54:09PM +0200, Peter Zijlstra wrote:
> > On Mon, Jul 09, 2018 at 04:45:50PM +0300, Alexey Brodkin wrote:
> > > diff --git a/drivers/base/devres.c b/drivers/base/devres.c
> > > index f98a097e73f2..d65327cb83c9 100644
> > > --- a/drivers/base/devres.c
> > > +++ b/drivers/base/devres.c
> > > @@ -24,8 +24,12 @@ struct devres_node {
> > >
> > >  struct devres {
> > >     struct devres_node              node;
> > > -   /* -- 3 pointers */
> > > -   unsigned long long              data[]; /* guarantee ull alignment */
> > > +   /*
> > > +    * data[] must be 64 bit aligned even on 32 bit architectures
> > > +    * because it might be accessed by instructions that require
> > > +    * aligned memory arguments such as atomic64_t.
> > > +    */
> > > +   u8 __aligned(8)                 data[];
> > >  };
> >
> > From a quick reading in Documentation/driver-model/devres.txt this
> > devres muck is supposed to be device memory, right?
>
> It's for associating resources (e.g. memory allocations) with a struct
> device.
>
> e.g. you do:
>
>         devm_kmalloc(dev, size, GFP_KERNEL);
>
> ... and that allocates sizeof(struct devres) + size, putting some
> accounting data into that devres, and returning a pointer to the
> remaining size bytes.
>
> The data[] thing is a hack to ensure that the structure is padded to
> 64-bit alignment, in case you'd done:
>
> struct foo {
>         atomic64_t counter;
> }
>
> struct foo *f = devm_kmalloc(dev, sizeof(*f), GFP_KERNEL);

So the big issue is that the minimum alignment of a buffer allocated with
devm_kmalloc() and friends is different (lower) than when allocated with
kmalloc().

On 32-bit, it's only aligned to 4 bytes. Ugh.
I wouldn't be surprised if some callers assume it to be cacheline-aligned...

Which means blind conversions to the devm_*() versions can be dangerous.

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

^ permalink raw reply	[flat|nested] 38+ messages in thread

* Re: [PATCH v3] devres: Explicitly align datai[] to 64-bit
  2018-07-09 14:10     ` Geert Uytterhoeven
@ 2018-07-09 14:10       ` Geert Uytterhoeven
  0 siblings, 0 replies; 38+ messages in thread
From: Geert Uytterhoeven @ 2018-07-09 14:10 UTC (permalink / raw)
  To: Mark Rutland
  Cc: Peter Zijlstra, Alexey Brodkin, Linux Kernel Mailing List, arcml,
	Linux-Arch, Greg KH, David Laight, Thomas Gleixner, Will Deacon,
	Greg KH, stable

On Mon, Jul 9, 2018 at 4:04 PM Mark Rutland <mark.rutland@arm.com> wrote:
> On Mon, Jul 09, 2018 at 03:54:09PM +0200, Peter Zijlstra wrote:
> > On Mon, Jul 09, 2018 at 04:45:50PM +0300, Alexey Brodkin wrote:
> > > diff --git a/drivers/base/devres.c b/drivers/base/devres.c
> > > index f98a097e73f2..d65327cb83c9 100644
> > > --- a/drivers/base/devres.c
> > > +++ b/drivers/base/devres.c
> > > @@ -24,8 +24,12 @@ struct devres_node {
> > >
> > >  struct devres {
> > >     struct devres_node              node;
> > > -   /* -- 3 pointers */
> > > -   unsigned long long              data[]; /* guarantee ull alignment */
> > > +   /*
> > > +    * data[] must be 64 bit aligned even on 32 bit architectures
> > > +    * because it might be accessed by instructions that require
> > > +    * aligned memory arguments such as atomic64_t.
> > > +    */
> > > +   u8 __aligned(8)                 data[];
> > >  };
> >
> > From a quick reading in Documentation/driver-model/devres.txt this
> > devres muck is supposed to be device memory, right?
>
> It's for associating resources (e.g. memory allocations) with a struct
> device.
>
> e.g. you do:
>
>         devm_kmalloc(dev, size, GFP_KERNEL);
>
> ... and that allocates sizeof(struct devres) + size, putting some
> accounting data into that devres, and returning a pointer to the
> remaining size bytes.
>
> The data[] thing is a hack to ensure that the structure is padded to
> 64-bit alignment, in case you'd done:
>
> struct foo {
>         atomic64_t counter;
> }
>
> struct foo *f = devm_kmalloc(dev, sizeof(*f), GFP_KERNEL);

So the big issue is that the minimum alignment of a buffer allocated with
devm_kmalloc() and friends is different (lower) than when allocated with
kmalloc().

On 32-bit, it's only aligned to 4 bytes. Ugh.
I wouldn't be surprised if some callers assume it to be cacheline-aligned...

Which means blind conversions to the devm_*() versions can be dangerous.

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

^ permalink raw reply	[flat|nested] 38+ messages in thread

* Re: [PATCH v3] devres: Explicitly align datai[] to 64-bit
  2018-07-09 14:07 ` Peter Zijlstra
  2018-07-09 14:07   ` Peter Zijlstra
@ 2018-07-09 14:10   ` Peter Zijlstra
  2018-07-09 14:10     ` Peter Zijlstra
  2018-07-09 14:33     ` Alexey Brodkin
  1 sibling, 2 replies; 38+ messages in thread
From: Peter Zijlstra @ 2018-07-09 14:10 UTC (permalink / raw)
  To: Alexey Brodkin
  Cc: linux-arch, Greg Kroah-Hartman, Will Deacon, linux-kernel,
	stable, David Laight, Geert Uytterhoeven, Greg KH,
	Thomas Gleixner, linux-snps-arc

On Mon, Jul 09, 2018 at 04:07:17PM +0200, Peter Zijlstra wrote:
> On Mon, Jul 09, 2018 at 04:45:50PM +0300, Alexey Brodkin wrote:
> > --- a/drivers/base/devres.c
> > +++ b/drivers/base/devres.c
> > @@ -24,8 +24,12 @@ struct devres_node {
> >  
> >  struct devres {
> >  	struct devres_node		node;
> > -	/* -- 3 pointers */
> > -	unsigned long long		data[];	/* guarantee ull alignment */
> > +	/*
> > +	 * data[] must be 64 bit aligned even on 32 bit architectures
> > +	 * because it might be accessed by instructions that require
> > +	 * aligned memory arguments such as atomic64_t.
> > +	 */
> > +	u8 __aligned(8)			data[];
> >  };
> 
> Seeing that this ends up in a semi generic allocation thing, I don't
> feel this should be different from ARCH_KMALLOC_MINALIGN.

In fact, since alloc_dr() uses kmalloc() to allocate the entire thing,
it is impossible to guarantee a larger alignment than kmalloc does.

^ permalink raw reply	[flat|nested] 38+ messages in thread

* Re: [PATCH v3] devres: Explicitly align datai[] to 64-bit
  2018-07-09 14:10   ` Peter Zijlstra
@ 2018-07-09 14:10     ` Peter Zijlstra
  2018-07-09 14:33     ` Alexey Brodkin
  1 sibling, 0 replies; 38+ messages in thread
From: Peter Zijlstra @ 2018-07-09 14:10 UTC (permalink / raw)
  To: Alexey Brodkin
  Cc: linux-kernel, linux-snps-arc, linux-arch, Greg Kroah-Hartman,
	Geert Uytterhoeven, David Laight, Thomas Gleixner, Will Deacon,
	Greg KH, stable

On Mon, Jul 09, 2018 at 04:07:17PM +0200, Peter Zijlstra wrote:
> On Mon, Jul 09, 2018 at 04:45:50PM +0300, Alexey Brodkin wrote:
> > --- a/drivers/base/devres.c
> > +++ b/drivers/base/devres.c
> > @@ -24,8 +24,12 @@ struct devres_node {
> >  
> >  struct devres {
> >  	struct devres_node		node;
> > -	/* -- 3 pointers */
> > -	unsigned long long		data[];	/* guarantee ull alignment */
> > +	/*
> > +	 * data[] must be 64 bit aligned even on 32 bit architectures
> > +	 * because it might be accessed by instructions that require
> > +	 * aligned memory arguments such as atomic64_t.
> > +	 */
> > +	u8 __aligned(8)			data[];
> >  };
> 
> Seeing that this ends up in a semi generic allocation thing, I don't
> feel this should be different from ARCH_KMALLOC_MINALIGN.

In fact, since alloc_dr() uses kmalloc() to allocate the entire thing,
it is impossible to guarantee a larger alignment than kmalloc does.

^ permalink raw reply	[flat|nested] 38+ messages in thread

* Re: [PATCH v3] devres: Explicitly align datai[] to 64-bit
  2018-07-09 14:10   ` Peter Zijlstra
  2018-07-09 14:10     ` Peter Zijlstra
@ 2018-07-09 14:33     ` Alexey Brodkin
  2018-07-09 14:33       ` Alexey Brodkin
  2018-07-09 14:49       ` Peter Zijlstra
  1 sibling, 2 replies; 38+ messages in thread
From: Alexey Brodkin @ 2018-07-09 14:33 UTC (permalink / raw)
  To: peterz
  Cc: linux-kernel, tglx, linux-snps-arc, stable, greg, will.deacon,
	gregkh, David.Laight, linux-arch, geert

Hi Peter,

On Mon, 2018-07-09 at 16:10 +0200, Peter Zijlstra wrote:
> On Mon, Jul 09, 2018 at 04:07:17PM +0200, Peter Zijlstra wrote:
> > On Mon, Jul 09, 2018 at 04:45:50PM +0300, Alexey Brodkin wrote:
> > > --- a/drivers/base/devres.c
> > > +++ b/drivers/base/devres.c
> > > @@ -24,8 +24,12 @@ struct devres_node {
> > >  
> > >  struct devres {
> > >  	struct devres_node		node;
> > > -	/* -- 3 pointers */
> > > -	unsigned long long		data[];	/* guarantee ull alignment */
> > > +	/*
> > > +	 * data[] must be 64 bit aligned even on 32 bit architectures
> > > +	 * because it might be accessed by instructions that require
> > > +	 * aligned memory arguments such as atomic64_t.
> > > +	 */
> > > +	u8 __aligned(8)			data[];
> > >  };
> > 
> > Seeing that this ends up in a semi generic allocation thing, I don't
> > feel this should be different from ARCH_KMALLOC_MINALIGN.
> 
> In fact, since alloc_dr() uses kmalloc() to allocate the entire thing,
> it is impossible to guarantee a larger alignment than kmalloc does.

Well but 4-bytes [which is critical for atomic64_t] should be much less
than a sane cache line length so above should work.

-Alexey

^ permalink raw reply	[flat|nested] 38+ messages in thread

* Re: [PATCH v3] devres: Explicitly align datai[] to 64-bit
  2018-07-09 14:33     ` Alexey Brodkin
@ 2018-07-09 14:33       ` Alexey Brodkin
  2018-07-09 14:49       ` Peter Zijlstra
  1 sibling, 0 replies; 38+ messages in thread
From: Alexey Brodkin @ 2018-07-09 14:33 UTC (permalink / raw)
  To: peterz
  Cc: linux-kernel, tglx, linux-snps-arc, stable, greg, will.deacon,
	gregkh, David.Laight, linux-arch, geert

Hi Peter,

On Mon, 2018-07-09 at 16:10 +0200, Peter Zijlstra wrote:
> On Mon, Jul 09, 2018 at 04:07:17PM +0200, Peter Zijlstra wrote:
> > On Mon, Jul 09, 2018 at 04:45:50PM +0300, Alexey Brodkin wrote:
> > > --- a/drivers/base/devres.c
> > > +++ b/drivers/base/devres.c
> > > @@ -24,8 +24,12 @@ struct devres_node {
> > >  
> > >  struct devres {
> > >  	struct devres_node		node;
> > > -	/* -- 3 pointers */
> > > -	unsigned long long		data[];	/* guarantee ull alignment */
> > > +	/*
> > > +	 * data[] must be 64 bit aligned even on 32 bit architectures
> > > +	 * because it might be accessed by instructions that require
> > > +	 * aligned memory arguments such as atomic64_t.
> > > +	 */
> > > +	u8 __aligned(8)			data[];
> > >  };
> > 
> > Seeing that this ends up in a semi generic allocation thing, I don't
> > feel this should be different from ARCH_KMALLOC_MINALIGN.
> 
> In fact, since alloc_dr() uses kmalloc() to allocate the entire thing,
> it is impossible to guarantee a larger alignment than kmalloc does.

Well but 4-bytes [which is critical for atomic64_t] should be much less
than a sane cache line length so above should work.

-Alexey

^ permalink raw reply	[flat|nested] 38+ messages in thread

* Re: [PATCH v3] devres: Explicitly align datai[] to 64-bit
  2018-07-09 14:33     ` Alexey Brodkin
  2018-07-09 14:33       ` Alexey Brodkin
@ 2018-07-09 14:49       ` Peter Zijlstra
  2018-07-09 14:49         ` Peter Zijlstra
                           ` (3 more replies)
  1 sibling, 4 replies; 38+ messages in thread
From: Peter Zijlstra @ 2018-07-09 14:49 UTC (permalink / raw)
  To: Alexey Brodkin
  Cc: linux-kernel, tglx, linux-snps-arc, stable, greg, will.deacon,
	gregkh, David.Laight, linux-arch, geert

On Mon, Jul 09, 2018 at 02:33:26PM +0000, Alexey Brodkin wrote:
> > In fact, since alloc_dr() uses kmalloc() to allocate the entire thing,
> > it is impossible to guarantee a larger alignment than kmalloc does.
> 
> Well but 4-bytes [which is critical for atomic64_t] should be much less
> than a sane cache line length so above should work.

AFAICT ARCH_KMALLOC_MINALIGN ends up being 4 on x86_32 (it doesn't
define ARCH_DMA_MINALIGN and doesn't seem to otherwise override the
thing).

So unconditionally setting the alignment of devres::data to 8 seems
broken.

^ permalink raw reply	[flat|nested] 38+ messages in thread

* Re: [PATCH v3] devres: Explicitly align datai[] to 64-bit
  2018-07-09 14:49       ` Peter Zijlstra
@ 2018-07-09 14:49         ` Peter Zijlstra
  2018-07-09 14:53         ` Alexey Brodkin
                           ` (2 subsequent siblings)
  3 siblings, 0 replies; 38+ messages in thread
From: Peter Zijlstra @ 2018-07-09 14:49 UTC (permalink / raw)
  To: Alexey Brodkin
  Cc: linux-kernel, tglx, linux-snps-arc, stable, greg, will.deacon,
	gregkh, David.Laight, linux-arch, geert

On Mon, Jul 09, 2018 at 02:33:26PM +0000, Alexey Brodkin wrote:
> > In fact, since alloc_dr() uses kmalloc() to allocate the entire thing,
> > it is impossible to guarantee a larger alignment than kmalloc does.
> 
> Well but 4-bytes [which is critical for atomic64_t] should be much less
> than a sane cache line length so above should work.

AFAICT ARCH_KMALLOC_MINALIGN ends up being 4 on x86_32 (it doesn't
define ARCH_DMA_MINALIGN and doesn't seem to otherwise override the
thing).

So unconditionally setting the alignment of devres::data to 8 seems
broken.

^ permalink raw reply	[flat|nested] 38+ messages in thread

* Re: [PATCH v3] devres: Explicitly align datai[] to 64-bit
  2018-07-09 14:49       ` Peter Zijlstra
  2018-07-09 14:49         ` Peter Zijlstra
@ 2018-07-09 14:53         ` Alexey Brodkin
  2018-07-09 14:53           ` Alexey Brodkin
  2018-10-04 16:19           ` Alexey Brodkin
  2018-07-09 15:02         ` David Laight
  2018-07-09 15:29         ` Mark Rutland
  3 siblings, 2 replies; 38+ messages in thread
From: Alexey Brodkin @ 2018-07-09 14:53 UTC (permalink / raw)
  To: peterz
  Cc: linux-kernel, tglx, linux-snps-arc, stable, will.deacon, greg,
	gregkh, David.Laight, Vineet Gupta, linux-arch, geert

Hi Peter,

On Mon, 2018-07-09 at 16:49 +0200, Peter Zijlstra wrote:
> On Mon, Jul 09, 2018 at 02:33:26PM +0000, Alexey Brodkin wrote:
> > > In fact, since alloc_dr() uses kmalloc() to allocate the entire thing,
> > > it is impossible to guarantee a larger alignment than kmalloc does.
> > 
> > Well but 4-bytes [which is critical for atomic64_t] should be much less
> > than a sane cache line length so above should work.
> 
> AFAICT ARCH_KMALLOC_MINALIGN ends up being 4 on x86_32 (it doesn't
> define ARCH_DMA_MINALIGN and doesn't seem to otherwise override the
> thing).
> 
> So unconditionally setting the alignment of devres::data to 8 seems
> broken.

Well but then what other options do we have to fix a problem with
misaligned access to atomic64_t in drm_gpu_scheduler?

-Alexey

^ permalink raw reply	[flat|nested] 38+ messages in thread

* Re: [PATCH v3] devres: Explicitly align datai[] to 64-bit
  2018-07-09 14:53         ` Alexey Brodkin
@ 2018-07-09 14:53           ` Alexey Brodkin
  2018-10-04 16:19           ` Alexey Brodkin
  1 sibling, 0 replies; 38+ messages in thread
From: Alexey Brodkin @ 2018-07-09 14:53 UTC (permalink / raw)
  To: peterz
  Cc: linux-kernel, tglx, linux-snps-arc, stable, will.deacon, greg,
	gregkh, David.Laight, Vineet Gupta, linux-arch, geert

Hi Peter,

On Mon, 2018-07-09 at 16:49 +0200, Peter Zijlstra wrote:
> On Mon, Jul 09, 2018 at 02:33:26PM +0000, Alexey Brodkin wrote:
> > > In fact, since alloc_dr() uses kmalloc() to allocate the entire thing,
> > > it is impossible to guarantee a larger alignment than kmalloc does.
> > 
> > Well but 4-bytes [which is critical for atomic64_t] should be much less
> > than a sane cache line length so above should work.
> 
> AFAICT ARCH_KMALLOC_MINALIGN ends up being 4 on x86_32 (it doesn't
> define ARCH_DMA_MINALIGN and doesn't seem to otherwise override the
> thing).
> 
> So unconditionally setting the alignment of devres::data to 8 seems
> broken.

Well but then what other options do we have to fix a problem with
misaligned access to atomic64_t in drm_gpu_scheduler?

-Alexey

^ permalink raw reply	[flat|nested] 38+ messages in thread

* RE: [PATCH v3] devres: Explicitly align datai[] to 64-bit
  2018-07-09 14:49       ` Peter Zijlstra
  2018-07-09 14:49         ` Peter Zijlstra
  2018-07-09 14:53         ` Alexey Brodkin
@ 2018-07-09 15:02         ` David Laight
  2018-07-09 15:02           ` David Laight
  2018-07-09 15:14           ` Peter Zijlstra
  2018-07-09 15:29         ` Mark Rutland
  3 siblings, 2 replies; 38+ messages in thread
From: David Laight @ 2018-07-09 15:02 UTC (permalink / raw)
  To: 'Peter Zijlstra', Alexey Brodkin
  Cc: linux-kernel, tglx, linux-snps-arc, stable, greg, will.deacon,
	gregkh, linux-arch, geert

From: Peter Zijlstra
> Sent: 09 July 2018 15:49
> On Mon, Jul 09, 2018 at 02:33:26PM +0000, Alexey Brodkin wrote:
> > > In fact, since alloc_dr() uses kmalloc() to allocate the entire thing,
> > > it is impossible to guarantee a larger alignment than kmalloc does.
> >
> > Well but 4-bytes [which is critical for atomic64_t] should be much less
> > than a sane cache line length so above should work.
> 
> AFAICT ARCH_KMALLOC_MINALIGN ends up being 4 on x86_32 (it doesn't
> define ARCH_DMA_MINALIGN and doesn't seem to otherwise override the
> thing).

That seems broken.

I wonder what the minimal alignment really is?
I suspect some code expects (and gets) 8-byte alignment.
The min alignment might even be 16 or 32 bytes.
There aren't many x86 instructions that fault on mis-aligned addresses,
but there are a few.
Mostly related to the fpu - probably including the fpu save area.

	David

^ permalink raw reply	[flat|nested] 38+ messages in thread

* RE: [PATCH v3] devres: Explicitly align datai[] to 64-bit
  2018-07-09 15:02         ` David Laight
@ 2018-07-09 15:02           ` David Laight
  2018-07-09 15:14           ` Peter Zijlstra
  1 sibling, 0 replies; 38+ messages in thread
From: David Laight @ 2018-07-09 15:02 UTC (permalink / raw)
  To: 'Peter Zijlstra', Alexey Brodkin
  Cc: linux-kernel, tglx, linux-snps-arc, stable, greg, will.deacon,
	gregkh, linux-arch, geert

From: Peter Zijlstra
> Sent: 09 July 2018 15:49
> On Mon, Jul 09, 2018 at 02:33:26PM +0000, Alexey Brodkin wrote:
> > > In fact, since alloc_dr() uses kmalloc() to allocate the entire thing,
> > > it is impossible to guarantee a larger alignment than kmalloc does.
> >
> > Well but 4-bytes [which is critical for atomic64_t] should be much less
> > than a sane cache line length so above should work.
> 
> AFAICT ARCH_KMALLOC_MINALIGN ends up being 4 on x86_32 (it doesn't
> define ARCH_DMA_MINALIGN and doesn't seem to otherwise override the
> thing).

That seems broken.

I wonder what the minimal alignment really is?
I suspect some code expects (and gets) 8-byte alignment.
The min alignment might even be 16 or 32 bytes.
There aren't many x86 instructions that fault on mis-aligned addresses,
but there are a few.
Mostly related to the fpu - probably including the fpu save area.

	David

^ permalink raw reply	[flat|nested] 38+ messages in thread

* Re: [PATCH v3] devres: Explicitly align datai[] to 64-bit
  2018-07-09 15:02         ` David Laight
  2018-07-09 15:02           ` David Laight
@ 2018-07-09 15:14           ` Peter Zijlstra
  2018-07-09 15:14             ` Peter Zijlstra
  1 sibling, 1 reply; 38+ messages in thread
From: Peter Zijlstra @ 2018-07-09 15:14 UTC (permalink / raw)
  To: David Laight
  Cc: linux-arch, greg, Alexey Brodkin, will.deacon, linux-kernel,
	stable, geert, gregkh, tglx, linux-snps-arc

On Mon, Jul 09, 2018 at 03:02:08PM +0000, David Laight wrote:

> Mostly related to the fpu - probably including the fpu save area.

So for the FPU save area in particular I know we play some horrific
games. As to the rest I really dont know.

I wouldn't mind it being changed, but from a cursory look, I couldn't
see it being anything other than 4 for x86_32.

^ permalink raw reply	[flat|nested] 38+ messages in thread

* Re: [PATCH v3] devres: Explicitly align datai[] to 64-bit
  2018-07-09 15:14           ` Peter Zijlstra
@ 2018-07-09 15:14             ` Peter Zijlstra
  0 siblings, 0 replies; 38+ messages in thread
From: Peter Zijlstra @ 2018-07-09 15:14 UTC (permalink / raw)
  To: David Laight
  Cc: Alexey Brodkin, linux-kernel, tglx, linux-snps-arc, stable, greg,
	will.deacon, gregkh, linux-arch, geert

On Mon, Jul 09, 2018 at 03:02:08PM +0000, David Laight wrote:

> Mostly related to the fpu - probably including the fpu save area.

So for the FPU save area in particular I know we play some horrific
games. As to the rest I really dont know.

I wouldn't mind it being changed, but from a cursory look, I couldn't
see it being anything other than 4 for x86_32.

^ permalink raw reply	[flat|nested] 38+ messages in thread

* Re: [PATCH v3] devres: Explicitly align datai[] to 64-bit
  2018-07-09 14:49       ` Peter Zijlstra
                           ` (2 preceding siblings ...)
  2018-07-09 15:02         ` David Laight
@ 2018-07-09 15:29         ` Mark Rutland
  2018-07-09 15:29           ` Mark Rutland
  2018-07-09 15:34           ` Peter Zijlstra
  3 siblings, 2 replies; 38+ messages in thread
From: Mark Rutland @ 2018-07-09 15:29 UTC (permalink / raw)
  To: Peter Zijlstra
  Cc: Alexey Brodkin, linux-kernel, tglx, linux-snps-arc, stable, greg,
	will.deacon, gregkh, David.Laight, linux-arch, geert

On Mon, Jul 09, 2018 at 04:49:25PM +0200, Peter Zijlstra wrote:
> On Mon, Jul 09, 2018 at 02:33:26PM +0000, Alexey Brodkin wrote:
> > > In fact, since alloc_dr() uses kmalloc() to allocate the entire thing,
> > > it is impossible to guarantee a larger alignment than kmalloc does.
> > 
> > Well but 4-bytes [which is critical for atomic64_t] should be much less
> > than a sane cache line length so above should work.
> 
> AFAICT ARCH_KMALLOC_MINALIGN ends up being 4 on x86_32 (it doesn't
> define ARCH_DMA_MINALIGN and doesn't seem to otherwise override the
> thing).

Shouldn't that be 8? AFAICT, __alignof__(unsigned long long) is 8 on
x86_32:

----
[mark@lakrids:~]% cat test.c     
#include <stdio.h>

#define PRINT_TYPE_INFO(t) \
        printf("%10s %5d %5d\n", #t, sizeof(t), __alignof__(t))

int main(int argc, char *argv[])
{
        printf("%10s %5s %5s\n", "TYPE", "SIZE", "ALIGN");
        PRINT_TYPE_INFO(int);
        PRINT_TYPE_INFO(long);
        PRINT_TYPE_INFO(long long);

        return 0;
}
[mark@lakrids:~]% gcc -m32 test.c -o test
[mark@lakrids:~]% ./test 
      TYPE  SIZE ALIGN
       int     4     4
      long     4     4
 long long     8     8
----

Mark.

^ permalink raw reply	[flat|nested] 38+ messages in thread

* Re: [PATCH v3] devres: Explicitly align datai[] to 64-bit
  2018-07-09 15:29         ` Mark Rutland
@ 2018-07-09 15:29           ` Mark Rutland
  2018-07-09 15:34           ` Peter Zijlstra
  1 sibling, 0 replies; 38+ messages in thread
From: Mark Rutland @ 2018-07-09 15:29 UTC (permalink / raw)
  To: Peter Zijlstra
  Cc: Alexey Brodkin, linux-kernel, tglx, linux-snps-arc, stable, greg,
	will.deacon, gregkh, David.Laight, linux-arch, geert

On Mon, Jul 09, 2018 at 04:49:25PM +0200, Peter Zijlstra wrote:
> On Mon, Jul 09, 2018 at 02:33:26PM +0000, Alexey Brodkin wrote:
> > > In fact, since alloc_dr() uses kmalloc() to allocate the entire thing,
> > > it is impossible to guarantee a larger alignment than kmalloc does.
> > 
> > Well but 4-bytes [which is critical for atomic64_t] should be much less
> > than a sane cache line length so above should work.
> 
> AFAICT ARCH_KMALLOC_MINALIGN ends up being 4 on x86_32 (it doesn't
> define ARCH_DMA_MINALIGN and doesn't seem to otherwise override the
> thing).

Shouldn't that be 8? AFAICT, __alignof__(unsigned long long) is 8 on
x86_32:

----
[mark@lakrids:~]% cat test.c     
#include <stdio.h>

#define PRINT_TYPE_INFO(t) \
        printf("%10s %5d %5d\n", #t, sizeof(t), __alignof__(t))

int main(int argc, char *argv[])
{
        printf("%10s %5s %5s\n", "TYPE", "SIZE", "ALIGN");
        PRINT_TYPE_INFO(int);
        PRINT_TYPE_INFO(long);
        PRINT_TYPE_INFO(long long);

        return 0;
}
[mark@lakrids:~]% gcc -m32 test.c -o test
[mark@lakrids:~]% ./test 
      TYPE  SIZE ALIGN
       int     4     4
      long     4     4
 long long     8     8
----

Mark.

^ permalink raw reply	[flat|nested] 38+ messages in thread

* Re: [PATCH v3] devres: Explicitly align datai[] to 64-bit
  2018-07-09 15:29         ` Mark Rutland
  2018-07-09 15:29           ` Mark Rutland
@ 2018-07-09 15:34           ` Peter Zijlstra
  2018-07-09 15:34             ` Peter Zijlstra
  2018-07-09 15:45             ` Peter Zijlstra
  1 sibling, 2 replies; 38+ messages in thread
From: Peter Zijlstra @ 2018-07-09 15:34 UTC (permalink / raw)
  To: Mark Rutland
  Cc: linux-arch, greg, Alexey Brodkin, will.deacon, linux-kernel,
	stable, David.Laight, geert, gregkh, tglx, linux-snps-arc

On Mon, Jul 09, 2018 at 04:29:58PM +0100, Mark Rutland wrote:
> Shouldn't that be 8? AFAICT, __alignof__(unsigned long long) is 8 on
> x86_32:

Curious, I wonder why we put that align in atomic64_32 then.

^ permalink raw reply	[flat|nested] 38+ messages in thread

* Re: [PATCH v3] devres: Explicitly align datai[] to 64-bit
  2018-07-09 15:34           ` Peter Zijlstra
@ 2018-07-09 15:34             ` Peter Zijlstra
  2018-07-09 15:45             ` Peter Zijlstra
  1 sibling, 0 replies; 38+ messages in thread
From: Peter Zijlstra @ 2018-07-09 15:34 UTC (permalink / raw)
  To: Mark Rutland
  Cc: Alexey Brodkin, linux-kernel, tglx, linux-snps-arc, stable, greg,
	will.deacon, gregkh, David.Laight, linux-arch, geert

On Mon, Jul 09, 2018 at 04:29:58PM +0100, Mark Rutland wrote:
> Shouldn't that be 8? AFAICT, __alignof__(unsigned long long) is 8 on
> x86_32:

Curious, I wonder why we put that align in atomic64_32 then.

^ permalink raw reply	[flat|nested] 38+ messages in thread

* Re: [PATCH v3] devres: Explicitly align datai[] to 64-bit
  2018-07-09 15:34           ` Peter Zijlstra
  2018-07-09 15:34             ` Peter Zijlstra
@ 2018-07-09 15:45             ` Peter Zijlstra
  2018-07-09 15:45               ` Peter Zijlstra
  2018-07-09 15:48               ` Mark Rutland
  1 sibling, 2 replies; 38+ messages in thread
From: Peter Zijlstra @ 2018-07-09 15:45 UTC (permalink / raw)
  To: Mark Rutland
  Cc: linux-arch, greg, Alexey Brodkin, will.deacon, linux-kernel,
	stable, David.Laight, geert, gregkh, tglx, linux-snps-arc

On Mon, Jul 09, 2018 at 05:34:27PM +0200, Peter Zijlstra wrote:
> On Mon, Jul 09, 2018 at 04:29:58PM +0100, Mark Rutland wrote:
> > Shouldn't that be 8? AFAICT, __alignof__(unsigned long long) is 8 on
> > x86_32:
> 
> Curious, I wonder why we put that align in atomic64_32 then.

Shiny, look at this: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=54188

^ permalink raw reply	[flat|nested] 38+ messages in thread

* Re: [PATCH v3] devres: Explicitly align datai[] to 64-bit
  2018-07-09 15:45             ` Peter Zijlstra
@ 2018-07-09 15:45               ` Peter Zijlstra
  2018-07-09 15:48               ` Mark Rutland
  1 sibling, 0 replies; 38+ messages in thread
From: Peter Zijlstra @ 2018-07-09 15:45 UTC (permalink / raw)
  To: Mark Rutland
  Cc: Alexey Brodkin, linux-kernel, tglx, linux-snps-arc, stable, greg,
	will.deacon, gregkh, David.Laight, linux-arch, geert

On Mon, Jul 09, 2018 at 05:34:27PM +0200, Peter Zijlstra wrote:
> On Mon, Jul 09, 2018 at 04:29:58PM +0100, Mark Rutland wrote:
> > Shouldn't that be 8? AFAICT, __alignof__(unsigned long long) is 8 on
> > x86_32:
> 
> Curious, I wonder why we put that align in atomic64_32 then.

Shiny, look at this: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=54188

^ permalink raw reply	[flat|nested] 38+ messages in thread

* Re: [PATCH v3] devres: Explicitly align datai[] to 64-bit
  2018-07-09 15:45             ` Peter Zijlstra
  2018-07-09 15:45               ` Peter Zijlstra
@ 2018-07-09 15:48               ` Mark Rutland
  2018-07-09 15:48                 ` Mark Rutland
  2018-07-09 16:10                 ` David Laight
  1 sibling, 2 replies; 38+ messages in thread
From: Mark Rutland @ 2018-07-09 15:48 UTC (permalink / raw)
  To: Peter Zijlstra
  Cc: Alexey Brodkin, linux-kernel, tglx, linux-snps-arc, stable, greg,
	will.deacon, gregkh, David.Laight, linux-arch, geert

On Mon, Jul 09, 2018 at 05:45:21PM +0200, Peter Zijlstra wrote:
> On Mon, Jul 09, 2018 at 05:34:27PM +0200, Peter Zijlstra wrote:
> > On Mon, Jul 09, 2018 at 04:29:58PM +0100, Mark Rutland wrote:
> > > Shouldn't that be 8? AFAICT, __alignof__(unsigned long long) is 8 on
> > > x86_32:
> > 
> > Curious, I wonder why we put that align in atomic64_32 then.
> 
> Shiny, look at this: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=54188
> 

Ouch.

[mark@lakrids:~]% cat test.c
#include <stdio.h>

#define PRINT_TYPE_INFO(t) \
        printf("%10s %5d %5d\n", #t, sizeof(t), __alignof__(t))

struct ull {
        unsigned long long v;
};

int main(int argc, char *argv[])
{
        printf("%10s %5s %5s\n", "TYPE", "SIZE", "ALIGN");
        PRINT_TYPE_INFO(int);
        PRINT_TYPE_INFO(long);
        PRINT_TYPE_INFO(long long);
        PRINT_TYPE_INFO(struct ull);

        return 0;
}
[mark@lakrids:~]% gcc -m32 test.c -o test
[mark@lakrids:~]% ./test 
      TYPE  SIZE ALIGN
       int     4     4
      long     4     4
 long long     8     8
struct ull     8     4

Mark.

^ permalink raw reply	[flat|nested] 38+ messages in thread

* Re: [PATCH v3] devres: Explicitly align datai[] to 64-bit
  2018-07-09 15:48               ` Mark Rutland
@ 2018-07-09 15:48                 ` Mark Rutland
  2018-07-09 16:10                 ` David Laight
  1 sibling, 0 replies; 38+ messages in thread
From: Mark Rutland @ 2018-07-09 15:48 UTC (permalink / raw)
  To: Peter Zijlstra
  Cc: Alexey Brodkin, linux-kernel, tglx, linux-snps-arc, stable, greg,
	will.deacon, gregkh, David.Laight, linux-arch, geert

On Mon, Jul 09, 2018 at 05:45:21PM +0200, Peter Zijlstra wrote:
> On Mon, Jul 09, 2018 at 05:34:27PM +0200, Peter Zijlstra wrote:
> > On Mon, Jul 09, 2018 at 04:29:58PM +0100, Mark Rutland wrote:
> > > Shouldn't that be 8? AFAICT, __alignof__(unsigned long long) is 8 on
> > > x86_32:
> > 
> > Curious, I wonder why we put that align in atomic64_32 then.
> 
> Shiny, look at this: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=54188
> 

Ouch.

[mark@lakrids:~]% cat test.c
#include <stdio.h>

#define PRINT_TYPE_INFO(t) \
        printf("%10s %5d %5d\n", #t, sizeof(t), __alignof__(t))

struct ull {
        unsigned long long v;
};

int main(int argc, char *argv[])
{
        printf("%10s %5s %5s\n", "TYPE", "SIZE", "ALIGN");
        PRINT_TYPE_INFO(int);
        PRINT_TYPE_INFO(long);
        PRINT_TYPE_INFO(long long);
        PRINT_TYPE_INFO(struct ull);

        return 0;
}
[mark@lakrids:~]% gcc -m32 test.c -o test
[mark@lakrids:~]% ./test 
      TYPE  SIZE ALIGN
       int     4     4
      long     4     4
 long long     8     8
struct ull     8     4

Mark.

^ permalink raw reply	[flat|nested] 38+ messages in thread

* RE: [PATCH v3] devres: Explicitly align datai[] to 64-bit
  2018-07-09 15:48               ` Mark Rutland
  2018-07-09 15:48                 ` Mark Rutland
@ 2018-07-09 16:10                 ` David Laight
  2018-07-09 16:10                   ` David Laight
  2018-07-09 20:28                   ` Vineet Gupta
  1 sibling, 2 replies; 38+ messages in thread
From: David Laight @ 2018-07-09 16:10 UTC (permalink / raw)
  To: 'Mark Rutland', Peter Zijlstra
  Cc: Alexey Brodkin, linux-kernel, tglx, linux-snps-arc, stable, greg,
	will.deacon, gregkh, linux-arch, geert

From: Mark Rutland
> Sent: 09 July 2018 16:49
> 
> On Mon, Jul 09, 2018 at 05:45:21PM +0200, Peter Zijlstra wrote:
> > On Mon, Jul 09, 2018 at 05:34:27PM +0200, Peter Zijlstra wrote:
> > > On Mon, Jul 09, 2018 at 04:29:58PM +0100, Mark Rutland wrote:
> > > > Shouldn't that be 8? AFAICT, __alignof__(unsigned long long) is 8 on
> > > > x86_32:
> > >
> > > Curious, I wonder why we put that align in atomic64_32 then.
> >
> > Shiny, look at this: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=54188
> >
> 
> Ouch.

Indeed.

changing the definition to:
struct ull {
        unsigned long long v __attribute__((aligned(__alignof__(long long))));
};

prints 8 for the structure alignment.

Time to audit uses of __alignof__().

#define actual_alignof(type) __alignof__(struct { type jsdjdhjdjh; })

	David

^ permalink raw reply	[flat|nested] 38+ messages in thread

* RE: [PATCH v3] devres: Explicitly align datai[] to 64-bit
  2018-07-09 16:10                 ` David Laight
@ 2018-07-09 16:10                   ` David Laight
  2018-07-09 20:28                   ` Vineet Gupta
  1 sibling, 0 replies; 38+ messages in thread
From: David Laight @ 2018-07-09 16:10 UTC (permalink / raw)
  To: 'Mark Rutland', Peter Zijlstra
  Cc: Alexey Brodkin, linux-kernel, tglx, linux-snps-arc, stable, greg,
	will.deacon, gregkh, linux-arch, geert

From: Mark Rutland
> Sent: 09 July 2018 16:49
> 
> On Mon, Jul 09, 2018 at 05:45:21PM +0200, Peter Zijlstra wrote:
> > On Mon, Jul 09, 2018 at 05:34:27PM +0200, Peter Zijlstra wrote:
> > > On Mon, Jul 09, 2018 at 04:29:58PM +0100, Mark Rutland wrote:
> > > > Shouldn't that be 8? AFAICT, __alignof__(unsigned long long) is 8 on
> > > > x86_32:
> > >
> > > Curious, I wonder why we put that align in atomic64_32 then.
> >
> > Shiny, look at this: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=54188
> >
> 
> Ouch.

Indeed.

changing the definition to:
struct ull {
        unsigned long long v __attribute__((aligned(__alignof__(long long))));
};

prints 8 for the structure alignment.

Time to audit uses of __alignof__().

#define actual_alignof(type) __alignof__(struct { type jsdjdhjdjh; })

	David

^ permalink raw reply	[flat|nested] 38+ messages in thread

* Re: [PATCH v3] devres: Explicitly align datai[] to 64-bit
  2018-07-09 16:10                 ` David Laight
  2018-07-09 16:10                   ` David Laight
@ 2018-07-09 20:28                   ` Vineet Gupta
  2018-07-09 20:28                     ` Vineet Gupta
  1 sibling, 1 reply; 38+ messages in thread
From: Vineet Gupta @ 2018-07-09 20:28 UTC (permalink / raw)
  To: David Laight, 'Mark Rutland', Peter Zijlstra
  Cc: Alexey Brodkin, linux-kernel, tglx, linux-snps-arc, stable, greg,
	will.deacon, gregkh, linux-arch, geert

On 07/09/2018 09:10 AM, David Laight wrote:
> From: Mark Rutland
>> Sent: 09 July 2018 16:49
>>
>> On Mon, Jul 09, 2018 at 05:45:21PM +0200, Peter Zijlstra wrote:
>>> On Mon, Jul 09, 2018 at 05:34:27PM +0200, Peter Zijlstra wrote:
>>>> On Mon, Jul 09, 2018 at 04:29:58PM +0100, Mark Rutland wrote:
>>>>> Shouldn't that be 8? AFAICT, __alignof__(unsigned long long) is 8 on
>>>>> x86_32:
>>>>
>>>> Curious, I wonder why we put that align in atomic64_32 then.
>>>
>>> Shiny, look at this: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=54188
>>>
>>
>> Ouch.
> 
> Indeed.
> 
> changing the definition to:
> struct ull {
>         unsigned long long v __attribute__((aligned(__alignof__(long long))));
> };
> 
> prints 8 for the structure alignment.

So this will help force alignment of outer structures triggered by inner members,
but only for globals and perhaps those on stack. I'm not sure how this helps
aligning such  a struct allocated dynamically - we are not passing this extra
alignment info the the allocator API here. Surely the size will increase and we
end up with extra padding in the end as intended originally and pointed to by
Mark, but how does it help with alignment ? What am I missing ?

> 
> Time to audit uses of __alignof__().
> 
> #define actual_alignof(type) __alignof__(struct { type jsdjdhjdjh; })

^ permalink raw reply	[flat|nested] 38+ messages in thread

* Re: [PATCH v3] devres: Explicitly align datai[] to 64-bit
  2018-07-09 20:28                   ` Vineet Gupta
@ 2018-07-09 20:28                     ` Vineet Gupta
  0 siblings, 0 replies; 38+ messages in thread
From: Vineet Gupta @ 2018-07-09 20:28 UTC (permalink / raw)
  To: David Laight, 'Mark Rutland', Peter Zijlstra
  Cc: Alexey Brodkin, linux-kernel, tglx, linux-snps-arc, stable, greg,
	will.deacon, gregkh, linux-arch, geert

On 07/09/2018 09:10 AM, David Laight wrote:
> From: Mark Rutland
>> Sent: 09 July 2018 16:49
>>
>> On Mon, Jul 09, 2018 at 05:45:21PM +0200, Peter Zijlstra wrote:
>>> On Mon, Jul 09, 2018 at 05:34:27PM +0200, Peter Zijlstra wrote:
>>>> On Mon, Jul 09, 2018 at 04:29:58PM +0100, Mark Rutland wrote:
>>>>> Shouldn't that be 8? AFAICT, __alignof__(unsigned long long) is 8 on
>>>>> x86_32:
>>>>
>>>> Curious, I wonder why we put that align in atomic64_32 then.
>>>
>>> Shiny, look at this: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=54188
>>>
>>
>> Ouch.
> 
> Indeed.
> 
> changing the definition to:
> struct ull {
>         unsigned long long v __attribute__((aligned(__alignof__(long long))));
> };
> 
> prints 8 for the structure alignment.

So this will help force alignment of outer structures triggered by inner members,
but only for globals and perhaps those on stack. I'm not sure how this helps
aligning such  a struct allocated dynamically - we are not passing this extra
alignment info the the allocator API here. Surely the size will increase and we
end up with extra padding in the end as intended originally and pointed to by
Mark, but how does it help with alignment ? What am I missing ?

> 
> Time to audit uses of __alignof__().
> 
> #define actual_alignof(type) __alignof__(struct { type jsdjdhjdjh; })

^ permalink raw reply	[flat|nested] 38+ messages in thread

* Re: [PATCH v3] devres: Explicitly align datai[] to 64-bit
  2018-07-09 14:53         ` Alexey Brodkin
  2018-07-09 14:53           ` Alexey Brodkin
@ 2018-10-04 16:19           ` Alexey Brodkin
  2018-10-04 16:19             ` Alexey Brodkin
  1 sibling, 1 reply; 38+ messages in thread
From: Alexey Brodkin @ 2018-10-04 16:19 UTC (permalink / raw)
  To: peterz
  Cc: linux-kernel, tglx, linux-snps-arc, stable, will.deacon, greg,
	gregkh, David.Laight, Vineet Gupta, linux-arch, geert

Hi Peter,

On Mon, 2018-07-09 at 17:53 +0300, Alexey Brodkin wrote:
> Hi Peter,
> 
> On Mon, 2018-07-09 at 16:49 +0200, Peter Zijlstra wrote:
> > On Mon, Jul 09, 2018 at 02:33:26PM +0000, Alexey Brodkin wrote:
> > > > In fact, since alloc_dr() uses kmalloc() to allocate the entire thing,
> > > > it is impossible to guarantee a larger alignment than kmalloc does.
> > > 
> > > Well but 4-bytes [which is critical for atomic64_t] should be much less
> > > than a sane cache line length so above should work.
> > 
> > AFAICT ARCH_KMALLOC_MINALIGN ends up being 4 on x86_32 (it doesn't
> > define ARCH_DMA_MINALIGN and doesn't seem to otherwise override the
> > thing).
> > 
> > So unconditionally setting the alignment of devres::data to 8 seems
> > broken.
> 
> Well but then what other options do we have to fix a problem with
> misaligned access to atomic64_t in drm_gpu_scheduler?

Ping!

Should I send v4 with ARCH_KMALLOC_MINALIGN used for alignment
if explicitly set __aligned__(8) will break x86_32?

-Alexey

^ permalink raw reply	[flat|nested] 38+ messages in thread

* Re: [PATCH v3] devres: Explicitly align datai[] to 64-bit
  2018-10-04 16:19           ` Alexey Brodkin
@ 2018-10-04 16:19             ` Alexey Brodkin
  0 siblings, 0 replies; 38+ messages in thread
From: Alexey Brodkin @ 2018-10-04 16:19 UTC (permalink / raw)
  To: peterz
  Cc: linux-kernel, tglx, linux-snps-arc, stable, will.deacon, greg,
	gregkh, David.Laight, Vineet Gupta, linux-arch, geert

Hi Peter,

On Mon, 2018-07-09 at 17:53 +0300, Alexey Brodkin wrote:
> Hi Peter,
> 
> On Mon, 2018-07-09 at 16:49 +0200, Peter Zijlstra wrote:
> > On Mon, Jul 09, 2018 at 02:33:26PM +0000, Alexey Brodkin wrote:
> > > > In fact, since alloc_dr() uses kmalloc() to allocate the entire thing,
> > > > it is impossible to guarantee a larger alignment than kmalloc does.
> > > 
> > > Well but 4-bytes [which is critical for atomic64_t] should be much less
> > > than a sane cache line length so above should work.
> > 
> > AFAICT ARCH_KMALLOC_MINALIGN ends up being 4 on x86_32 (it doesn't
> > define ARCH_DMA_MINALIGN and doesn't seem to otherwise override the
> > thing).
> > 
> > So unconditionally setting the alignment of devres::data to 8 seems
> > broken.
> 
> Well but then what other options do we have to fix a problem with
> misaligned access to atomic64_t in drm_gpu_scheduler?

Ping!

Should I send v4 with ARCH_KMALLOC_MINALIGN used for alignment
if explicitly set __aligned__(8) will break x86_32?

-Alexey

^ permalink raw reply	[flat|nested] 38+ messages in thread

end of thread, other threads:[~2018-10-04 23:13 UTC | newest]

Thread overview: 38+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-07-09 13:45 [PATCH v3] devres: Explicitly align datai[] to 64-bit Alexey Brodkin
2018-07-09 13:45 ` Alexey Brodkin
2018-07-09 13:54 ` Peter Zijlstra
2018-07-09 13:54   ` Peter Zijlstra
2018-07-09 14:04   ` Mark Rutland
2018-07-09 14:04     ` Mark Rutland
2018-07-09 14:10     ` Geert Uytterhoeven
2018-07-09 14:10       ` Geert Uytterhoeven
2018-07-09 14:07   ` Alexey Brodkin
2018-07-09 14:07     ` Alexey Brodkin
2018-07-09 14:07 ` Peter Zijlstra
2018-07-09 14:07   ` Peter Zijlstra
2018-07-09 14:10   ` Peter Zijlstra
2018-07-09 14:10     ` Peter Zijlstra
2018-07-09 14:33     ` Alexey Brodkin
2018-07-09 14:33       ` Alexey Brodkin
2018-07-09 14:49       ` Peter Zijlstra
2018-07-09 14:49         ` Peter Zijlstra
2018-07-09 14:53         ` Alexey Brodkin
2018-07-09 14:53           ` Alexey Brodkin
2018-10-04 16:19           ` Alexey Brodkin
2018-10-04 16:19             ` Alexey Brodkin
2018-07-09 15:02         ` David Laight
2018-07-09 15:02           ` David Laight
2018-07-09 15:14           ` Peter Zijlstra
2018-07-09 15:14             ` Peter Zijlstra
2018-07-09 15:29         ` Mark Rutland
2018-07-09 15:29           ` Mark Rutland
2018-07-09 15:34           ` Peter Zijlstra
2018-07-09 15:34             ` Peter Zijlstra
2018-07-09 15:45             ` Peter Zijlstra
2018-07-09 15:45               ` Peter Zijlstra
2018-07-09 15:48               ` Mark Rutland
2018-07-09 15:48                 ` Mark Rutland
2018-07-09 16:10                 ` David Laight
2018-07-09 16:10                   ` David Laight
2018-07-09 20:28                   ` Vineet Gupta
2018-07-09 20:28                     ` Vineet Gupta

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).