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 5CE47C4332F for ; Wed, 13 Dec 2023 22:05:17 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id F13EB8D0069; Wed, 13 Dec 2023 17:05:16 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id EC3FD8D0062; Wed, 13 Dec 2023 17:05:16 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id D63BF8D0069; Wed, 13 Dec 2023 17:05:16 -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 C10698D0062 for ; Wed, 13 Dec 2023 17:05:16 -0500 (EST) Received: from smtpin18.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay08.hostedemail.com (Postfix) with ESMTP id 89D30140193 for ; Wed, 13 Dec 2023 22:05:16 +0000 (UTC) X-FDA: 81563176632.18.CB0E044 Received: from mail-ed1-f47.google.com (mail-ed1-f47.google.com [209.85.208.47]) by imf02.hostedemail.com (Postfix) with ESMTP id 7CEB48003A for ; Wed, 13 Dec 2023 22:05:14 +0000 (UTC) Authentication-Results: imf02.hostedemail.com; dkim=pass header.d=google.com header.s=20230601 header.b=Lv43uYhu; spf=pass (imf02.hostedemail.com: domain of yosryahmed@google.com designates 209.85.208.47 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=1702505114; 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=hpRHgqfJh5BqpeJeGJK+ZkQ/hLtcXMW8feO7FrAZJh4=; b=ztvwyPU67Fzi2gff/JQR+GsvODYe6i3E6jPA7XOd3WGuEGAJVr2ywYAvgVzqdsdKrhgU3i wtH+qKI3WLv+he7KaVHQ/PqlUUeViSLDBSfvnUPQR5ctWawEyvVnnoNypXC6zRa/Rde+gi 6IAZnPtb+pDAOOf/uExjikya2luskQc= ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1702505114; a=rsa-sha256; cv=none; b=r57HPPTvdP5XPLMhPCFmbfjtF8UiHEx0J461tUBQkyPf0i9+azZRj/0OCgU+qCahdpqo99 /nacoa3UCaR66u5kkYb2RFtJ9vRQbSCtmdIxkhO7y7WLszCwTlyOKs0bnuDXjWr5t2K3wR v3U0muz56V5wJpUrNAY4IUpZQyHxo58= ARC-Authentication-Results: i=1; imf02.hostedemail.com; dkim=pass header.d=google.com header.s=20230601 header.b=Lv43uYhu; spf=pass (imf02.hostedemail.com: domain of yosryahmed@google.com designates 209.85.208.47 as permitted sender) smtp.mailfrom=yosryahmed@google.com; dmarc=pass (policy=reject) header.from=google.com Received: by mail-ed1-f47.google.com with SMTP id 4fb4d7f45d1cf-551ee7d5214so22573a12.0 for ; Wed, 13 Dec 2023 14:05:13 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20230601; t=1702505112; x=1703109912; 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=hpRHgqfJh5BqpeJeGJK+ZkQ/hLtcXMW8feO7FrAZJh4=; b=Lv43uYhuzh/S1zTiNspC+V1ROnZCf3i6Pc7hEPfMq7CM6q1cqD28lfh1k2qnGbMEd4 PuSbIvG8FIQmG37OsGat9rpRbdqgobh3dn9VewJAErOwJ4/AAZzYoLZOkj+vKhvHFsEV +Hc83fZb4WCYFrNbzMPafvbYxUdaDU+Y9RqBAxZdNpf0zHLF6vOhMn9iDPH19HRi+RdI CXb2uU5bb0b+1LC4fB76T6QvdIRbgr44+np5J1rST/zR91vj28sApRpV/okYk7Sj/bPx 3/Vpr/BrD9MunRUhz/kxKCRJwW+i2mYO3t1Lf3dBp3bcp4Ner6Hdt+Q++1BMippvMFze 6CaQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1702505112; x=1703109912; 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=hpRHgqfJh5BqpeJeGJK+ZkQ/hLtcXMW8feO7FrAZJh4=; b=nCqISqEKQ5M1Kq9QvEXfGNSiQvT3S459b2iiolZWeM0dzmaPO2xwOdEGvVvAZvm+X/ 14XmDpMc9+23ZAjeZeLbkNFeJb6QT3UyLIoETTMeawNrAPwrCVQjOz8sZO3q/gHrS2OA MVLu/DK03ibWfv+ioEW+NxEysmePFQ1Xyt3LJYEYlZwNnqtpNXQA+5v1YBO9QMcn3J9t 2fiWU0bZoIPmSHjspyqnxBLyGsYfcPTeB2h8kPsANlGjlWaBYk9O4pEZjoCnlWhsewlE k4T9ExY22VLV06bffepHq4jYDQtrE3rJuPSy4QHthwmHMPN8cRsFU3aEF1rKObrbiR4E XFzQ== X-Gm-Message-State: AOJu0YxSW8yUh1vXdehXK/H57fHJUy0dPxOyt5+cWTlj7K8TUz3d3/Av oIyC9iLTJbtl5KbKIYPbB/HHd0RrFyDzawprrwTBZg== X-Google-Smtp-Source: AGHT+IHj0D439xbVDCZV0vxrTEJ/YZrCU4ZbQgr9c1xscQDm2OJvU6/8IvaINH2YDnAjorfuUlFIqdcki5RouAKQ8X0= X-Received: by 2002:a17:906:8445:b0:a1c:7671:8806 with SMTP id e5-20020a170906844500b00a1c76718806mr8942187ejy.0.1702505112434; Wed, 13 Dec 2023 14:05:12 -0800 (PST) MIME-Version: 1.0 References: <20231213130414.353244-1-yosryahmed@google.com> <20231213202325.2cq3hwpycsvxcote@google.com> In-Reply-To: From: Yosry Ahmed Date: Wed, 13 Dec 2023 14:04:33 -0800 Message-ID: Subject: Re: [PATCH] mm: memcg: remove direct use of __memcg_kmem_uncharge_page To: Shakeel Butt Cc: Matthew Wilcox , Andrew Morton , Johannes Weiner , Michal Hocko , Roman Gushchin , Muchun Song , cgroups@vger.kernel.org, linux-mm@kvack.org Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable X-Rspamd-Queue-Id: 7CEB48003A X-Rspam-User: X-Stat-Signature: ooth3muchzy9rdmuokm6eg5bakiid4m9 X-Rspamd-Server: rspam03 X-HE-Tag: 1702505114-950201 X-HE-Meta: U2FsdGVkX1/LAO4odfHt001AA341J1CsW2fQO1+PHfLpHVzE6nld/AZ/BpA7kqqMTk75smOnqELPN+fuYs62RJEUzKYjjJeqH6miMK9qM4+fKx0vtedzjCxKyC0z+tkbt6ZZgfGqIhzTevXMI9oh58kx7gF67qKep6h47CUbsgpxYVN3kBF2Qb1/CcK7ro9bU1hokHwrqWOBHwbxLebMtCIqn9jtA5chWplHxKqcm5ISLQInVi3HT6XRAqMYFc3aaewL8ZRNiH0baQ0i+YvfZnnl193SlH/CM55vA3iYxD03KmUdVVzvYLPpqz+NLmHgWBWIsYpQomeCa2F7etzZi0skgVLI5PQS5miFVsqruQfVmf6g2qz7NlCJ/77El5O+o8S9Ua5QnRbAZ50w54erbNa7s0/6fQSnwbVeRxO3IXbTah1lk8vGzU5hjpLQ/uZZEq7p2Vkn0Hs0wBriwBIonfjkf+hfuKTmw5rFu2FY5Z6DHJsuSU7u6e6MGUzACDiZB8xrviQRRQT/rKWiBcHhaChtwzQKQ/6ltg+OBHGq2hW5bQ5DcHgXBTAHO/UUez3Kf+IfGrLZDAn35R4kdzKi1VedI1+/JDKcaqmoYlY7CJln1LF1fZ5NW5fk8eWkqCwxc7Pw7/7qg7RHQNUgrMOSO8McIl8UWCpQk/Jvvh/72j6eAzHij90sAmYdAvH6QNLpqEtlSI/ZxDt0z12qN1ctrG4RbkLzO5VGllABlch824qERz/q7sEfd1gAMDe6SRQrHZJ9xlvCBJUiJlMd2+xAcViyLiD22+2+hpv+6oQV+upeFabS5OhtcYO5aPs06hy7P3C0pIpMFafYi1aHNTjOXCsADV1FaYpWK4quVk17j4CLQ3Yr3swyNGyB9xjHM3P2ke1FKyAxrQMzK2wgZ8b4Oh0eWGy9+HdnDpGcI54+AMNH+Gr2GKwsOQz+3zUIbIPhzRjXz+Qu+cxT8gio5B9 X1AGfabA Xt68rBF9GhU4XvRxQB8L5awpuNSY4nogM2Dt0hX8M9kSkTH5AK/PPxsuFfDqayRyzA548Qkmr5v7a8LKsQ8ftJq+Mbn644znZaRYVJZw6UeRoVJSpW4iNCFh/aZtH81efCFxTc5QMNmz9jjOvuF4u15/KxiYlwGleaTzpsJ6HiGZCOy1JUmuaSVzKj1dBtao4tTZOxaxw6SEebh3hUeFk6h3WxWjgZkHwkxzUL7Bv+TktzvA4wq+YTX39PR9Ci+H/u51AkORBAyrD8QfhAEzbrXXxtUhaBjTe5VLC4IxL66kbOgIjMiS5AYcCqwPGR9AAcRKva3E8X8sEJfeogoMr9ez2+V3sdhMRnmkZ X-Bogosity: Ham, tests=bogofilter, spamicity=0.000008, 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 Wed, Dec 13, 2023 at 12:58=E2=80=AFPM Shakeel Butt = wrote: > > On Wed, Dec 13, 2023 at 12:41=E2=80=AFPM Shakeel Butt wrote: > > > > On Wed, Dec 13, 2023 at 12:27=E2=80=AFPM Matthew Wilcox wrote: > > > > > > On Wed, Dec 13, 2023 at 08:23:25PM +0000, Shakeel Butt wrote: > > > > On Wed, Dec 13, 2023 at 08:31:00AM -0800, Yosry Ahmed wrote: > > > > > On Wed, Dec 13, 2023 at 8:27=E2=80=AFAM Matthew Wilcox wrote: > > > > > > > > > > > > On Wed, Dec 13, 2023 at 08:24:04AM -0800, Yosry Ahmed wrote: > > > > > > > I doubt an extra compound_head() will matter in that path, bu= t if you > > > > > > > feel strongly about it that's okay. It's a nice cleanup that'= s all. > > > > > > > > > > > > i don't even understand why you think it's a nice cleanup. > > > > > > > > > > free_pages_prepare() is directly calling __memcg_kmem_uncharge_pa= ge() > > > > > instead of memcg_kmem_uncharge_page(), and open-coding checks tha= t > > > > > already exist in both of them to avoid the unnecessary function c= all > > > > > if possible. I think this should be the job of > > > > > memcg_kmem_uncharge_page(), but it's currently missing the > > > > > PageMemcgKmem() check (which is in __memcg_kmem_uncharge_page()). > > > > > > > > > > So I think moving that check to the wrapper allows > > > > > free_pages_prepare() to call memcg_kmem_uncharge_page() and witho= ut > > > > > worrying about those memcg-specific checks. > > > > > > > > There is a (performance) reason these open coded check are present = in > > > > page_alloc.c and that is very clear for __memcg_kmem_charge_page() = but > > > > not so much for __memcg_kmem_uncharge_page(). So, for uncharge path= , > > > > this seems ok. Now to resolve Willy's concern for the fork() path, = I > > > > think we can open code the checks there. > > > > > > > > Willy, any concern with that approach? > > > > > > The justification for this change is insufficient. Or really any cha= nge > > > in this area. It's fine the way it is. "The check is done twice" is > > > really weak, when the check is so cheap (much cheaper than calling > > > compound_head!) > > > > I think that is what Yosry is trying i.e. reducing two calls to > > page_folio() to one in the page free path. > > Actually no, there will still be two calls to page_folio() even after > Yosry's change. One for PageMemcgKmem() and second in > __memcg_kmem_uncharge_page(). > > I think I agree with Willy that this patch is actually adding one more > page_folio() call to the fork code path. It is adding one more page_folio(), yes, but to the process exit path. > > Maybe we just need to remove PageMemcgKmem() check in the > free_pages_prepare() and that's all. You mean call memcg_kmem_charge_page() directly in free_pages_prepare() without the PageMemcgKmem()? I think we can do that. My understanding is that this is not the case today because we want to avoid the function call if !PageMemcgKmem(). Do you believe the cost of the function call is negligible?