lkml.org 
[lkml]   [2018]   [Apr]   [10]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    SubjectRe: [PATCH 1/2] slab: __GFP_ZERO is incompatible with a constructor
    From
    Date
    On 04/10/2018 02:53 PM, Matthew Wilcox wrote:
    > From: Matthew Wilcox <mawilcox@microsoft.com>
    >
    > __GFP_ZERO requests that the object be initialised to all-zeroes,
    > while the purpose of a constructor is to initialise an object to a
    > particular pattern. We cannot do both. Add a warning to catch any
    > users who mistakenly pass a __GFP_ZERO flag when allocating a slab with
    > a constructor.
    >
    > Fixes: d07dbea46405 ("Slab allocators: support __GFP_ZERO in all allocators")
    > Signed-off-by: Matthew Wilcox <mawilcox@microsoft.com>
    > Cc: stable@vger.kernel.org

    It doesn't fix any known problem, does it? Then the stable tag seems too
    much IMHO. Especially for fixing a 2007 commit.

    Otherwise
    Acked-by: Vlastimil Babka <vbabka@suse.cz>

    > ---
    > mm/slab.c | 6 ++++--
    > mm/slob.c | 4 +++-
    > mm/slub.c | 6 ++++--
    > 3 files changed, 11 insertions(+), 5 deletions(-)
    >
    > diff --git a/mm/slab.c b/mm/slab.c
    > index 38d3f4fd17d7..8b2cb7db85db 100644
    > --- a/mm/slab.c
    > +++ b/mm/slab.c
    > @@ -3313,8 +3313,10 @@ slab_alloc_node(struct kmem_cache *cachep, gfp_t flags, int nodeid,
    > local_irq_restore(save_flags);
    > ptr = cache_alloc_debugcheck_after(cachep, flags, ptr, caller);
    >
    > - if (unlikely(flags & __GFP_ZERO) && ptr)
    > - memset(ptr, 0, cachep->object_size);
    > + if (unlikely(flags & __GFP_ZERO) && ptr) {
    > + if (!WARN_ON_ONCE(cachep->ctor))
    > + memset(ptr, 0, cachep->object_size);
    > + }
    >
    > slab_post_alloc_hook(cachep, flags, 1, &ptr);
    > return ptr;
    > diff --git a/mm/slob.c b/mm/slob.c
    > index 1a46181b675c..958173fd7c24 100644
    > --- a/mm/slob.c
    > +++ b/mm/slob.c
    > @@ -556,8 +556,10 @@ static void *slob_alloc_node(struct kmem_cache *c, gfp_t flags, int node)
    > flags, node);
    > }
    >
    > - if (b && c->ctor)
    > + if (b && c->ctor) {
    > + WARN_ON_ONCE(flags & __GFP_ZERO);
    > c->ctor(b);
    > + }
    >
    > kmemleak_alloc_recursive(b, c->size, 1, c->flags, flags);
    > return b;
    > diff --git a/mm/slub.c b/mm/slub.c
    > index 9e1100f9298f..0f55f0a0dcaa 100644
    > --- a/mm/slub.c
    > +++ b/mm/slub.c
    > @@ -2714,8 +2714,10 @@ static __always_inline void *slab_alloc_node(struct kmem_cache *s,
    > stat(s, ALLOC_FASTPATH);
    > }
    >
    > - if (unlikely(gfpflags & __GFP_ZERO) && object)
    > - memset(object, 0, s->object_size);
    > + if (unlikely(gfpflags & __GFP_ZERO) && object) {
    > + if (!WARN_ON_ONCE(s->ctor))
    > + memset(object, 0, s->object_size);
    > + }
    >
    > slab_post_alloc_hook(s, gfpflags, 1, &object);
    >
    >

    \
     
     \ /
      Last update: 2018-04-10 15:41    [W:4.316 / U:0.160 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site