linux-kernel - Re: [PATCH v2 15/46] mm/memcg: Add folio_uncharge

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <YNXYZTAx0JrTPfL2@dhcp22.suse.cz>
Date:   Fri, 25 Jun 2021 15:21:41 +0200
From:   Michal Hocko <mhocko@...e.com>
To:     Matthew Wilcox <willy@...radead.org>
Cc:     akpm@...ux-foundation.org, linux-fsdevel@...r.kernel.org,
        linux-mm@...ck.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v2 15/46] mm/memcg: Add folio_uncharge_cgroup()

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@...radead.org>
> > 
> > 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