Re: [PATCH 1/2] kmemcheck v3

[Christoph Lameter <clameter@sgi.com>]

On Thu, 7 Feb 2008, Vegard Nossum wrote:

> --- a/include/linux/slab.h
> +++ b/include/linux/slab.h
> @@ -28,6 +28,7 @@
>  #define SLAB_DESTROY_BY_RCU	0x00080000UL	/* Defer freeing slabs to RCU
> */
>  #define SLAB_MEM_SPREAD		0x00100000UL	/* Spread some memory
> over cpuset */
>  #define SLAB_TRACE		0x00200000UL	/* Trace allocations and frees
> */
> +#define SLAB_NOTRACK		0x00400000UL	/* Don't track use of
> uninitialized memory */

Ok new exception for tracking.

> index 3f05667..3b3dfb8 100644
> --- a/mm/slub.c
> +++ b/mm/slub.c
> +#ifdef CONFIG_KMEMCHECK
> +	if (s->flags & SLAB_NOTRACK)
> +		flags |= __GFP_NOTRACK;
> +
> +	/* Actually allocate twice as much, since we need to track the
> +	 * status of each byte within the allocation. */
> +	if (!(flags & __GFP_NOTRACK)) {
> +		pages += pages;
> +		order += 1;
> +	}

Hmmmm... You seem to assume that __GFP_NOTRACK can be passed to slab 
function calls like kmalloc. That is pretty unreliable. Could we add
__GFP_NOTRACK to the flags on which the slab allocators BUG?

> @@ -1551,9 +1586,7 @@ static __always_inline void *slab_alloc(struct
> kmem_cache *s,
>  	local_irq_save(flags);
>  	c = get_cpu_slab(s, smp_processor_id());
>  	if (unlikely(!c->freelist || !node_match(c, node)))
> -
>  		object = __slab_alloc(s, gfpflags, node, addr, c);
> -
>  	else {
>  		object = c->freelist;
>  		c->freelist = object[c->offset];

Drop this hunk.


> @@ -2344,6 +2393,8 @@ EXPORT_SYMBOL(kmem_cache_destroy);
>  struct kmem_cache kmalloc_caches[PAGE_SHIFT] __cacheline_aligned;
>  EXPORT_SYMBOL(kmalloc_caches);
> 
> +struct kmem_cache cache_cache;
> +

Why do we need a cache_cache?

>  #ifdef CONFIG_ZONE_DMA
>  static struct kmem_cache *kmalloc_caches_dma[PAGE_SHIFT];
>  #endif
> @@ -2391,6 +2442,9 @@ static struct kmem_cache *create_kmalloc_cache(struct
> kmem_cache *s,
>  	if (gfp_flags & SLUB_DMA)
>  		flags = SLAB_CACHE_DMA;
> 
> +	if (gfp_flags & __GFP_NOTRACK)
> +		flags |= SLAB_NOTRACK;
> +
>  	down_write(&slub_lock);
>  	if (!kmem_cache_open(s, gfp_flags, name, size, ARCH_KMALLOC_MINALIGN,
>  			flags, NULL))

Drop this one. create_kmalloc_cache is done only during bootstrap and 
kmalloc caches either all have SLAB_NOTRACK set or all do not have it 
set.

> @@ -2445,14 +2499,18 @@ static noinline struct kmem_cache
> *dma_kmalloc_cache(int index, gfp_t flags)
>  	if (kmalloc_caches_dma[index])
>  		goto unlock_out;
> 
> +#ifdef CONFIG_KMEMCHECK
> +	flags |= __GFP_NOTRACK;
> +#endif
> +

Same here.