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 9CFBFE8FDC1 for ; Tue, 3 Oct 2023 23:22:54 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 20B3D8D009B; Tue, 3 Oct 2023 19:22:54 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 1BAE88D0003; Tue, 3 Oct 2023 19:22:54 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 0836C8D009B; Tue, 3 Oct 2023 19:22:54 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0010.hostedemail.com [216.40.44.10]) by kanga.kvack.org (Postfix) with ESMTP id ECEE38D0003 for ; Tue, 3 Oct 2023 19:22:53 -0400 (EDT) Received: from smtpin27.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay09.hostedemail.com (Postfix) with ESMTP id B468C804B4 for ; Tue, 3 Oct 2023 23:22:53 +0000 (UTC) X-FDA: 81305727426.27.AC834DB Received: from mail-ej1-f43.google.com (mail-ej1-f43.google.com [209.85.218.43]) by imf30.hostedemail.com (Postfix) with ESMTP id D57B780006 for ; Tue, 3 Oct 2023 23:22:51 +0000 (UTC) Authentication-Results: imf30.hostedemail.com; dkim=pass header.d=google.com header.s=20230601 header.b=eIBTXed5; dmarc=pass (policy=reject) header.from=google.com; spf=pass (imf30.hostedemail.com: domain of yosryahmed@google.com designates 209.85.218.43 as permitted sender) smtp.mailfrom=yosryahmed@google.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1696375371; 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:content-transfer-encoding: in-reply-to:in-reply-to:references:references:dkim-signature; bh=4Z022+hD2oB//jAKqOrdZqf+zZSE40o5uTIGWJ6ucIE=; b=IBOlTm/95wxMMXrEdrX3OC89S4XduTYCJR8HCU5E2Vby3tT9elOyu0Ew1HrDygiHKVN7O0 6mCo+q0FBE4sP0i4dAQcy0jUueHaNwcT680sv9R+GXpoAoxyHeBgW0t2NgXtcN8aQhWnDM FqYb1iAfmYgGXEvqCis2RjfS8V3c+LU= ARC-Authentication-Results: i=1; imf30.hostedemail.com; dkim=pass header.d=google.com header.s=20230601 header.b=eIBTXed5; dmarc=pass (policy=reject) header.from=google.com; spf=pass (imf30.hostedemail.com: domain of yosryahmed@google.com designates 209.85.218.43 as permitted sender) smtp.mailfrom=yosryahmed@google.com ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1696375372; a=rsa-sha256; cv=none; b=0F3WYDl0hF5i/FSUX9tRmXuM5iV2hsxeHY6wkEFld5ocjKGGWjhwc6J4CFMhDoqKpII6n3 WRbEYWJ6Phs16MOe8La5KJCQbia/iOdZUREmdAZOY8EW7UQJdG7onvESq98sPJPM213+Zf VD6XCtgOdvMgH2YZpUzXf4+zqh9T9e0= Received: by mail-ej1-f43.google.com with SMTP id a640c23a62f3a-98377c5d53eso263290466b.0 for ; Tue, 03 Oct 2023 16:22:51 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20230601; t=1696375370; x=1696980170; darn=kvack.org; h=content-transfer-encoding:cc:to:subject:message-id:date:from :in-reply-to:references:mime-version:from:to:cc:subject:date :message-id:reply-to; bh=4Z022+hD2oB//jAKqOrdZqf+zZSE40o5uTIGWJ6ucIE=; b=eIBTXed54OQtfPxwai1s5lyGLAEpeXK+kmvHjAyVs9xr9vv7ed0MBP69Az8/J/V4bi TnvA0NN5yeIEmD2GV1V5OFQzxiXY4wvW3/w41EUQSsCX39uP7yaKXlMzIQlbTZoEbRUL a2dIEiCGXeaT05k/wjRpY80n/P6f/ZsMEqN8wyoslF9Wyjz0aBkRSUXFyCX3jHhKr7gd OCfOzQWgnQDXG4lNxVFAxzBaRtR6hIli7YFkH/9zI4zp5mrTjikH7oV/8VNZXM8v5Zfd OmXuM5NooaJhmiE4hhlnZpr1GTWrcsQRTaoK2dORHbnpoLmeGrnPS9G6qxnLN57a3xtU GftQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1696375370; x=1696980170; h=content-transfer-encoding:cc:to:subject:message-id:date:from :in-reply-to:references:mime-version:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=4Z022+hD2oB//jAKqOrdZqf+zZSE40o5uTIGWJ6ucIE=; b=kEu2Ig7FSu6rDyFEtihwKgwGbZhTLBBY+Iu7diqFJY3Sz+Dx8+gU172yv/SWWQRtxo BIPZVWJR66SMXD8CpN5kSSIzzP5SapSul+2LqZsBlnlkBu6xVf2IyoW/blv4brYUy+Lv ZcsoxIORJKER3zTzWuDqMjD9kuJSabs3bIJG3OjNbAfSAwx6KDKkmUaa7zLe7Nziub5u TITrc+dc3TVLfH8A/ljRc1W/bVI3Hs4QDX/l3yooL+oIQ4rZq3e17/vE7JzbTOpVUWt0 1osJv8Ak9PFqXv8QCb26OJyJHau+fpoxqAkm3pKk5mMGGB1m6/uU/jq9K3/rjyrlddC1 fxTw== X-Gm-Message-State: AOJu0YyyQhUoUMTSOl67QaGbjTAwVPJPxB9QVzutDfnX31ClJ0qtOZl6 WTUmhq4WD0GkieuN7B5pFlo+MZ/uTcpQGR3MTSO7pA== X-Google-Smtp-Source: AGHT+IGAyGev7nf+zBtH4Km6sxjwtNzFlkUturz3cFMrNbSh0v1YhO7orCqpkZDnkSFsw8ajzWEUmfVvX5MRV4zjWmk= X-Received: by 2002:a17:906:538f:b0:99e:f3b:2f78 with SMTP id g15-20020a170906538f00b0099e0f3b2f78mr610073ejo.67.1696375370218; Tue, 03 Oct 2023 16:22:50 -0700 (PDT) MIME-Version: 1.0 References: <20231003171329.GB314430@monkey> <20231003231422.4046187-1-nphamcs@gmail.com> In-Reply-To: <20231003231422.4046187-1-nphamcs@gmail.com> From: Yosry Ahmed Date: Tue, 3 Oct 2023 16:22:10 -0700 Message-ID: Subject: Re: [PATCH] memcontrol: only transfer the memcg data for migration To: Nhat Pham Cc: akpm@linux-foundation.org, riel@surriel.com, hannes@cmpxchg.org, mhocko@kernel.org, roman.gushchin@linux.dev, shakeelb@google.com, muchun.song@linux.dev, tj@kernel.org, lizefan.x@bytedance.com, shuah@kernel.org, mike.kravetz@oracle.com, fvdl@google.com, linux-mm@kvack.org, kernel-team@meta.com, linux-kernel@vger.kernel.org, cgroups@vger.kernel.org Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable X-Rspam-User: X-Rspamd-Server: rspam12 X-Rspamd-Queue-Id: D57B780006 X-Stat-Signature: z71ntqw6rx3ej9akb9yz65uderfq1exi X-HE-Tag: 1696375371-322844 X-HE-Meta: U2FsdGVkX19UOeYBIifbM6FIHj8aLmfa6ss5g4oz5ruBTjrRpK/kM8V32z1lPYIfS98Jjxtbpsxruo4QZ/CI4kAcDmhhK1TozOxrJcZG7Jh6DsMLJ97eFQn7fDvty4cPuFlNy/NrgvaTV3nEACrdUN68H6KnWifR7d7WvV4bl0yJ84VggqZINFrjl6n/hia8je8WRWXg/qnOtnC+vxHf2/wHF4mG0KWtI+3AzXGlbwEivNA/zyk60TL0jCm7O7KYHUEwA5pRwNQJPNUxWetDBYLVpxFKR87ZeQn5L25rTeGbLHbNso6WjneCgqcrQmLztYxSoIE9YgbpRjWdk6dS4tWaxkduUjfueY7j4ODC36aLF5fqflxjM0pmkBUwqBRup8JjMhX4yMjVVo/qHZYN7C4EkCOwQLc4o4vbEQIGQjqIjyYHHk7uj1sPcrhkda+cIlx1eQxExByuXYqKxotj7yi4VJMf9VMflV7unAO/zkqLv2phaQFe2aAeZbudqPM1pAMJtLhDwdshSflLhnFva23sRFjbkIiY5MshhblvfeMm7wI+1u2C1zjxblh01D6rUJh5OTREaMdZzLNw9AGMcOOLZ3oqluRQ9UXouSvFHLjkQWCLpAR5mz/XgayPHAXxjGam0iojSJz5bvF827YO/Z16VP3H7LAbJU3Ge/XvBYiQeic/mg4akSd2FVrIgQqXxMFxgJgaFzYVJcv4qk7rr3KnuBSc+yeILixsCo43pfeqNE+FjLuO18XLQNqnM0U88YQmmb76OMSSugFejL49pXhYHFVRnUWMjxGtpgPDL2NBMppuENcR7FpeWtZX/iXazWo6BQZZhRqShuGvnYQF0QPW1cMK9r3IeKLOs11YXeRbOab56CJygXKuWBCHlD4LnimnQO7I6eRT0T4lJPDOJDnSRJidUbzXYgg8aCvh3hp3RFNe4C6lGFOotWRlotSkoLfZ+j/XCqjVRrJ4CXX 19m+OFFw RVhUWU4Uk1napewU8/gqMeis8xYzLJoy8OCsl+YdPH4AgmrS60ZTD05PdMGsQlgXt4yc7yL59zFAsp1tGh7q321Nkp2gXQHudzANHVae1i9msn7ZPypoDD8p167rC+gBS/VGAEQzpTuXOEqvzNbentvL/S/bYfZkqMe4k1yE4EaG8VRbW8EuzN58wz1+VC0uzm7DIczkBhwrZtWbq4BwA/+PPqSHVzh4PvYtmKRmEE954hfkgyJFVE0cSKi0e/1sfwSf5iEu6l5qgoy0LN+mRLG/K5jVd85Sgjcjy3vFjfENyFdCTug8LUmrNA9q/BNYvuOelLIo0lDLDGBUvelNWyLOlc2m3dYd5yh/vk9XaHgf8oWZyphc9e5Su1PMb/cPik0Ukw44NXxM3q3M= 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 Tue, Oct 3, 2023 at 4:14=E2=80=AFPM Nhat Pham wrote: > > For most migration use cases, only transfer the memcg data from the old > folio to the new folio, and clear the old folio's memcg data. No > charging and uncharging will be done. These use cases include the new > hugetlb memcg accounting behavior (which was not previously handled). > > This shaves off some work on the migration path, and avoids the > temporary double charging of a folio during its migration. > > The only exception is replace_page_cache_folio(), which will use the old > mem_cgroup_migrate() (now renamed to mem_cgroup_replace_folio). In that > context, the isolation of the old page isn't quite as thorough as with > migration, so we cannot use our new implementation directly. > > This patch is the result of the following discussion on the new hugetlb > memcg accounting behavior: > > https://lore.kernel.org/lkml/20231003171329.GB314430@monkey/ > > Reported-by: Mike Kravetz > Closes: https://lore.kernel.org/lkml/20231003171329.GB314430@monkey/ > Suggested-by: Johannes Weiner > Signed-off-by: Nhat Pham Does this patch fit before or after your series? In both cases I think there might be a problem for bisectability. > --- > include/linux/memcontrol.h | 7 ++++++ > mm/filemap.c | 2 +- > mm/memcontrol.c | 45 +++++++++++++++++++++++++++++++++++--- > mm/migrate.c | 3 +-- > 4 files changed, 51 insertions(+), 6 deletions(-) > > diff --git a/include/linux/memcontrol.h b/include/linux/memcontrol.h > index a827e2129790..e3eaa123256b 100644 > --- a/include/linux/memcontrol.h > +++ b/include/linux/memcontrol.h > @@ -711,6 +711,8 @@ static inline void mem_cgroup_uncharge_list(struct li= st_head *page_list) > > void mem_cgroup_cancel_charge(struct mem_cgroup *memcg, unsigned int nr_= pages); > > +void mem_cgroup_replace_folio(struct folio *old, struct folio *new); > + > void mem_cgroup_migrate(struct folio *old, struct folio *new); > > /** > @@ -1294,6 +1296,11 @@ static inline void mem_cgroup_cancel_charge(struct= mem_cgroup *memcg, > { > } > > +static inline void mem_cgroup_replace_folio(struct folio *old, > + struct folio *new) > +{ > +} > + > static inline void mem_cgroup_migrate(struct folio *old, struct folio *n= ew) > { > } > diff --git a/mm/filemap.c b/mm/filemap.c > index 9481ffaf24e6..673745219c82 100644 > --- a/mm/filemap.c > +++ b/mm/filemap.c > @@ -819,7 +819,7 @@ void replace_page_cache_folio(struct folio *old, stru= ct folio *new) > new->mapping =3D mapping; > new->index =3D offset; > > - mem_cgroup_migrate(old, new); > + mem_cgroup_replace_folio(old, new); > > xas_lock_irq(&xas); > xas_store(&xas, new); > diff --git a/mm/memcontrol.c b/mm/memcontrol.c > index 6660684f6f97..cbaa26605b3d 100644 > --- a/mm/memcontrol.c > +++ b/mm/memcontrol.c > @@ -7316,16 +7316,17 @@ void __mem_cgroup_uncharge_list(struct list_head = *page_list) > } > > /** > - * mem_cgroup_migrate - Charge a folio's replacement. > + * mem_cgroup_replace_folio - Charge a folio's replacement. > * @old: Currently circulating folio. > * @new: Replacement folio. > * > * Charge @new as a replacement folio for @old. @old will > - * be uncharged upon free. > + * be uncharged upon free. This is only used by the page cache > + * (in replace_page_cache_folio()). > * > * Both folios must be locked, @new->mapping must be set up. > */ > -void mem_cgroup_migrate(struct folio *old, struct folio *new) > +void mem_cgroup_replace_folio(struct folio *old, struct folio *new) > { > struct mem_cgroup *memcg; > long nr_pages =3D folio_nr_pages(new); > @@ -7364,6 +7365,44 @@ void mem_cgroup_migrate(struct folio *old, struct = folio *new) > local_irq_restore(flags); > } > > +/** > + * mem_cgroup_migrate - Transfer the memcg data from the old to the new = folio. > + * @old: Currently circulating folio. > + * @new: Replacement folio. > + * > + * Transfer the memcg data from the old folio to the new folio for migra= tion. > + * The old folio's data info will be cleared. Note that the memory count= ers > + * will remain unchanged throughout the process. > + * > + * Both folios must be locked, @new->mapping must be set up. > + */ > +void mem_cgroup_migrate(struct folio *old, struct folio *new) > +{ > + struct mem_cgroup *memcg; > + > + VM_BUG_ON_FOLIO(!folio_test_locked(old), old); > + VM_BUG_ON_FOLIO(!folio_test_locked(new), new); > + VM_BUG_ON_FOLIO(folio_test_anon(old) !=3D folio_test_anon(new), n= ew); > + VM_BUG_ON_FOLIO(folio_nr_pages(old) !=3D folio_nr_pages(new), new= ); > + > + if (mem_cgroup_disabled()) > + return; > + > + memcg =3D folio_memcg(old); > + /* > + * Note that it is normal to see !memcg for a hugetlb folio. > + * It could have been allocated when memory_hugetlb_accounting wa= s not > + * selected, for e.g. > + */ > + VM_WARN_ON_ONCE_FOLIO(!memcg, old); > + if (!memcg) > + return; > + > + /* Transfer the charge and the css ref */ > + commit_charge(new, memcg); > + old->memcg_data =3D 0; > +} > + > DEFINE_STATIC_KEY_FALSE(memcg_sockets_enabled_key); > EXPORT_SYMBOL(memcg_sockets_enabled_key); > > diff --git a/mm/migrate.c b/mm/migrate.c > index 7d1804c4a5d9..6034c7ed1d65 100644 > --- a/mm/migrate.c > +++ b/mm/migrate.c > @@ -633,8 +633,7 @@ void folio_migrate_flags(struct folio *newfolio, stru= ct folio *folio) > > folio_copy_owner(newfolio, folio); > > - if (!folio_test_hugetlb(folio)) > - mem_cgroup_migrate(folio, newfolio); > + mem_cgroup_migrate(folio, newfolio); > } > EXPORT_SYMBOL(folio_migrate_flags); > > -- > 2.34.1 >