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 9C3BBC433F5 for ; Wed, 5 Oct 2022 14:55:05 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 0E83B6B0072; Wed, 5 Oct 2022 10:55:05 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 06FEF6B0073; Wed, 5 Oct 2022 10:55:05 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id E527B8E0001; Wed, 5 Oct 2022 10:55:04 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0012.hostedemail.com [216.40.44.12]) by kanga.kvack.org (Postfix) with ESMTP id CB1BD6B0072 for ; Wed, 5 Oct 2022 10:55:04 -0400 (EDT) Received: from smtpin19.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay02.hostedemail.com (Postfix) with ESMTP id 13252120F22 for ; Wed, 5 Oct 2022 14:55:04 +0000 (UTC) X-FDA: 79987193328.19.AC6D06E Received: from mail-wr1-f50.google.com (mail-wr1-f50.google.com [209.85.221.50]) by imf27.hostedemail.com (Postfix) with ESMTP id 9BF6040018 for ; Wed, 5 Oct 2022 14:55:03 +0000 (UTC) Received: by mail-wr1-f50.google.com with SMTP id r13so4843219wrj.11 for ; Wed, 05 Oct 2022 07:55:03 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20210112; h=cc:to:subject:message-id:date:from:in-reply-to:references :mime-version:from:to:cc:subject:date; bh=eV9mz6rLl17DvC65Faznhcr86ad30NDUHMTQXqdXEik=; b=fh69YG7QyQtTp9DHHvnSfm7aJT995Oy7tVzeev7VCPIuLTKIL8NqQRzHHpIzVlk8Vn k8ZAIfTR3qzR+0mswLdRFgy4jqyx/q3tAoqJ626pOumyRfpTTga7VFnd70Y+SRq03jFK CnHlrotw1YYFdAfE7NAfW0xc9tH2eQvDQoGuh3KHLENnFGyOCTeDkGPoG5Bl7v40RXX9 06GvTByA28GAbg3Xii0gYECRfYBw0HLy8y2lEBXOizEoNNgCXIXqk1sgbye6cQYxgz3Q uYmCp1r2rXb+5gu69J1d3AB3hD1e3nnxGFe4ce+E2WTOrmppdLtCMEMFTp7ftTOYQIS/ +STA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=cc:to:subject:message-id:date:from:in-reply-to:references :mime-version:x-gm-message-state:from:to:cc:subject:date; bh=eV9mz6rLl17DvC65Faznhcr86ad30NDUHMTQXqdXEik=; b=7uQGMgcInnrR8NHkYX+mAPv4gWJSZ2sE5as43r21kJ7qg7OmX5x93T8ykxUvDaairS QQ+hIxa2+R4+X6rX1xCy+Yvpw9MXTJa5JzbMSMAToCkLfXI3FanQKajChj2lZFRQkztc Zou18YObIv0jZAMywq35fnpeu8GGRlIe9+/geX1KiwXPTRRjJz+hUpkhfGXosgq8W2eN m+8I7rXKPKH6Vpwu2oXjSwjqUuhtYhORr3hv+1PDdN7+fuZIIOELiUhLhiU5u8FjcJj6 dcCA6owDkSqihoF77HtjA71gF+uTazUuTtEMfvlR2CuOyAbTI3fVrQx8RmiyqBkzDyGi n5Lg== X-Gm-Message-State: ACrzQf1yOzAXetAIugV/Xt3exefJnTRPev9IlVicaFR9sYYBsxJfYFam yI/hhxzI3f4nWVQelv8VPec+sjG7rEg1zJoTkl0XIw== X-Google-Smtp-Source: AMsMyM4pN1L/p0QyG89ATDWKg7KPb88Q+aLKViV0HctFaPVETTzaSC8A04oKFXmU4DwGESKMILdyuJHFXqutGPR8H/I= X-Received: by 2002:a5d:47a6:0:b0:22a:3862:86c8 with SMTP id 6-20020a5d47a6000000b0022a386286c8mr99881wrb.80.1664981702040; Wed, 05 Oct 2022 07:55:02 -0700 (PDT) MIME-Version: 1.0 References: <20221004233446.787056-1-yosryahmed@google.com> In-Reply-To: From: Yosry Ahmed Date: Wed, 5 Oct 2022 07:54:25 -0700 Message-ID: Subject: Re: [PATCH] mm/vmscan: check references from all memcgs for swapbacked memory To: Johannes Weiner Cc: Andrew Morton , Michal Hocko , Roman Gushchin , Shakeel Butt , Muchun Song , Greg Thelen , David Rientjes , Cgroups , Linux-MM Content-Type: text/plain; charset="UTF-8" ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1664981703; a=rsa-sha256; cv=none; b=WPjaDg0TIDXdT+reAbSrxEAL0N+nxCdjokI1iysoyydirvtAtdkoT3MT7KJoQeLHaMW1QL Jzd38BSiFWB6Slz/Y3NNRlPgzPDGLP2oEoGL4X0xGep22MOtLKFs/aAaYOiDTc8bG1NK0H ocxCQMfOfp1WNp0m5ZYFMAs/9vyqaFE= ARC-Authentication-Results: i=1; imf27.hostedemail.com; dkim=pass header.d=google.com header.s=20210112 header.b=fh69YG7Q; spf=pass (imf27.hostedemail.com: domain of yosryahmed@google.com designates 209.85.221.50 as permitted sender) smtp.mailfrom=yosryahmed@google.com; dmarc=pass (policy=reject) header.from=google.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1664981703; 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=eV9mz6rLl17DvC65Faznhcr86ad30NDUHMTQXqdXEik=; b=Q9xhqWchAcH2h+SPRduyUOLgR9GybWprsoI8vuq7L/rEVeC/B92nig250YpCH7ykqylRcZ yDbJaurf379En526vuGFHObFrRntYSgB7bwswX7m7VytHp7JMD71+vQn5AkZVHfVukkyI8 a3RFFv8Bv6+WfqGyLIu9Au0e09sPNFw= X-Rspamd-Server: rspam08 X-Rspam-User: X-Rspamd-Queue-Id: 9BF6040018 Authentication-Results: imf27.hostedemail.com; dkim=pass header.d=google.com header.s=20210112 header.b=fh69YG7Q; spf=pass (imf27.hostedemail.com: domain of yosryahmed@google.com designates 209.85.221.50 as permitted sender) smtp.mailfrom=yosryahmed@google.com; dmarc=pass (policy=reject) header.from=google.com X-Stat-Signature: 9k3m4faeknshwrsgyyjbqi3aenwdyaig X-HE-Tag: 1664981703-737638 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: On Wed, Oct 5, 2022 at 7:04 AM Johannes Weiner wrote: > > Hi Yosry, > > On Tue, Oct 04, 2022 at 11:34:46PM +0000, Yosry Ahmed wrote: > > During page/folio reclaim, we check folio is referenced using > > folio_referenced() to avoid reclaiming folios that have been recently > > accessed (hot memory). The ratinale is that this memory is likely to be > > accessed soon, and hence reclaiming it will cause a refault. > > > > For memcg reclaim, we pass in sc->target_mem_cgroup to > > folio_referenced(), which means we only check accesses to the folio > > from processes in the subtree of the target memcg. This behavior was > > originally introduced by commit bed7161a519a ("Memory controller: make > > page_referenced() cgroup aware") a long time ago. Back then, refaulted > > pages would get charged to the memcg of the process that was faulting them > > in. It made sense to only consider accesses coming from processes in the > > subtree of target_mem_cgroup. If a page was charged to memcg A but only > > being accessed by a sibling memcg B, we would reclaim it if memcg A is > > under pressure. memcg B can then fault it back in and get charged for it > > appropriately. > > > > Today, this behavior still makes sense for file pages. However, unlike > > file pages, when swapbacked pages are refaulted they are charged to the > > memcg that was originally charged for them during swapout. Which > > means that if a swapbacked page is charged to memcg A but only used by > > memcg B, and we reclaim it when memcg A is under pressure, it would > > simply be faulted back in and charged again to memcg A once memcg B > > accesses it. In that sense, accesses from all memcgs matter equally when > > considering if a swapbacked page/folio is a viable reclaim target. > > > > Add folio_referenced_memcg() which decides what memcg we should pass to > > folio_referenced() based on the folio type, and includes an elaborate > > comment about why we should do so. This should help reclaim make better > > decision and reduce refaults when reclaiming swapbacked memory that is > > used by multiple memcgs. > > Great observation, and I agree with this change. > > Just one nitpick: > > > Signed-off-by: Yosry Ahmed > > --- > > mm/vmscan.c | 38 ++++++++++++++++++++++++++++++++++---- > > 1 file changed, 34 insertions(+), 4 deletions(-) > > > > diff --git a/mm/vmscan.c b/mm/vmscan.c > > index c5a4bff11da6..f9fa0f9287e5 100644 > > --- a/mm/vmscan.c > > +++ b/mm/vmscan.c > > @@ -1443,14 +1443,43 @@ enum folio_references { > > FOLIOREF_ACTIVATE, > > }; > > > > +/* What memcg should we pass to folio_referenced()? */ > > +static struct mem_cgroup *folio_referenced_memcg(struct folio *folio, > > + struct mem_cgroup *target_memcg) > > +{ > > + /* > > + * We check references to folios to make sure we don't reclaim hot > > + * folios that are likely to be refaulted soon. We pass a memcg to > > + * folio_referenced() to only check references coming from processes in > > + * that memcg's subtree. > > + * > > + * For file folios, we only consider references from processes in the > > + * subtree of the target memcg. If a folio is charged to > > + * memcg A but is only referenced by processes in memcg B, we reclaim it > > + * if memcg A is under pressure. If it is later accessed by memcg B it > > + * will be faulted back in and charged to memcg B. For memcg A, this is > > + * called memory that should be reclaimed. > > + * > > + * On the other hand, when swapbacked folios are faulted in, they get > > + * charged to the memcg that was originally charged for them at the time > > + * of swapping out. This means that if a folio that is charged to > > + * memcg A gets swapped out, it will get charged back to A when *any* > > + * memcg accesses it. In that sense, we need to consider references from > > + * *all* processes when considering whether to reclaim a swapbacked > > + * folio. > > + */ > > + return folio_test_swapbacked(folio) ? NULL : target_memcg; > > +} > > + > > static enum folio_references folio_check_references(struct folio *folio, > > struct scan_control *sc) > > { > > int referenced_ptes, referenced_folio; > > unsigned long vm_flags; > > + struct mem_cgroup *memcg = folio_referenced_memcg(folio, > > + sc->target_mem_cgroup); > > > > - referenced_ptes = folio_referenced(folio, 1, sc->target_mem_cgroup, > > - &vm_flags); > > + referenced_ptes = folio_referenced(folio, 1, memcg, &vm_flags); > > referenced_folio = folio_test_clear_referenced(folio); > > > > /* > > @@ -2581,6 +2610,7 @@ static void shrink_active_list(unsigned long nr_to_scan, > > > > while (!list_empty(&l_hold)) { > > struct folio *folio; > > + struct mem_cgroup *memcg; > > > > cond_resched(); > > folio = lru_to_folio(&l_hold); > > @@ -2600,8 +2630,8 @@ static void shrink_active_list(unsigned long nr_to_scan, > > } > > > > /* Referenced or rmap lock contention: rotate */ > > - if (folio_referenced(folio, 0, sc->target_mem_cgroup, > > - &vm_flags) != 0) { > > + memcg = folio_referenced_memcg(folio, sc->target_mem_cgroup); > > + if (folio_referenced(folio, 0, memcg, &vm_flags) != 0) { > > Would you mind moving this to folio_referenced() directly? There is > already a comment and branch in there that IMO would extend quite > naturally to cover the new exception: > > /* > * If we are reclaiming on behalf of a cgroup, skip > * counting on behalf of references from different > * cgroups > */ > if (memcg) { > rwc.invalid_vma = invalid_folio_referenced_vma; > } > > That would keep the decision-making and doc in one place. Hi Johannes, Thanks for taking a look! I originally wanted to make the change in folio_referenced(). My only concern was that it wouldn't be clear for people looking at reclaim code in mm/vmscan.c. It would appear as if we are passing in the target memcg to folio_referenced(), and only if you look within you would realize that sometimes it ignores the passed memcg. It seemed to me that deciding whether we want to check references from one memcg or all of them is a reclaim decision, while folio_referenced() is just an rmap API that does what it is told: "if I am passed a memcg, I only look at references coming from this memcg". On the other hand, it looks like the doc has always lived in folio_referenced()/page_referenced(), so I might be overthinking this (I have been known to do this). Anyway, I don't feel strongly, just wanted to share my thoughts on the matter :) Let me know what you prefer. > > Thanks! > Johannnes