Re: [PATCH v3 24/30] mm: memcontrol: prepare for reparenting LRU pages for lruvec lock

[Shakeel Butt]

On Fri, Jan 16, 2026 at 05:50:22PM +0800, Qi Zheng wrote:
> 
> 
> On 1/16/26 5:43 PM, Muchun Song wrote:
> > 
> > 
> > On 2026/1/14 19:32, Qi Zheng wrote:
> > > From: Muchun Song <songmuchun@xxxxxxxxxxxxx>
> > > 
> > > The following diagram illustrates how to ensure the safety of the folio
> > > lruvec lock when LRU folios undergo reparenting.
> > > 
> > > In the folio_lruvec_lock(folio) function:
> > > ```
> > >      rcu_read_lock();
> > > retry:
> > >      lruvec = folio_lruvec(folio);
> > >      /* There is a possibility of folio reparenting at this point. */
> > >      spin_lock(&lruvec->lru_lock);
> > >      if (unlikely(lruvec_memcg(lruvec) != folio_memcg(folio))) {
> > >          /*
> > >           * The wrong lruvec lock was acquired, and a retry is required.
> > >           * This is because the folio resides on the parent memcg lruvec
> > >           * list.
> > >           */
> > >          spin_unlock(&lruvec->lru_lock);
> > >          goto retry;
> > >      }
> > > 
> > >      /* Reaching here indicates that folio_memcg() is stable. */
> > > ```
> > > 
> > > In the memcg_reparent_objcgs(memcg) function:
> > > ```
> > >      spin_lock(&lruvec->lru_lock);
> > >      spin_lock(&lruvec_parent->lru_lock);
> > >      /* Transfer folios from the lruvec list to the parent's. */
> > >      spin_unlock(&lruvec_parent->lru_lock);
> > >      spin_unlock(&lruvec->lru_lock);
> > > ```
> > > 
> > > After acquiring the lruvec lock, it is necessary to verify whether
> > > the folio has been reparented. If reparenting has occurred, the new
> > > lruvec lock must be reacquired. During the LRU folio reparenting
> > > process, the lruvec lock will also be acquired (this will be
> > > implemented in a subsequent patch). Therefore, folio_memcg() remains
> > > unchanged while the lruvec lock is held.
> > > 
> > > Given that lruvec_memcg(lruvec) is always equal to folio_memcg(folio)
> > > after the lruvec lock is acquired, the lruvec_memcg_debug() check is
> > > redundant. Hence, it is removed.
> > > 
> > > This patch serves as a preparation for the reparenting of LRU folios.
> > > 
> > > Signed-off-by: Muchun Song <songmuchun@xxxxxxxxxxxxx>
> > > Signed-off-by: Qi Zheng <zhengqi.arch@xxxxxxxxxxxxx>
> > > Acked-by: Johannes Weiner <hannes@xxxxxxxxxxx>
> > > ---
> > >   include/linux/memcontrol.h | 45 +++++++++++++++++++----------
> > >   include/linux/swap.h       |  1 +
> > >   mm/compaction.c            | 29 +++++++++++++++----
> > >   mm/memcontrol.c            | 59 +++++++++++++++++++++-----------------
> > >   mm/swap.c                  |  4 +++
> > >   5 files changed, 91 insertions(+), 47 deletions(-)
> > > 
> > > diff --git a/include/linux/memcontrol.h b/include/linux/memcontrol.h
> > > index 4b6f20dc694ba..26c3c0e375f58 100644
> > > --- a/include/linux/memcontrol.h
> > > +++ b/include/linux/memcontrol.h
> > > @@ -742,7 +742,15 @@ static inline struct lruvec
> > > *mem_cgroup_lruvec(struct mem_cgroup *memcg,
> > >    * folio_lruvec - return lruvec for isolating/putting an LRU folio
> > >    * @folio: Pointer to the folio.
> > >    *
> > > - * This function relies on folio->mem_cgroup being stable.
> > > + * Call with rcu_read_lock() held to ensure the lifetime of the
> > > returned lruvec.
> > > + * Note that this alone will NOT guarantee the stability of the
> > > folio->lruvec
> > > + * association; the folio can be reparented to an ancestor if this
> > > races with
> > > + * cgroup deletion.
> > > + *
> > > + * Use folio_lruvec_lock() to ensure both lifetime and stability of
> > > the binding.
> > > + * Once a lruvec is locked, folio_lruvec() can be called on other
> > > folios, and
> > > + * their binding is stable if the returned lruvec matches the one
> > > the caller has
> > > + * locked. Useful for lock batching.
> > >    */
> > >   static inline struct lruvec *folio_lruvec(struct folio *folio)
> > >   {
> > > @@ -761,18 +769,15 @@ struct mem_cgroup
> > > *get_mem_cgroup_from_current(void);
> > >   struct mem_cgroup *get_mem_cgroup_from_folio(struct folio *folio);
> > >   struct lruvec *folio_lruvec_lock(struct folio *folio);
> > > +    __acquires(&lruvec->lru_lock)
> > > +    __acquires(rcu)
> > >   struct lruvec *folio_lruvec_lock_irq(struct folio *folio);
> > > +    __acquires(&lruvec->lru_lock)
> > > +    __acquires(rcu)
> > >   struct lruvec *folio_lruvec_lock_irqsave(struct folio *folio,
> > >                           unsigned long *flags);
> > > -
> > > -#ifdef CONFIG_DEBUG_VM
> > > -void lruvec_memcg_debug(struct lruvec *lruvec, struct folio *folio);
> > > -#else
> > > -static inline
> > > -void lruvec_memcg_debug(struct lruvec *lruvec, struct folio *folio)
> > > -{
> > > -}
> > > -#endif
> > > +    __acquires(&lruvec->lru_lock)
> > > +    __acquires(rcu)
> > >   static inline
> > >   struct mem_cgroup *mem_cgroup_from_css(struct cgroup_subsys_state
> > > *css){
> > > @@ -1199,11 +1204,6 @@ static inline struct lruvec
> > > *folio_lruvec(struct folio *folio)
> > >       return &pgdat->__lruvec;
> > >   }
> > > -static inline
> > > -void lruvec_memcg_debug(struct lruvec *lruvec, struct folio *folio)
> > > -{
> > > -}
> > > -
> > >   static inline struct mem_cgroup *parent_mem_cgroup(struct
> > > mem_cgroup *memcg)
> > >   {
> > >       return NULL;
> > > @@ -1262,6 +1262,7 @@ static inline struct lruvec
> > > *folio_lruvec_lock(struct folio *folio)
> > >   {
> > >       struct pglist_data *pgdat = folio_pgdat(folio);
> > > +    rcu_read_lock();
> > >       spin_lock(&pgdat->__lruvec.lru_lock);
> > >       return &pgdat->__lruvec;
> > >   }
> > > @@ -1270,6 +1271,7 @@ static inline struct lruvec
> > > *folio_lruvec_lock_irq(struct folio *folio)
> > >   {
> > >       struct pglist_data *pgdat = folio_pgdat(folio);
> > > +    rcu_read_lock();
> > >       spin_lock_irq(&pgdat->__lruvec.lru_lock);
> > >       return &pgdat->__lruvec;
> > >   }
> > > @@ -1279,6 +1281,7 @@ static inline struct lruvec
> > > *folio_lruvec_lock_irqsave(struct folio *folio,
> > >   {
> > >       struct pglist_data *pgdat = folio_pgdat(folio);
> > > +    rcu_read_lock();
> > >       spin_lock_irqsave(&pgdat->__lruvec.lru_lock, *flagsp);
> > >       return &pgdat->__lruvec;
> > >   }
> > > @@ -1500,24 +1503,36 @@ static inline struct lruvec
> > > *parent_lruvec(struct lruvec *lruvec)
> > >   }
> > >   static inline void lruvec_lock_irq(struct lruvec *lruvec)
> > > +    __acquires(&lruvec->lru_lock)
> > > +    __acquires(rcu)
> > 
> > It seems that functions marked as `inline` cannot be decorated with
> > `__acquires`? We’ve had to move these little helpers into `memcontrol.c`
> > and declare them as extern, but they’re so short that it hardly feels
> 
> Right, I received a compilation error reported LKP:
> 
> All errors (new ones prefixed by >>):
> 
>    In file included from crypto/ahash.c:26:
>    In file included from include/net/netlink.h:6:
>    In file included from include/linux/netlink.h:9:
>    In file included from include/net/scm.h:9:
>    In file included from include/linux/security.h:35:
>    In file included from include/linux/bpf.h:32:
> >> include/linux/memcontrol.h:772:14: error: use of undeclared identifier
> 'lruvec'
>      772 |         __acquires(&lruvec->lru_lock)
>          |                     ^~~~~~
>    include/linux/memcontrol.h:773:13: error: use of undeclared identifier
> 'rcu'
>      773 |         __acquires(rcu)
>          |                    ^~~
>    include/linux/memcontrol.h:775:14: error: use of undeclared identifier
> 'lruvec'
>      775 |         __acquires(&lruvec->lru_lock)
>          |                     ^~~~~~
>    include/linux/memcontrol.h:776:13: error: use of undeclared identifier
> 'rcu'
>      776 |         __acquires(rcu)
>          |                    ^~~
>    include/linux/memcontrol.h:779:14: error: use of undeclared identifier
> 'lruvec'
>      779 |         __acquires(&lruvec->lru_lock)
>          |                     ^~~~~~
>    include/linux/memcontrol.h:780:13: error: use of undeclared identifier
> 'rcu'
>      780 |         __acquires(rcu)
>          |                    ^~~
>    include/linux/memcontrol.h:1507:13: error: use of undeclared identifier
> 'rcu'
>     1507 |         __acquires(rcu)
>          |                    ^~~
>    include/linux/memcontrol.h:1515:13: error: use of undeclared identifier
> 'rcu'
>     1515 |         __releases(rcu)
>          |                    ^~~
>    include/linux/memcontrol.h:1523:13: error: use of undeclared identifier
> 'rcu'
>     1523 |         __releases(rcu)
>          |                    ^~~
>    include/linux/memcontrol.h:1532:13: error: use of undeclared identifier
> 'rcu'
>     1532 |         __releases(rcu)
> 
> And I reproduced this error with the following configuration:
> 
> 1. enable CONFIG_WARN_CONTEXT_ANALYSIS_ALL
> 2. make CC=clang bzImage (clang version >= 22)
> 
> > worth the trouble. My own inclination is to drop the `__acquires`
> > annotations—mainly for performance reasons.
> 
> If no one else objects, I will drop __acquires/__releases in the next
> version.
> 

If you drop these annotations from header file and keep in the C file,
do you still get the compilation error?