Messages in this thread | | | Date | Thu, 23 Mar 2006 13:21:08 +1100 | From | Nick Piggin <> | Subject | Re: [PATCH 00/34] mm: Page Replacement Policy Framework |
| |
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/
| |