Re: [PATCH v2 15/46] mm/memcg: Add folio_uncharge_cgroup()

[Michal Hocko]

On Tue 22-06-21 13:15:20, Matthew Wilcox wrote:
> Reimplement mem_cgroup_uncharge() as a wrapper around
> folio_uncharge_cgroup().
> 
> Signed-off-by: Matthew Wilcox (Oracle) <willy@xxxxxxxxxxxxx>

Similar to the previous patch. Is there any reason why we cannot simply
stick with mem_cgroup_{un}charge and only change the parameter to folio?

> ---
>  include/linux/memcontrol.h |  5 +++++
>  mm/folio-compat.c          |  5 +++++
>  mm/memcontrol.c            | 14 +++++++-------
>  3 files changed, 17 insertions(+), 7 deletions(-)
> 
> diff --git a/include/linux/memcontrol.h b/include/linux/memcontrol.h
> index a50e5cee6d2c..d4b2bc939eee 100644
> --- a/include/linux/memcontrol.h
> +++ b/include/linux/memcontrol.h
> @@ -705,6 +705,7 @@ static inline bool mem_cgroup_below_min(struct mem_cgroup *memcg)
>  }
>  
>  int folio_charge_cgroup(struct folio *, struct mm_struct *, gfp_t);
> +void folio_uncharge_cgroup(struct folio *);
>  
>  int mem_cgroup_charge(struct page *page, struct mm_struct *mm, gfp_t gfp_mask);
>  int mem_cgroup_swapin_charge_page(struct page *page, struct mm_struct *mm,
> @@ -1224,6 +1225,10 @@ static inline int folio_charge_cgroup(struct folio *folio,
>  	return 0;
>  }
>  
> +static inline void folio_uncharge_cgroup(struct folio *folio)
> +{
> +}
> +
>  static inline int mem_cgroup_charge(struct page *page, struct mm_struct *mm,
>  				    gfp_t gfp_mask)
>  {
> diff --git a/mm/folio-compat.c b/mm/folio-compat.c
> index 1d71b8b587f8..d229b979b00d 100644
> --- a/mm/folio-compat.c
> +++ b/mm/folio-compat.c
> @@ -54,4 +54,9 @@ int mem_cgroup_charge(struct page *page, struct mm_struct *mm, gfp_t gfp)
>  {
>  	return folio_charge_cgroup(page_folio(page), mm, gfp);
>  }
> +
> +void mem_cgroup_uncharge(struct page *page)
> +{
> +	folio_uncharge_cgroup(page_folio(page));
> +}
>  #endif
> diff --git a/mm/memcontrol.c b/mm/memcontrol.c
> index 69638f84d11b..a6befc0843e7 100644
> --- a/mm/memcontrol.c
> +++ b/mm/memcontrol.c
> @@ -6717,24 +6717,24 @@ static void uncharge_page(struct page *page, struct uncharge_gather *ug)
>  }
>  
>  /**
> - * mem_cgroup_uncharge - uncharge a page
> - * @page: page to uncharge
> + * folio_uncharge_cgroup - Uncharge a folio.
> + * @folio: Folio to uncharge.
>   *
> - * Uncharge a page previously charged with mem_cgroup_charge().
> + * Uncharge a folio previously charged with folio_charge_cgroup().
>   */
> -void mem_cgroup_uncharge(struct page *page)
> +void folio_uncharge_cgroup(struct folio *folio)
>  {
>  	struct uncharge_gather ug;
>  
>  	if (mem_cgroup_disabled())
>  		return;
>  
> -	/* Don't touch page->lru of any random page, pre-check: */
> -	if (!page_memcg(page))
> +	/* Don't touch folio->lru of any random page, pre-check: */
> +	if (!folio_memcg(folio))
>  		return;
>  
>  	uncharge_gather_clear(&ug);
> -	uncharge_page(page, &ug);
> +	uncharge_page(&folio->page, &ug);
>  	uncharge_batch(&ug);
>  }
>  
> -- 
> 2.30.2

-- 
Michal Hocko
SUSE Labs