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 E0D9BC61DA4 for ; Wed, 15 Mar 2023 04:54:59 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 4EB676B0072; Wed, 15 Mar 2023 00:54:59 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 49B216B0074; Wed, 15 Mar 2023 00:54:59 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 389C98E0001; Wed, 15 Mar 2023 00:54:59 -0400 (EDT) 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 2A8856B0072 for ; Wed, 15 Mar 2023 00:54:59 -0400 (EDT) Received: from smtpin30.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay07.hostedemail.com (Postfix) with ESMTP id F04E3161020 for ; Wed, 15 Mar 2023 04:54:58 +0000 (UTC) X-FDA: 80569917876.30.37828E9 Received: from casper.infradead.org (casper.infradead.org [90.155.50.34]) by imf06.hostedemail.com (Postfix) with ESMTP id A3AC3180013 for ; Wed, 15 Mar 2023 04:54:56 +0000 (UTC) Authentication-Results: imf06.hostedemail.com; dkim=pass header.d=infradead.org header.s=casper.20170209 header.b=UWGc5iC7; spf=none (imf06.hostedemail.com: domain of willy@infradead.org has no SPF policy when checking 90.155.50.34) smtp.mailfrom=willy@infradead.org; dmarc=none ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1678856097; 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=+5eAySqKu8quKYezY1WSwvPjQogNyS4CVabFURW422s=; b=bV4G7goRjuS5meH9P6hVpI+t4w12m6l9o7zYQmZNoEp2G0K5CbQy5Gqujn9eEKrLrN8aQl IYCk5nyGWHLvyaLXe/o8xGetlRWYAMSzhah7qBo4dP11dL/f8R70r5p5iQMaqi0oBQbgAq CLQDe5PjBOQpd0X7+mUIRYt7+VAqUbA= ARC-Authentication-Results: i=1; imf06.hostedemail.com; dkim=pass header.d=infradead.org header.s=casper.20170209 header.b=UWGc5iC7; spf=none (imf06.hostedemail.com: domain of willy@infradead.org has no SPF policy when checking 90.155.50.34) smtp.mailfrom=willy@infradead.org; dmarc=none ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1678856097; a=rsa-sha256; cv=none; b=hTEcnIP9EPOctbUu2aVs1FQOsqVGo2KD/VxUHAICae03k2P1eWkdzTsfKfzdsOs1v6iuOW IkX+0L6LTKeYKnpqH3O7pAp34Se4Sl4ED5hvg0AX8qI11wK0RAT4TWh4zh/4BTaL5h9jKp UvUZ+eDNYiJJtAXzIVCS5K0yU2WDFIM= 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=+5eAySqKu8quKYezY1WSwvPjQogNyS4CVabFURW422s=; b=UWGc5iC75MwzhUe32/obZHoEYG 9HjdVRVyOT+5NNcsHuT4FIPSntudhDB+Hgm6nvg3LhHaV6+X/rY0guINYcagEHA+zoOv3XH2qHZz9 LbyQhGU+JajvQR9XMZcIpjljLyVBelZ9bFku4mgr0ZuPjIrTQ9yWFJBFp7sJ5aM5DjqfU//miQHgN 4ocintfUcutJkUpO4tA/63ctHJK09HPIxI8yc1uClbZx6j9x5L1Tn+7bS9w8NIW5u8MYMU0bMNeeH vpLBwN4QDES2yrycumMTX+yG7sWHV1ffrOAsXryr1kYMDPKEom0KAGk845XZhgiliAum9WYD/DM9R cfNTeG5A==; Received: from willy by casper.infradead.org with local (Exim 4.94.2 #2 (Red Hat Linux)) id 1pcJ9j-00DXS8-Fq; Wed, 15 Mar 2023 04:54:31 +0000 Date: Wed, 15 Mar 2023 04:54:31 +0000 From: Matthew Wilcox To: Yosry Ahmed 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 Subject: Re: [PATCH] memcg: page_cgroup_ino() get memcg from compound_head(page) Message-ID: References: <20230313083452.1319968-1-yosryahmed@google.com> <20230313124431.fe901d79bc8c7dc96582539c@linux-foundation.org> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: X-Rspamd-Server: rspam05 X-Rspamd-Queue-Id: A3AC3180013 X-Stat-Signature: nb5t36npy1bq5itgyj6h894ejswts6br X-Rspam-User: X-HE-Tag: 1678856096-730877 X-HE-Meta: U2FsdGVkX1/bLlWTERtd/aAbCQkUCEtwCHpc0FQ+/5N4uQZf2Zqtb5/EDTyPTp3WnFaRPDwzFVV9iFVkEYYX1davUm7OnDUeFQifd62Q1cn94Q7r+bKRbrmBOk84N24pE3cizT+l3ybzV8FChj2CKkShJQ5tlDqB2p/YRCv49hto1VDWgbmsfFP4Hed8gnb+HImdVIrHrkK1k1+JpbQdkefv/CUU3NsAxF0aP+v0Pl7XXGF2M0D7xc0b4S2wjqZXB5k62+w5Q6eQZAS7Jtjqq2ppza2dIfcbR22SAwKkUOtUI0zl0bvUBVv1GBX4UueY8s/tckcHP4uyyx2rnmckmtg0AvVqc4ApGyG3NS6HesUpsjhqZKeAYzAaSn+NXc3PqQae0s9ewYN+5xr4L577ftTitA+PLJLEtn37+mxipF7X8otAkebRd/l8IpXUsot1W0zx3KTSN1OdDPhGT4c5eFJbNyj/jK6jiehmp7wxnHRE7DGP36/mxwyE8u3l70xoaJOlbPb1lKfIxmfajJlNYVmHIFE9bk5CHmlmF+tDGJn98ArGkejkVPiVTJy233nJm3CBfIRfq/KwzesI63Ezc0MGT13aVXO6Qws0aLuQ8V6va4eDcPNiC8SJvlzF/IrvEW7WVFOmUUkcPBrca4hdRzEE51dPYClLoFYvtCC7j2o9CGcOeu2IxVAOUKrLzzo0Ms/f4Q3XA9tqZDiLaMjahJfiRw4KEaW80iWuvPSk7Xsnr+sLscBLCe0t9MOjXuatLij6Bh1PoT9WWjGzhrVckm6PuUJFip8WmkdqZXAVpHJDVajdKp0+FdXC4eKFAc/5ymndJwju8hREMbdVzdZnMlBApw6lTfGnX7Hh0YyILRWRLvFBXrA1dEDY9tpYdI+SbumOR1CwnzqYXwgX5wvOgaBMfbeizFOdPojE/IubV/1OGb/rY6dUnxjQ4EH+o9qtVKOItmMQFTUnNke18X1 69pXmJwX cbHS4eGYQzrSfYHe4chnmPSHhopmPOh05F4jWuEf9figtzMZ2fjI3W/ddzlkmJh9VZSHzTJNBTsc3L5lDO3zbrIz6FUiD3s5IDtyEAX/S4Mx5LPWf652aq7dmHDZg+Ej4WPnrWFHPsH1MQQlak2iABNmZfWKNlzLW9A3lqdpShWq7lqteEDj/TFqKpiResg4ahcJpCeq5E3oe+IDGWUDXx0TAy+ZLR3qnsH10n2QWFFkal3/iSaA4k+loRX3McezwdMpv6kujRj/TsCZzm4wzlnA6eSu7z8PF6wBDIXRmnoUZ26JCE8LOyT0SiOsvQQFg2Yb1Zm0gspo0eeHXMFxCQRf6tW5UKR1S4wVH X-Bogosity: Ham, tests=bogofilter, spamicity=0.000009, version=1.2.4 Sender: owner-linux-mm@kvack.org Precedence: bulk X-Loop: owner-majordomo@kvack.org List-ID: On Mon, Mar 13, 2023 at 02:08:53PM -0700, Yosry Ahmed wrote: > On Mon, Mar 13, 2023 at 12:44 PM 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. 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 understand 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.