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

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

[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)'

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

Re: [bug report] mm, compaction: round-robin the order while searching the free lists for a target

[Mel Gorman]

On Wed, Jan 09, 2019 at 11:27:33AM +0300, Dan Carpenter wrote:
> 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.

diff --git a/mm/compaction.c b/mm/compaction.c
index cc17f0c01811..a3b665e15ab2 100644
--- a/mm/compaction.c
+++ b/mm/compaction.c
@@ -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.
diff --git a/mm/internal.h b/mm/internal.h
index 6b1e5e313855..bebfb4b655dd 100644
--- a/mm/internal.h
+++ b/mm/internal.h
@@ -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 */

Re: [bug report] mm, compaction: round-robin the order while searching the free lists for a target

[Dan Carpenter]

On Wed, Jan 09, 2019 at 10:25:46AM +0000, Mel Gorman wrote:
> On Wed, Jan 09, 2019 at 11:27:33AM +0300, Dan Carpenter wrote:
> > 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.
> 

Yes.  Thanks.

regards,
dan carpenter

Re: [bug report] mm, compaction: round-robin the order while searching the free lists for a target

[Mel Gorman]

On Wed, Jan 09, 2019 at 10:25:46AM +0000, Mel Gorman wrote:
> On Wed, Jan 09, 2019 at 11:27:33AM +0300, Dan Carpenter wrote:
> > 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.
> 

Sending a version that was actually committed might help.

diff --git a/mm/compaction.c b/mm/compaction.c
index 6720234dc701..399dea80d09b 100644
--- a/mm/compaction.c
+++ b/mm/compaction.c
@@ -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.
diff --git a/mm/internal.h b/mm/internal.h
index 6b1e5e313855..bebfb4b655dd 100644
--- a/mm/internal.h
+++ b/mm/internal.h
@@ -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 */

-- 
Mel Gorman
SUSE Labs