Re: [PATCH v1 09/26] writeback: prevent memory cgroup release in writeback module

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

On Tue, Oct 28, 2025 at 09:58:22PM +0800, Qi Zheng wrote:
> From: Muchun Song <songmuchun@bytedance.com>
> 
> In the near future, a folio will no longer pin its corresponding
> memory cgroup. To ensure safety, it will only be appropriate to
> hold the rcu read lock or acquire a reference to the memory cgroup
> returned by folio_memcg(), thereby preventing it from being released.
> 
> In the current patch, the function get_mem_cgroup_css_from_folio()
> and the rcu read lock are employed to safeguard against the release
> of the memory cgroup.
> 
> This serves as a preparatory measure for the reparenting of the
> LRU pages.
> 
> Signed-off-by: Muchun Song <songmuchun@bytedance.com>
> Signed-off-by: Qi Zheng <zhengqi.arch@bytedance.com>
> ---

Looks good to me,
Reviewed-by: Harry Yoo <harry.yoo@oracle.com>

>  fs/fs-writeback.c                | 22 +++++++++++-----------
>  include/linux/memcontrol.h       |  9 +++++++--
>  include/trace/events/writeback.h |  3 +++
>  mm/memcontrol.c                  | 14 ++++++++------
>  4 files changed, 29 insertions(+), 19 deletions(-)
> 
> diff --git a/include/linux/memcontrol.h b/include/linux/memcontrol.h
> index 9fdbd4970021d..174e52d8e7039 100644
> --- a/include/linux/memcontrol.h
> +++ b/include/linux/memcontrol.h
> @@ -895,7 +895,7 @@ static inline bool mm_match_cgroup(struct mm_struct *mm,
>  	return match;
>  }
>  
> -struct cgroup_subsys_state *mem_cgroup_css_from_folio(struct folio *folio);
> +struct cgroup_subsys_state *get_mem_cgroup_css_from_folio(struct folio *folio);
>  ino_t page_cgroup_ino(struct page *page);
>  
>  static inline bool mem_cgroup_online(struct mem_cgroup *memcg)
> @@ -1577,9 +1577,14 @@ static inline void mem_cgroup_track_foreign_dirty(struct folio *folio,
>  	if (mem_cgroup_disabled())
>  		return;
>  
> +	if (!folio_memcg_charged(folio))
> +		return;
> +
> +	rcu_read_lock();
>  	memcg = folio_memcg(folio);
> -	if (unlikely(memcg && &memcg->css != wb->memcg_css))
> +	if (unlikely(&memcg->css != wb->memcg_css))
>  		mem_cgroup_track_foreign_dirty_slowpath(folio, wb);
> +	rcu_read_unlock();

I was wondering if it's fine to record foreign dirty info to a dying cgroup
and lose this info, but mem_cgroup_track_foreign_dirty_slowpath() says:

  "The mechanism only remembers IDs and doesn't hold any object references.
   As being wrong occasionally doesn't matter, updates and accesses to the
   records are lockless and racy"

so this looks fine.

-- 
Cheers,
Harry / Hyeonggon