Linux-mm Archive on lore.kernel.org
 help / color / Atom feed
From: Vlastimil Babka <vbabka@suse.cz>
To: Andrew Morton <akpm@linux-foundation.org>,
	Michal Hocko <mhocko@suse.com>
Cc: Yafang Shao <laoar.shao@gmail.com>,
	linux-mm@kvack.org, Mel Gorman <mgorman@techsingularity.net>,
	Yafang Shao <shaoyafang@didiglobal.com>
Subject: Re: [PATCH] mm/compaction: introduce a helper compact_zone_counters_init()
Date: Wed, 24 Jul 2019 15:35:12 +0200
Message-ID: <1fb6f7da-f776-9e42-22f8-bbb79b030b98@suse.cz> (raw)
In-Reply-To: <20190723144007.9660c3c98068caeba2109ded@linux-foundation.org>

On 7/23/19 11:40 PM, Andrew Morton wrote:
> On Tue, 23 Jul 2019 10:12:18 +0200 Michal Hocko <mhocko@suse.com> wrote:
> 
>> On Tue 23-07-19 04:08:15, Yafang Shao wrote:
>>> This is the follow-up of the
>>> commit "mm/compaction.c: clear total_{migrate,free}_scanned before scanning a new zone".
>>>
>>> These counters are used to track activities during compacting a zone,
>>> and they will be set to zero before compacting a new zone in all compact
>>> paths. Move all these common settings into compact_zone() for better
>>> management. A new helper compact_zone_counters_init() is introduced for
>>> this purpose.
>>
>> The helper seems excessive a bit because we have a single call site but
>> other than that this is an improvement to the current fragile and
>> duplicated code.
>>
>> I would just get rid of the helper and squash it to your previous patch
>> which Andrew already took to the mm tree.

I have squashed everything locally, and for the result:

Reviewed-by: Vlastimil Babka <vbabka@suse.cz>

Also, why not squash some more?

----8<----
diff --git a/mm/compaction.c b/mm/compaction.c
index dcbe95fa8e28..cfe1457352f9 100644
--- a/mm/compaction.c
+++ b/mm/compaction.c
@@ -2083,6 +2083,8 @@ compact_zone(struct compact_control *cc, struct capture_control *capc)
 	cc->total_free_scanned = 0;
 	cc->nr_migratepages = 0;
 	cc->nr_freepages = 0;
+	INIT_LIST_HEAD(&cc->freepages);
+	INIT_LIST_HEAD(&cc->migratepages);
 
 	cc->migratetype = gfpflags_to_migratetype(cc->gfp_mask);
 	ret = compaction_suitable(cc->zone, cc->order, cc->alloc_flags,
@@ -2307,8 +2309,6 @@ static enum compact_result compact_zone_order(struct zone *zone, int order,
 
 	if (capture)
 		current->capture_control = &capc;
-	INIT_LIST_HEAD(&cc.freepages);
-	INIT_LIST_HEAD(&cc.migratepages);
 
 	ret = compact_zone(&cc, &capc);
 
@@ -2424,8 +2424,6 @@ static void compact_node(int nid)
 			continue;
 
 		cc.zone = zone;
-		INIT_LIST_HEAD(&cc.freepages);
-		INIT_LIST_HEAD(&cc.migratepages);
 
 		compact_zone(&cc, NULL);
 
@@ -2551,8 +2549,6 @@ static void kcompactd_do_work(pg_data_t *pgdat)
 			continue;
 
 		cc.zone = zone;
-		INIT_LIST_HEAD(&cc.freepages);
-		INIT_LIST_HEAD(&cc.migratepages);
 
 		if (kthread_should_stop())
 			return;


  reply index

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-07-23  8:08 Yafang Shao
2019-07-23  8:12 ` Michal Hocko
2019-07-23  8:24   ` Yafang Shao
2019-07-23 21:40   ` Andrew Morton
2019-07-24 13:35     ` Vlastimil Babka [this message]
2019-07-25  0:19       ` Andrew Morton
2019-07-25  0:25         ` Andrew Morton
2019-07-25 11:42           ` Vlastimil Babka

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1fb6f7da-f776-9e42-22f8-bbb79b030b98@suse.cz \
    --to=vbabka@suse.cz \
    --cc=akpm@linux-foundation.org \
    --cc=laoar.shao@gmail.com \
    --cc=linux-mm@kvack.org \
    --cc=mgorman@techsingularity.net \
    --cc=mhocko@suse.com \
    --cc=shaoyafang@didiglobal.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

Linux-mm Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-mm/0 linux-mm/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-mm linux-mm/ https://lore.kernel.org/linux-mm \
		linux-mm@kvack.org
	public-inbox-index linux-mm

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kvack.linux-mm


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git