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

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

[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);

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