Re: [RFC PATCH 1/5] mm/page_ext: resurrect struct page extending code for debugging

[Joonsoo Kim]

On Thu, Nov 13, 2014 at 12:40:35PM -0800, Andrew Morton wrote:
> On Thu, 13 Nov 2014 15:40:35 +0900 Joonsoo Kim <iamjoonsoo.kim@xxxxxxx> wrote:
> 
> > On Wed, Nov 12, 2014 at 08:33:40AM -0800, Dave Hansen wrote:
> > > On 11/12/2014 12:27 AM, Joonsoo Kim wrote:
> > > > @@ -1092,6 +1096,14 @@ struct mem_section {
> > > >  
> > > >  	/* See declaration of similar field in struct zone */
> > > >  	unsigned long *pageblock_flags;
> > > > +#ifdef CONFIG_PAGE_EXTENSION
> > > > +	/*
> > > > +	 * If !SPARSEMEM, pgdat doesn't have page_ext pointer. We use
> > > > +	 * section. (see page_ext.h about this.)
> > > > +	 */
> > > > +	struct page_ext *page_ext;
> > > > +	unsigned long pad;
> > > > +#endif
> > > 
> > > Will the distributions be amenable to enabling this?  If so, I'm all for
> > > it if it gets us things like page_owner at runtime.
> > 
> > Yes, I hope so.
> > At least, I can make it default to our product. But, how distributions
> > will do is beyond my power. :)
> > 
> 
> >From my reading of the code, the overhead is very low if nobody is
> using it.  In which case things should be OK and we can perhaps do away
> with CONFIG_PAGE_EXTENSION altogether.

Yeap!

> 
> But my reading of the code may be wrong.  It is very poorly documented.
> As far as I can tell, invoke_need_callbacks() works out whether there
> are any clients of this feature and if not, we avoid allocating that
> huge chunk of memory.
> 

Your understanding is correct.

> And the way we register clients is to enter a pointer into the global
> page_ext_ops[].  So this requires a kernel rebuild anyway, so there's
> no point in distros enabling CONFIG_PAGE_EXTENSION.  The way to do this
> is for CONFIG_PAGE_OWNER (for example) to `select'
> CONFIG_PAGE_EXTENSION.
> 

Yes. Without any client, CONFIG_PAGE_EXTENSION has no point to be
enabled. So, each client, DEBUG_PAGEALLOC, PAGE_OWNER has 'select
CONFIG_PAGE_EXTENSION' in it's Kconfig, repectively. (Patch 2, 5)

> It's unclear to me why invoke_need_callbacks() walks the page_ext_ops[]
> entries, inspecting them.  Perhaps this is so that clients of
> CONFIG_PAGE_EXTENSION can be enabled/disabled at boot time, dunno.

Yes, if user decides not to use certain debugging feature at boot time,
it's very wasting to have that huge memory chunk for page extension.
invoke_need_callbacks() is introduced to determine and reduce this
overhead in boot time.

> Please, can we get all this design and behaviour appropriately
> documented in the code and changelogs?

Yes! 
With pleasure, I will do it. :)

Thanks.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/