damon.lists.linux.dev archive mirror
 help / color / mirror / Atom feed
From: "李佑鸿 " <dayou5941@163.com>
To: sashiko-reviews@lists.linux.dev
Cc: damon@lists.linux.dev
Subject: Re:Re: [PATCH] mm/damon/ops-common: factor out damon_putback_folio_list()
Date: Mon, 17 Aug 2026 15:14:36 +0800 (CST)	[thread overview]
Message-ID: <7f87a4e0.54d4.1a00e927ecd.Coremail.dayou5941@163.com> (raw)
In-Reply-To: <20260817064117.3581D1F000E9@smtp.kernel.org>


















At 2026-08-17 14:41:16, sashiko-bot@kernel.org wrote:
>Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
>- [Low] Removal of `struct folio *folio;` declaration causes a compilation failure in `damon_migrate_folio_list()`.
>--
>
>--- Patch [1]: [PATCH] mm/damon/ops-common: factor out damon_putback_folio_list() ---
>commit b95b21fa4d0d8990d47609474f8cee2aa7d640e1
>Author: Li Youhong <liyouhong@kylinos.cn>
>
>mm/damon/ops-common: factor out damon_putback_folio_list()
>
>This commit factors out the duplicated putback loop in
>damon_migrate_folio_list() and damon_migrate_pages() into a new
>damon_putback_folio_list() helper. The commit notes state this removes
>the need for the local folio variable.
>
>Link: https://lore.kernel.org/all/20260724060707.CE1F81F000E9@smtp.kernel.org/
>
>> diff --git a/mm/damon/ops-common.c b/mm/damon/ops-common.c
>> index fbda70d8ea4d0..f9cfd35bee722 100644
>> --- a/mm/damon/ops-common.c
>> +++ b/mm/damon/ops-common.c
>[ ... ]
>>  static unsigned int damon_migrate_folio_list(struct list_head *folio_list,
>>  						struct pglist_data *pgdat,
>>  						int target_nid)
>>  {
>>  	unsigned int nr_migrated = 0;
>> -	struct folio *folio;
>>  	LIST_HEAD(ret_folios);
>>  	LIST_HEAD(migrate_folios);
>
>[Severity: Low]
>The commit message notes that removing the folio variable here won't cause
>a build failure. However, does damon_migrate_folio_list() still use the
>folio variable in its first while-loop?

>

Thanks for the review.
Removing the local variable "struct folio *folio" in damon_migrate_folio_list() will
not cause build failures, since this variable is only used inside the while‑loop.

@@ -372,13 +384,7 @@ static unsigned int damon_migrate_folio_list(struct list_head *folio_list,
 
 	list_splice(&ret_folios, folio_list);
 
-	while (!list_empty(folio_list)) {
-		folio = lru_to_folio(folio_list);  //This variable is only used here.
-		list_del(&folio->lru);
-		node_stat_sub_folio(folio, NR_ISOLATED_ANON +
-				folio_is_file_lru(folio));
-		folio_putback_lru(folio);
-	}
+	damon_putback_folio_list(folio_list);

>Looking at the remaining code in damon_migrate_folio_list():
>
>	while (!list_empty(folio_list)) {
>		cond_resched();
>
>		folio = lru_to_folio(folio_list);
>		list_del(&folio->lru);
>
>		if (!folio_trylock(folio))
>			goto keep;
>
>Will this cause an undeclared identifier compilation error since the variable
>was removed from the function scope but not re-declared locally inside this
>loop block?
>
The latest code is as shown below; folio is already declared locally.

	while (!list_empty(folio_list)) {
		struct folio *folio;

		cond_resched();

		folio = lru_to_folio(folio_list);
		list_del(&folio->lru);

		if (!folio_trylock(folio))
			goto keep;

link: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/mm/damon/ops-common.c?id=v7.2

Thanks,
Youhong

>-- 
>Sashiko AI review · https://sashiko.dev/#/patchset/20260817063458.351063-1-dayou5941@163.com?part=1

  reply	other threads:[~2026-08-17  7:14 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-08-17  6:34 [PATCH] mm/damon/ops-common: factor out damon_putback_folio_list() dayou5941
2026-08-17  6:41 ` sashiko-bot
2026-08-17  7:14   ` 李佑鸿  [this message]
2026-08-17 13:42 ` SJ Park

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=7f87a4e0.54d4.1a00e927ecd.Coremail.dayou5941@163.com \
    --to=dayou5941@163.com \
    --cc=damon@lists.linux.dev \
    --cc=sashiko-reviews@lists.linux.dev \
    /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
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).