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

From: Christopher Lameter <cl@linux.com>
To: Matthew Wilcox <willy@infradead.org>
Cc: linux-mm@kvack.org, Matthew Wilcox <mawilcox@microsoft.com>,
	Pekka Enberg <penberg@kernel.org>,
	David Rientjes <rientjes@google.com>,
	Joonsoo Kim <iamjoonsoo.kim@lge.com>,
	Andrew Morton <akpm@linux-foundation.org>,
	linux-kernel@vger.kernel.org, Jan Kara <jack@suse.cz>,
	Jeff Layton <jlayton@redhat.com>,
	Mel Gorman <mgorman@techsingularity.net>
Subject: Re: [PATCH v2 2/2] slab: __GFP_ZERO is incompatible with a constructor
Date: Wed, 11 Apr 2018 16:11:17 -0500 (CDT)	[thread overview]
Message-ID: <alpine.DEB.2.20.1804111601090.7458@nuc-kabylake> (raw)
In-Reply-To: <20180411192448.GD22494@bombadil.infradead.org>

On Wed, 11 Apr 2018, Matthew Wilcox wrote:

> > >  	slab_post_alloc_hook(s, gfpflags, 1, &object);
> >
> > 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 & __GFP_ZERO) && object && !WARN_ON_ONCE(s->ctor))
>
> and you didn't like that because it was putting checking into a (semi)fast
> path.  Now you want me to add a check for slub_debug somewhere?  I dont
> see an existing one I can leverage that will hit on every allocation.
> Perhaps I'm missing something.

The WARN_ON is only enabled when you configure and build the kernel with
debugging enabled (CONFIG_VM_DEBUG). That is a compile time debugging
feature like supported by SLAB.

SLUB debugging is different because we had problems isolating memory
corruption bugs in the production kernels for years. The debug code is
always included in the build but kept out of the hotpaths.

The debug can be enabled when needed to find memory corruption errors
without the need to rebuild a kernel for a prod environment (which may
change race conditions etc) because we only then need to add a kernel
parameter.

"slub_debug" enables kmem_cache->flags & SLAB_DEBUG and that forces all
fastpath processing to be disabled. Thus you can check reliably in the
slow path only for the GFP_ZERO problem.

Add the check to the other debug stuff already there. F.e. in
alloc_debug_processing() or after

if (kmem_cache_debug(s) ...

in ____slab_alloc()