| Message ID | 20240208022508.1771534-1-wangkefeng.wang@huawei.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | mm: compaction: early termination in compact_nodes() | expand |
On Thu, 8 Feb 2024 10:25:08 +0800 Kefeng Wang <wangkefeng.wang@huawei.com> wrote: > No need to continue try compact memory if pending fatal signal, > allow loop termination earlier in compact_nodes(). Seems sensible, but... why? Is there some problem which we can demonstrate with the existing code? In other words, does this change provide any observable benefit under any circumstances?
On 08.02.24 22:14, Andrew Morton wrote: > On Thu, 8 Feb 2024 10:25:08 +0800 Kefeng Wang <wangkefeng.wang@huawei.com> wrote: > >> No need to continue try compact memory if pending fatal signal, >> allow loop termination earlier in compact_nodes(). > > Seems sensible, but... why? Is there some problem which we can > demonstrate with the existing code? In other words, does this change > provide any observable benefit under any circumstances? I'd also be curious why the existing fatal_signal_pending() calls are insufficient.
On 2024/2/12 22:22, David Hildenbrand wrote: > On 08.02.24 22:14, Andrew Morton wrote: >> On Thu, 8 Feb 2024 10:25:08 +0800 Kefeng Wang >> <wangkefeng.wang@huawei.com> wrote: >> >>> No need to continue try compact memory if pending fatal signal, >>> allow loop termination earlier in compact_nodes(). >> >> Seems sensible, but... why? Is there some problem which we can >> demonstrate with the existing code? In other words, does this change >> provide any observable benefit under any circumstances? > > I'd also be curious why the existing fatal_signal_pending() calls are > insufficient. The existing fatal_signal_pending() does make compact_zone() breakout of the while loop, but it still enter the next zone/next nid, and some unnecessary functions(eg, lru_add_drain) called, no observable benefit from test, it is just found from code inspection when refactor compact_node().
On Sat, 17 Feb 2024 10:55:10 +0800 Kefeng Wang <wangkefeng.wang@huawei.com> wrote: > On 2024/2/12 22:22, David Hildenbrand wrote: > > On 08.02.24 22:14, Andrew Morton wrote: > >> On Thu, 8 Feb 2024 10:25:08 +0800 Kefeng Wang > >> <wangkefeng.wang@huawei.com> wrote: > >> > >>> No need to continue try compact memory if pending fatal signal, > >>> allow loop termination earlier in compact_nodes(). > >> > >> Seems sensible, but... why? Is there some problem which we can > >> demonstrate with the existing code? In other words, does this change > >> provide any observable benefit under any circumstances? > > > > I'd also be curious why the existing fatal_signal_pending() calls are > > insufficient. > > The existing fatal_signal_pending() does make compact_zone() breakout of > the while loop, but it still enter the next zone/next nid, and some > unnecessary functions(eg, lru_add_drain) called, no observable benefit > from test, it is just found from code inspection when refactor Fair enough. I added the above words to the changelog (this material should have been communicated in the original!) and I'll plan to move this change into mm-stable next week unless someone stops me.
On 2024/2/22 6:34, Andrew Morton wrote: > On Sat, 17 Feb 2024 10:55:10 +0800 Kefeng Wang <wangkefeng.wang@huawei.com> wrote: > >> On 2024/2/12 22:22, David Hildenbrand wrote: >>> On 08.02.24 22:14, Andrew Morton wrote: >>>> On Thu, 8 Feb 2024 10:25:08 +0800 Kefeng Wang >>>> <wangkefeng.wang@huawei.com> wrote: >>>> >>>>> No need to continue try compact memory if pending fatal signal, >>>>> allow loop termination earlier in compact_nodes(). >>>> >>>> Seems sensible, but... why? Is there some problem which we can >>>> demonstrate with the existing code? In other words, does this change >>>> provide any observable benefit under any circumstances? >>> >>> I'd also be curious why the existing fatal_signal_pending() calls are >>> insufficient. >> >> The existing fatal_signal_pending() does make compact_zone() breakout of >> the while loop, but it still enter the next zone/next nid, and some >> unnecessary functions(eg, lru_add_drain) called, no observable benefit >> from test, it is just found from code inspection when refactor > > Fair enough. I added the above words to the changelog (this material > should have been communicated in the original!) and I'll plan to move > this change into mm-stable next week unless someone stops me. Indeed, thanks for the update, you're so kind.
diff --git a/mm/compaction.c b/mm/compaction.c index de882ecb61c5..52e75f8ac7e7 100644 --- a/mm/compaction.c +++ b/mm/compaction.c @@ -2895,7 +2895,7 @@ enum compact_result try_to_compact_pages(gfp_t gfp_mask, unsigned int order, * reaching score targets due to various back-off conditions, such as, * contention on per-node or per-zone locks. */ -static void compact_node(pg_data_t *pgdat, bool proactive) +static int compact_node(pg_data_t *pgdat, bool proactive) { int zoneid; struct zone *zone; @@ -2913,6 +2913,9 @@ static void compact_node(pg_data_t *pgdat, bool proactive) if (!populated_zone(zone)) continue; + if (fatal_signal_pending(current)) + return -EINTR; + cc.zone = zone; compact_zone(&cc, NULL); @@ -2924,18 +2927,25 @@ static void compact_node(pg_data_t *pgdat, bool proactive) cc.total_free_scanned); } } + + return 0; } /* Compact all zones of all nodes in the system */ -static void compact_nodes(void) +static int compact_nodes(void) { - int nid; + int ret, nid; /* Flush pending updates to the LRU lists */ lru_add_drain_all(); - for_each_online_node(nid) - compact_node(NODE_DATA(nid), false); + for_each_online_node(nid) { + ret = compact_node(NODE_DATA(nid), false); + if (ret) + return ret; + } + + return 0; } static int compaction_proactiveness_sysctl_handler(struct ctl_table *table, int write, @@ -2981,9 +2991,9 @@ static int sysctl_compaction_handler(struct ctl_table *table, int write, return -EINVAL; if (write) - compact_nodes(); + ret = compact_nodes(); - return 0; + return ret; } #if defined(CONFIG_SYSFS) && defined(CONFIG_NUMA)
No need to continue try compact memory if pending fatal signal, allow loop termination earlier in compact_nodes(). Signed-off-by: Kefeng Wang <wangkefeng.wang@huawei.com> --- mm/compaction.c | 24 +++++++++++++++++------- 1 file changed, 17 insertions(+), 7 deletions(-)