Re: [PATCH 2/5] cpuset memory spread page cache implementation and hooks

[Andrew Morton <akpm@osdl.org>]

Paul Jackson <pj@sgi.com> wrote:
>
> Andrew wrote:
> > That's a no-op.
> 
> agreed.
> 
> > The problem remains that for CONFIG_NUMA=y, this function is too big to inline.
> 
> A clear statement of the problem.  Good.
> 
> But I'm still being a stupid git.  Is the following variant of
> page_cache_alloc_cold() still bigger than you would prefer inlined
> (where cpuset_mem_spread_check() is an inline current->flags test)
> (ditto for page_cache_alloc())?
> 
> ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
> static struct page *page_cache_alloc_mem_spread_cold(struct address_space *x)
> {
> 	int n = cpuset_mem_spread_node();
> 	return alloc_pages_node(n, mapping_gfp_mask(x)|__GFP_COLD, 0);
> }

That's an almost-equivalent transformation.  If the compiler's good enough,
it'll generate the same code here I think.

If so then there's probably not much point in optimising it - but one needs
to look at the numbers.

> static inline struct page *page_cache_alloc_cold(struct address_space *x)
> {
> 	if (cpuset_mem_spread_check())
> 		return page_cache_alloc_mem_spread_cold(x);
> 	return alloc_pages(mapping_gfp_mask(x)|__GFP_COLD, 0);
> }
> ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
> 
> Are you recommending taking the whole thing, both page_cache_alloc*()
> calls, for the CONFIG_NUMA case, out of line, instead of even the above?

I'm saying "gee, that looks big.  Do you have time to investigate possible
improvements?"   They may come to naught.

> If so, fine ... then the rest of your explanations make sense to
> me on how to go about coding this, and I'll try coding it up.

Neato.  Please also have a think about __cache_alloc(), see if we can
improve it further - that's a real hotspot.