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

At 2026-08-17 14:41:16, [email protected] 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 <[email protected]>
>
>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/[email protected]/
>
>> 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/[email protected]?part=1