From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by smtp.lore.kernel.org (Postfix) with ESMTP id 1AC29D64097 for ; Fri, 8 Nov 2024 22:43:06 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 9232D6B00A0; Fri, 8 Nov 2024 17:43:05 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id 8D25D6B00A2; Fri, 8 Nov 2024 17:43:05 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 79A236B00B2; Fri, 8 Nov 2024 17:43:05 -0500 (EST) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0015.hostedemail.com [216.40.44.15]) by kanga.kvack.org (Postfix) with ESMTP id 589EC6B00A0 for ; Fri, 8 Nov 2024 17:43:05 -0500 (EST) Received: from smtpin13.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay06.hostedemail.com (Postfix) with ESMTP id CD377ACA74 for ; Fri, 8 Nov 2024 22:43:04 +0000 (UTC) X-FDA: 82764402798.13.3BEA296 Received: from out-175.mta0.migadu.com (out-175.mta0.migadu.com [91.218.175.175]) by imf17.hostedemail.com (Postfix) with ESMTP id 315F040009 for ; Fri, 8 Nov 2024 22:42:33 +0000 (UTC) Authentication-Results: imf17.hostedemail.com; dkim=pass header.d=linux.dev header.s=key1 header.b=TEQtUttB; spf=pass (imf17.hostedemail.com: domain of shakeel.butt@linux.dev designates 91.218.175.175 as permitted sender) smtp.mailfrom=shakeel.butt@linux.dev; dmarc=pass (policy=none) header.from=linux.dev ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1731105644; h=from:from:sender:reply-to:subject:subject:date:date: message-id:message-id:to:to:cc:cc:mime-version:mime-version: content-type:content-type:content-transfer-encoding: in-reply-to:in-reply-to:references:references:dkim-signature; bh=EaU4/A6qN+s4NwESrU06mXqhjeqkwS2os0mJlrPgmQI=; b=PS2zH6pK9otkdZuZucKfVXXINx+YMTFo1xAXWWM5E1IEeBXWGJwZ1PK93bgcL+C3QZL48I TyuSpz/byA8Ad0fT21+sxQ29vWFWVmNjn5GMHQGx08DGjPMLmzWY3DXO2M95yRd3wO9eBi Yf1bRPdzFjF0tiQ54MEIniYNox8FxG0= ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1731105644; a=rsa-sha256; cv=none; b=B6qqs9rLA2zfopCH1e9ENhH8Uhz6A0O775w6HRuKhDn2iJiKGwuFpOWxtwAsId/3OxuCWw ERyiEjt7fVhPGqH3axjP/6EqRW0otQLB2fjvfQBhlXqtkfQnjrMt9sal7evtjAM5NvmDX7 VEk6qmt2Lvx6M2bdJ6apIE68ou39stM= ARC-Authentication-Results: i=1; imf17.hostedemail.com; dkim=pass header.d=linux.dev header.s=key1 header.b=TEQtUttB; spf=pass (imf17.hostedemail.com: domain of shakeel.butt@linux.dev designates 91.218.175.175 as permitted sender) smtp.mailfrom=shakeel.butt@linux.dev; dmarc=pass (policy=none) header.from=linux.dev Date: Fri, 8 Nov 2024 14:42:48 -0800 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linux.dev; s=key1; t=1731105779; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: in-reply-to:in-reply-to:references:references; bh=EaU4/A6qN+s4NwESrU06mXqhjeqkwS2os0mJlrPgmQI=; b=TEQtUttB7t3S6AALgjFubWKeTXnbmbztwdNlefzVXB8eWM0fLE+0abM2Yz35F5K4BoRd+i 93IOMjCmdltbyLl4a1yWt+Ir0jrzU/2ISpeLaqyk96dp30k/1OesU+nqJZIS7o+7JP4IMj b869/3ZD3/+6UzLWdqGjnrRwg9ql8yw= X-Report-Abuse: Please report any abuse attempt to abuse@migadu.com and include these headers. From: Shakeel Butt To: Joshua Hahn Cc: hannes@cmpxchg.org, mhocko@kernel.org, roman.gushchin@linux.dev, muchun.song@linux.dev, akpm@linux-foundation.org, cgroups@vger.kernel.org, linux-mm@kvack.org, linux-kernel@vger.kernel.org, kernel-team@meta.com Subject: Re: [PATCH 2/3] memcg/hugetlb: Introduce mem_cgroup_charge_hugetlb Message-ID: References: <20241108212946.2642085-1-joshua.hahnjy@gmail.com> <20241108212946.2642085-3-joshua.hahnjy@gmail.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20241108212946.2642085-3-joshua.hahnjy@gmail.com> X-Migadu-Flow: FLOW_OUT X-Rspam-User: X-Rspamd-Server: rspam12 X-Rspamd-Queue-Id: 315F040009 X-Stat-Signature: 4gfidwsibefn4ioymjtxi6q5arbmtaoy X-HE-Tag: 1731105753-751180 X-HE-Meta: U2FsdGVkX19gUtqEtQ7+cA6xDu5+1e0iZaXsmnRUUpXnprwsgQucrZ4cXKnyreofmFM/m0DaWoa/slxc6Ijbfu+JJbvXTMZIrDa7dOC5vUpBEWaDnaE42/Cv+B9mdhW/an9+Ojh86oaEWsTfWddm8dNumwkaNRjogKxvjbIOE6fNKV/RD1jPa6nD8kXDYXt07cBTKJ/wG0czAWdcTEFz8sOMbsLu5WFu3sAQjCXrOw3b7EXmFEceYOkD9pId5rlFr/n7RY3WTrpePFQnlldhGaBk51p2bMSEsj1hT4DSdjDqBKfaiwT03fkekp+vS/uDTsN6pTXhP4pSoM4zTrrldxWaNsGRWllaLQQIGI/1cWEfZf+anAemGeb3eWZb/QoP4ybC0C90wSFptAhRCQaFjUfmjKUtPIYXqiQU+zXhEQKSGWVn7GIfxIpenxZORxdSAhO67OpCJ+R1xzt3MIHkguzSECWsTI70B6txOSNQyxqI/qPSW0RLCWuN9md9h5jLGc3Uj7hRcrhmcf7F1Irbwu/cSyC9U0ROpgOcfE3x5f5uINV7aTRQd6hn5TahsRR8zGM0bTKcnh3j56OC9TYvPLNdOfXkPAUlYhZWCv/CbjUFNnDMgopUXkHE6pmFjK0cMyCXeueGrgEvurffJcUc89bmPcOm4Esc6QRrakgd5hF9wr6HTw9qoaOR+Q5P2zL02QPu1OM1VY3LtQnMLHeVF0JgiAdrx04gb39mhSK4BMMzljCiWgqy/JRSK6n8OWOH5maDq2G6q39/LvSHZ5aAU4B8qyP4eZEtXKc5Lf4I4mM0Xh57UPyF+Skn/1X3b5EUkrAX7KzUhCMGbtUzAdANmBmMS/MwMfnV5zgDiZQGNyX0irXZQ1dtcMjeFLeWBwjIiMj4jEFPMmWYIJU5/+aBRUon357goftRiFjvVFF533vVl/0xYzWMAJWSmhiP7pmcndemrjH5vvCOhTwhJjU p44w1MhD JlINS9CUxh1EWYQgLmJwu3TEf4dZOa4CAmTUv57nIyKy4+9EWXxi0aLRPDVtvO6K1zJ0wuM7NhB9JG4VyxIMRDTmt2qosYeARhWzP7SC4WzYu15r7BTWWgu6ZC15iLKRpUEHo5yf6ovTLls6XVxFolzviTnxNzxqco8XfNqyzKp2Tbk0nA7imohOgXK5I3CyRiRSIuZu9j1Bv1++0CcHx6k0eIjMCymb3DikQfEBCWSAglwLJaqCCT+IE+1/YTlhYVMUpCXtLUY42oulKU8rWMoAwJSQn6/Q+ojzrHtMulf6iOIVeNvP+KBWYmK/Pt4j93nFNRgoAnWIWFJA= X-Bogosity: Ham, tests=bogofilter, spamicity=0.000000, version=1.2.4 Sender: owner-linux-mm@kvack.org Precedence: bulk X-Loop: owner-majordomo@kvack.org List-ID: List-Subscribe: List-Unsubscribe: On Fri, Nov 08, 2024 at 01:29:45PM -0800, Joshua Hahn wrote: > This patch introduces mem_cgroup_charge_hugetlb, which combines the > logic of mem_cgroup{try,commit}_hugetlb. This reduces the footprint of > memcg in hugetlb code, and also consolidates the error path that memcg > can take into just one point. > > Signed-off-by: Joshua Hahn > > --- > include/linux/memcontrol.h | 2 ++ > mm/hugetlb.c | 34 ++++++++++++---------------------- > mm/memcontrol.c | 19 +++++++++++++++++++ > 3 files changed, 33 insertions(+), 22 deletions(-) > > diff --git a/include/linux/memcontrol.h b/include/linux/memcontrol.h > index bb49e0d4b377..d90c1ac791f1 100644 > --- a/include/linux/memcontrol.h > +++ b/include/linux/memcontrol.h > @@ -678,6 +678,8 @@ static inline int mem_cgroup_charge(struct folio *folio, struct mm_struct *mm, > int mem_cgroup_hugetlb_try_charge(struct mem_cgroup *memcg, gfp_t gfp, > long nr_pages); > > +int mem_cgroup_charge_hugetlb(struct folio* folio, gfp_t gfp); > + > int mem_cgroup_swapin_charge_folio(struct folio *folio, struct mm_struct *mm, > gfp_t gfp, swp_entry_t entry); > > diff --git a/mm/hugetlb.c b/mm/hugetlb.c > index fbb10e52d7ea..6f6841483039 100644 > --- a/mm/hugetlb.c > +++ b/mm/hugetlb.c > @@ -2967,21 +2967,13 @@ struct folio *alloc_hugetlb_folio(struct vm_area_struct *vma, > struct hugepage_subpool *spool = subpool_vma(vma); > struct hstate *h = hstate_vma(vma); > struct folio *folio; > - long map_chg, map_commit, nr_pages = pages_per_huge_page(h); > + long map_chg, map_commit; > long gbl_chg; > - int memcg_charge_ret, ret, idx; > + int ret, idx; > struct hugetlb_cgroup *h_cg = NULL; > - struct mem_cgroup *memcg; > bool deferred_reserve; > gfp_t gfp = htlb_alloc_mask(h) | __GFP_RETRY_MAYFAIL; > > - memcg = get_mem_cgroup_from_current(); > - memcg_charge_ret = mem_cgroup_hugetlb_try_charge(memcg, gfp, nr_pages); > - if (memcg_charge_ret == -ENOMEM) { > - mem_cgroup_put(memcg); > - return ERR_PTR(-ENOMEM); > - } > - > idx = hstate_index(h); > /* > * Examine the region/reserve map to determine if the process > @@ -2989,12 +2981,8 @@ struct folio *alloc_hugetlb_folio(struct vm_area_struct *vma, > * code of zero indicates a reservation exists (no change). > */ > map_chg = gbl_chg = vma_needs_reservation(h, vma, addr); > - if (map_chg < 0) { > - if (!memcg_charge_ret) > - mem_cgroup_cancel_charge(memcg, nr_pages); > - mem_cgroup_put(memcg); > + if (map_chg < 0) > return ERR_PTR(-ENOMEM); > - } > > /* > * Processes that did not create the mapping will have no > @@ -3092,10 +3080,15 @@ struct folio *alloc_hugetlb_folio(struct vm_area_struct *vma, > } > } > > - if (!memcg_charge_ret) > - mem_cgroup_commit_charge(folio, memcg); > - lruvec_stat_mod_folio(folio, NR_HUGETLB, pages_per_huge_page(h)); > - mem_cgroup_put(memcg); > + ret = mem_cgroup_charge_hugetlb(folio, gfp); > + if (ret == -ENOMEM) { > + spin_unlock_irq(&hugetlb_lock); spin_unlock_irq?? > + free_huge_folio(folio); free_huge_folio() will call lruvec_stat_mod_folio() unconditionally but you are only calling it on success. This may underflow the metric. > + return ERR_PTR(-ENOMEM); > + } > + else if (!ret) > + lruvec_stat_mod_folio(folio, NR_HUGETLB, > + pages_per_huge_page(h)); > > return folio; > > @@ -3110,9 +3103,6 @@ struct folio *alloc_hugetlb_folio(struct vm_area_struct *vma, > hugepage_subpool_put_pages(spool, 1); > out_end_reservation: > vma_end_reservation(h, vma, addr); > - if (!memcg_charge_ret) > - mem_cgroup_cancel_charge(memcg, nr_pages); > - mem_cgroup_put(memcg); > return ERR_PTR(-ENOSPC); > } > > diff --git a/mm/memcontrol.c b/mm/memcontrol.c > index 97f63ec9c9fb..95ee77fe27af 100644 > --- a/mm/memcontrol.c > +++ b/mm/memcontrol.c > @@ -4529,6 +4529,25 @@ int mem_cgroup_hugetlb_try_charge(struct mem_cgroup *memcg, gfp_t gfp, > return 0; > } > > +int mem_cgroup_charge_hugetlb(struct folio *folio, gfp_t gfp) > +{ > + struct mem_cgroup *memcg = get_mem_cgroup_from_current(); > + int ret = 0; > + > + if (mem_cgroup_disabled() || !memcg_accounts_hugetlb() || > + !memcg || !cgroup_subsys_on_dfl(memory_cgrp_subsys)) { > + ret = -EOPNOTSUPP; why EOPNOTSUPP? You need to return 0 here. We do want lruvec_stat_mod_folio() to be called. > + goto out; > + } > + > + if (charge_memcg(folio, memcg, gfp)) > + ret = -ENOMEM; > + > +out: > + mem_cgroup_put(memcg); > + return ret; > +} > + > /** > * mem_cgroup_swapin_charge_folio - Charge a newly allocated folio for swapin. > * @folio: folio to charge. > -- > 2.43.5 >