All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 00/12] A few cleanup and fixup patches for compaction
@ 2022-04-18 14:12 Miaohe Lin
  2022-04-18 14:12 ` [PATCH 01/12] mm: compaction: remove unneeded return value of kcompactd_run Miaohe Lin
                   ` (11 more replies)
  0 siblings, 12 replies; 17+ messages in thread
From: Miaohe Lin @ 2022-04-18 14:12 UTC (permalink / raw)
  To: akpm; +Cc: vbabka, pintu, charante, linux-mm, linux-kernel, linmiaohe

Hi everyone,
This series contains a few patches to clean up some obsolete comment,
remove unneeded return value and so on. Also we fix the possible NULL
pointer dereference. More details can be found in the respective
changelogs. Thanks!

Miaohe Lin (12):
  mm: compaction: remove unneeded return value of kcompactd_run
  mm: compaction: remove unneeded pfn update
  mm: compaction: remove unneeded assignment to isolate_start_pfn
  mm: compaction: clean up comment for sched contention
  mm: compaction: clean up comment about suitable migration target
    recheck
  mm: compaction: use COMPACT_CLUSTER_MAX in compaction.c
  mm: compaction: use helper compound_nr in isolate_migratepages_block
  mm: compaction: clean up comment about async compaction in
    isolate_migratepages
  mm: compaction: avoid possible NULL pointer dereference in
    kcompactd_cpu_online
  mm: compaction: make compaction_zonelist_suitable return false when
    COMPACT_SUCCESS
  mm: compaction: simplify the code in __compact_finished
  mm: compaction: make sure highest is above the min_pfn

 include/linux/compaction.h |  5 +--
 mm/compaction.c            | 87 ++++++++++++++------------------------
 mm/internal.h              |  2 +-
 3 files changed, 34 insertions(+), 60 deletions(-)

-- 
2.23.0


^ permalink raw reply	[flat|nested] 17+ messages in thread

* [PATCH 01/12] mm: compaction: remove unneeded return value of kcompactd_run
  2022-04-18 14:12 [PATCH 00/12] A few cleanup and fixup patches for compaction Miaohe Lin
@ 2022-04-18 14:12 ` Miaohe Lin
  2022-04-18 14:12 ` [PATCH 02/12] mm: compaction: remove unneeded pfn update Miaohe Lin
                   ` (10 subsequent siblings)
  11 siblings, 0 replies; 17+ messages in thread
From: Miaohe Lin @ 2022-04-18 14:12 UTC (permalink / raw)
  To: akpm; +Cc: vbabka, pintu, charante, linux-mm, linux-kernel, linmiaohe

The return value of kcompactd_run() is unused now.  Clean it up.

Signed-off-by: Miaohe Lin <linmiaohe@huawei.com>
---
 include/linux/compaction.h | 5 ++---
 mm/compaction.c            | 7 ++-----
 2 files changed, 4 insertions(+), 8 deletions(-)

diff --git a/include/linux/compaction.h b/include/linux/compaction.h
index 34bce35c808d..52a9ff65faee 100644
--- a/include/linux/compaction.h
+++ b/include/linux/compaction.h
@@ -177,7 +177,7 @@ static inline bool compaction_withdrawn(enum compact_result result)
 bool compaction_zonelist_suitable(struct alloc_context *ac, int order,
 					int alloc_flags);
 
-extern int kcompactd_run(int nid);
+extern void kcompactd_run(int nid);
 extern void kcompactd_stop(int nid);
 extern void wakeup_kcompactd(pg_data_t *pgdat, int order, int highest_zoneidx);
 
@@ -212,9 +212,8 @@ static inline bool compaction_withdrawn(enum compact_result result)
 	return true;
 }
 
-static inline int kcompactd_run(int nid)
+static inline void kcompactd_run(int nid)
 {
-	return 0;
 }
 static inline void kcompactd_stop(int nid)
 {
diff --git a/mm/compaction.c b/mm/compaction.c
index baf654b5e073..2e838992b324 100644
--- a/mm/compaction.c
+++ b/mm/compaction.c
@@ -3016,21 +3016,18 @@ static int kcompactd(void *p)
  * This kcompactd start function will be called by init and node-hot-add.
  * On node-hot-add, kcompactd will moved to proper cpus if cpus are hot-added.
  */
-int kcompactd_run(int nid)
+void kcompactd_run(int nid)
 {
 	pg_data_t *pgdat = NODE_DATA(nid);
-	int ret = 0;
 
 	if (pgdat->kcompactd)
-		return 0;
+		return;
 
 	pgdat->kcompactd = kthread_run(kcompactd, pgdat, "kcompactd%d", nid);
 	if (IS_ERR(pgdat->kcompactd)) {
 		pr_err("Failed to start kcompactd on node %d\n", nid);
-		ret = PTR_ERR(pgdat->kcompactd);
 		pgdat->kcompactd = NULL;
 	}
-	return ret;
 }
 
 /*
-- 
2.23.0


^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCH 02/12] mm: compaction: remove unneeded pfn update
  2022-04-18 14:12 [PATCH 00/12] A few cleanup and fixup patches for compaction Miaohe Lin
  2022-04-18 14:12 ` [PATCH 01/12] mm: compaction: remove unneeded return value of kcompactd_run Miaohe Lin
@ 2022-04-18 14:12 ` Miaohe Lin
  2022-04-18 14:12 ` [PATCH 03/12] mm: compaction: remove unneeded assignment to isolate_start_pfn Miaohe Lin
                   ` (9 subsequent siblings)
  11 siblings, 0 replies; 17+ messages in thread
From: Miaohe Lin @ 2022-04-18 14:12 UTC (permalink / raw)
  To: akpm; +Cc: vbabka, pintu, charante, linux-mm, linux-kernel, linmiaohe

pfn is unused in this do while loop. Remove the unneeded pfn update.

Signed-off-by: Miaohe Lin <linmiaohe@huawei.com>
---
 mm/compaction.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/mm/compaction.c b/mm/compaction.c
index 2e838992b324..f9e628a5306a 100644
--- a/mm/compaction.c
+++ b/mm/compaction.c
@@ -317,7 +317,6 @@ __reset_isolation_pfn(struct zone *zone, unsigned long pfn, bool check_source,
 		}
 
 		page += (1 << PAGE_ALLOC_COSTLY_ORDER);
-		pfn += (1 << PAGE_ALLOC_COSTLY_ORDER);
 	} while (page <= end_page);
 
 	return false;
-- 
2.23.0


^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCH 03/12] mm: compaction: remove unneeded assignment to isolate_start_pfn
  2022-04-18 14:12 [PATCH 00/12] A few cleanup and fixup patches for compaction Miaohe Lin
  2022-04-18 14:12 ` [PATCH 01/12] mm: compaction: remove unneeded return value of kcompactd_run Miaohe Lin
  2022-04-18 14:12 ` [PATCH 02/12] mm: compaction: remove unneeded pfn update Miaohe Lin
@ 2022-04-18 14:12 ` Miaohe Lin
  2022-04-18 14:12 ` [PATCH 04/12] mm: compaction: clean up comment for sched contention Miaohe Lin
                   ` (8 subsequent siblings)
  11 siblings, 0 replies; 17+ messages in thread
From: Miaohe Lin @ 2022-04-18 14:12 UTC (permalink / raw)
  To: akpm; +Cc: vbabka, pintu, charante, linux-mm, linux-kernel, linmiaohe

isolate_start_pfn is unused when cc->nr_freepages ! = 0. Otherwise
cc->free_pfn will overwrite it unconditionally. So we should remove
this unneeded and somewhat misleading assignment.

Signed-off-by: Miaohe Lin <linmiaohe@huawei.com>
---
 mm/compaction.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/mm/compaction.c b/mm/compaction.c
index f9e628a5306a..ee2ddf77191f 100644
--- a/mm/compaction.c
+++ b/mm/compaction.c
@@ -1586,7 +1586,7 @@ static void isolate_freepages(struct compact_control *cc)
 	unsigned int stride;
 
 	/* Try a small search of the free lists for a candidate */
-	isolate_start_pfn = fast_isolate_freepages(cc);
+	fast_isolate_freepages(cc);
 	if (cc->nr_freepages)
 		goto splitmap;
 
-- 
2.23.0


^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCH 04/12] mm: compaction: clean up comment for sched contention
  2022-04-18 14:12 [PATCH 00/12] A few cleanup and fixup patches for compaction Miaohe Lin
                   ` (2 preceding siblings ...)
  2022-04-18 14:12 ` [PATCH 03/12] mm: compaction: remove unneeded assignment to isolate_start_pfn Miaohe Lin
@ 2022-04-18 14:12 ` Miaohe Lin
  2022-04-18 14:12 ` [PATCH 05/12] mm: compaction: clean up comment about suitable migration target recheck Miaohe Lin
                   ` (7 subsequent siblings)
  11 siblings, 0 replies; 17+ messages in thread
From: Miaohe Lin @ 2022-04-18 14:12 UTC (permalink / raw)
  To: akpm; +Cc: vbabka, pintu, charante, linux-mm, linux-kernel, linmiaohe

Since commit cf66f0700c8f ("mm, compaction: do not consider a need to
reschedule as contention"), async compaction won't abort when scheduling
is needed. Correct the relevant comment accordingly.

Signed-off-by: Miaohe Lin <linmiaohe@huawei.com>
---
 mm/compaction.c | 11 ++++-------
 mm/internal.h   |  2 +-
 2 files changed, 5 insertions(+), 8 deletions(-)

diff --git a/mm/compaction.c b/mm/compaction.c
index ee2ddf77191f..e839b26fb3d8 100644
--- a/mm/compaction.c
+++ b/mm/compaction.c
@@ -513,15 +513,12 @@ static bool compact_lock_irqsave(spinlock_t *lock, unsigned long *flags,
  * very heavily contended. The lock should be periodically unlocked to avoid
  * having disabled IRQs for a long time, even when there is nobody waiting on
  * the lock. It might also be that allowing the IRQs will result in
- * need_resched() becoming true. If scheduling is needed, async compaction
- * aborts. Sync compaction schedules.
+ * need_resched() becoming true. If scheduling is needed, compaction schedules.
  * Either compaction type will also abort if a fatal signal is pending.
  * In either case if the lock was locked, it is dropped and not regained.
  *
- * Returns true if compaction should abort due to fatal signal pending, or
- *		async compaction due to need_resched()
- * Returns false when compaction can continue (sync compaction might have
- *		scheduled)
+ * Returns true if compaction should abort due to fatal signal pending.
+ * Returns false when compaction can continue.
  */
 static bool compact_unlock_should_abort(spinlock_t *lock,
 		unsigned long flags, bool *locked, struct compact_control *cc)
@@ -574,7 +571,7 @@ static unsigned long isolate_freepages_block(struct compact_control *cc,
 		/*
 		 * Periodically drop the lock (if held) regardless of its
 		 * contention, to give chance to IRQs. Abort if fatal signal
-		 * pending or async compaction detects need_resched()
+		 * pending.
 		 */
 		if (!(blockpfn % SWAP_CLUSTER_MAX)
 		    && compact_unlock_should_abort(&cc->zone->lock, flags,
diff --git a/mm/internal.h b/mm/internal.h
index 48eb2d24fcd2..70cc61af06de 100644
--- a/mm/internal.h
+++ b/mm/internal.h
@@ -403,7 +403,7 @@ struct compact_control {
 	bool direct_compaction;		/* False from kcompactd or /proc/... */
 	bool proactive_compaction;	/* kcompactd proactive compaction */
 	bool whole_zone;		/* Whole zone should/has been scanned */
-	bool contended;			/* Signal lock or sched contention */
+	bool contended;			/* Signal lock contention */
 	bool rescan;			/* Rescanning the same pageblock */
 	bool alloc_contig;		/* alloc_contig_range allocation */
 };
-- 
2.23.0


^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCH 05/12] mm: compaction: clean up comment about suitable migration target recheck
  2022-04-18 14:12 [PATCH 00/12] A few cleanup and fixup patches for compaction Miaohe Lin
                   ` (3 preceding siblings ...)
  2022-04-18 14:12 ` [PATCH 04/12] mm: compaction: clean up comment for sched contention Miaohe Lin
@ 2022-04-18 14:12 ` Miaohe Lin
  2022-04-18 14:12 ` [PATCH 06/12] mm: compaction: use COMPACT_CLUSTER_MAX in compaction.c Miaohe Lin
                   ` (6 subsequent siblings)
  11 siblings, 0 replies; 17+ messages in thread
From: Miaohe Lin @ 2022-04-18 14:12 UTC (permalink / raw)
  To: akpm; +Cc: vbabka, pintu, charante, linux-mm, linux-kernel, linmiaohe

checked_pageblock is already removed and suitable_migration_target is not
rechecked under the zone lock since commit f8224aa5a0a4 ("mm, compaction:
do not recheck suitable_migration_target under lock"). Correct the comment
accordingly.

Signed-off-by: Miaohe Lin <linmiaohe@huawei.com>
---
 mm/compaction.c | 8 +-------
 1 file changed, 1 insertion(+), 7 deletions(-)

diff --git a/mm/compaction.c b/mm/compaction.c
index e839b26fb3d8..97821b7d7a4f 100644
--- a/mm/compaction.c
+++ b/mm/compaction.c
@@ -599,13 +599,7 @@ static unsigned long isolate_freepages_block(struct compact_control *cc,
 		if (!PageBuddy(page))
 			goto isolate_fail;
 
-		/*
-		 * If we already hold the lock, we can skip some rechecking.
-		 * Note that if we hold the lock now, checked_pageblock was
-		 * already set in some previous iteration (or strict is true),
-		 * so it is correct to skip the suitable migration target
-		 * recheck as well.
-		 */
+		/* If we already hold the lock, we can skip some rechecking. */
 		if (!locked) {
 			locked = compact_lock_irqsave(&cc->zone->lock,
 								&flags, cc);
-- 
2.23.0


^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCH 06/12] mm: compaction: use COMPACT_CLUSTER_MAX in compaction.c
  2022-04-18 14:12 [PATCH 00/12] A few cleanup and fixup patches for compaction Miaohe Lin
                   ` (4 preceding siblings ...)
  2022-04-18 14:12 ` [PATCH 05/12] mm: compaction: clean up comment about suitable migration target recheck Miaohe Lin
@ 2022-04-18 14:12 ` Miaohe Lin
  2022-04-18 14:12 ` [PATCH 07/12] mm: compaction: use helper compound_nr in isolate_migratepages_block Miaohe Lin
                   ` (5 subsequent siblings)
  11 siblings, 0 replies; 17+ messages in thread
From: Miaohe Lin @ 2022-04-18 14:12 UTC (permalink / raw)
  To: akpm; +Cc: vbabka, pintu, charante, linux-mm, linux-kernel, linmiaohe

Always use COMPACT_CLUSTER_MAX here as we're doing the compaction.
Minor improvements in readability.

Signed-off-by: Miaohe Lin <linmiaohe@huawei.com>
---
 mm/compaction.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/mm/compaction.c b/mm/compaction.c
index 97821b7d7a4f..1fc912ec3a98 100644
--- a/mm/compaction.c
+++ b/mm/compaction.c
@@ -573,7 +573,7 @@ static unsigned long isolate_freepages_block(struct compact_control *cc,
 		 * contention, to give chance to IRQs. Abort if fatal signal
 		 * pending.
 		 */
-		if (!(blockpfn % SWAP_CLUSTER_MAX)
+		if (!(blockpfn % COMPACT_CLUSTER_MAX)
 		    && compact_unlock_should_abort(&cc->zone->lock, flags,
 								&locked, cc))
 			break;
@@ -862,7 +862,7 @@ isolate_migratepages_block(struct compact_control *cc, unsigned long low_pfn,
 		 * contention, to give chance to IRQs. Abort completely if
 		 * a fatal signal is pending.
 		 */
-		if (!(low_pfn % SWAP_CLUSTER_MAX)) {
+		if (!(low_pfn % COMPACT_CLUSTER_MAX)) {
 			if (locked) {
 				unlock_page_lruvec_irqrestore(locked, flags);
 				locked = NULL;
@@ -1614,7 +1614,7 @@ static void isolate_freepages(struct compact_control *cc)
 		 * This can iterate a massively long zone without finding any
 		 * suitable migration targets, so periodically check resched.
 		 */
-		if (!(block_start_pfn % (SWAP_CLUSTER_MAX * pageblock_nr_pages)))
+		if (!(block_start_pfn % (COMPACT_CLUSTER_MAX * pageblock_nr_pages)))
 			cond_resched();
 
 		page = pageblock_pfn_to_page(block_start_pfn, block_end_pfn,
@@ -1921,7 +1921,7 @@ static isolate_migrate_t isolate_migratepages(struct compact_control *cc)
 		 * many pageblocks unsuitable, so periodically check if we
 		 * need to schedule.
 		 */
-		if (!(low_pfn % (SWAP_CLUSTER_MAX * pageblock_nr_pages)))
+		if (!(low_pfn % (COMPACT_CLUSTER_MAX * pageblock_nr_pages)))
 			cond_resched();
 
 		page = pageblock_pfn_to_page(block_start_pfn,
-- 
2.23.0


^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCH 07/12] mm: compaction: use helper compound_nr in isolate_migratepages_block
  2022-04-18 14:12 [PATCH 00/12] A few cleanup and fixup patches for compaction Miaohe Lin
                   ` (5 preceding siblings ...)
  2022-04-18 14:12 ` [PATCH 06/12] mm: compaction: use COMPACT_CLUSTER_MAX in compaction.c Miaohe Lin
@ 2022-04-18 14:12 ` Miaohe Lin
  2022-04-18 14:12 ` [PATCH 08/12] mm: compaction: clean up comment about async compaction in isolate_migratepages Miaohe Lin
                   ` (4 subsequent siblings)
  11 siblings, 0 replies; 17+ messages in thread
From: Miaohe Lin @ 2022-04-18 14:12 UTC (permalink / raw)
  To: akpm; +Cc: vbabka, pintu, charante, linux-mm, linux-kernel, linmiaohe

Use helper compound_nr to make use of compound_nr when CONFIG_64BIT and
simplify the code a bit.

Signed-off-by: Miaohe Lin <linmiaohe@huawei.com>
---
 mm/compaction.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/mm/compaction.c b/mm/compaction.c
index 1fc912ec3a98..55013a2af817 100644
--- a/mm/compaction.c
+++ b/mm/compaction.c
@@ -908,7 +908,7 @@ isolate_migratepages_block(struct compact_control *cc, unsigned long low_pfn,
 				 /* Do not report -EBUSY down the chain */
 				if (ret == -EBUSY)
 					ret = 0;
-				low_pfn += (1UL << compound_order(page)) - 1;
+				low_pfn += compound_nr(page) - 1;
 				goto isolate_fail;
 			}
 
-- 
2.23.0


^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCH 08/12] mm: compaction: clean up comment about async compaction in isolate_migratepages
  2022-04-18 14:12 [PATCH 00/12] A few cleanup and fixup patches for compaction Miaohe Lin
                   ` (6 preceding siblings ...)
  2022-04-18 14:12 ` [PATCH 07/12] mm: compaction: use helper compound_nr in isolate_migratepages_block Miaohe Lin
@ 2022-04-18 14:12 ` Miaohe Lin
  2022-04-18 14:12 ` [PATCH 09/12] mm: compaction: avoid possible NULL pointer dereference in kcompactd_cpu_online Miaohe Lin
                   ` (3 subsequent siblings)
  11 siblings, 0 replies; 17+ messages in thread
From: Miaohe Lin @ 2022-04-18 14:12 UTC (permalink / raw)
  To: akpm; +Cc: vbabka, pintu, charante, linux-mm, linux-kernel, linmiaohe

Since commit 282722b0d258 ("mm, compaction: restrict async compaction to
pageblocks of same migratetype"), async direct compaction is restricted to
scan the pageblocks of same migratetype. Correct the comment accordingly.

Signed-off-by: Miaohe Lin <linmiaohe@huawei.com>
---
 mm/compaction.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/mm/compaction.c b/mm/compaction.c
index 55013a2af817..562f274b2c51 100644
--- a/mm/compaction.c
+++ b/mm/compaction.c
@@ -1941,12 +1941,12 @@ static isolate_migrate_t isolate_migratepages(struct compact_control *cc)
 			continue;
 
 		/*
-		 * For async compaction, also only scan in MOVABLE blocks
-		 * without huge pages. Async compaction is optimistic to see
-		 * if the minimum amount of work satisfies the allocation.
-		 * The cached PFN is updated as it's possible that all
-		 * remaining blocks between source and target are unsuitable
-		 * and the compaction scanners fail to meet.
+		 * For async direct compaction, only scan the pageblocks of the
+		 * same migratetype without huge pages. Async direct compaction
+		 * is optimistic to see if the minimum amount of work satisfies
+		 * the allocation. The cached PFN is updated as it's possible
+		 * that all remaining blocks between source and target are
+		 * unsuitable and the compaction scanners fail to meet.
 		 */
 		if (!suitable_migration_source(cc, page)) {
 			update_cached_migrate(cc, block_end_pfn);
-- 
2.23.0


^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCH 09/12] mm: compaction: avoid possible NULL pointer dereference in kcompactd_cpu_online
  2022-04-18 14:12 [PATCH 00/12] A few cleanup and fixup patches for compaction Miaohe Lin
                   ` (7 preceding siblings ...)
  2022-04-18 14:12 ` [PATCH 08/12] mm: compaction: clean up comment about async compaction in isolate_migratepages Miaohe Lin
@ 2022-04-18 14:12 ` Miaohe Lin
  2022-04-19  3:55   ` Andrew Morton
  2022-04-18 14:12 ` [PATCH 10/12] mm: compaction: make compaction_zonelist_suitable return false when COMPACT_SUCCESS Miaohe Lin
                   ` (2 subsequent siblings)
  11 siblings, 1 reply; 17+ messages in thread
From: Miaohe Lin @ 2022-04-18 14:12 UTC (permalink / raw)
  To: akpm; +Cc: vbabka, pintu, charante, linux-mm, linux-kernel, linmiaohe

It's possible that kcompactd_run could fail to run kcompactd for a hot
added node and leave pgdat->kcompactd as NULL. So pgdat->kcompactd should
be checked here to avoid possible NULL pointer dereference.

Signed-off-by: Miaohe Lin <linmiaohe@huawei.com>
---
 mm/compaction.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/mm/compaction.c b/mm/compaction.c
index 562f274b2c51..82c54d70a978 100644
--- a/mm/compaction.c
+++ b/mm/compaction.c
@@ -3052,7 +3052,8 @@ static int kcompactd_cpu_online(unsigned int cpu)
 
 		if (cpumask_any_and(cpu_online_mask, mask) < nr_cpu_ids)
 			/* One of our CPUs online: restore mask */
-			set_cpus_allowed_ptr(pgdat->kcompactd, mask);
+			if (pgdat->kcompactd)
+				set_cpus_allowed_ptr(pgdat->kcompactd, mask);
 	}
 	return 0;
 }
-- 
2.23.0


^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCH 10/12] mm: compaction: make compaction_zonelist_suitable return false when COMPACT_SUCCESS
  2022-04-18 14:12 [PATCH 00/12] A few cleanup and fixup patches for compaction Miaohe Lin
                   ` (8 preceding siblings ...)
  2022-04-18 14:12 ` [PATCH 09/12] mm: compaction: avoid possible NULL pointer dereference in kcompactd_cpu_online Miaohe Lin
@ 2022-04-18 14:12 ` Miaohe Lin
  2022-04-18 14:12 ` [PATCH 11/12] mm: compaction: simplify the code in __compact_finished Miaohe Lin
  2022-04-18 14:12 ` [PATCH 12/12] mm: compaction: make sure highest is above the min_pfn Miaohe Lin
  11 siblings, 0 replies; 17+ messages in thread
From: Miaohe Lin @ 2022-04-18 14:12 UTC (permalink / raw)
  To: akpm; +Cc: vbabka, pintu, charante, linux-mm, linux-kernel, linmiaohe

When compact_result indicates that the allocation should now succeed,
i.e. compact_result = COMPACT_SUCCESS, compaction_zonelist_suitable
should return false because there is no need to do compaction now.

Signed-off-by: Miaohe Lin <linmiaohe@huawei.com>
---
 mm/compaction.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/mm/compaction.c b/mm/compaction.c
index 82c54d70a978..334a573485fe 100644
--- a/mm/compaction.c
+++ b/mm/compaction.c
@@ -2291,7 +2291,7 @@ bool compaction_zonelist_suitable(struct alloc_context *ac, int order,
 		available += zone_page_state_snapshot(zone, NR_FREE_PAGES);
 		compact_result = __compaction_suitable(zone, order, alloc_flags,
 				ac->highest_zoneidx, available);
-		if (compact_result != COMPACT_SKIPPED)
+		if (compact_result == COMPACT_CONTINUE)
 			return true;
 	}
 
-- 
2.23.0


^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCH 11/12] mm: compaction: simplify the code in __compact_finished
  2022-04-18 14:12 [PATCH 00/12] A few cleanup and fixup patches for compaction Miaohe Lin
                   ` (9 preceding siblings ...)
  2022-04-18 14:12 ` [PATCH 10/12] mm: compaction: make compaction_zonelist_suitable return false when COMPACT_SUCCESS Miaohe Lin
@ 2022-04-18 14:12 ` Miaohe Lin
  2022-04-18 14:12 ` [PATCH 12/12] mm: compaction: make sure highest is above the min_pfn Miaohe Lin
  11 siblings, 0 replies; 17+ messages in thread
From: Miaohe Lin @ 2022-04-18 14:12 UTC (permalink / raw)
  To: akpm; +Cc: vbabka, pintu, charante, linux-mm, linux-kernel, linmiaohe

Since commit efe771c7603b ("mm, compaction: always finish scanning of a
full pageblock"), compaction will always finish scanning a pageblock. And
migrate_pfn is assured to align with pageblock_nr_pages when we reach here.
So we will always return COMPACT_SUCCESS if a suitable fallback is found
due to the below IS_ALIGNED check of migrate_pfn. Simplify the code to make
this clear and improve the readability. No functional change intended.

Signed-off-by: Miaohe Lin <linmiaohe@huawei.com>
---
 mm/compaction.c | 29 ++++++++---------------------
 1 file changed, 8 insertions(+), 21 deletions(-)

diff --git a/mm/compaction.c b/mm/compaction.c
index 334a573485fe..609a76d7e051 100644
--- a/mm/compaction.c
+++ b/mm/compaction.c
@@ -2134,29 +2134,16 @@ static enum compact_result __compact_finished(struct compact_control *cc)
 		 * other migratetype buddy lists.
 		 */
 		if (find_suitable_fallback(area, order, migratetype,
-						true, &can_steal) != -1) {
-
-			/* movable pages are OK in any pageblock */
-			if (migratetype == MIGRATE_MOVABLE)
-				return COMPACT_SUCCESS;
-
+						true, &can_steal) != -1)
 			/*
-			 * We are stealing for a non-movable allocation. Make
-			 * sure we finish compacting the current pageblock
-			 * first so it is as free as possible and we won't
-			 * have to steal another one soon. This only applies
-			 * to sync compaction, as async compaction operates
-			 * on pageblocks of the same migratetype.
+			 * Movable pages are OK in any pageblock. If we are
+			 * stealing for a non-movable allocation, make sure
+			 * we finish compacting the current pageblock first
+			 * (which is assured by the above migrate_pfn align
+			 * check) so it is as free as possible and we won't
+			 * have to steal another one soon.
 			 */
-			if (cc->mode == MIGRATE_ASYNC ||
-					IS_ALIGNED(cc->migrate_pfn,
-							pageblock_nr_pages)) {
-				return COMPACT_SUCCESS;
-			}
-
-			ret = COMPACT_CONTINUE;
-			break;
-		}
+			return COMPACT_SUCCESS;
 	}
 
 out:
-- 
2.23.0


^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCH 12/12] mm: compaction: make sure highest is above the min_pfn
  2022-04-18 14:12 [PATCH 00/12] A few cleanup and fixup patches for compaction Miaohe Lin
                   ` (10 preceding siblings ...)
  2022-04-18 14:12 ` [PATCH 11/12] mm: compaction: simplify the code in __compact_finished Miaohe Lin
@ 2022-04-18 14:12 ` Miaohe Lin
  11 siblings, 0 replies; 17+ messages in thread
From: Miaohe Lin @ 2022-04-18 14:12 UTC (permalink / raw)
  To: akpm; +Cc: vbabka, pintu, charante, linux-mm, linux-kernel, linmiaohe

It's not guaranteed that highest will be above the min_pfn. If highest is
below the min_pfn, migrate_pfn and free_pfn can meet prematurely and lead
to some useless work. Make sure highest is above min_pfn to avoid making
a futile effort.

Signed-off-by: Miaohe Lin <linmiaohe@huawei.com>
---
 mm/compaction.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/mm/compaction.c b/mm/compaction.c
index 609a76d7e051..65970107b789 100644
--- a/mm/compaction.c
+++ b/mm/compaction.c
@@ -1532,7 +1532,7 @@ fast_isolate_freepages(struct compact_control *cc)
 			 * not found, be pessimistic for direct compaction
 			 * and use the min mark.
 			 */
-			if (highest) {
+			if (highest >= min_pfn) {
 				page = pfn_to_page(highest);
 				cc->free_pfn = highest;
 			} else {
-- 
2.23.0


^ permalink raw reply related	[flat|nested] 17+ messages in thread

* Re: [PATCH 09/12] mm: compaction: avoid possible NULL pointer dereference in kcompactd_cpu_online
  2022-04-18 14:12 ` [PATCH 09/12] mm: compaction: avoid possible NULL pointer dereference in kcompactd_cpu_online Miaohe Lin
@ 2022-04-19  3:55   ` Andrew Morton
  2022-04-19  6:47     ` Miaohe Lin
  0 siblings, 1 reply; 17+ messages in thread
From: Andrew Morton @ 2022-04-19  3:55 UTC (permalink / raw)
  To: Miaohe Lin
  Cc: vbabka, pintu, charante, linux-mm, linux-kernel, David Hildenbrand

Please cc David H on memhotplug stuff.

On Mon, 18 Apr 2022 22:12:50 +0800 Miaohe Lin <linmiaohe@huawei.com> wrote:

> It's possible that kcompactd_run could fail to run kcompactd for a hot
> added node and leave pgdat->kcompactd as NULL. So pgdat->kcompactd should
> be checked here to avoid possible NULL pointer dereference.
>
> ..
>
> --- a/mm/compaction.c
> +++ b/mm/compaction.c
> @@ -3052,7 +3052,8 @@ static int kcompactd_cpu_online(unsigned int cpu)
>  
>  		if (cpumask_any_and(cpu_online_mask, mask) < nr_cpu_ids)
>  			/* One of our CPUs online: restore mask */
> -			set_cpus_allowed_ptr(pgdat->kcompactd, mask);
> +			if (pgdat->kcompactd)
> +				set_cpus_allowed_ptr(pgdat->kcompactd, mask);
>  	}
>  	return 0;
>  }

Why not fail to bring the node online if kcompactd_run() failed?

Also, should we panic the system if kcompactd_run() failed in
kcompactd_init()?


^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH 09/12] mm: compaction: avoid possible NULL pointer dereference in kcompactd_cpu_online
  2022-04-19  3:55   ` Andrew Morton
@ 2022-04-19  6:47     ` Miaohe Lin
  2022-04-19  7:42       ` David Hildenbrand
  0 siblings, 1 reply; 17+ messages in thread
From: Miaohe Lin @ 2022-04-19  6:47 UTC (permalink / raw)
  To: Andrew Morton
  Cc: vbabka, pintu, charante, linux-mm, linux-kernel, David Hildenbrand

On 2022/4/19 11:55, Andrew Morton wrote:
> Please cc David H on memhotplug stuff.

I'm relying on the scripts/get_maintainer.pl to take me the right people now.
Will take care of it. Many thanks for your remind!

> 
> On Mon, 18 Apr 2022 22:12:50 +0800 Miaohe Lin <linmiaohe@huawei.com> wrote:
> 
>> It's possible that kcompactd_run could fail to run kcompactd for a hot
>> added node and leave pgdat->kcompactd as NULL. So pgdat->kcompactd should
>> be checked here to avoid possible NULL pointer dereference.
>>
>> ..
>>
>> --- a/mm/compaction.c
>> +++ b/mm/compaction.c
>> @@ -3052,7 +3052,8 @@ static int kcompactd_cpu_online(unsigned int cpu)
>>  
>>  		if (cpumask_any_and(cpu_online_mask, mask) < nr_cpu_ids)
>>  			/* One of our CPUs online: restore mask */
>> -			set_cpus_allowed_ptr(pgdat->kcompactd, mask);
>> +			if (pgdat->kcompactd)
>> +				set_cpus_allowed_ptr(pgdat->kcompactd, mask);
>>  	}
>>  	return 0;
>>  }
> 
> Why not fail to bring the node online if kcompactd_run() failed?

kcompactd_run() is allowed to fail since it's introduced via commit 698b1b30642 ("mm, compaction: introduce kcompactd").

> 
> Also, should we panic the system if kcompactd_run() failed in
> kcompactd_init()?

So I think this might not be a critical issue. We could live with it anyway.
And in fact, CONFIG_COMPACTION is even not enabled in some systems.

Thanks!

> 
> .
> 


^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH 09/12] mm: compaction: avoid possible NULL pointer dereference in kcompactd_cpu_online
  2022-04-19  6:47     ` Miaohe Lin
@ 2022-04-19  7:42       ` David Hildenbrand
  2022-04-19 11:05         ` Miaohe Lin
  0 siblings, 1 reply; 17+ messages in thread
From: David Hildenbrand @ 2022-04-19  7:42 UTC (permalink / raw)
  To: Miaohe Lin, Andrew Morton; +Cc: vbabka, pintu, charante, linux-mm, linux-kernel

On 19.04.22 08:47, Miaohe Lin wrote:
> On 2022/4/19 11:55, Andrew Morton wrote:
>> Please cc David H on memhotplug stuff.
> 
> I'm relying on the scripts/get_maintainer.pl to take me the right people now.
> Will take care of it. Many thanks for your remind!

Yes, we should add add a memhotplug in MAINTAINERS subsections for good
so I can punch in my name. Let me add that to my TODO list.

-- 
Thanks,

David / dhildenb


^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH 09/12] mm: compaction: avoid possible NULL pointer dereference in kcompactd_cpu_online
  2022-04-19  7:42       ` David Hildenbrand
@ 2022-04-19 11:05         ` Miaohe Lin
  0 siblings, 0 replies; 17+ messages in thread
From: Miaohe Lin @ 2022-04-19 11:05 UTC (permalink / raw)
  To: David Hildenbrand, Andrew Morton
  Cc: vbabka, pintu, charante, linux-mm, linux-kernel

On 2022/4/19 15:42, David Hildenbrand wrote:
> On 19.04.22 08:47, Miaohe Lin wrote:
>> On 2022/4/19 11:55, Andrew Morton wrote:
>>> Please cc David H on memhotplug stuff.
>>
>> I'm relying on the scripts/get_maintainer.pl to take me the right people now.
>> Will take care of it. Many thanks for your remind!
> 
> Yes, we should add add a memhotplug in MAINTAINERS subsections for good
> so I can punch in my name. Let me add that to my TODO list.
> 

That will be really convenient. Thanks!


^ permalink raw reply	[flat|nested] 17+ messages in thread

end of thread, other threads:[~2022-04-19 11:05 UTC | newest]

Thread overview: 17+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-04-18 14:12 [PATCH 00/12] A few cleanup and fixup patches for compaction Miaohe Lin
2022-04-18 14:12 ` [PATCH 01/12] mm: compaction: remove unneeded return value of kcompactd_run Miaohe Lin
2022-04-18 14:12 ` [PATCH 02/12] mm: compaction: remove unneeded pfn update Miaohe Lin
2022-04-18 14:12 ` [PATCH 03/12] mm: compaction: remove unneeded assignment to isolate_start_pfn Miaohe Lin
2022-04-18 14:12 ` [PATCH 04/12] mm: compaction: clean up comment for sched contention Miaohe Lin
2022-04-18 14:12 ` [PATCH 05/12] mm: compaction: clean up comment about suitable migration target recheck Miaohe Lin
2022-04-18 14:12 ` [PATCH 06/12] mm: compaction: use COMPACT_CLUSTER_MAX in compaction.c Miaohe Lin
2022-04-18 14:12 ` [PATCH 07/12] mm: compaction: use helper compound_nr in isolate_migratepages_block Miaohe Lin
2022-04-18 14:12 ` [PATCH 08/12] mm: compaction: clean up comment about async compaction in isolate_migratepages Miaohe Lin
2022-04-18 14:12 ` [PATCH 09/12] mm: compaction: avoid possible NULL pointer dereference in kcompactd_cpu_online Miaohe Lin
2022-04-19  3:55   ` Andrew Morton
2022-04-19  6:47     ` Miaohe Lin
2022-04-19  7:42       ` David Hildenbrand
2022-04-19 11:05         ` Miaohe Lin
2022-04-18 14:12 ` [PATCH 10/12] mm: compaction: make compaction_zonelist_suitable return false when COMPACT_SUCCESS Miaohe Lin
2022-04-18 14:12 ` [PATCH 11/12] mm: compaction: simplify the code in __compact_finished Miaohe Lin
2022-04-18 14:12 ` [PATCH 12/12] mm: compaction: make sure highest is above the min_pfn Miaohe Lin

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.