Re: [PATCH v2 2/2] slab: __GFP_ZERO is incompatible with a constructor

On Thu, 12 Apr 2018, Matthew Wilcox wrote: > > Thus the next invocation of the fastpath will find that c->freelist is > > NULL and go to the slowpath. ... > > _ah_. I hadn't figured out that c->page was always NULL in the debugging > case too, so ___slab_alloc() always hits the 'new_slab' case.

Re: [PATCH v2 2/2] slab: __GFP_ZERO is incompatible with a constructor

On Thu, 12 Apr 2018, Matthew Wilcox wrote: > > Thus the next invocation of the fastpath will find that c->freelist is > > NULL and go to the slowpath. ... > > _ah_. I hadn't figured out that c->page was always NULL in the debugging > case too, so ___slab_alloc() always hits the 'new_slab' case.

Re: [PATCH v2 2/2] slab: __GFP_ZERO is incompatible with a constructor

On Thu, Apr 12, 2018 at 09:10:23AM -0500, Christopher Lameter wrote: > On Wed, 11 Apr 2018, Matthew Wilcox wrote: > > I don't see how that works ... can you explain a little more? > > c->freelist is NULL and thus ___slab_alloc (slowpath) is called. > ___slab_alloc populates c->freelist and gets

Re: [PATCH v2 2/2] slab: __GFP_ZERO is incompatible with a constructor

On Thu, Apr 12, 2018 at 09:10:23AM -0500, Christopher Lameter wrote: > On Wed, 11 Apr 2018, Matthew Wilcox wrote: > > I don't see how that works ... can you explain a little more? > > c->freelist is NULL and thus ___slab_alloc (slowpath) is called. > ___slab_alloc populates c->freelist and gets

Re: [PATCH v2 2/2] slab: __GFP_ZERO is incompatible with a constructor

On Wed, 11 Apr 2018, Matthew Wilcox wrote: > > I don't see how that works ... can you explain a little more? > > I see ___slab_alloc() is called from __slab_alloc(). And I see > slab_alloc_node does this: > > object = c->freelist; > page = c->page; > if (unlikely(!object

Re: [PATCH v2 2/2] slab: __GFP_ZERO is incompatible with a constructor

On Wed, 11 Apr 2018, Matthew Wilcox wrote: > > I don't see how that works ... can you explain a little more? > > I see ___slab_alloc() is called from __slab_alloc(). And I see > slab_alloc_node does this: > > object = c->freelist; > page = c->page; > if (unlikely(!object

Re: [PATCH v2 2/2] slab: __GFP_ZERO is incompatible with a constructor

On Wed, Apr 11, 2018 at 04:11:17PM -0500, Christopher Lameter wrote: > On Wed, 11 Apr 2018, Matthew Wilcox wrote: > > > Please put this in a code path that is enabled by specifying > > > > > > slub_debug > > > > > > on the kernel command line. > > > > I don't understand. First, I had: > > > >

Re: [PATCH v2 2/2] slab: __GFP_ZERO is incompatible with a constructor

On Wed, Apr 11, 2018 at 04:11:17PM -0500, Christopher Lameter wrote: > On Wed, 11 Apr 2018, Matthew Wilcox wrote: > > > Please put this in a code path that is enabled by specifying > > > > > > slub_debug > > > > > > on the kernel command line. > > > > I don't understand. First, I had: > > > >

Re: [PATCH v2 2/2] slab: __GFP_ZERO is incompatible with a constructor

On Wed, 11 Apr 2018, Matthew Wilcox wrote: > > > slab_post_alloc_hook(s, gfpflags, 1, ); > > > > Please put this in a code path that is enabled by specifying > > > > slub_debug > > > > on the kernel command line. > > I don't understand. First, I had: > > if (unlikely(gfpflags &

Re: [PATCH v2 2/2] slab: __GFP_ZERO is incompatible with a constructor

On Wed, 11 Apr 2018, Matthew Wilcox wrote: > > > slab_post_alloc_hook(s, gfpflags, 1, ); > > > > Please put this in a code path that is enabled by specifying > > > > slub_debug > > > > on the kernel command line. > > I don't understand. First, I had: > > if (unlikely(gfpflags &

Re: [PATCH v2 2/2] slab: __GFP_ZERO is incompatible with a constructor

On Wed, Apr 11, 2018 at 08:44:23AM -0500, Christopher Lameter wrote: > > +++ b/mm/slub.c > > @@ -2725,7 +2726,7 @@ static __always_inline void *slab_alloc_node(struct > > kmem_cache *s, > > stat(s, ALLOC_FASTPATH); > > } > > > > - if (unlikely(gfpflags & __GFP_ZERO) && object) >

Re: [PATCH v2 2/2] slab: __GFP_ZERO is incompatible with a constructor

On Wed, Apr 11, 2018 at 08:44:23AM -0500, Christopher Lameter wrote: > > +++ b/mm/slub.c > > @@ -2725,7 +2726,7 @@ static __always_inline void *slab_alloc_node(struct > > kmem_cache *s, > > stat(s, ALLOC_FASTPATH); > > } > > > > - if (unlikely(gfpflags & __GFP_ZERO) && object) >

Re: [PATCH v2 2/2] slab: __GFP_ZERO is incompatible with a constructor

On Tue, 10 Apr 2018, Matthew Wilcox wrote: > diff --git a/mm/slab.h b/mm/slab.h > index 3cd4677953c6..896818c7b30a 100644 > --- a/mm/slab.h > +++ b/mm/slab.h > @@ -515,6 +515,13 @@ static inline void dump_unreclaimable_slab(void) > > void ___cache_free(struct kmem_cache *cache, void *x, unsigned

Re: [PATCH v2 2/2] slab: __GFP_ZERO is incompatible with a constructor

On Tue, 10 Apr 2018, Matthew Wilcox wrote: > diff --git a/mm/slab.h b/mm/slab.h > index 3cd4677953c6..896818c7b30a 100644 > --- a/mm/slab.h > +++ b/mm/slab.h > @@ -515,6 +515,13 @@ static inline void dump_unreclaimable_slab(void) > > void ___cache_free(struct kmem_cache *cache, void *x, unsigned

Re: [PATCH v2 2/2] slab: __GFP_ZERO is incompatible with a constructor

On Tue 10-04-18 23:03:20, Matthew Wilcox wrote: > diff --git a/mm/slab.c b/mm/slab.c > index 58c8cecc26ab..9ad85fd9fca8 100644 > --- a/mm/slab.c > +++ b/mm/slab.c > @@ -2661,6 +2661,7 @@ static struct page *cache_grow_begin(struct kmem_cache > *cachep, >

Re: [PATCH v2 2/2] slab: __GFP_ZERO is incompatible with a constructor

On Tue 10-04-18 23:03:20, Matthew Wilcox wrote: > diff --git a/mm/slab.c b/mm/slab.c > index 58c8cecc26ab..9ad85fd9fca8 100644 > --- a/mm/slab.c > +++ b/mm/slab.c > @@ -2661,6 +2661,7 @@ static struct page *cache_grow_begin(struct kmem_cache > *cachep, >

[PATCH v2 2/2] slab: __GFP_ZERO is incompatible with a constructor

From: Matthew Wilcox __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

[PATCH v2 2/2] slab: __GFP_ZERO is incompatible with a constructor

From: Matthew Wilcox __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