Re: [PATCH 00/34] mm: Page Replacement Policy Framework

[Nick Piggin <nickpiggin@yahoo.com.au>]

Andrew Morton wrote:
> Peter Zijlstra <a.p.zijlstra@chello.nl> wrote:
>
> > This patch-set introduces a page replacement policy framework and 4 new 
> > experimental policies.
>
>
> Holy cow.
>
>
> > The page replacement algorithm determines which pages to swap out.
> > The current algorithm has some problems that are increasingly noticable, even
> > on desktop workloads.
>
>
> Rather than replacing the whole lot four times I'd really prefer to see
> precise descriptions of these problems, see if we can improve the situation
> incrementally rather than wholesale slash-n-burn...

The other thing is that a lot of the "policy" stuff you've abstracted
out is actually low-level "mechanism" stuff that has implications beyond
page reclaim. Taking a refcount on lru pages, for example.

Also, as you work and find incremental improvements to the current code,
you should be submitting them (eg. patch 25, or patch 1) rather than
sitting on them and sending them in a huge patchset where they don't
really belong.

Some of the API names aren't very nice either. It's great that you want
to keep the namespace consistent, but it shouldn't be at the expense of
more descriptive names, and having the page_replace_ prefix itself makes
many functions read like crap. I'd suggest something like a pgrep_
prefix and try to make the rest of the name make sense.

Aside from all that, I'm with Andrew in that problems need to be
identified first and foremost. But also I don't like the chances of this
whole framework flying at all -- Linus vetoed a similar framework for
sched.c that was actually a reasonable API, with little or no
consequences outside sched.c. With good reason.

Nice work, though :)

--
SUSE Labs, Novell Inc.
Send instant messages to your online friends http://au.messenger.yahoo.com 
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/