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 14526C61DA4 for ; Wed, 15 Mar 2023 07:04:51 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 8F5C26B007B; Wed, 15 Mar 2023 03:04:51 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 8A5D06B007D; Wed, 15 Mar 2023 03:04:51 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 76F306B007E; Wed, 15 Mar 2023 03:04:51 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0013.hostedemail.com [216.40.44.13]) by kanga.kvack.org (Postfix) with ESMTP id 666F46B007B for ; Wed, 15 Mar 2023 03:04:51 -0400 (EDT) Received: from smtpin27.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay03.hostedemail.com (Postfix) with ESMTP id 333CCA10F0 for ; Wed, 15 Mar 2023 07:04:51 +0000 (UTC) X-FDA: 80570245182.27.554F49F Received: from mail-ed1-f52.google.com (mail-ed1-f52.google.com [209.85.208.52]) by imf15.hostedemail.com (Postfix) with ESMTP id 5EBDDA0009 for ; Wed, 15 Mar 2023 07:04:49 +0000 (UTC) Authentication-Results: imf15.hostedemail.com; dkim=pass header.d=google.com header.s=20210112 header.b=tmOmoMo5; spf=pass (imf15.hostedemail.com: domain of yosryahmed@google.com designates 209.85.208.52 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=1678863889; 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=ByTkjvP85l0HIVb9EAEVi9c7ngFNS0G/NftKOOEY9V0=; b=nGY+Ntk0WHFFaX4qchpwd+gjzDae6h/nUL4yIwBtL7ssHv9GeFTiScV0L2d7+NVR5sRB8y n76XRaFmWsTCL8csdMUxTOINx3CxEp0uvPykATQo1Uo88Itk04ACxbr6WydrJmMBP+qILI OyE+JWyO3XTDFWzFjrdbx6BzA4B4JGU= ARC-Authentication-Results: i=1; imf15.hostedemail.com; dkim=pass header.d=google.com header.s=20210112 header.b=tmOmoMo5; spf=pass (imf15.hostedemail.com: domain of yosryahmed@google.com designates 209.85.208.52 as permitted sender) smtp.mailfrom=yosryahmed@google.com; dmarc=pass (policy=reject) header.from=google.com ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1678863889; a=rsa-sha256; cv=none; b=wyw+GOaBPwaNkhCkjDcjlEM4SGKcnasvnqpXcAmx9V4aK/g3t0ZtECcTlRy9GBEoOgXLnj eV9s8PhHA0tSFKMdAuFtQteVko1YciEVoqXo/DMG1C2KsT5xUKIBge2E3gKXmoJ7Y77GVO r0xq3lahqwqhM4Fv0hSQW64i0I+P32M= Received: by mail-ed1-f52.google.com with SMTP id o12so71558367edb.9 for ; Wed, 15 Mar 2023 00:04:49 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20210112; t=1678863888; 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=ByTkjvP85l0HIVb9EAEVi9c7ngFNS0G/NftKOOEY9V0=; b=tmOmoMo5SXJRy2GbAyCO/7pDf4tsNZHjiNk4mq8ny24PEy55JBsMaiiVOuk6duqUCV 0EZBRHTMNIvHMtPfVe8QxTuNJds0srJzHdW8HzAwf/1pnNuCyXLXOkJ9vEyYCEfmVOGn d6frD03KpktEzpd8oAj61qo3WN7CihVoWMW5+ftcgVQ+6OG4C2MMzwbwJsQckFMTxZLm e5XQ+h4Xt5JTsH21uV0QbSYd8ns9pqRT4pHiyYN1Ic//wX7O+cfb7Ao1f5wTg5ssRrs7 5gmizMOJ9G1btbiLwPPWznDGgBvBFKnY1+bV9x6cqTefZs8ncUAx8HKI80eKQh4NLvhT 9+Vw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; t=1678863888; 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=ByTkjvP85l0HIVb9EAEVi9c7ngFNS0G/NftKOOEY9V0=; b=vpMldXmMDYCmqsF0v5ISkVwiPmgo1LOXN6J+aofwimO4fVItB3qwBlkb8XCWdsggtd UfY7eD6czNurglIseR0MOjqw8SSq/X5Gtpdny8/xbKB5G0fXLFiU7KCK2FYae8m40bEt FYkyOpkxomFeOKQQoG2ZSb+M0Oa8ZxZOepbbBTydbvUjlIkVg6uLmd1FKBGNkPGx0RLp J6aQlUHkPvs1K94DhbfhGJJGBbvNbXZoDwVsS46N9lUR9YUI2ensdQpkvZ452OSzrO16 sYZi8v64vanhgxQCIjKlekLHrI/F/ln3MM5ewmH9sbdQV38221dWrQhKtKt+C93ME1NR Llgw== X-Gm-Message-State: AO0yUKUCQ+BhBLH73mDemdf44CtXnHDWZiA/xEgWea8jqPSEU1Q6jxyM ZvR7TyiRyuYiEt7okNvgkoUy4qqJRAS1OZIK+4aq6Q== X-Google-Smtp-Source: AK7set+xTWvXGEK35WOuZE4h3qPA478af/USZfgD8M3WEO7/5P9BPf6EQk8IN4jjVk2U8XHusXWtqDfIrVV5aEIPWGQ= X-Received: by 2002:a50:8a93:0:b0:4fc:e5c:902 with SMTP id j19-20020a508a93000000b004fc0e5c0902mr839303edj.8.1678863887534; Wed, 15 Mar 2023 00:04:47 -0700 (PDT) MIME-Version: 1.0 References: <20230313083452.1319968-1-yosryahmed@google.com> <20230313124431.fe901d79bc8c7dc96582539c@linux-foundation.org> In-Reply-To: From: Yosry Ahmed Date: Wed, 15 Mar 2023 00:04:10 -0700 Message-ID: Subject: Re: [PATCH] memcg: page_cgroup_ino() get memcg from compound_head(page) To: Matthew Wilcox Cc: Andrew Morton , Hugh Dickins , Johannes Weiner , Michal Hocko , Roman Gushchin , Shakeel Butt , Muchun Song , Naoya Horiguchi , Miaohe Lin , Vladimir Davydov , linux-mm@kvack.org, cgroups@vger.kernel.org Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable X-Rspamd-Queue-Id: 5EBDDA0009 X-Stat-Signature: riryib89rb659dhrynterfr69ge1kzai X-Rspam-User: X-Rspamd-Server: rspam08 X-HE-Tag: 1678863889-261076 X-HE-Meta: U2FsdGVkX1/o4nZ3tYXrCQCCt3TbjXPqTd4kpGoHGdOMGShIujJQNw6Nqt/Ftn9g3nLbBNk0FrM6BimfVXfjnZ4KZIh0tL0Ou9GfiSiehEcQ4dS4RMKzb88SQMUNMEW+2OIhPVknwSbpw/r8vLDggHj5kTUeHPSHUAzzYaLeKikdcy2Uo2NeXNHFbfe2u8IY+so/i3/I5sMp1wIPzyEPzJVj8Mo0wzErv+UcABFtcfO8WagVX+6vDrqkRUKReTnkWw/KOfKMzVJLbwJt1d5eiYA8zzbz9Tvh8qgeQiiGhrMF+rX4TtiA1mPQs40qc7bHndu2j2Yc7lbKs7iitvgjqC4BQlTrO9ukSrCc38b5NidUhSFbqhfZaUAOejkxViMplvDiLfjLh4hRJ1M8Gwj/wUVe9VNXe/o7jO78ybcTWlApYCVJqnStDBBHM4s9Oz9y1UhIcleQTpnkMYvBEnToPFeBDuXuOm56FDmDpVFsvKSpAVzlzRMy6C52o5ATSVOgqYqvwgj+v4uYPa5K5b8YWd13T1Az+Ah8uPPSeo2ykTTwNAwtGDpD+7/pfwfJGjbkscXskzkmXwqtdxKnizXG23pKSEzRtmP/c3ay/9aOTvyYmKGj3SsMLkJD7T7vU1+erkV6hyitDIcfTzC4Q5WW5jJYuyinlBRrjgvVEn3EyvP8PnQpIypRHhi1pPQd2WjCZDgpo+LwfRd5w5p17m3pDmEClHCcB1k8q0lBSKSZn18hLkvKcyiHDJ/+pZMgv26fZPBeHXAYd/27MZ6B3PvpgpJy1z53JewtSGUtFF+6cwo9J0u1oTJcrpZ/ysixaIgQO2bIe8oCu1eLeIkLgu5BuLHN1eTHF+Cdfc6WkET/AI03eDtpR1nGzxtyfJ/BADhGg6EKisDfSbJVX9wuPrLiiuIQPSh1+X4dhxjnlcQ83+uW5z3AgjLIr8+LBFDCOqJEorbX3U27vXV/gpKTD0o mI0mgeZ7 lHsKu6uM21bMrXi7j3KrRU/5WQY39iAsfp+6Cy8DD+EB9tLK89e3tc2NiVzRGoO0mxyfgvPFun2+nZGFmHSMAFVbr9NDclYm3m82SRS45oYgioCVn9CwlTCuWe/BShb2UUlFr5qBzHp0JWmrE+trD6QXJDXDxO1NylxNDaL2cnLJHJwJQ/9nHpuk/hY5JUU+Mc/vaItEvMGFed6+wWOBafBISJrvxwo2veSImm7WXTWQ4T2Bx7nM628osMJDA46jVLXVtmaSjzkaZoXlelRFHMT8b92DWU/q6zXbtJtkDqkFrqe2l6LDeit4rpuf3opL4Z6UkCTWSpQpD29Lb+cbgl0JGLs4THMmgvcGnV5cXU1rTfXa1qyAjvAkXQVJOCLa5e45zRXErgTJtM10r7mluNsCnarwjxtaf04/Q7FPw8b7yI6l795SLeQiztdmoXLxjimmxEdjRwBt9LkkVU9P5Nh7XNHBMMDFUeZBFgwp8I+3Hlb4d0lyA9Va+1Q== X-Bogosity: Ham, tests=bogofilter, spamicity=0.000003, version=1.2.4 Sender: owner-linux-mm@kvack.org Precedence: bulk X-Loop: owner-majordomo@kvack.org List-ID: On Tue, Mar 14, 2023 at 9:54=E2=80=AFPM Matthew Wilcox wrote: > > On Mon, Mar 13, 2023 at 02:08:53PM -0700, Yosry Ahmed wrote: > > On Mon, Mar 13, 2023 at 12:44=E2=80=AFPM Andrew Morton > > wrote: > > > > > > On Mon, 13 Mar 2023 08:34:52 +0000 Yosry Ahmed wrote: > > > > > > > From: Hugh Dickins > > > > > > > > In a kernel with added WARN_ON_ONCE(PageTail) in page_memcg_check()= , we > > > > observed a warning from page_cgroup_ino() when reading > > > > /proc/kpagecgroup. > > > > > > If this is the only known situation in which page_memcg_check() is > > > passed a tail page, why does page_memcg_check() have > > > > > > if (PageTail(page)) > > > return NULL; > > > > > > ? Can we remove this to simplify, streamline and clarify? > > > > I guess it's a safety check so that we don't end up trying to cast a > > tail page to a folio. My opinion is to go one step further and change > > page_memcg_check() to do return the memcg of the head page, i.e: > > > > static inline struct mem_cgroup *page_memcg_check(struct page *page) > > { > > return folio_memcg_check(page_folio(page)); > > } > > If you look at my commit becacb04fdd4, I was preserving the existing > behaviour of page_memcg_check() when passed a tail page. It would > previously, rightly or wrongly, read the memcg_data from the tail page > and get back NULL. Right, I looked at that. I also looked at 1b7e4464d43a which added folio_memcg() and changed page_memcg()'s behavior to use page_folio() to retrieve the memcg from the head, which made me wonder why different decisions were made for these 2 helpers. Were the users of page_memcg() already passing in head pages only? > > I suspect that was not the intended behaviour, but I do not think this > patch is the right fix; it simply papers over the problem and maybe > creates a new one. Callers of page_memcg_check() should be eliminated, > precisely because of this ambiguity. It's up to the people who understan= d > each of the callers who need to make the decision to always convert the > page that they have to a folio and ask about its memcg, or whether they > want to preserve the existing behaviour of returning NULL for tail pages. > > So, I say NACK to this patch as it does not preserve existing behaviour, > and does not advance our understanding of what we have wrought. I am not sure which patch you are NACKing, the original patch from Hugh (adding compound_head() to page_cgroup_ino()) or the suggested alternative patch which changes page_memcg_check() to use page_folio(). I am assuming the latter (based on where this reply was placed). If that's the case, are you okay with the original patch? If yes, then changing page_memcg_check() to use page_folio() should be a NOP as the only 2 callers today are page_cgroup_ino() and print_page_owner_memcg(). The latter already implicitly excludes tail pages by checking if memcg_data =3D=3D 0, and we can add an explicit PageTail() check there instead. Or is it the former (original patch)?