Re: [RFC PATCH v6 2/5] mm: migrate: Add migrate_misplaced_folios_batch()

[Bharata B Rao]

Replies to review from Shashiko
[https://sashiko.dev/#/patchset/20260323095104.238982-1-bharata%40amd.com]

>> diff --git a/mm/migrate.c b/mm/migrate.c
>> --- a/mm/migrate.c
>> +++ b/mm/migrate.c
>[ ... ]
>> @@ -2751,5 +2751,53 @@ int migrate_misplaced_folio(struct folio *folio, int node)
>[ ... ]
>> +	if (nr_remaining)
>> +		putback_movable_pages(folio_list);
>> +
>> +	if (nr_succeeded) {
>> +		count_vm_numa_events(NUMA_PAGE_MIGRATE, nr_succeeded);
>> +		mod_node_page_state(pgdat, PGPROMOTE_SUCCESS, nr_succeeded);
>
>Should this use mod_lruvec_state() instead of mod_node_page_state()?
>Using mod_node_page_state() updates the global NUMA node statistic but bypasses
>the memcg-specific lruvec statistic. This can cause the pgpromote_success
>metric in memory.stat to undercount for batched promotions.

That's correct. Earlier, memcg wasn't available in this routine but that
is no longer the case in v6. Hence I can get lruvec and should be able
to use mod_lruvec_state() itself for PGPROMOTE_SUCCESS accounting. This will
be part of next iteration.

>
>Are there missing topology and sysctl checks before incrementing
>PGPROMOTE_SUCCESS here?
>
>In the single-folio variant, this statistic is only incremented when:
> if ((sysctl_numa_balancing_mode & NUMA_BALANCING_MEMORY_TIERING)
>	    && !node_is_toptier(folio_nid(folio))
>	    && node_is_toptier(node))
>		mod_lruvec_state(lruvec, PGPROMOTE_SUCCESS, nr_succeeded);
>Unconditionally recording the stat inside the migration helper could
>corrupt promotion statistics if the function is ever used for lateral NUMA
>migrations in the future.

This routine is called only for hot page promotion by pghot kmigrated thread
and has been documented as such. So unconditional call should be fine.

Regards,
Bharata.