Re: [PATCH v8 2/8] mm: migrate: Add promote_misplaced_memcg_folios()

[Bharata B Rao]

[Reply to Shashiko review]

On 28-Jul-26 11:13 AM, Bharata B Rao wrote:
> diff --git a/mm/migrate.c b/mm/migrate.c
> index ab7227376757..58a8a0cf6fa3 100644
> --- a/mm/migrate.c
> +++ b/mm/migrate.c
> @@ -2773,4 +2773,62 @@ int migrate_misplaced_folio(struct folio *folio, int node)
>  	BUG_ON(!list_empty(&migratepages));
>  	return nr_remaining ? -EAGAIN : 0;
>  }
> +
> +/**
> + * promote_misplaced_memcg_folios() - Batch variant of migrate_misplaced_folio
> + * Attempts to promote a folio list to the specified destination.
> + * @folio_list: Isolated list of folios to be batch-promoted.
> + * @node: The NUMA node ID to where the folios should be promoted.
> + *
> + * Caller is expected to have isolated the folios by calling
> + * migrate_misplaced_folio_prepare(), which will result in an
> + * elevated reference count on the folios. All the isolated folios
> + * in the list must belong to the same memcg so that NUMA_PAGE_MIGRATE
> + * stat can be attributed correctly to the memcg.
> + *
> + * This function will un-isolate the folios, drop the elevated reference
> + * and remove them from the list before returning. This should be called
> + * only for batched promotion of hot pages from lower tier nodes.
> + *
> + * Return: 0 on success and -EAGAIN on failure or partial promotion.
> + *         On return, @folio_list will be empty regardless of success/failure.
> + */
> +int promote_misplaced_memcg_folios(struct list_head *folio_list, int node)
> +{
> +	struct mem_cgroup *memcg = NULL;
> +	unsigned int nr_succeeded = 0;
> +	struct folio *first;
> +	int nr_remaining;
> +
> +	if (list_empty(folio_list))
> +		return 0;
> +
> +	first = list_first_entry(folio_list, struct folio, lru);
> +#ifdef CONFIG_DEBUG_VM
> +	{
> +		struct folio *f;
> +
> +		list_for_each_entry(f, folio_list, lru)
> +			VM_WARN_ON_ONCE(folio_memcg(f) != folio_memcg(first));

> Does this iteration need to hold the RCU read lock?
> Looking at folio_memcg(), it calls obj_cgroup_memcg(), which executes a
> lockdep assertion requiring either rcu_read_lock or cgroup_mutex. Calling it
> without these locks will cause a lockdep splat when CONFIG_PROVE_LOCKING is
> enabled.
> Additionally, without the RCU lock, can the returned memcg pointer be
> concurrently updated due to memcg reparenting? If so, this race might cause
> folio_memcg(f) and folio_memcg(first) to evaluate to different pointers
> even if both folios belong to the same original memcg, potentially triggering
> a spurious VM_WARN_ON_ONCE().

That list walk needs RCU read lock protection. Will fix.

Regards,
Bharata.