Re: [patch] mm: compaction: make compact_control order signed

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 02/01/2012 04:24 PM, Andrew Morton wrote:
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) {

Agreed, with that and Dan's patch, things should all be
as expected.

--
All rights reversed

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@xxxxxxxxx.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/
Don't email: <a href=mailto:"dont@xxxxxxxxx";> email@xxxxxxxxx </a>


[Index of Archives]     [Linux ARM Kernel]     [Linux ARM]     [Linux Omap]     [Fedora ARM]     [IETF Annouce]     [Bugtraq]     [Linux]     [Linux OMAP]     [Linux MIPS]     [ECOS]     [Asterisk Internet PBX]     [Linux API]