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

From: Andrew Morton
Date: Sun Feb 05 2006 - 01:14:57 EST


Paul Jackson <pj@xxxxxxx> 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.
-
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/