Re: [PATCH] Bias the location of pages freed for min_free_kbytes inthe same MAX_ORDER_NR_PAGES blocks

[Mel Gorman]

On Sun, 18 Mar 2007, Andrew Morton wrote:

> On Sat, 17 Mar 2007 18:26:41 +0000 mel@xxxxxxxxx (Mel Gorman) wrote:
> >
>
> I still haven't got onto reviewing all your mm patches :(

There are a lot them admittadly.

> Nor has anyone else, afaik.

They have been reviewed at various points in their development and most of 
the feedback was fed back in. Marcelo Tosatti commented on version 19 for 
example. Joel Schopp commented heavily on earlier versions around the 
v15-v19 mark as well as Dave Hansen around the same time. Christoph 
Lameter has commented on recent versions but I'm not sure how detailed his 
review was of if the comments were based on the patch description. There 
have been comments from various other people as well, mainly around the 
v19-v20 mark. Andy Whitcroft has reviewed most versions of the patches 
including the most recent ones.

I suspect there may not have been detailed review recently because so many 
versions have been released.

> But let me leap ahead of myself.
>
> > CONFIG_PAGE_GROUP_BY_MOBILITY
>
> Why does this config item exist?  It's not good to have some mysterious
> knob which affects mm behaviour at compile time.  We need to make up our
> minds and stick with it.

The configuration item exists because there were concerns over the memory 
footprint and cache line footprint. It was introduced to address that 
concern and also so that it would be possible to compare the performance 
behavior of anti-fragmentation. Your comment rang a bell though so I 
searched the archives to see this comment from Andi Kleen;

===
If anything this should be a boot time option or perhaps sysctl, not a 
config. In general CONFIGs that change runtime behaviour are evil - just 
makes changing the option more painful, causes problems for distribution 
users, doesn't make much sense, etc.etc.

Also #ifdef as a documentation device is a really really scary concept.
Yuck.
===

A sysctl would avoid any cache line footprint but not the memory overhead 
because the freelists in struct zone as those freelists would still exist. 
I could make the option depend on CONFIG_EMBEDDED for the zone overhead. 
Would that make sense or would it be preferable to ditch the option 
altogether?

I'll start looking at doing a sysctl so it can be disabled at runtime if 
necessary. I strongly suspect that it cannot be enabled again once 
disabled but I don't see that as a problem as such.

--
Mel Gorman
Part-time Phd Student                          Linux Technology Center
University of Limerick                         IBM Dublin Software Lab
-
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/