Re: [PATCH v2 2/2] slab: __GFP_ZERO is incompatible with a constructor
From: Michal Hocko
Date: Wed Apr 11 2018 - 02:38:46 EST
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,
> invalid_mask, &invalid_mask, flags, &flags);
> dump_stack();
> }
> + BUG_ON(cachep->ctor && (flags & __GFP_ZERO));
NAK. We really do not want to blow the whole kernel just because
somebody is doing something stupid. Make it WARN_ON_ONCE and fix up the
flag.
> +static inline bool slab_no_ctor(struct kmem_cache *s)
> +{
> + if (IS_ENABLED(CONFIG_DEBUG_VM))
> + return !WARN_ON_ONCE(s->ctor);
> + return true;
> +}
I do realize that you want to keep the hotpath without additional checks
but if for nothing else this is a really bad misnomer.
debug_slab_no_ctor()? I can clearly see how somebody uses this blindly
for a different purpose.
[...]
> diff --git a/mm/slub.c b/mm/slub.c
> index a28488643603..9f8f38a552e5 100644
> --- a/mm/slub.c
> +++ b/mm/slub.c
> @@ -1576,6 +1576,7 @@ static struct page *allocate_slab(struct kmem_cache *s, gfp_t flags, int node)
>
> if (gfpflags_allow_blocking(flags))
> local_irq_enable();
> + BUG_ON(s->ctor && (flags & __GFP_ZERO));
No no on this as well.
Othe than that. Once those are fixed, feel free to add
Acked-by: Michal Hocko <mhocko@xxxxxxxx>
--
Michal Hocko
SUSE Labs