[PATCH] mm/damon/ops-common: factor out damon_putback_folio_list()

[PATCH] mm/damon/ops-common: factor out damon_putback_folio_list()

[dayou5941]

From: Li Youhong <liyouhong@kylinos.cn>

The putback loop is duplicated in damon_migrate_folio_list() and on the
invalid-nid path of damon_migrate_pages().  Factor it into a small helper
for readability.  No functional change.

Signed-off-by: Li Youhong <liyouhong@kylinos.cn>

---
Note:
As previously pointed out by the AI regarding whether removing the local
variable struct folio *folio in the damon_migrate_folio_list()function
would introduce build-time issues: no build failure will actually occur.
This is because struct folio *folio is only used inside the while-loop.
link: https://lore.kernel.org/all/20260724060707.CE1F81F000E9@smtp.kernel.org/

---
 mm/damon/ops-common.c | 31 +++++++++++++++----------------
 1 file changed, 15 insertions(+), 16 deletions(-)

diff --git a/mm/damon/ops-common.c b/mm/damon/ops-common.c
index 0bcad6b1e5b9..5c0c941b7f75 100644
--- a/mm/damon/ops-common.c
+++ b/mm/damon/ops-common.c
@@ -329,12 +329,24 @@ static unsigned int __damon_migrate_folio_list(
 	return nr_succeeded;
 }
 
+static void damon_putback_folio_list(struct list_head *folio_list)
+{
+	struct folio *folio;
+
+	while (!list_empty(folio_list)) {
+		folio = lru_to_folio(folio_list);
+		list_del(&folio->lru);
+		node_stat_sub_folio(folio, NR_ISOLATED_ANON +
+				folio_is_file_lru(folio));
+		folio_putback_lru(folio);
+	}
+}
+
 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);
 
@@ -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);
-		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);
 
 	return nr_migrated;
 }
@@ -395,14 +401,7 @@ unsigned long damon_migrate_pages(struct list_head *folio_list, int target_nid)
 
 	if (target_nid < 0 || target_nid >= MAX_NUMNODES ||
 			!node_state(target_nid, N_MEMORY)) {
-		while (!list_empty(folio_list)) {
-			struct folio *folio = lru_to_folio(folio_list);
-
-			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);
 		return nr_migrated;
 	}
 
-- 
2.25.1

Re: [PATCH] mm/damon/ops-common: factor out damon_putback_folio_list()

[sashiko-bot]

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?

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?

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

Re:Re: [PATCH] mm/damon/ops-common: factor out damon_putback_folio_list()

[李佑鸿]

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