On Wed, 01 Feb 2012 16:17:10 -0500
Rik van Riel <riel@xxxxxxxxxx> wrote:
> >> @@ -35,7 +35,7 @@ struct compact_control {
> >> unsigned long migrate_pfn; /* isolate_migratepages search base */
> >> bool sync; /* Synchronous migration */
> >>
> >> - unsigned int order; /* order a direct compactor needs */
> >> + int order; /* order a direct compactor needs */
> >> int migratetype; /* MOVABLE, RECLAIMABLE etc */
> >> struct zone *zone;
> >> };
> >
> > One would expect this to significantly change the behaviour of
> > /proc/sys/vm/compact_memory. Enfeebled minds want to know: is
> > the new behaviour better or worse than the old behaviour?
>
> The old behaviour and the behaviour post Dan's fix are the
> same.
>
> My patch temporarily broke things, by testing for order < 0,
> instead of the explicit cc->order == -1 used elsewhere in
> the code.
>
> I did not notice it in my own testing because I tested on
> 3.2.0 and sent you patches against 3.3-current. It looks
> like this line of code is the one difference between both
> trees I was working on :(
>
> In my test tree, I had (cc->sync || !compaction_deferred(zone, cc->order)).
>
> Arguably, testing for cc->order == -1 (or cc->order < 0) is
> better anyway.
I suppose it would be nicer to make the code in __compact_pgdat() match
all the other places whcih do this:
--- a/mm/compaction.c~mm-compaction-make-compact_control-order-signed-fix
+++ a/mm/compaction.c
@@ -686,7 +686,7 @@ static int __compact_pgdat(pg_data_t *pg
INIT_LIST_HEAD(&cc->freepages);
INIT_LIST_HEAD(&cc->migratepages);
- if (cc->order < 0 || !compaction_deferred(zone, cc->order))
+ if (cc->order == -1 || !compaction_deferred(zone, cc->order))
compact_zone(zone, cc);
if (cc->order > 0) {
_
--
To unsubscribe from this list: send the line "unsubscribe kernel-janitors" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
