* [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; 5+ 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] 5+ messages in thread* [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
0 siblings, 1 reply; 5+ 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] 5+ messages in thread* [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
2012-10-31 10:01 ` Matthieu CASTET
0 siblings, 1 reply; 5+ 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] 5+ messages in thread* [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
2013-07-11 6:48 ` Pekka Enberg
0 siblings, 1 reply; 5+ 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] 5+ messages in thread* [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
0 siblings, 0 replies; 5+ 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] 5+ messages in thread
end of thread, other threads:[~2013-07-11 6:48 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-01-21 9:56 [PATCH] slab : allow SLAB_RED_ZONE and SLAB_STORE_USER to work on arm Matthieu CASTET
-- strict thread matches above, loose matches on Subject: below --
2012-10-16 11:17 Matthieu CASTET
2012-10-31 7:59 ` Pekka Enberg
2012-10-31 10:01 ` Matthieu CASTET
2013-07-11 6:48 ` Pekka Enberg
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).