Re: [PATCH] Re: Sparc32: random invalid instruction occourances onsparc32 (sun4c)

From: Mark Fortescue
Date: Tue Jul 03 2007 - 18:48:08 EST


Hi David,

I will try out your patch shortly.

On Tue, 3 Jul 2007, David Woodhouse wrote:

On Tue, 2007-07-03 at 22:25 +0100, Mark Fortescue wrote:
The problem is that sun4c Sparc32 can't handle un-aligned variables so
having a 64bit readzone word that is not aligned on a 64bit boundary is a
problem.

Surely, it can. You just have to tell the compiler that it's not
properly aligned, and it'll emit code to cope. Hence the suggestion that
you use 'unsigned long long __attribute__((aligned(BYTES_PER_WORD))'.
But it's probably better just to make sure it remains aligned; you're
right.

In addition, having looked at the size calculations, it looks to me as if
not all of them got updated to handle 64bit redzone words.

Really? Other than the alignment of the second redzone, what's wrong?
Remember that the 'user word' is still not necessarily 64-bit. And in
fact I suspect that's what is causing the problem -- your object _size_
will be aligned to 8 bytes, including the user word, and then we look
for the second redzone word 12 bytes before the end of the object.


Yes, the user word is a 32bit word and this is part of the issue.

I may be wrong about the size calculations but if you take a look at lines 2174 to 2188 and 2207 to 2203, reading the comments suggest to me that these need to be changed to match the changes to the RedZone words. Failing to change these means that 32bit aligned access of the 64bit RedZone words is still posible and this will kill sun4c.

For the 64bit RedZone word to be 64bit aligned (required by sun4c), the User word must be 64bit aligned. I don't see where in your patch, this is enforced.

Does this fix it?

diff --git a/mm/slab.c b/mm/slab.c
index 6d65cf4..3b15671 100644
--- a/mm/slab.c
+++ b/mm/slab.c
@@ -547,7 +547,7 @@ static unsigned long long *dbg_redzone2(struct kmem_cache *cachep, void *objp)
if (cachep->flags & SLAB_STORE_USER)
return (unsigned long long *)(objp + cachep->buffer_size -
sizeof(unsigned long long) -
- BYTES_PER_WORD);
+ max(BYTES_PER_WORD, __alignof__(unsigned long long)));
return (unsigned long long *) (objp + cachep->buffer_size -
sizeof(unsigned long long));
}
@@ -2262,9 +2262,14 @@ kmem_cache_create (const char *name, size_t size, size_t align,
}
if (flags & SLAB_STORE_USER) {
/* user store requires one word storage behind the end of
- * the real object.
+ * the real object. But if the second red zone must be
+ * aligned 'better' than that, allow for it.
*/
- size += BYTES_PER_WORD;
+ if (flags & SLAB_RED_ZONE
+ && BYTES_PER_WORD < __alignof__(unsigned long long))
+ size += __alignof__(unsigned long long);
+ else
+ size += BYTES_PER_WORD;
}
#if FORCED_DEBUG && defined(CONFIG_DEBUG_PAGEALLOC)
if (size >= malloc_sizes[INDEX_L3 + 1].cs_size


--
dwmw2


-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/