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 BD187C4332F for ; Wed, 13 Dec 2023 20:27:21 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 505818D0058; Wed, 13 Dec 2023 15:27:21 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id 4B3EB8D0049; Wed, 13 Dec 2023 15:27:21 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 356668D0058; Wed, 13 Dec 2023 15:27:21 -0500 (EST) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0011.hostedemail.com [216.40.44.11]) by kanga.kvack.org (Postfix) with ESMTP id 216358D0049 for ; Wed, 13 Dec 2023 15:27:21 -0500 (EST) Received: from smtpin12.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay05.hostedemail.com (Postfix) with ESMTP id EA0F64032E for ; Wed, 13 Dec 2023 20:27:20 +0000 (UTC) X-FDA: 81562929840.12.DFA9A0E Received: from casper.infradead.org (casper.infradead.org [90.155.50.34]) by imf08.hostedemail.com (Postfix) with ESMTP id D8A48160023 for ; Wed, 13 Dec 2023 20:27:17 +0000 (UTC) Authentication-Results: imf08.hostedemail.com; dkim=pass header.d=infradead.org header.s=casper.20170209 header.b=D8qVKtWz; dmarc=none; spf=none (imf08.hostedemail.com: domain of willy@infradead.org has no SPF policy when checking 90.155.50.34) smtp.mailfrom=willy@infradead.org ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1702499239; 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=EzlB/43el/VrIkuquoM6aTBQ8QwT/OicCQ4lPZjUMvw=; b=rH+fAZPXmNtMgporhsBtwNTKdMT2+UB7Gkzslj/WkYRExtRoNm941gU7iZRGIMi/pC/QAf /cMDvQ/YJ4l8AQSADgnBjQ7sRzbvGDA3ucxWnGJL8kA5WWewfMIRU2JiFElKPPlfRWyVas /IAguC99GpAERipWNPaSr2wxdNsBjBU= ARC-Authentication-Results: i=1; imf08.hostedemail.com; dkim=pass header.d=infradead.org header.s=casper.20170209 header.b=D8qVKtWz; dmarc=none; spf=none (imf08.hostedemail.com: domain of willy@infradead.org has no SPF policy when checking 90.155.50.34) smtp.mailfrom=willy@infradead.org ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1702499239; a=rsa-sha256; cv=none; b=pOpEbq7PfwT/+H8AaDEYJ1LA/AawDHjdYBb2B+QLR34dTqKrpSKJCoaHIwq4YkDMG48Cv0 BBSMfyYhprhdEZcOPF0fDYwgMU7b2IMb7xM9Mnb5tGgtiMnYVVJcu+selrL+Yg3LEnlp8T 5PZT4cR8Ps3t/1YSc6ntBsVav96UGyo= DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=infradead.org; s=casper.20170209; h=In-Reply-To:Content-Transfer-Encoding: Content-Type:MIME-Version:References:Message-ID:Subject:Cc:To:From:Date: Sender:Reply-To:Content-ID:Content-Description; bh=EzlB/43el/VrIkuquoM6aTBQ8QwT/OicCQ4lPZjUMvw=; b=D8qVKtWz3cgbsiAIjWU6UIIOyh dJE/yQCyEZIimgSiurlrZZG6Kby+3V/PUSlf/uMyryJg9VAOsZqUW25QzF+HO5ETI+NVsR9P/YFDz mR46KCwmMcD/dJRRCvQMPFiG79+na3PLCjY6kgGGz1ApFhDzoZeP7Z3OhYaZig+eMFLGBWYaXHO2j NT7i3O9ctYvO+7GoHA4qlcP/eOn5tpVA9sAcDm6h64SQSGUaZh0pjpCR8ws+usMWxlZ6Mu9fUSujY vC2U7tWQu4wqyG8Nrptos/lnylAawlJLzJgk6jW0cRzvP/c2+M1nZN50yWAdElhogNew6OMyvNHiX t7/Dl6yA==; Received: from willy by casper.infradead.org with local (Exim 4.94.2 #2 (Red Hat Linux)) id 1rDVox-002eoA-LS; Wed, 13 Dec 2023 20:27:07 +0000 Date: Wed, 13 Dec 2023 20:27:07 +0000 From: Matthew Wilcox To: Shakeel Butt Cc: Yosry Ahmed , Andrew Morton , Johannes Weiner , Michal Hocko , Roman Gushchin , Muchun Song , cgroups@vger.kernel.org, linux-mm@kvack.org Subject: Re: [PATCH] mm: memcg: remove direct use of __memcg_kmem_uncharge_page Message-ID: References: <20231213130414.353244-1-yosryahmed@google.com> <20231213202325.2cq3hwpycsvxcote@google.com> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: <20231213202325.2cq3hwpycsvxcote@google.com> X-Rspam-User: X-Stat-Signature: xmgj5zpa8zk45w1dbb8pfi1gw8ecacrs X-Rspamd-Server: rspam07 X-Rspamd-Queue-Id: D8A48160023 X-HE-Tag: 1702499237-644397 X-HE-Meta: U2FsdGVkX1/SaipYRAJLrTxjPVix/qANQYbCjsMJoL4nqoFLnOgzErVt7wqF2wxNwmkzPE5EpuT4af2pxrPX1XZDdz/eovSiaoGmdy/G6okNnWvGxCRauehWF/SM+bZwrVn4en7AzZqR4KamXfh9ZC6NVsEV66HSQJprWVzLQvJmsR7e5euMFrc18FROEVTFXIg8plLmwGVJCP3RWTiSRiDKkI4lYSEu8/5jDEe7GjF0zZ66j4D7fpCBkSnJ3y5FMmGhfNIntRHR1ArT1hzOOc7YwI4mnKnlGyGly2u4pWRCKy8Q+Kt/Ny1sP7SXOyKAOfld5dJVbvb3nQTJWv5zrHPhpCZTUXtS9lTdci/6t3T0pWCbcWd7gJPzKhlXP/SkvcoIl5OmzRVSn4zPttQ+3nqORUKbh/IXaYk+xojrwBzXuZAWz+NNPCwo/WK6PMOq6x5RFgQj/KyaYaPzrjqgL1CUubxT1h0cugMBJxssCP3cqTz3RpxoMVTm7djxE3sx6aotozwfJsyYTOe9F4a/ATp0ZabFhmIMTiqmYBFpaH4QW+0n6lxH17vX7+E3yPYO7ACTGtnQ/XC/4OiEdDKgfLLd/hnqoDkxhpkgQbwG2bkE9UGm4ndY5RMu64R2VvS4pIEJcxBreys2xDPo6VqAmuSC9LI29p+5IwsElqB4gnqlZEc7ht3qE4Sxrzl3qjvkrQzpIxKBe3MhEQotBxYWIOuWXS2rtFWwX4CgEkDYTyLPO1w/b8z9jWueX4dpv0lgxlBH959tWo3kV1r9T1OieMP1pp4XjDosJOzu2kcdjJcjT1Hgiq40dHRMfeGUhNFvB4creLNcJuyQd9ul9LrHv1Sbao95hg/cMelYSGZYx/xUzcMLRx3gQiacDxTeMMV23SwhzUQBiYjnNV2Evkd4jx2B3ral09BABn9ldLrYUuhhFiFoYLvsl0lxOO1/JjzX2Fk9lxLrRM1a5BTkd/2 zcpQ5z9+ vcE8EFrYIIQhSBoJ10EiCFbExx4L2Ng96VsNLXjsGcT0qmdD5jSkHa/6lZ9F4N90EjFMlULf4Rvpl7YqYI8hiNgtr/zMGc3GTUOTdbrCMExs+STDP8NUZQQkgt+PieYMc3bPvCoFVp+FT5bX4kvROzekLbmYEpw8+n/Kiz/Xi9+f8AOJFDttomzioFDd8gofieSiLy4h1NzTJPPOLo/MlIh42siYCpJxDm7B+P6nNtUChcgNg0Qk2UQ54eA== 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 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 AM 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, but 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_page() > > instead of memcg_kmem_uncharge_page(), and open-coding checks that > > already exist in both of them to avoid the unnecessary function call > > 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 without > > 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 change 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!)