All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] slab : allow SLAB_RED_ZONE and SLAB_STORE_USER to work on arm
@ 2012-10-16 11:17 ` Matthieu CASTET
  0 siblings, 0 replies; 11+ messages in thread
From: Matthieu CASTET @ 2012-10-16 11:17 UTC (permalink / raw)
  To: linux-kernel, 'linux-arm-kernel'
  Cc: Matthieu CASTET, Matthieu Castet, Russell King, Pekka Enberg

From: Matthieu CASTET <castet.matthieu@free.fr>

on cortexA8 (omap3) ralign is 64 and __alignof__(unsigned long long) is 8.
So we always disable debug.

This patch is based on 5c5e3b33b7cb959a401f823707bee006caadd76e, but fix
case were align < sizeof(unsigned long long).

Signed-off-by: Matthieu Castet <matthieu.castet@parrot.com>
CC: Russell King <rmk@arm.linux.org.uk>
CC: Pekka Enberg <penberg@cs.helsinki.fi>
---
 mm/slab.c |    8 +++-----
 1 file changed, 3 insertions(+), 5 deletions(-)

diff --git a/mm/slab.c b/mm/slab.c
index c685475..8427901 100644
--- a/mm/slab.c
+++ b/mm/slab.c
@@ -2462,9 +2462,6 @@ __kmem_cache_create (const char *name, size_t size, size_t align,
 	if (ralign < align) {
 		ralign = align;
 	}
-	/* disable debug if necessary */
-	if (ralign > __alignof__(unsigned long long))
-		flags &= ~(SLAB_RED_ZONE | SLAB_STORE_USER);
 	/*
 	 * 4) Store it.
 	 */
@@ -2491,8 +2488,9 @@ __kmem_cache_create (const char *name, size_t size, size_t align,
 	 */
 	if (flags & SLAB_RED_ZONE) {
 		/* add space for red zone words */
-		cachep->obj_offset += sizeof(unsigned long long);
-		size += 2 * sizeof(unsigned long long);
+		int offset = max(align, sizeof(unsigned long long));
+		cachep->obj_offset += offset;
+		size += offset + sizeof(unsigned long long);
 	}
 	if (flags & SLAB_STORE_USER) {
 		/* user store requires one word storage behind the end of
-- 
1.7.10.4


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

* [PATCH] slab : allow SLAB_RED_ZONE and SLAB_STORE_USER to work on arm
@ 2012-10-16 11:17 ` Matthieu CASTET
  0 siblings, 0 replies; 11+ messages in thread
From: Matthieu CASTET @ 2012-10-16 11:17 UTC (permalink / raw)
  To: linux-arm-kernel

From: Matthieu CASTET <castet.matthieu@free.fr>

on cortexA8 (omap3) ralign is 64 and __alignof__(unsigned long long) is 8.
So we always disable debug.

This patch is based on 5c5e3b33b7cb959a401f823707bee006caadd76e, but fix
case were align < sizeof(unsigned long long).

Signed-off-by: Matthieu Castet <matthieu.castet@parrot.com>
CC: Russell King <rmk@arm.linux.org.uk>
CC: Pekka Enberg <penberg@cs.helsinki.fi>
---
 mm/slab.c |    8 +++-----
 1 file changed, 3 insertions(+), 5 deletions(-)

diff --git a/mm/slab.c b/mm/slab.c
index c685475..8427901 100644
--- a/mm/slab.c
+++ b/mm/slab.c
@@ -2462,9 +2462,6 @@ __kmem_cache_create (const char *name, size_t size, size_t align,
 	if (ralign < align) {
 		ralign = align;
 	}
-	/* disable debug if necessary */
-	if (ralign > __alignof__(unsigned long long))
-		flags &= ~(SLAB_RED_ZONE | SLAB_STORE_USER);
 	/*
 	 * 4) Store it.
 	 */
@@ -2491,8 +2488,9 @@ __kmem_cache_create (const char *name, size_t size, size_t align,
 	 */
 	if (flags & SLAB_RED_ZONE) {
 		/* add space for red zone words */
-		cachep->obj_offset += sizeof(unsigned long long);
-		size += 2 * sizeof(unsigned long long);
+		int offset = max(align, sizeof(unsigned long long));
+		cachep->obj_offset += offset;
+		size += offset + sizeof(unsigned long long);
 	}
 	if (flags & SLAB_STORE_USER) {
 		/* user store requires one word storage behind the end of
-- 
1.7.10.4

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

* Re: [PATCH] slab : allow SLAB_RED_ZONE and SLAB_STORE_USER to work on arm
  2012-10-16 11:17 ` Matthieu CASTET
@ 2012-10-31  7:59   ` Pekka Enberg
  -1 siblings, 0 replies; 11+ messages in thread
From: Pekka Enberg @ 2012-10-31  7:59 UTC (permalink / raw)
  To: Matthieu CASTET
  Cc: linux-kernel, linux-arm-kernel, Matthieu CASTET, Russell King,
	Shiyong Li, Christoph Lameter, David Rientjes, Andrew Morton

Hi,

(Adding more people to CC.)

On Tue, Oct 16, 2012 at 2:17 PM, Matthieu CASTET
<matthieu.castet@parrot.com> wrote:
> From: Matthieu CASTET <castet.matthieu@free.fr>
>
> on cortexA8 (omap3) ralign is 64 and __alignof__(unsigned long long) is 8.
> So we always disable debug.
>
> This patch is based on 5c5e3b33b7cb959a401f823707bee006caadd76e, but fix
> case were align < sizeof(unsigned long long).
>
> Signed-off-by: Matthieu Castet <matthieu.castet@parrot.com>
> CC: Russell King <rmk@arm.linux.org.uk>
> CC: Pekka Enberg <penberg@cs.helsinki.fi>
> ---
>  mm/slab.c |    8 +++-----
>  1 file changed, 3 insertions(+), 5 deletions(-)
>
> diff --git a/mm/slab.c b/mm/slab.c
> index c685475..8427901 100644
> --- a/mm/slab.c
> +++ b/mm/slab.c
> @@ -2462,9 +2462,6 @@ __kmem_cache_create (const char *name, size_t size, size_t align,
>         if (ralign < align) {
>                 ralign = align;
>         }
> -       /* disable debug if necessary */
> -       if (ralign > __alignof__(unsigned long long))
> -               flags &= ~(SLAB_RED_ZONE | SLAB_STORE_USER);
>         /*
>          * 4) Store it.
>          */
> @@ -2491,8 +2488,9 @@ __kmem_cache_create (const char *name, size_t size, size_t align,
>          */
>         if (flags & SLAB_RED_ZONE) {
>                 /* add space for red zone words */
> -               cachep->obj_offset += sizeof(unsigned long long);
> -               size += 2 * sizeof(unsigned long long);
> +               int offset = max(align, sizeof(unsigned long long));
> +               cachep->obj_offset += offset;
> +               size += offset + sizeof(unsigned long long);
>         }
>         if (flags & SLAB_STORE_USER) {
>                 /* user store requires one word storage behind the end of

This piece of code tends to break in peculiar ways every time someone
touches it. I could use some more convincing in the changelog this
time it won't...

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

* [PATCH] slab : allow SLAB_RED_ZONE and SLAB_STORE_USER to work on arm
@ 2012-10-31  7:59   ` Pekka Enberg
  0 siblings, 0 replies; 11+ messages in thread
From: Pekka Enberg @ 2012-10-31  7:59 UTC (permalink / raw)
  To: linux-arm-kernel

Hi,

(Adding more people to CC.)

On Tue, Oct 16, 2012 at 2:17 PM, Matthieu CASTET
<matthieu.castet@parrot.com> wrote:
> From: Matthieu CASTET <castet.matthieu@free.fr>
>
> on cortexA8 (omap3) ralign is 64 and __alignof__(unsigned long long) is 8.
> So we always disable debug.
>
> This patch is based on 5c5e3b33b7cb959a401f823707bee006caadd76e, but fix
> case were align < sizeof(unsigned long long).
>
> Signed-off-by: Matthieu Castet <matthieu.castet@parrot.com>
> CC: Russell King <rmk@arm.linux.org.uk>
> CC: Pekka Enberg <penberg@cs.helsinki.fi>
> ---
>  mm/slab.c |    8 +++-----
>  1 file changed, 3 insertions(+), 5 deletions(-)
>
> diff --git a/mm/slab.c b/mm/slab.c
> index c685475..8427901 100644
> --- a/mm/slab.c
> +++ b/mm/slab.c
> @@ -2462,9 +2462,6 @@ __kmem_cache_create (const char *name, size_t size, size_t align,
>         if (ralign < align) {
>                 ralign = align;
>         }
> -       /* disable debug if necessary */
> -       if (ralign > __alignof__(unsigned long long))
> -               flags &= ~(SLAB_RED_ZONE | SLAB_STORE_USER);
>         /*
>          * 4) Store it.
>          */
> @@ -2491,8 +2488,9 @@ __kmem_cache_create (const char *name, size_t size, size_t align,
>          */
>         if (flags & SLAB_RED_ZONE) {
>                 /* add space for red zone words */
> -               cachep->obj_offset += sizeof(unsigned long long);
> -               size += 2 * sizeof(unsigned long long);
> +               int offset = max(align, sizeof(unsigned long long));
> +               cachep->obj_offset += offset;
> +               size += offset + sizeof(unsigned long long);
>         }
>         if (flags & SLAB_STORE_USER) {
>                 /* user store requires one word storage behind the end of

This piece of code tends to break in peculiar ways every time someone
touches it. I could use some more convincing in the changelog this
time it won't...

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

* Re: [PATCH] slab : allow SLAB_RED_ZONE and SLAB_STORE_USER to work on arm
  2012-10-31  7:59   ` Pekka Enberg
@ 2012-10-31 10:01     ` Matthieu CASTET
  -1 siblings, 0 replies; 11+ messages in thread
From: Matthieu CASTET @ 2012-10-31 10:01 UTC (permalink / raw)
  To: Pekka Enberg
  Cc: linux-kernel, linux-arm-kernel, Matthieu CASTET, Russell King,
	Shiyong Li, Christoph Lameter, David Rientjes, Andrew Morton

Pekka Enberg a écrit :
> Hi,
> 
> (Adding more people to CC.)
> 
> On Tue, Oct 16, 2012 at 2:17 PM, Matthieu CASTET
> <matthieu.castet@parrot.com> wrote:
>> From: Matthieu CASTET <castet.matthieu@free.fr>
>>
>> on cortexA8 (omap3) ralign is 64 and __alignof__(unsigned long long) is 8.
>> So we always disable debug.
>>
>> This patch is based on 5c5e3b33b7cb959a401f823707bee006caadd76e, but fix
>> case were align < sizeof(unsigned long long).
>>
>> Signed-off-by: Matthieu Castet <matthieu.castet@parrot.com>
>> CC: Russell King <rmk@arm.linux.org.uk>
>> CC: Pekka Enberg <penberg@cs.helsinki.fi>
>> ---
>>  mm/slab.c |    8 +++-----
>>  1 file changed, 3 insertions(+), 5 deletions(-)
>>
>> diff --git a/mm/slab.c b/mm/slab.c
>> index c685475..8427901 100644
>> --- a/mm/slab.c
>> +++ b/mm/slab.c
>> @@ -2462,9 +2462,6 @@ __kmem_cache_create (const char *name, size_t size, size_t align,
>>         if (ralign < align) {
>>                 ralign = align;
>>         }
>> -       /* disable debug if necessary */
>> -       if (ralign > __alignof__(unsigned long long))
>> -               flags &= ~(SLAB_RED_ZONE | SLAB_STORE_USER);
>>         /*
>>          * 4) Store it.
>>          */
>> @@ -2491,8 +2488,9 @@ __kmem_cache_create (const char *name, size_t size, size_t align,
>>          */
>>         if (flags & SLAB_RED_ZONE) {
>>                 /* add space for red zone words */
>> -               cachep->obj_offset += sizeof(unsigned long long);
>> -               size += 2 * sizeof(unsigned long long);
>> +               int offset = max(align, sizeof(unsigned long long));
>> +               cachep->obj_offset += offset;
>> +               size += offset + sizeof(unsigned long long);
>>         }
>>         if (flags & SLAB_STORE_USER) {
>>                 /* user store requires one word storage behind the end of
> 
> This piece of code tends to break in peculiar ways every time someone
> touches it. I could use some more convincing in the changelog this
> time it won't...
> 
Ok, is the following changelog is ok ?

The current slab code only allow to put redzone( and user store) info if "buffer
alignment(ralign) <= __alignof__(unsigned long long)". This was done because we
want to keep the buffer aligned for user even after adding redzone at the
beginning of the buffer (the user store is stored after user buffer) [1]

But instead of disabling this feature when "ralign > __alignof__(unsigned long
long)", we can force the alignment by allocating ralign before user buffer.

This is done by setting ralign in obj_offset when "ralign > __alignof__(unsigned
long long)" and keeping the old behavior when  "ralign <= __alignof__(unsigned
long long)" (we set sizeof(unsigned long long)).

The 5c5e3b33b7cb959a401f823707bee006caadd76e commit wasn't handling "ralign <=
__alignof__(unsigned long long)", that's why it broked some configuration.

This was tested on omap3 (ralign is 64 and __alignof__(unsigned long long) is 8)


[1]
/*
 * memory layout of objects:
 * 0        : objp
 * 0 .. cachep->obj_offset - BYTES_PER_WORD - 1: padding. This ensures that
 *      the end of an object is aligned with the end of the real
 *      allocation. Catches writes behind the end of the allocation.
 * cachep->obj_offset - BYTES_PER_WORD .. cachep->obj_offset - 1:
 *      redzone word.
 * cachep->obj_offset: The real object.
 * cachep->buffer_size - 2* BYTES_PER_WORD: redzone word [BYTES_PER_WORD long]
 * cachep->buffer_size - 1* BYTES_PER_WORD: last caller address
 *                  [BYTES_PER_WORD long]
 */

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

* [PATCH] slab : allow SLAB_RED_ZONE and SLAB_STORE_USER to work on arm
@ 2012-10-31 10:01     ` Matthieu CASTET
  0 siblings, 0 replies; 11+ messages in thread
From: Matthieu CASTET @ 2012-10-31 10:01 UTC (permalink / raw)
  To: linux-arm-kernel

Pekka Enberg a ?crit :
> Hi,
> 
> (Adding more people to CC.)
> 
> On Tue, Oct 16, 2012 at 2:17 PM, Matthieu CASTET
> <matthieu.castet@parrot.com> wrote:
>> From: Matthieu CASTET <castet.matthieu@free.fr>
>>
>> on cortexA8 (omap3) ralign is 64 and __alignof__(unsigned long long) is 8.
>> So we always disable debug.
>>
>> This patch is based on 5c5e3b33b7cb959a401f823707bee006caadd76e, but fix
>> case were align < sizeof(unsigned long long).
>>
>> Signed-off-by: Matthieu Castet <matthieu.castet@parrot.com>
>> CC: Russell King <rmk@arm.linux.org.uk>
>> CC: Pekka Enberg <penberg@cs.helsinki.fi>
>> ---
>>  mm/slab.c |    8 +++-----
>>  1 file changed, 3 insertions(+), 5 deletions(-)
>>
>> diff --git a/mm/slab.c b/mm/slab.c
>> index c685475..8427901 100644
>> --- a/mm/slab.c
>> +++ b/mm/slab.c
>> @@ -2462,9 +2462,6 @@ __kmem_cache_create (const char *name, size_t size, size_t align,
>>         if (ralign < align) {
>>                 ralign = align;
>>         }
>> -       /* disable debug if necessary */
>> -       if (ralign > __alignof__(unsigned long long))
>> -               flags &= ~(SLAB_RED_ZONE | SLAB_STORE_USER);
>>         /*
>>          * 4) Store it.
>>          */
>> @@ -2491,8 +2488,9 @@ __kmem_cache_create (const char *name, size_t size, size_t align,
>>          */
>>         if (flags & SLAB_RED_ZONE) {
>>                 /* add space for red zone words */
>> -               cachep->obj_offset += sizeof(unsigned long long);
>> -               size += 2 * sizeof(unsigned long long);
>> +               int offset = max(align, sizeof(unsigned long long));
>> +               cachep->obj_offset += offset;
>> +               size += offset + sizeof(unsigned long long);
>>         }
>>         if (flags & SLAB_STORE_USER) {
>>                 /* user store requires one word storage behind the end of
> 
> This piece of code tends to break in peculiar ways every time someone
> touches it. I could use some more convincing in the changelog this
> time it won't...
> 
Ok, is the following changelog is ok ?

The current slab code only allow to put redzone( and user store) info if "buffer
alignment(ralign) <= __alignof__(unsigned long long)". This was done because we
want to keep the buffer aligned for user even after adding redzone@the
beginning of the buffer (the user store is stored after user buffer) [1]

But instead of disabling this feature when "ralign > __alignof__(unsigned long
long)", we can force the alignment by allocating ralign before user buffer.

This is done by setting ralign in obj_offset when "ralign > __alignof__(unsigned
long long)" and keeping the old behavior when  "ralign <= __alignof__(unsigned
long long)" (we set sizeof(unsigned long long)).

The 5c5e3b33b7cb959a401f823707bee006caadd76e commit wasn't handling "ralign <=
__alignof__(unsigned long long)", that's why it broked some configuration.

This was tested on omap3 (ralign is 64 and __alignof__(unsigned long long) is 8)


[1]
/*
 * memory layout of objects:
 * 0        : objp
 * 0 .. cachep->obj_offset - BYTES_PER_WORD - 1: padding. This ensures that
 *      the end of an object is aligned with the end of the real
 *      allocation. Catches writes behind the end of the allocation.
 * cachep->obj_offset - BYTES_PER_WORD .. cachep->obj_offset - 1:
 *      redzone word.
 * cachep->obj_offset: The real object.
 * cachep->buffer_size - 2* BYTES_PER_WORD: redzone word [BYTES_PER_WORD long]
 * cachep->buffer_size - 1* BYTES_PER_WORD: last caller address
 *                  [BYTES_PER_WORD long]
 */

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

* Re: [PATCH] slab : allow SLAB_RED_ZONE and SLAB_STORE_USER to work on arm
  2012-10-31 10:01     ` Matthieu CASTET
@ 2013-07-11  6:48       ` Pekka Enberg
  -1 siblings, 0 replies; 11+ messages in thread
From: Pekka Enberg @ 2013-07-11  6:48 UTC (permalink / raw)
  To: Matthieu CASTET
  Cc: linux-kernel, linux-arm-kernel, Matthieu CASTET, Russell King,
	Shiyong Li, Christoph Lameter, David Rientjes, Andrew Morton

(nine months later...)

On 10/31/2012 12:01 PM, Matthieu CASTET wrote:
>> This piece of code tends to break in peculiar ways every time someone
>> touches it. I could use some more convincing in the changelog this
>> time it won't...
>
> Ok, is the following changelog is ok ?
>
> The current slab code only allow to put redzone( and user store) info if "buffer
> alignment(ralign) <= __alignof__(unsigned long long)". This was done because we
> want to keep the buffer aligned for user even after adding redzone at the
> beginning of the buffer (the user store is stored after user buffer) [1]
>
> But instead of disabling this feature when "ralign > __alignof__(unsigned long
> long)", we can force the alignment by allocating ralign before user buffer.
>
> This is done by setting ralign in obj_offset when "ralign > __alignof__(unsigned
> long long)" and keeping the old behavior when  "ralign <= __alignof__(unsigned
> long long)" (we set sizeof(unsigned long long)).
>
> The 5c5e3b33b7cb959a401f823707bee006caadd76e commit wasn't handling "ralign <=
> __alignof__(unsigned long long)", that's why it broked some configuration.
>
> This was tested on omap3 (ralign is 64 and __alignof__(unsigned long long) is 8)

Care to resend this patch with the above changelog? I'll see
if I can find someone to ACK it for v3.12.

			Pekka

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

* [PATCH] slab : allow SLAB_RED_ZONE and SLAB_STORE_USER to work on arm
@ 2013-07-11  6:48       ` Pekka Enberg
  0 siblings, 0 replies; 11+ messages in thread
From: Pekka Enberg @ 2013-07-11  6:48 UTC (permalink / raw)
  To: linux-arm-kernel

(nine months later...)

On 10/31/2012 12:01 PM, Matthieu CASTET wrote:
>> This piece of code tends to break in peculiar ways every time someone
>> touches it. I could use some more convincing in the changelog this
>> time it won't...
>
> Ok, is the following changelog is ok ?
>
> The current slab code only allow to put redzone( and user store) info if "buffer
> alignment(ralign) <= __alignof__(unsigned long long)". This was done because we
> want to keep the buffer aligned for user even after adding redzone at the
> beginning of the buffer (the user store is stored after user buffer) [1]
>
> But instead of disabling this feature when "ralign > __alignof__(unsigned long
> long)", we can force the alignment by allocating ralign before user buffer.
>
> This is done by setting ralign in obj_offset when "ralign > __alignof__(unsigned
> long long)" and keeping the old behavior when  "ralign <= __alignof__(unsigned
> long long)" (we set sizeof(unsigned long long)).
>
> The 5c5e3b33b7cb959a401f823707bee006caadd76e commit wasn't handling "ralign <=
> __alignof__(unsigned long long)", that's why it broked some configuration.
>
> This was tested on omap3 (ralign is 64 and __alignof__(unsigned long long) is 8)

Care to resend this patch with the above changelog? I'll see
if I can find someone to ACK it for v3.12.

			Pekka

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

* Re: [PATCH] slab : allow SLAB_RED_ZONE and SLAB_STORE_USER to work on arm
  2013-01-21  9:56 ` Matthieu CASTET
  (?)
@ 2013-03-14 14:03 ` Castet Matthieu
  -1 siblings, 0 replies; 11+ messages in thread
From: Castet Matthieu @ 2013-03-14 14:03 UTC (permalink / raw)
  To: linux-kernel

On Mon, Jan 21, 2013 at 10:56 AM, Matthieu CASTET
<matthieu.castet@parrot.com> wrote:
> The current slab code only allow to put redzone( and user store) info if "buffer
> alignment(ralign) <= __alignof__(unsigned long long)". This was done because we
> want to keep the buffer aligned for user even after adding redzone at the
> beginning of the buffer (the user store is stored after user buffer) [1]
>
> But instead of disabling this feature when "ralign > __alignof__(unsigned long
> long)", we can force the alignment by allocating ralign before user buffer.
>
> This is done by setting ralign in obj_offset when "ralign > __alignof__(unsigned
> long long)" and keeping the old behavior when  "ralign <= __alignof__(unsigned
> long long)" (we set sizeof(unsigned long long)).
>
> The 5c5e3b33b7cb959a401f823707bee006caadd76e commit wasn't handling "ralign <=
> __alignof__(unsigned long long)", that's why it broked some configuration.
>
> This was tested on omap3 (ralign is 64 and __alignof__(unsigned long long) is 8)
>
> [1]
> /*
>  * memory layout of objects:
>  * 0        : objp
>  * 0 .. cachep->obj_offset - BYTES_PER_WORD - 1: padding. This ensures that
>  *      the end of an object is aligned with the end of the real
>  *      allocation. Catches writes behind the end of the allocation.
>  * cachep->obj_offset - BYTES_PER_WORD .. cachep->obj_offset - 1:
>  *      redzone word.
>  * cachep->obj_offset: The real object.
>  * cachep->buffer_size - 2* BYTES_PER_WORD: redzone word [BYTES_PER_WORD long]
>  * cachep->buffer_size - 1* BYTES_PER_WORD: last caller address
>  *                  [BYTES_PER_WORD long]
>  */

Ping,

Thanks,

Matthieu


>
> Signed-off-by: Matthieu Castet <matthieu.castet@parrot.com>
> CC: Russell King <rmk@arm.linux.org.uk>
> CC: Pekka Enberg <penberg@cs.helsinki.fi>
> ---
>  mm/slab.c |    8 +++-----
>  1 file changed, 3 insertions(+), 5 deletions(-)
>
> diff --git a/mm/slab.c b/mm/slab.c
> index e7667a3..d2380d9 100644
> --- a/mm/slab.c
> +++ b/mm/slab.c
> @@ -2392,9 +2392,6 @@ __kmem_cache_create (struct kmem_cache *cachep, unsigned long flags)
>         if (ralign < cachep->align) {
>                 ralign = cachep->align;
>         }
> -       /* disable debug if necessary */
> -       if (ralign > __alignof__(unsigned long long))
> -               flags &= ~(SLAB_RED_ZONE | SLAB_STORE_USER);
>         /*
>          * 4) Store it.
>          */
> @@ -2414,8 +2411,9 @@ __kmem_cache_create (struct kmem_cache *cachep, unsigned long flags)
>          */
>         if (flags & SLAB_RED_ZONE) {
>                 /* add space for red zone words */
> -               cachep->obj_offset += sizeof(unsigned long long);
> -               size += 2 * sizeof(unsigned long long);
> +               int offset = max(ralign, sizeof(unsigned long long));
> +               cachep->obj_offset += offset;
> +               size += offset + sizeof(unsigned long long);
>         }
>         if (flags & SLAB_STORE_USER) {
>                 /* user store requires one word storage behind the end of
> --
> 1.7.10.4
>

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

* [PATCH] slab : allow SLAB_RED_ZONE and SLAB_STORE_USER to work on arm
@ 2013-01-21  9:56 ` Matthieu CASTET
  0 siblings, 0 replies; 11+ messages in thread
From: Matthieu CASTET @ 2013-01-21  9:56 UTC (permalink / raw)
  To: linux-kernel
  Cc: akpm, rientjes, cl, rmk, linux-arm-kernel, Matthieu CASTET, Pekka Enberg

The current slab code only allow to put redzone( and user store) info if "buffer
alignment(ralign) <= __alignof__(unsigned long long)". This was done because we
want to keep the buffer aligned for user even after adding redzone at the
beginning of the buffer (the user store is stored after user buffer) [1]

But instead of disabling this feature when "ralign > __alignof__(unsigned long
long)", we can force the alignment by allocating ralign before user buffer.

This is done by setting ralign in obj_offset when "ralign > __alignof__(unsigned
long long)" and keeping the old behavior when  "ralign <= __alignof__(unsigned
long long)" (we set sizeof(unsigned long long)).

The 5c5e3b33b7cb959a401f823707bee006caadd76e commit wasn't handling "ralign <=
__alignof__(unsigned long long)", that's why it broked some configuration.

This was tested on omap3 (ralign is 64 and __alignof__(unsigned long long) is 8)

[1]
/*
 * memory layout of objects:
 * 0        : objp
 * 0 .. cachep->obj_offset - BYTES_PER_WORD - 1: padding. This ensures that
 *      the end of an object is aligned with the end of the real
 *      allocation. Catches writes behind the end of the allocation.
 * cachep->obj_offset - BYTES_PER_WORD .. cachep->obj_offset - 1:
 *      redzone word.
 * cachep->obj_offset: The real object.
 * cachep->buffer_size - 2* BYTES_PER_WORD: redzone word [BYTES_PER_WORD long]
 * cachep->buffer_size - 1* BYTES_PER_WORD: last caller address
 *                  [BYTES_PER_WORD long]
 */

Signed-off-by: Matthieu Castet <matthieu.castet@parrot.com>
CC: Russell King <rmk@arm.linux.org.uk>
CC: Pekka Enberg <penberg@cs.helsinki.fi>
---
 mm/slab.c |    8 +++-----
 1 file changed, 3 insertions(+), 5 deletions(-)

diff --git a/mm/slab.c b/mm/slab.c
index e7667a3..d2380d9 100644
--- a/mm/slab.c
+++ b/mm/slab.c
@@ -2392,9 +2392,6 @@ __kmem_cache_create (struct kmem_cache *cachep, unsigned long flags)
 	if (ralign < cachep->align) {
 		ralign = cachep->align;
 	}
-	/* disable debug if necessary */
-	if (ralign > __alignof__(unsigned long long))
-		flags &= ~(SLAB_RED_ZONE | SLAB_STORE_USER);
 	/*
 	 * 4) Store it.
 	 */
@@ -2414,8 +2411,9 @@ __kmem_cache_create (struct kmem_cache *cachep, unsigned long flags)
 	 */
 	if (flags & SLAB_RED_ZONE) {
 		/* add space for red zone words */
-		cachep->obj_offset += sizeof(unsigned long long);
-		size += 2 * sizeof(unsigned long long);
+		int offset = max(ralign, sizeof(unsigned long long));
+		cachep->obj_offset += offset;
+		size += offset + sizeof(unsigned long long);
 	}
 	if (flags & SLAB_STORE_USER) {
 		/* user store requires one word storage behind the end of
-- 
1.7.10.4


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

* [PATCH] slab : allow SLAB_RED_ZONE and SLAB_STORE_USER to work on arm
@ 2013-01-21  9:56 ` Matthieu CASTET
  0 siblings, 0 replies; 11+ messages in thread
From: Matthieu CASTET @ 2013-01-21  9:56 UTC (permalink / raw)
  To: linux-arm-kernel

The current slab code only allow to put redzone( and user store) info if "buffer
alignment(ralign) <= __alignof__(unsigned long long)". This was done because we
want to keep the buffer aligned for user even after adding redzone@the
beginning of the buffer (the user store is stored after user buffer) [1]

But instead of disabling this feature when "ralign > __alignof__(unsigned long
long)", we can force the alignment by allocating ralign before user buffer.

This is done by setting ralign in obj_offset when "ralign > __alignof__(unsigned
long long)" and keeping the old behavior when  "ralign <= __alignof__(unsigned
long long)" (we set sizeof(unsigned long long)).

The 5c5e3b33b7cb959a401f823707bee006caadd76e commit wasn't handling "ralign <=
__alignof__(unsigned long long)", that's why it broked some configuration.

This was tested on omap3 (ralign is 64 and __alignof__(unsigned long long) is 8)

[1]
/*
 * memory layout of objects:
 * 0        : objp
 * 0 .. cachep->obj_offset - BYTES_PER_WORD - 1: padding. This ensures that
 *      the end of an object is aligned with the end of the real
 *      allocation. Catches writes behind the end of the allocation.
 * cachep->obj_offset - BYTES_PER_WORD .. cachep->obj_offset - 1:
 *      redzone word.
 * cachep->obj_offset: The real object.
 * cachep->buffer_size - 2* BYTES_PER_WORD: redzone word [BYTES_PER_WORD long]
 * cachep->buffer_size - 1* BYTES_PER_WORD: last caller address
 *                  [BYTES_PER_WORD long]
 */

Signed-off-by: Matthieu Castet <matthieu.castet@parrot.com>
CC: Russell King <rmk@arm.linux.org.uk>
CC: Pekka Enberg <penberg@cs.helsinki.fi>
---
 mm/slab.c |    8 +++-----
 1 file changed, 3 insertions(+), 5 deletions(-)

diff --git a/mm/slab.c b/mm/slab.c
index e7667a3..d2380d9 100644
--- a/mm/slab.c
+++ b/mm/slab.c
@@ -2392,9 +2392,6 @@ __kmem_cache_create (struct kmem_cache *cachep, unsigned long flags)
 	if (ralign < cachep->align) {
 		ralign = cachep->align;
 	}
-	/* disable debug if necessary */
-	if (ralign > __alignof__(unsigned long long))
-		flags &= ~(SLAB_RED_ZONE | SLAB_STORE_USER);
 	/*
 	 * 4) Store it.
 	 */
@@ -2414,8 +2411,9 @@ __kmem_cache_create (struct kmem_cache *cachep, unsigned long flags)
 	 */
 	if (flags & SLAB_RED_ZONE) {
 		/* add space for red zone words */
-		cachep->obj_offset += sizeof(unsigned long long);
-		size += 2 * sizeof(unsigned long long);
+		int offset = max(ralign, sizeof(unsigned long long));
+		cachep->obj_offset += offset;
+		size += offset + sizeof(unsigned long long);
 	}
 	if (flags & SLAB_STORE_USER) {
 		/* user store requires one word storage behind the end of
-- 
1.7.10.4

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

end of thread, other threads:[~2013-07-11  6:48 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2012-10-16 11:17 [PATCH] slab : allow SLAB_RED_ZONE and SLAB_STORE_USER to work on arm Matthieu CASTET
2012-10-16 11:17 ` Matthieu CASTET
2012-10-31  7:59 ` Pekka Enberg
2012-10-31  7:59   ` Pekka Enberg
2012-10-31 10:01   ` Matthieu CASTET
2012-10-31 10:01     ` Matthieu CASTET
2013-07-11  6:48     ` Pekka Enberg
2013-07-11  6:48       ` Pekka Enberg
2013-01-21  9:56 Matthieu CASTET
2013-01-21  9:56 ` Matthieu CASTET
2013-03-14 14:03 ` Castet Matthieu

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.