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

Mel Gorman
2007-03-18T11:35:41Z

On Sun, 18 Mar 2007, Andrew Morton wrote:

> On Sat, 17 Mar 2007 18:26:41 +0000 mel@skynet.ie (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