LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
From: Christoph Lameter <clameter@sgi.com>
To: Vegard Nossum <vegard.nossum@gmail.com>
Cc: Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
	Ingo Molnar <mingo@elte.hu>,
	Pekka Enberg <penberg@cs.helsinki.fi>,
	Andi Kleen <andi@firstfloor.org>,
	Richard Knutsson <ricknu-0@student.ltu.se>
Subject: Re: [PATCH 1/2] kmemcheck v3
Date: Thu, 7 Feb 2008 13:53:27 -0800 (PST)	[thread overview]
Message-ID: <Pine.LNX.4.64.0802071341500.2544@schroedinger.engr.sgi.com> (raw)
In-Reply-To: <47AB79D4.2070605@gmail.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.

  parent reply	other threads:[~2008-02-07 21:53 UTC|newest]

Thread overview: 25+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2008-02-07 21:36 Vegard Nossum
2008-02-07 21:39 ` [PATCH 2/2] " Vegard Nossum
2008-02-07 21:53 ` Christoph Lameter [this message]
2008-02-07 22:12   ` [PATCH 1/2] " Vegard Nossum
2008-02-07 22:53     ` Christoph Lameter
2008-02-07 23:18       ` Vegard Nossum
2008-02-07 23:32         ` Christoph Lameter
2008-02-08  6:40           ` Pekka Enberg
2008-02-08  8:09             ` Ingo Molnar
2008-02-08  6:30       ` Pekka Enberg
2008-02-08  6:33         ` Pekka Enberg
2008-02-08  7:10 ` Christoph Lameter
2008-02-08  7:48   ` Pekka Enberg
2008-02-08 11:55 ` Andi Kleen
2008-02-08 11:31   ` Pekka Enberg
2008-02-08 12:10     ` Andi Kleen
2008-02-08 11:39       ` Pekka Enberg
2008-02-08 11:37   ` Pekka Enberg
2008-02-08 12:15     ` Andi Kleen
2008-02-08 11:43       ` Pekka Enberg
2008-02-08 12:18   ` Vegard Nossum
2008-02-08 13:20     ` Andi Kleen
2008-02-08 12:59       ` Vegard Nossum
2008-02-08 13:48         ` Andi Kleen
2008-02-09  9:33           ` Ingo Molnar

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=Pine.LNX.4.64.0802071341500.2544@schroedinger.engr.sgi.com \
    --to=clameter@sgi.com \
    --cc=andi@firstfloor.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mingo@elte.hu \
    --cc=penberg@cs.helsinki.fi \
    --cc=ricknu-0@student.ltu.se \
    --cc=vegard.nossum@gmail.com \
    --subject='Re: [PATCH 1/2] kmemcheck v3' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).