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 vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id 35C35E7B61F for ; Wed, 4 Oct 2023 14:17:27 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S242878AbjJDOR2 (ORCPT ); Wed, 4 Oct 2023 10:17:28 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:58686 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S242770AbjJDOR0 (ORCPT ); Wed, 4 Oct 2023 10:17:26 -0400 Received: from mail-qk1-x72d.google.com (mail-qk1-x72d.google.com [IPv6:2607:f8b0:4864:20::72d]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 8EFF2C0 for ; Wed, 4 Oct 2023 07:17:22 -0700 (PDT) Received: by mail-qk1-x72d.google.com with SMTP id af79cd13be357-7743448d88eso150621185a.2 for ; Wed, 04 Oct 2023 07:17:22 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=cmpxchg-org.20230601.gappssmtp.com; s=20230601; t=1696429041; x=1697033841; darn=vger.kernel.org; h=in-reply-to:content-disposition:mime-version:references:message-id :subject:cc:to:from:date:from:to:cc:subject:date:message-id:reply-to; bh=0iVKRRlivbZawLpQE+aqhCrFfCHAJWFRH4b1xYHHXmU=; b=Uen7Uk+OcZfpZ6NOOR/CtLouBvKuDLkL6h8xwIoLLP0AGP0SztviHiyReGovSM0ChO OKaUwFlIZpHQBfmAC+cVABbNwf3z7vDWdme8oLUAlgiJVADLkO7y9/bLsZEFmUFk6/yr /6U0ze/AsCJ0YnMvWgYSUbQ0KWaC29CjGlWGd/5VVgIshzon5YffLqZI0O4VbveobISn bwKnRKlu+XX7S39ZvTlgz7wWzJJcLy35tXa/BGQGyHjaI5ls1S7lkygPk53EwcYOU6VX YsapwNZRtocPIFQU7gVrKZdiV4Wjr3v21kcwJyn14tkcVpmksMAAxN19w2BgrslkGy99 G3qw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1696429041; x=1697033841; h=in-reply-to:content-disposition:mime-version:references:message-id :subject:cc:to:from:date:x-gm-message-state:from:to:cc:subject:date :message-id:reply-to; bh=0iVKRRlivbZawLpQE+aqhCrFfCHAJWFRH4b1xYHHXmU=; b=FSkMPU8ymFVrok/8rsb66dZX8R899D057eFvccGED9Xchzl+wKGr7NhMmJNMe7HL9c RT2Pqc9z3gXb9U04Be3p79UC+eAC2WABPKo4B9gwbMKYUtv4FGEheOqkgHUr13AqTMBo +dVwi5CsOXN3tpzZJL2OGlTH06d3UcpzIlQLaLP8xJDLTbnmAD5SvQeicP5vHdKeg+0a 9J/5fbOjoZikIf0cZl8DQlALBCbnnWOI/+Qgs6Yxd+h8spnNCLwsNVToEDtuZdOHzIRh U300MG6djPX3WSIv7p2sIXcIaTts6kke4ed0Vi5gOoUSatoaHg9/NVHff9vgooV/F6mw EkDw== X-Gm-Message-State: AOJu0YxuFiOiIxUXVROJQjOYhcbahhi/4b9iVjOc7PFHMse9CT89WvnS RLE4KQqqhTBAhqFHuJKPCjKufA== X-Google-Smtp-Source: AGHT+IHwbweLrbblB6DFXPXXebkAKQNIMi3Tti+rzdpJw79HDk06ZZMQubaZZkRYtiU5APTTeVDCUg== X-Received: by 2002:a05:620a:454c:b0:773:b623:72f7 with SMTP id u12-20020a05620a454c00b00773b62372f7mr3286274qkp.50.1696429041546; Wed, 04 Oct 2023 07:17:21 -0700 (PDT) Received: from localhost ([2620:10d:c091:400::5:753d]) by smtp.gmail.com with ESMTPSA id g23-20020a05620a109700b007756736aee0sm1280773qkk.115.2023.10.04.07.17.21 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 04 Oct 2023 07:17:21 -0700 (PDT) Date: Wed, 4 Oct 2023 10:17:20 -0400 From: Johannes Weiner To: Nhat Pham Cc: akpm@linux-foundation.org, riel@surriel.com, 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, yosryahmed@google.com, fvdl@google.com, linux-mm@kvack.org, kernel-team@meta.com, linux-kernel@vger.kernel.org, cgroups@vger.kernel.org Subject: Re: [PATCH] memcontrol: only transfer the memcg data for migration Message-ID: <20231004141720.GA35281@cmpxchg.org> References: <20231003171329.GB314430@monkey> <20231003231422.4046187-1-nphamcs@gmail.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20231003231422.4046187-1-nphamcs@gmail.com> Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Tue, Oct 03, 2023 at 04:14:22PM -0700, 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 For squashing, the patch title should be: hugetlb: memcg: account hugetlb-backed memory in memory controller fix However, I think this should actually be split out. It changes how all pages are cgroup-migrated, which is a bit too large of a side effect for the hugetlb accounting patch itself. Especially because the reasoning outlined above will get lost once this fixup is folded. IOW, send one prep patch, to go before the series, which splits mem_cgroup_replace_folio() and does the mem_cgroup_migrate() optimization() with the above explanation. Then send a fixlet for the hugetlb accounting patch that removes the !hugetlb-conditional for the mem_cgroup_migrate() call. If you're clear in the queueing instructions for both patches, Andrew can probably do it in-place without having to resend everything :) > +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) != folio_test_anon(new), new); > + VM_BUG_ON_FOLIO(folio_nr_pages(old) != folio_nr_pages(new), new); > + > + if (mem_cgroup_disabled()) > + return; > + > + memcg = folio_memcg(old); > + /* > + * Note that it is normal to see !memcg for a hugetlb folio. > + * It could have been allocated when memory_hugetlb_accounting was not > + * selected, for e.g. Is that sentence truncated? > + */ > + VM_WARN_ON_ONCE_FOLIO(!memcg, old); > + if (!memcg) > + return; If this is expected to happen, it shouldn't warn: VM_WARN_ON_ONCE(!folio_test_hugetlb(old) && !memcg, old);