Re: [PATCH RFC 00/28] Eliminate Dying Memory Cgroup

[Harry Yoo <harry.yoo@oracle.com>]

On Tue, Apr 15, 2025 at 10:45:04AM +0800, Muchun Song wrote:
> This patchset is based on v6.15-rc2. It functions correctly only when
> CONFIG_LRU_GEN (Multi-Gen LRU) is disabled. Several issues were encountered
> during rebasing onto the latest code. For more details and assistance, refer
> to the "Challenges" section. This is the reason for adding the RFC tag.
> 

[...snip...]

> ## Fundamentals
> 
> A folio will no longer pin its corresponding memory cgroup. It is necessary
> to ensure that the memory cgroup or the lruvec associated with the memory
> cgroup is not released when a user obtains a pointer to the memory cgroup
> or lruvec returned by folio_memcg() or folio_lruvec(). Users are required
> to hold the RCU read lock or acquire a reference to the memory cgroup
> associated with the folio to prevent its release if they are not concerned
> about the binding stability between the folio and its corresponding memory
> cgroup. However, some users of folio_lruvec() (i.e., the lruvec lock)
> desire a stable binding between the folio and its corresponding memory
> cgroup. An approach is needed to ensure the stability of the binding while
> the lruvec lock is held, and to detect the situation of holding the
> incorrect lruvec lock when there is a race condition during memory cgroup
> reparenting. The following four steps are taken to achieve these goals.
> 
> 1. The first step  to be taken is to identify all users of both functions
>    (folio_memcg() and folio_lruvec()) who are not concerned about binding
>    stability and implement appropriate measures (such as holding a RCU read
>    lock or temporarily obtaining a reference to the memory cgroup for a
>    brief period) to prevent the release of the memory cgroup.
> 
> 2. Secondly, the following refactoring of folio_lruvec_lock() demonstrates
>    how to ensure the binding stability from the user's perspective of
>    folio_lruvec().
> 
>    struct lruvec *folio_lruvec_lock(struct folio *folio)
>    {
>            struct lruvec *lruvec;
> 
>            rcu_read_lock();
>    retry:
>            lruvec = folio_lruvec(folio);
>            spin_lock(&lruvec->lru_lock);
>            if (unlikely(lruvec_memcg(lruvec) != folio_memcg(folio))) {
>                    spin_unlock(&lruvec->lru_lock);
>                    goto retry;
>            }
> 
>            return lruvec;
>    }

Is it still required to hold RCU read lock after binding stability
between folio and memcg? 

In the previous version of this series, folio_lruvec_lock() is implemented:

 struct lruvec *folio_lruvec_lock(struct folio *folio)
 {
	struct lruvec *lruvec;

	rcu_read_lock();
retry:
	lruvec = folio_lruvec(folio);
 	spin_lock(&lruvec->lru_lock);

	if (unlikely(lruvec_memcg(lruvec) != folio_memcg(folio))) {
		spin_unlock(&lruvec->lru_lock);
		goto retry;
	}
	rcu_read_unlock();

 	return lruvec;
 }

And then this version calls rcu_read_unlock() in lruvec_unlock(),
instead of folio_lruvec_lock().

I wonder if this is because the memcg or objcg can be released without
rcu_read_lock(), or just to silence the warning in
folio_memcg()->obj_cgroup_memcg()->lockdep_assert_once(rcu_read_lock_is_held())?

>    From the perspective of memory cgroup removal, the entire reparenting
>    process (altering the binding relationship between folio and its memory
>    cgroup and moving the LRU lists to its parental memory cgroup) should be
>    carried out under both the lruvec lock of the memory cgroup being removed
>    and the lruvec lock of its parent.
> 
> 3. Thirdly, another lock that requires the same approach is the split-queue
>    lock of THP.
> 
> 4. Finally, transfer the LRU pages to the object cgroup without holding a
>    reference to the original memory cgroup.

-- 
Cheers,
Harry / Hyeonggon