linux-um archives
 help / color / mirror / Atom feed
From: Steve VanDeBogart <vandebo-lkml@NerdBox.Net>
To: Pekka Enberg <penberg@cs.helsinki.fi>
Cc: user-mode-linux-devel@lists.sourceforge.net,
	John Reiser <jreiser@BitWagon.com>,
	linux-kernel@vger.kernel.org,
	"Paul E. McKenney" <paulmck@us.ibm.com>,
	Ingo Molnar <mingo@elte.hu>,
	dkegel@google.com, jiayingz@google.com
Subject: Re: [uml-devel] [PATCH 5/6] slab: Annotate slab
Date: Tue, 2 Sep 2008 22:08:48 -0700 (PDT)	[thread overview]
Message-ID: <alpine.DEB.1.00.0809022036560.15543@abydos.NerdBox.Net> (raw)
In-Reply-To: <84144f020808300350i7cb2d23aj6d1ffcbeceaf7e9c@mail.gmail.com>

On Sat, 30 Aug 2008, Pekka Enberg wrote:

> Hi Steve,
>
> On Sat, Aug 30, 2008 at 2:17 AM, Steve VanDeBogart
> <vandebo-lkml@nerdbox.net> wrote:
>> Valgrind annotations for the slab allocator:  Malloc-like and free-like
>> for cache_alloc and free.  Telling Valgrind a region is free-like clears
>> all the valid bits, so slabs with constructors need different treatment;
>> tell Valgrind about slab objects when first constructed and free them
>> when the slab is destroyed.
>
> OK, I'm biased (I'm one of the kmemcheck developers) but these hooks
> to SLAB are too ugly to live with. My preferred solution is that you
> reuse the kmemcheck annotations
> kmemcheck_slab_alloc()/kmemcheck_slab_free() we have:
>
> http://git.kernel.org/?p=linux/kernel/git/x86/linux-2.6-tip.git;a=commitdiff;h=30532cb3c49a2a9fed94127aab26003c52398a51

Thank you for pointing out the location of the kmemcheck code.  I looked
around briefly but didn't find the latest version.  It does seem that
kmemcheck and Valgrind annotations want to live in the same places so it
makes perfect sense to combine them where possible.  I'm not exactly sure
what you find so ugly about the Valgrind annotations, please elaborate.

>> @@ -3578,6 +3595,9 @@
>>  {
>>        struct array_cache *ac = cpu_cache_get(cachep);
>>
>> +       if (!cachep->ctor)
>> +               VALGRIND_FREELIKE_BLOCK(objp, 0);
>> +
>>        check_irq_off();
>>        objp = cache_free_debugcheck(cachep, objp,
>> __builtin_return_address(0));
>
> I'm not sure why you want to treat caches with constructor
> differently. Sure, the memory regions *are* initialized but from
> programmer's point of view you're not supposed to be touching the
> memory unless you got it from kmalloc() or kmem_cache_alloc(). Same
> goes for kfree() and kmem_cache_free() -- no touchy touchy after you
> pass a pointer to either of the functions (unless you're RCU, of
> course).

It is true that code above the allocator should not be touching free'd
slab objects.  However, it is also true that objects from slabs that
have a constructor should retain their per byte un/initialized state
through allocation and free cycles (just the semantic of slabs with
constructors AFAICT).

Ideally, we'd tell Valgrind that the bytes of a free'd slab object are
no longer accessible, but the initialized state should remain the same
until the object is made accessible again by the next allocation of
the object.  Unfortunately, the compression method for A & V bits in
Valgrind doesn't allow a region to be inaccessible and retain validness
bits.

The best we can do without spending extra space or extra cycles it to
leave the memory accessible while it is free in a slab.

--
Steve

-------------------------------------------------------------------------
This SF.Net email is sponsored by the Moblin Your Move Developer's challenge
Build the coolest Linux based applications with Moblin SDK & win great prizes
Grand prize is a trip for two to an Open Source event anywhere in the world
http://moblin-contest.org/redirect.php?banner_id=100&url=/
_______________________________________________
User-mode-linux-devel mailing list
User-mode-linux-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/user-mode-linux-devel

  parent reply	other threads:[~2008-09-03  5:08 UTC|newest]

Thread overview: 25+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2008-08-29 23:12 [uml-devel] [PATCH 0/6] support valgrinding uml Steve VanDeBogart
2008-08-29 23:14 ` [uml-devel] [PATCH 1/6] base: Valgrind headers and Kconfig Steve VanDeBogart
2008-09-01  9:32   ` Andi Kleen
2008-09-01 14:06     ` Jeff Dike
2008-09-01 14:22       ` Andi Kleen
2008-09-01 15:47         ` Jeff Dike
2008-08-29 23:15 ` [uml-devel] [PATCH 2/6] UML: Don't valgrind userspace Steve VanDeBogart
2008-09-05 16:37   ` Jeff Dike
2008-09-06 20:55     ` John Reiser
2008-09-06 22:12       ` Jeff Dike
2008-08-29 23:16 ` [uml-devel] [PATCH 3/6] UML and sched: Annotate stacks Steve VanDeBogart
2008-08-29 23:16 ` [uml-devel] [PATCH 4/6] VM: Annotate pagealloc Steve VanDeBogart
2008-08-30 10:57   ` Pekka Enberg
2008-09-03  5:25     ` Steve VanDeBogart
2008-09-03  9:35       ` Pekka Enberg
2008-08-29 23:17 ` [uml-devel] [PATCH 5/6] slab: Annotate slab Steve VanDeBogart
2008-08-30 10:50   ` Pekka Enberg
2008-09-03  2:54     ` John Reiser
2008-09-03  9:39       ` Pekka J Enberg
2008-09-03  5:08     ` Steve VanDeBogart [this message]
2008-09-03  9:27       ` Pekka Enberg
2008-09-03  9:40         ` Pekka Enberg
2008-09-03 15:42         ` Steve VanDeBogart
2008-09-04  7:33           ` Pekka Enberg
2008-08-29 23:18 ` [uml-devel] [PATCH 6/6] VM: Annotate vmalloc Steve VanDeBogart

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=alpine.DEB.1.00.0809022036560.15543@abydos.NerdBox.Net \
    --to=vandebo-lkml@nerdbox.net \
    --cc=dkegel@google.com \
    --cc=jiayingz@google.com \
    --cc=jreiser@BitWagon.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mingo@elte.hu \
    --cc=paulmck@us.ibm.com \
    --cc=penberg@cs.helsinki.fi \
    --cc=user-mode-linux-devel@lists.sourceforge.net \
    /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
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox