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 <mho...@suse.com>
-- 
Michal Hocko
SUSE Labs

Reply via email to