Skip to contentSkip to conversation

mm, compaction: round-robin the order while searching the free lists for a target

bug report Dan Carpenter 2 participants 4 messages Jan 9, 2019

Subsystems (3)
Memory management MM core Page allocator
lore
Share & tools
lore

Dan Carpenter

author
#0 lore raw

Hello Mel Gorman,

The patch 1688e2896de4: "mm, compaction: round-robin the order while searching the free lists for a target" from Jan 8, 2019, leads to the following static checker warning:

	mm/compaction.c:1252 next_search_order()
	warn: impossible condition '(cc->search_order < 0) => (0-u16max < 0)'

mm/compaction.c

    1243 static int next_search_order(struct compact_control *cc, int order)
    1244 {
    1245 	order--;
    1246 	if (order < 0)
    1247 		order = cc->order - 1;
    1248 
    1249 	/* Search wrapped around? */
    1250 	if (order == cc->search_order) {
    1251 		cc->search_order--;
--> 1252 		if (cc->search_order < 0)
                            ^^^^^^^^^^^^^^^^^^^^

u16 can't be negative.

    1253 			cc->search_order = cc->order - 1;
    1254 		return -1;
    1255 	}
    1256 
    1257 	return order;
    1258 }

regards,
dan carpenter

#1 lore raw
Quoted 9 lines · Dan Carpenter

Hello Mel Gorman,

The patch 1688e2896de4: "mm, compaction: round-robin the order while searching the free lists for a target" from Jan 8, 2019, leads to the following static checker warning:

	mm/compaction.c:1252 next_search_order()
	warn: impossible condition '(cc->search_order < 0) => (0-u16max < 0)'

Thanks Dan!

Does the following combination of two patches address it? The two patches address separate problems with two patches in the series.

mm/compaction.c +4 -0
index cc17f0c01811..a3b665e15ab2 100644
@@ -1269,6 +1269,10 @@ fast_isolate_freepages(struct compact_control *cc)
 	bool scan_start = false;
 	int order;
 
+	/* Full compaction passes in a negative order */
+	if (order <= 0)
+		return cc->free_pfn;
+
 	/*
 	 * If starting the scan, use a deeper search and use the highest
 	 * PFN found if a suitable one is not found.
mm/internal.h +1 -1
index 6b1e5e313855..bebfb4b655dd 100644
@@ -192,7 +192,7 @@ struct compact_control {
 	unsigned long total_migrate_scanned;
 	unsigned long total_free_scanned;
 	unsigned short fast_search_fail;/* failures to use free list searches */
-	unsigned short search_order;	/* order to start a fast search at */
+	short search_order;		/* order to start a fast search at */
 	const gfp_t gfp_mask;		/* gfp mask of a direct compactor */
 	int order;			/* order a direct compactor needs */
 	int migratetype;		/* migratetype of direct compactor */
author
#2 lore raw
Quoted 7 lines · Mel Gorman
Quoted 6 lines · Dan Carpenter

Hello Mel Gorman,

The patch 1688e2896de4: "mm, compaction: round-robin the order while searching the free lists for a target" from Jan 8, 2019, leads to the following static checker warning:

Quoted 9 lines
Quoted 3 lines
	mm/compaction.c:1252 next_search_order()
	warn: impossible condition '(cc->search_order < 0) => (0-u16max < 0)'

Thanks Dan!

Does the following combination of two patches address it? The two patches address separate problems with two patches in the series.

Yes. Thanks.

regards,
dan carpenter

#3 lore raw
In reply to Mel Gorman
Quoted 7 lines · Mel Gorman
Quoted 6 lines · Dan Carpenter

Hello Mel Gorman,

The patch 1688e2896de4: "mm, compaction: round-robin the order while searching the free lists for a target" from Jan 8, 2019, leads to the following static checker warning:

Quoted 9 lines
Quoted 3 lines
	mm/compaction.c:1252 next_search_order()
	warn: impossible condition '(cc->search_order < 0) => (0-u16max < 0)'

Thanks Dan!

Does the following combination of two patches address it? The two patches address separate problems with two patches in the series.

Sending a version that was actually committed might help.

mm/compaction.c +4 -0
index 6720234dc701..399dea80d09b 100644
@@ -1269,6 +1269,10 @@ fast_isolate_freepages(struct compact_control *cc)
 	bool scan_start = false;
 	int order;
 
+	/* Full compaction passes in a negative order */
+	if (cc->order <= 0)
+		return cc->free_pfn;
+
 	/*
 	 * If starting the scan, use a deeper search and use the highest
 	 * PFN found if a suitable one is not found.
mm/internal.h +1 -1
index 6b1e5e313855..bebfb4b655dd 100644
@@ -192,7 +192,7 @@ struct compact_control {
 	unsigned long total_migrate_scanned;
 	unsigned long total_free_scanned;
 	unsigned short fast_search_fail;/* failures to use free list searches */
-	unsigned short search_order;	/* order to start a fast search at */
+	short search_order;		/* order to start a fast search at */
 	const gfp_t gfp_mask;		/* gfp mask of a direct compactor */
 	int order;			/* order a direct compactor needs */
 	int migratetype;		/* migratetype of direct compactor */
Signature · 2 lines
Mel Gorman
SUSE Labs