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 43128C4332F for ; Wed, 13 Dec 2023 20:58:28 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id BCF1C8D005A; Wed, 13 Dec 2023 15:58:27 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id B7ECE8D0049; Wed, 13 Dec 2023 15:58:27 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id A1F438D005A; Wed, 13 Dec 2023 15:58:27 -0500 (EST) 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 8EEDA8D0049 for ; Wed, 13 Dec 2023 15:58:27 -0500 (EST) Received: from smtpin21.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay08.hostedemail.com (Postfix) with ESMTP id 58ABB140317 for ; Wed, 13 Dec 2023 20:58:27 +0000 (UTC) X-FDA: 81563008254.21.9D128CA Received: from mail-pl1-f171.google.com (mail-pl1-f171.google.com [209.85.214.171]) by imf07.hostedemail.com (Postfix) with ESMTP id 8105540021 for ; Wed, 13 Dec 2023 20:58:25 +0000 (UTC) Authentication-Results: imf07.hostedemail.com; dkim=pass header.d=google.com header.s=20230601 header.b=gl7CEY9e; spf=pass (imf07.hostedemail.com: domain of shakeelb@google.com designates 209.85.214.171 as permitted sender) smtp.mailfrom=shakeelb@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=1702501105; 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=vjA89IEVHAVYT/DUQohpH3j1+gx/EjaXpeh999f1dzc=; b=FMp/jayKkCj2Wx2hViyXxLSRqpjYcgMh5sfbyRQxq4J2K63EC9BFXV5EfkUnLJkhQU6U66 7rqXzlfiYQmAuhuCz4eC9Fiuei+pNB36IcgmTrKAvvyUSiIKndlbRflhavB66udLsd0n4I khh5dbz79ZQP064c0xT1mdT3VlvFm4g= ARC-Authentication-Results: i=1; imf07.hostedemail.com; dkim=pass header.d=google.com header.s=20230601 header.b=gl7CEY9e; spf=pass (imf07.hostedemail.com: domain of shakeelb@google.com designates 209.85.214.171 as permitted sender) smtp.mailfrom=shakeelb@google.com; dmarc=pass (policy=reject) header.from=google.com ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1702501105; a=rsa-sha256; cv=none; b=KQJwyqBD7lMC6X7yd7E3u2S++kYFOiNTDgS+z2aPQIHD2ER3wyK5pddiDk03lm2CVTNgNy uAZr+CmWjqJCs7pbHrCj8fm2dWDyb31w3fkmMIjtv60BSUWiPEW3anYRQA9/K0Zh6Y3nFP +oNSfO3PXfP2aJRWqQObm3lTl7Pwc3I= Received: by mail-pl1-f171.google.com with SMTP id d9443c01a7336-1cc79f73e58so2655ad.1 for ; Wed, 13 Dec 2023 12:58:25 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20230601; t=1702501104; x=1703105904; 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=vjA89IEVHAVYT/DUQohpH3j1+gx/EjaXpeh999f1dzc=; b=gl7CEY9eQcbZs/2Lnjb2bQJIAulWD5vi6T7u0vaT+l797/sxxcQ23CUOA0YKLvH35B +nmvebqjNnWmAyyqxShwtLUFCRK63ufnlFuW6Xl56ABcYjRImqd6cfCf4Rl18L9jbLgu t4eYpEMJApA/wXnbO6mKDsK6IXRQi8nKXUQ+0o74CaHzFDDvRVeFCsJFskMNuIA7A7du UQjH/KHnITENjpMY5v1vBicWRUcZgprqS3QVLgFdjwt3NFCs5GKMekqZQaoPvIcxO5B1 5j+gfcKkigLyYXr9W9X30BwN5NaLQSC00RsGeC6Tw4MCF5Md6iV7gsIzUAXZf9oXBGMx Uzcg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1702501104; x=1703105904; 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=vjA89IEVHAVYT/DUQohpH3j1+gx/EjaXpeh999f1dzc=; b=kTVzh56SliwMWahzyg6e8uLQKb0so3qW8gZYN2G363VZJJMDMn3GBWzD+CUy7hPaPm FRAws45CRjHNH7KLwPpFAQ3ni8LKiJzM4rShV9PyWrXfar7ZCbHpm0Leeo/H+LIDmWAP 9L/cHyPR51z/gEE5tMHB5fiGXaqqcGp5B3PaIBi7RuEqAw8NzFRviAq3N6p7oIjWiMEi wabsJCpcdtL747EYD5aQSddpTbcoRA3IWmYEvaktSgxIbPBBIYdbe6PqqoKfg/V/UXlG LZBN/kUVzX23uyP2FI5KEN2zgH2++RfbK1gZJwbHhIitH3LMbzkzRar+SmWHnpplRl5I 12cw== X-Gm-Message-State: AOJu0YxswaEa/N3H8vsBY3HWU5Od/5PAkFeore2GIIjqPidZFDc3NEaa Qr3I8Q0+U7fqgKcH/gi6NTS40pSfgLOvt0utDFhwfw== X-Google-Smtp-Source: AGHT+IEiL4ergZMc81Oh9/1PFa2OeHSgr/9ucV1Bi0ZD5svsZR+k+77JXGoBnVON/ikzDWK1GyrNDq/Q4ZRU3bbgWi0= X-Received: by 2002:a17:902:ecc3:b0:1d0:55ef:4f76 with SMTP id a3-20020a170902ecc300b001d055ef4f76mr1289567plh.9.1702501104079; Wed, 13 Dec 2023 12:58:24 -0800 (PST) MIME-Version: 1.0 References: <20231213130414.353244-1-yosryahmed@google.com> <20231213202325.2cq3hwpycsvxcote@google.com> In-Reply-To: From: Shakeel Butt Date: Wed, 13 Dec 2023 12:58:11 -0800 Message-ID: Subject: Re: [PATCH] mm: memcg: remove direct use of __memcg_kmem_uncharge_page To: Matthew Wilcox Cc: Yosry Ahmed , 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: 8105540021 X-Rspam-User: X-Stat-Signature: joqkgrbsmem4if8j6w3xbreg58863mb7 X-Rspamd-Server: rspam01 X-HE-Tag: 1702501105-438741 X-HE-Meta: U2FsdGVkX1/TaJ/m2sjdl8UaWbzNmT2i0+JPP8uwTHBipOm4wonLSdF2bTZn3Cxqhbmd38XL8dmmsETm4aQ68XPAPKw4UAZz81XhZB2duyB0XqTXtYNgCNMVwPhHsRJUsxYv1eQRD/AJyTY7SIHRpDrsUVkhHfhhBNI7MXGoR+xCpLliavIUgI/lJW+75QySKLWlQ+CIBAUdgIu5FEnzHzEVSwQM8ZJJ8LOpRZR0kOFh6FfXM+GZbC6cNvJeloP5zrnVGzV1lEdAjGNm+Ca6bEqFbaUPG25CGGdOD3UVvEU16UqyLEFjsX4bxisNjqANr+a86cqf6Xj1MZXmVdMTIx4B9rU8r9EyHH7Pp3YLx8DdpTW4hLRKOREIEpx6rdta/eroqD2Dh4FpmRq3YZrEbXN8DMSNtZEGHKdadj22dcRn/RefQmZMQQT7YJq/Hjw+jOA0Hd3k/oh8MsfrhPjXX8l5lxXYCueIEvCQ2j+TvVG5BvD5plV5f8tV5+gY8YnUizwPXN5GS+TwtCA2iJcs4mYbsDteDaKwso3jAGPvQ2MscjzTSHZcmdYZ4+8urTDgAaRGyOW6Sm4ML9gG8Ox786/JSdEJ/xNvGu6NtdTytJo1UWCc5OAV6CD2uyO+oxEkkaCa+75fFDRk/rWZZjvxMlBjpitltTvHlASAXzAMoFYT5QidU6ObVw3Hh0ICkIoxFfh8tv9W3MNp2+L/mpTlrJWD8zb5NfmDz+Vg3tF9yC5lyFoYs383RzsluRPJ/69eBsEQhmQpAyaunQfA0QcjzEOOJDs0Kj5hnnBZlbQaszqHs/Pe9IJzEEyGqDVdeaaSX/s9wRQO2NDQyO2ItkYtnESNblehaI9w8MiA/TCi+eQGqubxKnKQFepqtrOk9+/JcJvCvC2g0T5ZgKYkOB4Io2TSekFu8hXiXCl+UscttfSV0iPpdF+00C/Y40Pun03q0dMOlUiGad2ocjsd2U6 Ladrqzle mxJcmQbp/MvZjzJrES1j8/3F+0fOXerJ7TnJrAIJdpk+BC9APjLeWxxMkmtkv85Npe4Q4g8hrNXPFVSwHmzNoK/kctx9klHrEzdeCkWJkzCJbIoY+2GVRHY/HVt5K3heWNz683RQzWX4kAa9l9er8n9jfehW45RuzTmS5qLYnjL9ujPAvpPhTWyvGn+a3mfdeJxqNIypS2kDG9qpBRF8de4pSi5MkEG2kJcU3BA+mfr20fv3aOeGib9YefHQzUg0kG8aU61LrnMGk7PH89VscE8F5Zva+V0Lx+lk/knM68KvXyomnT/6vOIObcqKCydT+bLe96ZPEj9qwiJ4TO75YGNhyPHj0vQIoE2X+JOWPRWoz71E= X-Bogosity: Ham, tests=bogofilter, spamicity=0.000183, 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: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, 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 cal= l > > > > 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() bu= t > > > 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 chang= e > > 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. Maybe we just need to remove PageMemcgKmem() check in the free_pages_prepare() and that's all.