[lkml]   [2008]   [Sep]   [2]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
Messages in this thread
    SubjectRe: [uml-devel] [PATCH 5/6] slab: Annotate slab
    Pekka Enberg wrote:
    > Steve VanDeBogart proposed
    >> Index: linux-2.6.27-rc5/mm/slab.c
    >> ===================================================================
    >> --- linux-2.6.27-rc5.orig/mm/slab.c 2008-08-29 14:24:25.000000000 -0700
    >> +++ linux-2.6.27-rc5/mm/slab.c 2008-08-29 14:24:42.000000000 -0700
    > @@ -3466,6 +3481,8 @@
    >> objp = cache_alloc_debugcheck_after(cachep, flags, objp, caller);
    >> prefetchw(objp);
    >> + if (!cachep->ctor)
    >> + VALGRIND_MALLOCLIKE_BLOCK(objp, cachep->buffer_size, 0, 0);
    >> if (unlikely((flags & __GFP_ZERO) && objp))
    >> memset(objp, 0, obj_size(cachep));
    > 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).

    Hi, I developed the predecessor to Steve's patches. The patch treats
    a cache with a constructor differently because the semantics of kmalloc
    are different in that case. If a cache has a constructor then the
    object returned by kmalloc has been initialized for certain. Namely,
    the constructor initialized the object before it was kmalloc'ed the first
    time, and each corresponding caller [re-]initialized the object before
    calling kfree. This is emphasized by comments in mm/slab.c:
    * This means, that your constructor is used only for newly allocated
    * slabs and you must pass objects with the same initializations to
    * kmem_cache_free.

    The slab allocator has the property that the contents of a block [object]
    that is returned by kmalloc are identical to the contents that were passed
    to kfree (or back as result from the constructor) unless something special
    happens, such as SLAB_POISON. A block [object] which is handed back and
    forth between kfree and kmalloc builds and retains history throughout the
    lifetime of the cache. This property is directly contrary to the semantics
    of libc malloc. The contents of the result of libc malloc must be considered
    to be *un*initialized always.

    Yes, there is at least one kmalloc+kfree slab object cache that depends on
    retaining state from kfree to kmalloc. My first attempt treated kmalloc+kfree
    like libc malloc+free, but I soon learned that was not the actual semantics.

    Because the slab allocator deals in objects that persist through kfree
    and kmalloc, then there are only two indications that objects ever become
    uninitialized: SLAB_POISON, and not having a constructor. Obviously the
    application of SLAB_POISON means that the object becomes [logically] uninit.
    Not having a constructor *tends* to indicate that kmalloc() implies uninit,
    by chain of reasoning based on the comment quoted above from mm/slab.c.
    If the object was uninit upon first kmalloc, and if kfree is supposed to
    receive objects that are "ready for kmalloc", and because a caller cannot
    discern the first-ever return of an object by kmalloc, then the caller
    must treat each result of kmalloc (from that particular cache) as uninit.
    It would be helpful if each cache had a documenting comment such as "This
    cache depends on objects whose contents persist from kfree() to kmalloc()"
    or a comment such as "An object in this cache is considered to become
    uninitialized when the object is passed to kfree."

    With regard to Read-Copy-Update, I anticipate that RCU will require
    special annotations because of the deliberate references after kfree
    and before the next corresponding kmalloc. These special annotations
    will require great care, because bugs in this area have high value.

    John Reiser,

     \ /
      Last update: 2008-09-03 04:57    [W:0.025 / U:4.116 seconds]
    ©2003-2017 Jasper Spaans. hosted at Digital OceanAdvertise on this site