[PATCH] slob: push the min alignment to long long

[PATCH] slob: push the min alignment to long long

[Sebastian Andrzej Siewior]

In SLOB ARCH_KMALLOC_MINALIGN is 4 on 32bit platforms by default. On
powerpc and some other architectures except x86 the default alignment of
u64 is 8. The leads to __alignof__(struct ipt_entry) being 8 instead of 4
which is enforced by SLOB.
This leads funny behavior where "iptables -nvL -t nat" does not work on
the first invocation but on the second. The network code has more than one
check of this kind for the correct alignment of the allocated struct.
I personally don't understand why u64 needs 8byte alignment on a 32bit
platform since all access happens via two 4byte reads/writes. I know that
x86_32 has a 64bit cmpxchg instruction but they have also the 4byte
alignment of u64, remember?
David S. Miller says "An allocator needs to provide memory with the maximum
alignment that might be required for types on a given architecture. " [0]
and the fact that gcc on x86 doesn't do it is actually gcc's fault.
Therefore I'm changing the default alignment of SLOB to 8. This fixes my
netfilter problems (and probably other) and we have consistent behavior
across all SL*B allocators.

[0]  http://www.spinics.net/lists/netfilter/msg51123.html

Cc: stable@kernel.org
Cc: David S. Miller <davem@davemloft.net>
Signed-off-by: Sebastian Andrzej Siewior <sebastian@breakpoint.cc>
---
 include/linux/slob_def.h |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/include/linux/slob_def.h b/include/linux/slob_def.h
index 4382db0..019f713 100644
--- a/include/linux/slob_def.h
+++ b/include/linux/slob_def.h
@@ -4,11 +4,11 @@
 #ifdef ARCH_DMA_MINALIGN
 #define ARCH_KMALLOC_MINALIGN ARCH_DMA_MINALIGN
 #else
-#define ARCH_KMALLOC_MINALIGN __alignof__(unsigned long)
+#define ARCH_KMALLOC_MINALIGN __alignof__(unsigned long long)
 #endif
 
 #ifndef ARCH_SLAB_MINALIGN
-#define ARCH_SLAB_MINALIGN __alignof__(unsigned long)
+#define ARCH_SLAB_MINALIGN __alignof__(unsigned long long)
 #endif
 
 void *kmem_cache_alloc_node(struct kmem_cache *, gfp_t flags, int node);
-- 
1.7.4.4

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

[PATCH] slob: push the min alignment to long long

[Sebastian Andrzej Siewior]

In SLOB ARCH_KMALLOC_MINALIGN is 4 on 32bit platforms by default. On
powerpc and some other architectures except x86 the default alignment of
u64 is 8. The leads to __alignof__(struct ipt_entry) being 8 instead of 4
which is enforced by SLOB.
This leads funny behavior where "iptables -nvL -t nat" does not work on
the first invocation but on the second. The network code has more than one
check of this kind for the correct alignment of the allocated struct.
I personally don't understand why u64 needs 8byte alignment on a 32bit
platform since all access happens via two 4byte reads/writes. I know that
x86_32 has a 64bit cmpxchg instruction but they have also the 4byte
alignment of u64, remember?
David S. Miller says "An allocator needs to provide memory with the maximum
alignment that might be required for types on a given architecture. " [0]
and the fact that gcc on x86 doesn't do it is actually gcc's fault.
Therefore I'm changing the default alignment of SLOB to 8. This fixes my
netfilter problems (and probably other) and we have consistent behavior
across all SL*B allocators.

[0]  http://www.spinics.net/lists/netfilter/msg51123.html

Cc: stable@kernel.org
Cc: David S. Miller <davem@davemloft.net>
Signed-off-by: Sebastian Andrzej Siewior <sebastian@breakpoint.cc>
---
 include/linux/slob_def.h |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/include/linux/slob_def.h b/include/linux/slob_def.h
index 4382db0..019f713 100644
--- a/include/linux/slob_def.h
+++ b/include/linux/slob_def.h
@@ -4,11 +4,11 @@
 #ifdef ARCH_DMA_MINALIGN
 #define ARCH_KMALLOC_MINALIGN ARCH_DMA_MINALIGN
 #else
-#define ARCH_KMALLOC_MINALIGN __alignof__(unsigned long)
+#define ARCH_KMALLOC_MINALIGN __alignof__(unsigned long long)
 #endif
 
 #ifndef ARCH_SLAB_MINALIGN
-#define ARCH_SLAB_MINALIGN __alignof__(unsigned long)
+#define ARCH_SLAB_MINALIGN __alignof__(unsigned long long)
 #endif
 
 void *kmem_cache_alloc_node(struct kmem_cache *, gfp_t flags, int node);
-- 
1.7.4.4

Re: [PATCH] slob: push the min alignment to long long

[Christoph Lameter]

On Tue, 14 Jun 2011, Sebastian Andrzej Siewior wrote:

> Therefore I'm changing the default alignment of SLOB to 8. This fixes my
> netfilter problems (and probably other) and we have consistent behavior
> across all SL*B allocators.

If you do that then all slab allocators do the same and we may move
that alignment stuff into include/linux/slab.h instead.

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH] slob: push the min alignment to long long

[Christoph Lameter]

Maybe this would work too?


Subject: slauob: Unify alignment definition

Every slab has its on alignment definition in include/linux/sl?b_def.h. Extract those
and define a common set in include/linux/slab.h.

SLOB: As notes sometimes we need double word alignment on 32 bit. This gives all
structures allocated by SLOB a unsigned long long alignment like the others do.

SLAB: If ARCH_SLAB_MINALIGN is not set SLAB would set ARCH_SLAB_MINALIGN to
zero meaning no alignment at all. Give it the default unsigned long long alignment.

Signed-off-by: Christoph Lameter <cl@linux.com>


---
 include/linux/slab.h     |   10 ++++++++++
 include/linux/slab_def.h |   26 --------------------------
 include/linux/slob_def.h |   10 ----------
 include/linux/slub_def.h |   10 ----------
 4 files changed, 10 insertions(+), 46 deletions(-)

Index: linux-2.6/include/linux/slab.h
===================================================================
--- linux-2.6.orig/include/linux/slab.h	2011-06-14 15:46:38.000000000 -0500
+++ linux-2.6/include/linux/slab.h	2011-06-14 15:46:59.000000000 -0500
@@ -133,6 +133,16 @@ unsigned int kmem_cache_size(struct kmem
 #define KMALLOC_MAX_SIZE	(1UL << KMALLOC_SHIFT_HIGH)
 #define KMALLOC_MAX_ORDER	(KMALLOC_SHIFT_HIGH - PAGE_SHIFT)

+#ifdef ARCH_DMA_MINALIGN
+#define ARCH_KMALLOC_MINALIGN ARCH_DMA_MINALIGN
+#else
+#define ARCH_KMALLOC_MINALIGN __alignof__(unsigned long long)
+#endif
+
+#ifndef ARCH_SLAB_MINALIGN
+#define ARCH_SLAB_MINALIGN __alignof__(unsigned long long)
+#endif
+
 /*
  * Common kmalloc functions provided by all allocators
  */
Index: linux-2.6/include/linux/slab_def.h
===================================================================
--- linux-2.6.orig/include/linux/slab_def.h	2011-06-14 15:47:04.000000000 -0500
+++ linux-2.6/include/linux/slab_def.h	2011-06-14 15:50:04.000000000 -0500
@@ -18,32 +18,6 @@
 #include <trace/events/kmem.h>

 /*
- * Enforce a minimum alignment for the kmalloc caches.
- * Usually, the kmalloc caches are cache_line_size() aligned, except when
- * DEBUG and FORCED_DEBUG are enabled, then they are BYTES_PER_WORD aligned.
- * Some archs want to perform DMA into kmalloc caches and need a guaranteed
- * alignment larger than the alignment of a 64-bit integer.
- * ARCH_KMALLOC_MINALIGN allows that.
- * Note that increasing this value may disable some debug features.
- */
-#ifdef ARCH_DMA_MINALIGN
-#define ARCH_KMALLOC_MINALIGN ARCH_DMA_MINALIGN
-#else
-#define ARCH_KMALLOC_MINALIGN __alignof__(unsigned long long)
-#endif
-
-#ifndef ARCH_SLAB_MINALIGN
-/*
- * Enforce a minimum alignment for all caches.
- * Intended for archs that get misalignment faults even for BYTES_PER_WORD
- * aligned buffers. Includes ARCH_KMALLOC_MINALIGN.
- * If possible: Do not enable this flag for CONFIG_DEBUG_SLAB, it disables
- * some debug features.
- */
-#define ARCH_SLAB_MINALIGN 0
-#endif
-
-/*
  * struct kmem_cache
  *
  * manages a cache.
Index: linux-2.6/include/linux/slob_def.h
===================================================================
--- linux-2.6.orig/include/linux/slob_def.h	2011-06-14 15:47:33.000000000 -0500
+++ linux-2.6/include/linux/slob_def.h	2011-06-14 15:47:40.000000000 -0500
@@ -1,16 +1,6 @@
 #ifndef __LINUX_SLOB_DEF_H
 #define __LINUX_SLOB_DEF_H

-#ifdef ARCH_DMA_MINALIGN
-#define ARCH_KMALLOC_MINALIGN ARCH_DMA_MINALIGN
-#else
-#define ARCH_KMALLOC_MINALIGN __alignof__(unsigned long)
-#endif
-
-#ifndef ARCH_SLAB_MINALIGN
-#define ARCH_SLAB_MINALIGN __alignof__(unsigned long)
-#endif
-
 void *kmem_cache_alloc_node(struct kmem_cache *, gfp_t flags, int node);

 static __always_inline void *kmem_cache_alloc(struct kmem_cache *cachep,
Index: linux-2.6/include/linux/slub_def.h
===================================================================
--- linux-2.6.orig/include/linux/slub_def.h	2011-06-14 15:46:24.000000000 -0500
+++ linux-2.6/include/linux/slub_def.h	2011-06-14 15:46:32.000000000 -0500
@@ -113,16 +113,6 @@ struct kmem_cache {

 #define KMALLOC_SHIFT_LOW ilog2(KMALLOC_MIN_SIZE)

-#ifdef ARCH_DMA_MINALIGN
-#define ARCH_KMALLOC_MINALIGN ARCH_DMA_MINALIGN
-#else
-#define ARCH_KMALLOC_MINALIGN __alignof__(unsigned long long)
-#endif
-
-#ifndef ARCH_SLAB_MINALIGN
-#define ARCH_SLAB_MINALIGN __alignof__(unsigned long long)
-#endif
-
 /*
  * Maximum kmalloc object size handled by SLUB. Larger object allocations
  * are passed through to the page allocator. The page allocator "fastpath"

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH] slob: push the min alignment to long long

[Matt Mackall]

On Tue, 2011-06-14 at 22:10 +0200, Sebastian Andrzej Siewior wrote:
> In SLOB ARCH_KMALLOC_MINALIGN is 4 on 32bit platforms by default. On
> powerpc and some other architectures except x86 the default alignment of
> u64 is 8. The leads to __alignof__(struct ipt_entry) being 8 instead of 4
> which is enforced by SLOB.

Ok, so you claim that ARCH_KMALLOC_MINALIGN is not set on some
architectures, and thus SLOB does the wrong thing.

Doesn't that rather obviously mean that the affected architectures
should define ARCH_KMALLOC_MINALIGN? Because, well, they have an
"architecture-specific minimum kmalloc alignment"?

This change will regress SLOB everywhere where '4' was the right answer.

-- 
Mathematics is the supreme nostalgia of our time.


--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH] slob: push the min alignment to long long

[Sebastian Andrzej Siewior]

* Christoph Lameter | 2011-06-14 16:16:36 [-0500]:

>Maybe this would work too?

Yep, it does. Would you please be so kind to push this stable?

Sebastian

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH] slob: push the min alignment to long long

[Sebastian Andrzej Siewior]

* Matt Mackall | 2011-06-14 17:05:40 [-0500]:

>Ok, so you claim that ARCH_KMALLOC_MINALIGN is not set on some
>architectures, and thus SLOB does the wrong thing.
>
>Doesn't that rather obviously mean that the affected architectures
>should define ARCH_KMALLOC_MINALIGN? Because, well, they have an
>"architecture-specific minimum kmalloc alignment"?

nope, if nothing is defined SLOB asumes that alignment of long is the way
go. Unfortunately alignment of u64 maybe larger than of u32.

>This change will regress SLOB everywhere where '4' was the right answer.
I doubt that 4 was the correct answer. On x86_32 you still get 4.
Everything else might be miss-aligned for u64 types.

Sebastian

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH] slob: push the min alignment to long long

[Matt Mackall]

On Wed, 2011-06-15 at 22:12 +0200, Sebastian Andrzej Siewior wrote:
> * Matt Mackall | 2011-06-14 17:05:40 [-0500]:
> 
> >Ok, so you claim that ARCH_KMALLOC_MINALIGN is not set on some
> >architectures, and thus SLOB does the wrong thing.
> >
> >Doesn't that rather obviously mean that the affected architectures
> >should define ARCH_KMALLOC_MINALIGN? Because, well, they have an
> >"architecture-specific minimum kmalloc alignment"?
> 
> nope, if nothing is defined SLOB asumes that alignment of long is the way
> go. Unfortunately alignment of u64 maybe larger than of u32.

I understand that. I guess we have a different idea of what constitutes
"architecture-specific" and what constitutes "normal".

But I guess I can be persuaded that most architectures now expect 64-bit
alignment of u64s.

-- 
Mathematics is the supreme nostalgia of our time.


--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH] slob: push the min alignment to long long

[Pekka Enberg]

On Wed, Jun 15, 2011 at 11:24 PM, Matt Mackall <mpm@selenic.com> wrote:
> On Wed, 2011-06-15 at 22:12 +0200, Sebastian Andrzej Siewior wrote:
>> * Matt Mackall | 2011-06-14 17:05:40 [-0500]:
>>
>> >Ok, so you claim that ARCH_KMALLOC_MINALIGN is not set on some
>> >architectures, and thus SLOB does the wrong thing.
>> >
>> >Doesn't that rather obviously mean that the affected architectures
>> >should define ARCH_KMALLOC_MINALIGN? Because, well, they have an
>> >"architecture-specific minimum kmalloc alignment"?
>>
>> nope, if nothing is defined SLOB asumes that alignment of long is the way
>> go. Unfortunately alignment of u64 maybe larger than of u32.
>
> I understand that. I guess we have a different idea of what constitutes
> "architecture-specific" and what constitutes "normal".
>
> But I guess I can be persuaded that most architectures now expect 64-bit
> alignment of u64s.

Changing the alignment for everyone is likely to cause less problems
in the future. Matt, are there any practical reasons why we shouldn't
do that?

                         Pekka

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH] slob: push the min alignment to long long

[Matt Mackall]

On Wed, 2011-06-15 at 23:40 +0300, Pekka Enberg wrote:
> On Wed, Jun 15, 2011 at 11:24 PM, Matt Mackall <mpm@selenic.com> wrote:
> > On Wed, 2011-06-15 at 22:12 +0200, Sebastian Andrzej Siewior wrote:
> >> * Matt Mackall | 2011-06-14 17:05:40 [-0500]:
> >>
> >> >Ok, so you claim that ARCH_KMALLOC_MINALIGN is not set on some
> >> >architectures, and thus SLOB does the wrong thing.
> >> >
> >> >Doesn't that rather obviously mean that the affected architectures
> >> >should define ARCH_KMALLOC_MINALIGN? Because, well, they have an
> >> >"architecture-specific minimum kmalloc alignment"?
> >>
> >> nope, if nothing is defined SLOB asumes that alignment of long is the way
> >> go. Unfortunately alignment of u64 maybe larger than of u32.
> >
> > I understand that. I guess we have a different idea of what constitutes
> > "architecture-specific" and what constitutes "normal".
> >
> > But I guess I can be persuaded that most architectures now expect 64-bit
> > alignment of u64s.
> 
> Changing the alignment for everyone is likely to cause less problems
> in the future. Matt, are there any practical reasons why we shouldn't
> do that?

Unless you audit all architectures to check that things are sensible,
it's a trade: regressing performance on one arch to improve correctness
on another. On the one hand, regressions trump improvement. On the
other, correctness trumps performance.

In general, I think the right thing is to require every arch to
explicitly document its alignment requirements via defines in the kernel
headers so that random hackers don't have to scour the internet for
datasheets on obscure architectures they don't care about. We should
have no defaults and refuse to compile on any arch that doesn't have the
define which will ensure someone somewhere actually thinks about it for
each arch.

But as I don't have time to push that vision, I'll let it slide.

-- 
Mathematics is the supreme nostalgia of our time.


--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH] slob: push the min alignment to long long

[David Miller]

From: Sebastian Andrzej Siewior <sebastian@breakpoint.cc>
Date: Wed, 15 Jun 2011 22:12:02 +0200

> I doubt that 4 was the correct answer. On x86_32 you still get 4.

No in certain circumstances with current gcc versions.

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH] slob: push the min alignment to long long

[David Miller]

From: Matt Mackall <mpm@selenic.com>
Date: Wed, 15 Jun 2011 15:55:55 -0500

> In general, I think the right thing is to require every arch to
> explicitly document its alignment requirements via defines in the kernel
> headers so that random hackers don't have to scour the internet for
> datasheets on obscure architectures they don't care about.

Blink... because the compiler doesn't provide a portable way to
do this, right? :-)

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH] slob: push the min alignment to long long

[Matt Mackall]

On Wed, 2011-06-15 at 18:11 -0400, David Miller wrote:
> From: Matt Mackall <mpm@selenic.com>
> Date: Wed, 15 Jun 2011 15:55:55 -0500
> 
> > In general, I think the right thing is to require every arch to
> > explicitly document its alignment requirements via defines in the kernel
> > headers so that random hackers don't have to scour the internet for
> > datasheets on obscure architectures they don't care about.
> 
> Blink... because the compiler doesn't provide a portable way to
> do this, right? :-)

Because I, on x86, cannot deduce the alignment requirements of, say,
CRIS without doing significant research. So answering a question like
"are there any architectures where assumption X fails" is obnoxiously
hard, rather than being a grep.

I also don't think it's a given there's a portable way to deduce the
alignment requirements due to the existence of arch-specific quirks. If
an arch wants to kmalloc its weird crypto or SIMD context and those want
128-bit alignment, we're not going to want to embed that knowledge in
the generic code, but instead tweak an arch define.

Also note that not having generic defaults forces each new architectures
to (nominally) examine each assumption rather than discover they
inherited an incorrect default somewhere down the road.

-- 
Mathematics is the supreme nostalgia of our time.


--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH] slob: push the min alignment to long long

[Pekka Enberg]

On Thu, Jun 16, 2011 at 1:53 AM, Matt Mackall <mpm@selenic.com> wrote:
>> Blink... because the compiler doesn't provide a portable way to
>> do this, right? :-)
>
> Because I, on x86, cannot deduce the alignment requirements of, say,
> CRIS without doing significant research. So answering a question like
> "are there any architectures where assumption X fails" is obnoxiously
> hard, rather than being a grep.
>
> I also don't think it's a given there's a portable way to deduce the
> alignment requirements due to the existence of arch-specific quirks. If
> an arch wants to kmalloc its weird crypto or SIMD context and those want
> 128-bit alignment, we're not going to want to embed that knowledge in
> the generic code, but instead tweak an arch define.
>
> Also note that not having generic defaults forces each new architectures
> to (nominally) examine each assumption rather than discover they
> inherited an incorrect default somewhere down the road.

I don't agree. I think we should either provide defaults that work for
everyone and let architectures override them (which AFAICT Christoph's
patch does) or we flat out #error if architectures don't specify
alignment requirements. The current solution seems to be the worst one
from practical point of view.

This doesn't seem to be a *regression* per se so I'll queue
Christoph's patch for 3.1 and mark it for 3.0-stable.

                            Pekka

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH] slob: push the min alignment to long long

[Pekka Enberg]

On Thu, Jun 16, 2011 at 1:53 AM, Matt Mackall <mpm@selenic.com> wrote:
>> Blink... because the compiler doesn't provide a portable way to
>> do this, right? :-)
>
> Because I, on x86, cannot deduce the alignment requirements of, say,
> CRIS without doing significant research. So answering a question like
> "are there any architectures where assumption X fails" is obnoxiously
> hard, rather than being a grep.
>
> I also don't think it's a given there's a portable way to deduce the
> alignment requirements due to the existence of arch-specific quirks. If
> an arch wants to kmalloc its weird crypto or SIMD context and those want
> 128-bit alignment, we're not going to want to embed that knowledge in
> the generic code, but instead tweak an arch define.
>
> Also note that not having generic defaults forces each new architectures
> to (nominally) examine each assumption rather than discover they
> inherited an incorrect default somewhere down the road.

I don't agree. I think we should either provide defaults that work for
everyone and let architectures override them (which AFAICT Christoph's
patch does) or we flat out #error if architectures don't specify
alignment requirements. The current solution seems to be the worst one
from practical point of view.

This doesn't seem to be a *regression* per se so I'll queue
Christoph's patch for 3.1 and mark it for 3.0-stable.

                            Pekka

Re: [PATCH] slob: push the min alignment to long long

[Matt Mackall]

On Thu, 2011-06-16 at 09:59 +0300, Pekka Enberg wrote:
> On Thu, Jun 16, 2011 at 1:53 AM, Matt Mackall <mpm@selenic.com> wrote:
> >> Blink... because the compiler doesn't provide a portable way to
> >> do this, right? :-)
> >
> > Because I, on x86, cannot deduce the alignment requirements of, say,
> > CRIS without doing significant research. So answering a question like
> > "are there any architectures where assumption X fails" is obnoxiously
> > hard, rather than being a grep.
> >
> > I also don't think it's a given there's a portable way to deduce the
> > alignment requirements due to the existence of arch-specific quirks. If
> > an arch wants to kmalloc its weird crypto or SIMD context and those want
> > 128-bit alignment, we're not going to want to embed that knowledge in
> > the generic code, but instead tweak an arch define.
> >
> > Also note that not having generic defaults forces each new architectures
> > to (nominally) examine each assumption rather than discover they
> > inherited an incorrect default somewhere down the road.
> 
> I don't agree. I think we should either provide defaults that work for
> everyone and let architectures override them (which AFAICT Christoph's
> patch does) or we flat out #error if architectures don't specify
> alignment requirements.

Uh, isn't the latter precisely what I say above?

>  The current solution seems to be the worst one
> from practical point of view.

Good, because no one's advocating for it.

> This doesn't seem to be a *regression* per se so I'll queue
> Christoph's patch for 3.1 and mark it for 3.0-stable.

>                             Pekka


-- 
Mathematics is the supreme nostalgia of our time.


--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH] slob: push the min alignment to long long

[Pekka Enberg]

On Thu, 2011-06-16 at 10:23 -0500, Matt Mackall wrote:
> > I don't agree. I think we should either provide defaults that work for
> > everyone and let architectures override them (which AFAICT Christoph's
> > patch does) or we flat out #error if architectures don't specify
> > alignment requirements.
> 
> Uh, isn't the latter precisely what I say above?
> 
> >  The current solution seems to be the worst one
> > from practical point of view.
> 
> Good, because no one's advocating for it.

Sorry, I totally misunderstood what you wrote!

			Pekka

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH] slob: push the min alignment to long long

[Pekka Enberg]

On Wed, Jun 15, 2011 at 12:16 AM, Christoph Lameter <cl@linux.com> wrote:
> Maybe this would work too?
>
>
> Subject: slauob: Unify alignment definition
>
> Every slab has its on alignment definition in include/linux/sl?b_def.h. Extract those
> and define a common set in include/linux/slab.h.
>
> SLOB: As notes sometimes we need double word alignment on 32 bit. This gives all
> structures allocated by SLOB a unsigned long long alignment like the others do.
>
> SLAB: If ARCH_SLAB_MINALIGN is not set SLAB would set ARCH_SLAB_MINALIGN to
> zero meaning no alignment at all. Give it the default unsigned long long alignment.
>
> Signed-off-by: Christoph Lameter <cl@linux.com>

Applied, thanks!

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH] slob: push the min alignment to long long

[David Rientjes]

On Tue, 14 Jun 2011, Christoph Lameter wrote:

> Index: linux-2.6/include/linux/slab.h
> ===================================================================
> --- linux-2.6.orig/include/linux/slab.h	2011-06-14 15:46:38.000000000 -0500
> +++ linux-2.6/include/linux/slab.h	2011-06-14 15:46:59.000000000 -0500
> @@ -133,6 +133,16 @@ unsigned int kmem_cache_size(struct kmem
>  #define KMALLOC_MAX_SIZE	(1UL << KMALLOC_SHIFT_HIGH)
>  #define KMALLOC_MAX_ORDER	(KMALLOC_SHIFT_HIGH - PAGE_SHIFT)
> 
> +#ifdef ARCH_DMA_MINALIGN
> +#define ARCH_KMALLOC_MINALIGN ARCH_DMA_MINALIGN
> +#else
> +#define ARCH_KMALLOC_MINALIGN __alignof__(unsigned long long)
> +#endif
> +
> +#ifndef ARCH_SLAB_MINALIGN
> +#define ARCH_SLAB_MINALIGN __alignof__(unsigned long long)
> +#endif
> +
>  /*
>   * Common kmalloc functions provided by all allocators
>   */
> Index: linux-2.6/include/linux/slab_def.h
> ===================================================================
> --- linux-2.6.orig/include/linux/slab_def.h	2011-06-14 15:47:04.000000000 -0500
> +++ linux-2.6/include/linux/slab_def.h	2011-06-14 15:50:04.000000000 -0500
> @@ -18,32 +18,6 @@
>  #include <trace/events/kmem.h>
> 
>  /*
> - * Enforce a minimum alignment for the kmalloc caches.
> - * Usually, the kmalloc caches are cache_line_size() aligned, except when
> - * DEBUG and FORCED_DEBUG are enabled, then they are BYTES_PER_WORD aligned.
> - * Some archs want to perform DMA into kmalloc caches and need a guaranteed
> - * alignment larger than the alignment of a 64-bit integer.
> - * ARCH_KMALLOC_MINALIGN allows that.
> - * Note that increasing this value may disable some debug features.
> - */
> -#ifdef ARCH_DMA_MINALIGN
> -#define ARCH_KMALLOC_MINALIGN ARCH_DMA_MINALIGN
> -#else
> -#define ARCH_KMALLOC_MINALIGN __alignof__(unsigned long long)
> -#endif
> -
> -#ifndef ARCH_SLAB_MINALIGN
> -/*
> - * Enforce a minimum alignment for all caches.
> - * Intended for archs that get misalignment faults even for BYTES_PER_WORD
> - * aligned buffers. Includes ARCH_KMALLOC_MINALIGN.
> - * If possible: Do not enable this flag for CONFIG_DEBUG_SLAB, it disables
> - * some debug features.
> - */
> -#define ARCH_SLAB_MINALIGN 0
> -#endif
> -
> -/*
>   * struct kmem_cache
>   *
>   * manages a cache.

Looks like we lost some valuable information in the comments when this got 
moved to slab.h :(

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH] slob: push the min alignment to long long

[David Rientjes]

On Tue, 14 Jun 2011, Christoph Lameter wrote:

> Index: linux-2.6/include/linux/slab.h
> ===================================================================
> --- linux-2.6.orig/include/linux/slab.h	2011-06-14 15:46:38.000000000 -0500
> +++ linux-2.6/include/linux/slab.h	2011-06-14 15:46:59.000000000 -0500
> @@ -133,6 +133,16 @@ unsigned int kmem_cache_size(struct kmem
>  #define KMALLOC_MAX_SIZE	(1UL << KMALLOC_SHIFT_HIGH)
>  #define KMALLOC_MAX_ORDER	(KMALLOC_SHIFT_HIGH - PAGE_SHIFT)
> 
> +#ifdef ARCH_DMA_MINALIGN
> +#define ARCH_KMALLOC_MINALIGN ARCH_DMA_MINALIGN
> +#else
> +#define ARCH_KMALLOC_MINALIGN __alignof__(unsigned long long)
> +#endif
> +
> +#ifndef ARCH_SLAB_MINALIGN
> +#define ARCH_SLAB_MINALIGN __alignof__(unsigned long long)
> +#endif
> +
>  /*
>   * Common kmalloc functions provided by all allocators
>   */
> Index: linux-2.6/include/linux/slab_def.h
> ===================================================================
> --- linux-2.6.orig/include/linux/slab_def.h	2011-06-14 15:47:04.000000000 -0500
> +++ linux-2.6/include/linux/slab_def.h	2011-06-14 15:50:04.000000000 -0500
> @@ -18,32 +18,6 @@
>  #include <trace/events/kmem.h>
> 
>  /*
> - * Enforce a minimum alignment for the kmalloc caches.
> - * Usually, the kmalloc caches are cache_line_size() aligned, except when
> - * DEBUG and FORCED_DEBUG are enabled, then they are BYTES_PER_WORD aligned.
> - * Some archs want to perform DMA into kmalloc caches and need a guaranteed
> - * alignment larger than the alignment of a 64-bit integer.
> - * ARCH_KMALLOC_MINALIGN allows that.
> - * Note that increasing this value may disable some debug features.
> - */
> -#ifdef ARCH_DMA_MINALIGN
> -#define ARCH_KMALLOC_MINALIGN ARCH_DMA_MINALIGN
> -#else
> -#define ARCH_KMALLOC_MINALIGN __alignof__(unsigned long long)
> -#endif
> -
> -#ifndef ARCH_SLAB_MINALIGN
> -/*
> - * Enforce a minimum alignment for all caches.
> - * Intended for archs that get misalignment faults even for BYTES_PER_WORD
> - * aligned buffers. Includes ARCH_KMALLOC_MINALIGN.
> - * If possible: Do not enable this flag for CONFIG_DEBUG_SLAB, it disables
> - * some debug features.
> - */
> -#define ARCH_SLAB_MINALIGN 0
> -#endif
> -
> -/*
>   * struct kmem_cache
>   *
>   * manages a cache.

Looks like we lost some valuable information in the comments when this got 
moved to slab.h :(

Re: [PATCH] slob: push the min alignment to long long

[Christoph Lameter]

On Wed, 22 Jun 2011, David Rientjes wrote:

> >   * struct kmem_cache
> >   *
> >   * manages a cache.
>
> Looks like we lost some valuable information in the comments when this got
> moved to slab.h :(

Ok. If we want a description for these defines then lets use this.


Subject: slab allocators: Provide generic description of alignment defines

Provide description for alignment defines.

Signed-off-by: Christoph Lameter <cl@linux.com>

---
 include/linux/slab.h |   10 ++++++++++
 1 file changed, 10 insertions(+)

Index: linux-2.6/include/linux/slab.h
===================================================================
--- linux-2.6.orig/include/linux/slab.h	2011-06-23 09:26:55.000000000 -0500
+++ linux-2.6/include/linux/slab.h	2011-06-23 09:33:46.000000000 -0500
@@ -133,12 +133,22 @@ unsigned int kmem_cache_size(struct kmem
 #define KMALLOC_MAX_SIZE	(1UL << KMALLOC_SHIFT_HIGH)
 #define KMALLOC_MAX_ORDER	(KMALLOC_SHIFT_HIGH - PAGE_SHIFT)

+/*
+ * Some archs want to perform DMA into kmalloc caches and need a guaranteed
+ * alignment larger than the alignment of a 64-bit integer.
+ * Setting ARCH_KMALLOC_MINALIGN in arch headers allows that.
+ */
 #ifdef ARCH_DMA_MINALIGN
 #define ARCH_KMALLOC_MINALIGN ARCH_DMA_MINALIGN
 #else
 #define ARCH_KMALLOC_MINALIGN __alignof__(unsigned long long)
 #endif

+/*
+ * Setting ARCH_SLAB_MINALIGN in arch headers allows a different alignment.
+ * Intended for arches that get misalignment faults even for 64 bit integer
+ * aligned buffers.
+ */
 #ifndef ARCH_SLAB_MINALIGN
 #define ARCH_SLAB_MINALIGN __alignof__(unsigned long long)
 #endif

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH] slob: push the min alignment to long long

[Christoph Lameter]

On Wed, 22 Jun 2011, David Rientjes wrote:

> >   * struct kmem_cache
> >   *
> >   * manages a cache.
>
> Looks like we lost some valuable information in the comments when this got
> moved to slab.h :(

Ok. If we want a description for these defines then lets use this.


Subject: slab allocators: Provide generic description of alignment defines

Provide description for alignment defines.

Signed-off-by: Christoph Lameter <cl@linux.com>

---
 include/linux/slab.h |   10 ++++++++++
 1 file changed, 10 insertions(+)

Index: linux-2.6/include/linux/slab.h
===================================================================
--- linux-2.6.orig/include/linux/slab.h	2011-06-23 09:26:55.000000000 -0500
+++ linux-2.6/include/linux/slab.h	2011-06-23 09:33:46.000000000 -0500
@@ -133,12 +133,22 @@ unsigned int kmem_cache_size(struct kmem
 #define KMALLOC_MAX_SIZE	(1UL << KMALLOC_SHIFT_HIGH)
 #define KMALLOC_MAX_ORDER	(KMALLOC_SHIFT_HIGH - PAGE_SHIFT)

+/*
+ * Some archs want to perform DMA into kmalloc caches and need a guaranteed
+ * alignment larger than the alignment of a 64-bit integer.
+ * Setting ARCH_KMALLOC_MINALIGN in arch headers allows that.
+ */
 #ifdef ARCH_DMA_MINALIGN
 #define ARCH_KMALLOC_MINALIGN ARCH_DMA_MINALIGN
 #else
 #define ARCH_KMALLOC_MINALIGN __alignof__(unsigned long long)
 #endif

+/*
+ * Setting ARCH_SLAB_MINALIGN in arch headers allows a different alignment.
+ * Intended for arches that get misalignment faults even for 64 bit integer
+ * aligned buffers.
+ */
 #ifndef ARCH_SLAB_MINALIGN
 #define ARCH_SLAB_MINALIGN __alignof__(unsigned long long)
 #endif

Re: [PATCH] slob: push the min alignment to long long

[David Rientjes]

On Thu, 23 Jun 2011, Christoph Lameter wrote:

> Subject: slab allocators: Provide generic description of alignment defines
> 
> Provide description for alignment defines.
> 
> Signed-off-by: Christoph Lameter <cl@linux.com>

Acked-by: David Rientjes <rientjes@google.com>

Thanks!

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>