[PATCH] slab: fix generic PAGE_POISONING conflict with SLAB_RED_ZONE

[PATCH] slab: fix generic PAGE_POISONING conflict with SLAB_RED_ZONE

[Ron]

A generic page poisoning mechanism was added with commit:
 6a11f75b6a17b5d9ac5025f8d048382fd1f47377
which destructively poisons full pages with a bitpattern.

On arches where PAGE_POISONING is used, this conflicts with the slab
redzone checking enabled by DEBUG_SLAB, scribbling bits all over its
magic words and making it complain about that quite emphatically.

On x86 (and I presume at present all the other arches which set
ARCH_SUPPORTS_DEBUG_PAGEALLOC too), the kernel_map_pages() operation
is non destructive so it can coexist with the other DEBUG_SLAB
mechanisms just fine.

This patch favours the expensive full page destruction test for
cases where there is a collision and it is explicitly selected.

Signed-off-by: Ron Lee <ron@debian.org>


diff --git a/mm/slab.c b/mm/slab.c
index 9a90b00..b5e5b27 100644
--- a/mm/slab.c
+++ b/mm/slab.c
@@ -2353,6 +2353,15 @@ kmem_cache_create (const char *name, size_t size, size_t align,
 		/* really off slab. No need for manual alignment */
 		slab_size =
 		    cachep->num * sizeof(kmem_bufctl_t) + sizeof(struct slab);
+
+#ifdef CONFIG_PAGE_POISONING
+		/* If we're going to use the generic kernel_map_pages()
+		 * poisoning, then it's going to smash the contents of
+		 * the redzone and userword anyhow, so switch them off.
+		 */
+		if (size % PAGE_SIZE == 0 && flags & SLAB_POISON)
+			flags &= ~(SLAB_RED_ZONE | SLAB_STORE_USER);
+#endif
 	}
 
 	cachep->colour_off = cache_line_size();

--
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/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH] slab: fix generic PAGE_POISONING conflict with SLAB_RED_ZONE

[Pekka Enberg]

On Fri, 2009-05-22 at 04:58 +0930, Ron wrote:
> A generic page poisoning mechanism was added with commit:
>  6a11f75b6a17b5d9ac5025f8d048382fd1f47377
> which destructively poisons full pages with a bitpattern.
> 
> On arches where PAGE_POISONING is used, this conflicts with the slab
> redzone checking enabled by DEBUG_SLAB, scribbling bits all over its
> magic words and making it complain about that quite emphatically.
> 
> On x86 (and I presume at present all the other arches which set
> ARCH_SUPPORTS_DEBUG_PAGEALLOC too), the kernel_map_pages() operation
> is non destructive so it can coexist with the other DEBUG_SLAB
> mechanisms just fine.
> 
> This patch favours the expensive full page destruction test for
> cases where there is a collision and it is explicitly selected.
> 
> Signed-off-by: Ron Lee <ron@debian.org>

Applied, thanks!

			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/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

[PATCH] slab: add missing guard for kernel_map_pages() use

[Ron]

All other uses of kernel_map_pages() are explicitly excluded without
CONFIG_DEBUG_PAGEALLOC, this one should be too.

Signed-off-by: Ron Lee <ron@debian.org>


diff --git a/mm/slab.c b/mm/slab.c
index 9a90b00..b5e5b27 100644
--- a/mm/slab.c
+++ b/mm/slab.c
@@ -2674,10 +2683,12 @@ static void cache_init_objs(struct kmem_cache *cachep,
 				slab_error(cachep, "constructor overwrote the"
 					   " start of an object");
 		}
+#ifdef CONFIG_DEBUG_PAGEALLOC
 		if ((cachep->buffer_size % PAGE_SIZE) == 0 &&
 			    OFF_SLAB(cachep) && cachep->flags & SLAB_POISON)
 			kernel_map_pages(virt_to_page(objp),
 					 cachep->buffer_size / PAGE_SIZE, 0);
+#endif
 #else
 		if (cachep->ctor)
 			cachep->ctor(objp);


--
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/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH] slab: add missing guard for kernel_map_pages() use

[Ron]

On Fri, May 22, 2009 at 06:20:40PM +0930, Ron wrote:
> 
> All other uses of kernel_map_pages() are explicitly excluded without
> CONFIG_DEBUG_PAGEALLOC, this one should be too.
> 
> Signed-off-by: Ron Lee <ron@debian.org>
> 
> 
> diff --git a/mm/slab.c b/mm/slab.c
> index 9a90b00..b5e5b27 100644
> --- a/mm/slab.c
> +++ b/mm/slab.c
> @@ -2674,10 +2683,12 @@ static void cache_init_objs(struct kmem_cache *cachep,
>  				slab_error(cachep, "constructor overwrote the"
>  					   " start of an object");
>  		}
> +#ifdef CONFIG_DEBUG_PAGEALLOC
>  		if ((cachep->buffer_size % PAGE_SIZE) == 0 &&
>  			    OFF_SLAB(cachep) && cachep->flags & SLAB_POISON)
>  			kernel_map_pages(virt_to_page(objp),
>  					 cachep->buffer_size / PAGE_SIZE, 0);
> +#endif
>  #else
>  		if (cachep->ctor)
>  			cachep->ctor(objp);

Actually, no.  I'm wrong on this one.  The compiler will already take that
one away because kernel_map_pages() is an empty function in that case.

It's subtle, but this extra guard actually adds nothing (unless you consider
more #ifdefs to be 'clarifying');

Sorry for the extra noise,
Ron


--
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/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>