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

[Michal Hocko]

On Fri 25-06-21 12:21:32, Matthew Wilcox wrote:
> On Fri, Jun 25, 2021 at 10:25:44AM +0200, Michal Hocko wrote:
> > 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?
> 
> There are a dozen callers of mem_cgroup_charge() and most of them
> aren't quite ready to convert to folios at this point in the patch
> series.  So either we need a new name for the variant that takes a
> folio, or we need to play fun games with _Generic to allow
> mem_cgroup_charge() to take either a folio or a page, or we convert
> all callers to open-code their call to page_folio, like this:
> 
> -	if (mem_cgroup_charge(vmf->cow_page, vma->vm_mm, GFP_KERNEL)) {
> +	if (mem_cgroup_charge(page_folio(vmf->cow_page), vma->vm_mm,
> +			GFP_KERNEL)) {
> 
> I've generally gone with creating compat functions to minimise the
> merge conflicts when people are adding new callers or changing code near
> existing ones.  But if you don't like the new name, we have options.

Well, I will not insist because I can see how the conversion is PITA in
general.
mem_cgroup_charge should be something to be added very often so if you
do not mind I would go with your above example of direct usage of
page_folio() rather than wrappers.

Thanks!
-- 
Michal Hocko
SUSE Labs