Re: [PATCH - V2] Fix missing of last user while dumping slab corruption log

Previous thread: linux-next: manual merge of the drm tree with Linus' tree by Stephen Rothwell on Sunday, April 11, 2010 - 10:06 pm. (1 message)

Next thread: [PATCH 1/23] Make register values available to panic notifiers by David VomLehn on Sunday, April 11, 2010 - 11:03 pm. (3 messages)
From: ShiYong LI
Date: Sunday, April 11, 2010 - 10:50 pm

Hi,

Compared to previous version, add alignment checking to make sure
memory space storing redzone2 and last user tags is 8 byte alignment.

From 949e8c29e8681a2359e23a8fbd8b9d4833f42344 Mon Sep 17 00:00:00 2001
From: Shiyong Li <shi-yong.li@motorola.com>
Date: Mon, 12 Apr 2010 13:48:21 +0800
Subject: [PATCH] Fix missing of last user info while getting
DEBUG_SLAB config enabled.

Even with SLAB_RED_ZONE and SLAB_STORE_USER enabled, kernel would NOT
store redzone and last user data around allocated memory space if arch
cache line > sizeof(unsigned long long). As a result, last user information
is unexpectedly MISSED while dumping slab corruption log.

This fix makes sure that redzone and last user tags get stored unless
the required alignment breaks redzone's.

Signed-off-by: Shiyong Li <shi-yong.li@motorola.com>
---
 mm/slab.c |    8 ++++----
 1 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/mm/slab.c b/mm/slab.c
index a8a38ca..b97c57e 100644
--- a/mm/slab.c
+++ b/mm/slab.c
@@ -2267,8 +2267,8 @@ 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))
+	/* disable debug if not aligning with REDZONE_ALIGN */
+	if (ralign & (__alignof__(unsigned long long) - 1))
 		flags &= ~(SLAB_RED_ZONE | SLAB_STORE_USER);
 	/*
 	 * 4) Store it.
@@ -2289,8 +2289,8 @@ 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);
+		cachep->obj_offset += align;
+		size += align + sizeof(unsigned long long);
 	}
 	if (flags & SLAB_STORE_USER) {
 		/* user store requires one word storage behind the end of
-- 
1.6.0.4





-- 
Thanks & Best Regards
Shiyong
--

From: TAO HU
Date: Tuesday, April 13, 2010 - 12:05 am

Hi,  Pekka Enberg

Actually we greped "kmem_cache_create" in whole kernel souce tree
(2.6.29 and 2.6.32).

Either "align" equal to "0" or flag SLAB_HWCACHE_ALIGN is used when
calling kmem_cache_create().
Seems all of arch's cache-line-size is multiple of 64-bit/8-byte
(sizeof(long long)) except  arch-microblaze (4-byte).
The smallest (except arch-microblaze) cache-line-size is 2^4= 16-byte
as I can see.
So even considering possible sizeof(long long) == 128-bit/16-byte, it
is still safe to apply Shiyong's original version.

Anyway, Shiyong's new patch check the weired situation that "align >
sizeof(long long) && align is NOT multiple of sizeof (long long)"
Let us know whether the new version address your concerns.

-- 
Best Regards
Hu Tao





--

From: Pekka Enberg
Date: Tuesday, April 13, 2010 - 2:32 am

I don't think that's correct. The "task_xstate" has alignof(struct 
task_xstate) and there seems to be so GCC attributes that force 

Yeah, sorry for dragging this issue on. I've been looking at the patch 
but haven't been able to convince myself that it's correct. Nick, David, 
Christoph, Matt, could you also please take a look at this?

			Pekka
--

From: Pekka Enberg
Date: Wednesday, April 14, 2010 - 10:56 am

From: TAO HU
Date: Wednesday, April 14, 2010 - 8:04 pm

Hi, Pekka Enberg

Thanks!
If we hadn't seen the last user info, we would not have fixed a
difficult bug in our system.
So very glad to know.

-- 
Best Regards
Hu Tao

--

Previous thread: linux-next: manual merge of the drm tree with Linus' tree by Stephen Rothwell on Sunday, April 11, 2010 - 10:06 pm. (1 message)

Next thread: [PATCH 1/23] Make register values available to panic notifiers by David VomLehn on Sunday, April 11, 2010 - 11:03 pm. (3 messages)