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 EB35EC6FD1D for ; Wed, 15 Mar 2023 03:11:34 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 81E5E8E0003; Tue, 14 Mar 2023 23:11:34 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 7A7C18E0001; Tue, 14 Mar 2023 23:11:34 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 620D18E0003; Tue, 14 Mar 2023 23:11:34 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0010.hostedemail.com [216.40.44.10]) by kanga.kvack.org (Postfix) with ESMTP id 4E1F08E0001 for ; Tue, 14 Mar 2023 23:11:34 -0400 (EDT) Received: from smtpin07.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay10.hostedemail.com (Postfix) with ESMTP id 14E29C0C5C for ; Wed, 15 Mar 2023 03:11:34 +0000 (UTC) X-FDA: 80569657308.07.9319ADB Received: from mail-ed1-f47.google.com (mail-ed1-f47.google.com [209.85.208.47]) by imf29.hostedemail.com (Postfix) with ESMTP id 46B87120013 for ; Wed, 15 Mar 2023 03:11:32 +0000 (UTC) Authentication-Results: imf29.hostedemail.com; dkim=pass header.d=google.com header.s=20210112 header.b=gvJCiPYb; spf=pass (imf29.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=1678849892; 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=YXqCKGMyW/C+0Hogx41I4dQn/i/iOzKSbxMPqIV3gX0=; b=VvXwyOzd85wDeeqAhcLSrxBqDMsvvS+CCd6V7EKoR9anOjS/aTQXRw8G0FhqmGvc9nzwJR qmcKFTCITHeCMdLhfS/Ifv93tiZBkHQ82Q8RNvwZj5vOpggQFL1q09KlFHUcpSG0LAJzj6 1LSwVpB2LslWbG8rq5u6dwDByQXg0fQ= ARC-Authentication-Results: i=1; imf29.hostedemail.com; dkim=pass header.d=google.com header.s=20210112 header.b=gvJCiPYb; spf=pass (imf29.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-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1678849892; a=rsa-sha256; cv=none; b=k3xIgkS9iTvXNe5mM8i7MF5TCgzIE4Ne15likMkbXjZuoQF+xobFciD0xGIeF4f7bRlyWu eOlRgaTBG9mhHQQJYdn9sW2vw8upKagzc7S9mZjKgfmqCUPJR8urFCGPbj9aVoBYRMcSBs gCqf3kgt64wbFOS+JxDrzN73ZDGjnlU= Received: by mail-ed1-f47.google.com with SMTP id cy23so70042441edb.12 for ; Tue, 14 Mar 2023 20:11:31 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20210112; t=1678849891; 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=YXqCKGMyW/C+0Hogx41I4dQn/i/iOzKSbxMPqIV3gX0=; b=gvJCiPYbBgWxA/3y98DWHdOpJYKUc0mYgM4igQE4cMa+S1vjhhHBTFcV3pd3erEvNX o9tufmTIVgzVU4Zc9pbYdrPnCy7vstq4ZMLRRzjPCFFenh/sJlNtSc7XITQUGrjSp+dD OIdhh+SFVgd8DT2bpIBTp7WaL8V2svJCYwVW2x492qCSsxGu8Llgr3OBGCEbpx4PMPA/ Ai2jMvvMwqW+sy+LdMt8lseB2EVAYv4YeN3QAgTFEQYa7a4U64tSzPo9xvhX5uO7IBFJ 4oKB9rGM9KM7U9TG6BtcDrqQCcJeGyLioy4i1JAMcFm2211dRy+TaJPMauys4R9mV1Nr EA9w== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; t=1678849891; 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=YXqCKGMyW/C+0Hogx41I4dQn/i/iOzKSbxMPqIV3gX0=; b=tgthAGIdvBFjFK9giMoBrRWx6Y7P9r5pkcX6P/pKzA0VRmtFlLrCN+/iRu+PapRmlA 37GMeJrcSHui8j5EB/F4FzAg8bjTG+CChAUsF8FVGm4GVTvJq9opRr8z/96B5jXWJoJd qL6Wk90px5y81aiJBreTIZIF80gYPaZwjxuLP00b7dirKT2wJO+LOItSXNvpRqZbqnyO IS7MHmON2fRHsV52t8cem182RTzlYAjUAcyYwMnYFgYcrvbaV9zjC8t/GtEAdIHsQJhb UMRJ995aNup3/8LpS/NrV5UG1LzdsZe5U3/fh7USa2krgOlz7VsBx+6ne0pI7b0XYPRX nnYw== X-Gm-Message-State: AO0yUKXyg6ZrHJJ/EmSDJ2VLFIkXN6E+PHX0qI2++QG9egXsOfv9lwVE vAUptJeoNgJ8j/VlMigQ3iWQVA4AkjFltO5/oedQEg== X-Google-Smtp-Source: AK7set9QCtoouRDiZmo0H0lx5dNDudt+NxwGawmXylQ936F9n1Uuf2tm9YjmxP6LkSSaBz9A1vETQzmGqqdlfe7Iz6E= X-Received: by 2002:a17:907:2069:b0:8af:4963:fb08 with SMTP id qp9-20020a170907206900b008af4963fb08mr2483530ejb.15.1678849890539; Tue, 14 Mar 2023 20:11:30 -0700 (PDT) MIME-Version: 1.0 References: <20230313083452.1319968-1-yosryahmed@google.com> <20230313124431.fe901d79bc8c7dc96582539c@linux-foundation.org> <61e8e6d3-697e-f9a5-a1fb-45a3448ee5db@redhat.com> In-Reply-To: <61e8e6d3-697e-f9a5-a1fb-45a3448ee5db@redhat.com> From: Yosry Ahmed Date: Tue, 14 Mar 2023 20:10:54 -0700 Message-ID: Subject: Re: [PATCH] memcg: page_cgroup_ino() get memcg from compound_head(page) To: Waiman Long 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, Matthew Wilcox Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable X-Rspamd-Server: rspam07 X-Rspamd-Queue-Id: 46B87120013 X-Rspam-User: X-Stat-Signature: aaaokez3ui4jm1b3fx7hwhuptjtmrrxy X-HE-Tag: 1678849892-739428 X-HE-Meta: U2FsdGVkX1/dEN27k41w1cyc7yx9kpTSVD++D+/Az4D21RIKhX4I/5LDTEDKSNLhU3D4PU9vQrUxQkvHkMEahCUw+9APSGCITF9P/UnF8snCTwJtvqHa/036aSnkrNC4mHqj+naf9Ksr3Mw9yMk5wEeU7CT5ZbWbSqMD1yIEBVze1TRzgMXomS1WtENMY0lwETqXddXENjv4bnX9k77NOMW5n4+RF7m4nTBF903oo37arKl2ZAgfHdt/2kgISaNNpC3g+u1xifGW6K6HkUafKnWBNsaGGPIp7N+6r+2vK775zsaCvAW1CQRA5ksY6Toz3sBjYNKosRazCXvJ10+ZJP/4ndB5Hp9xh9kJNdOU5AshKjLZrF9J/YH9YdsMYWjBGb6eipbQAFRcGGjS9jkF5fl7rP3eIqp0vEOasMsVkZSpnTI8hFTQsoPWed0Et4CH08SyEQkcYyHIoFe9HHe6YVWC7/K1EYCIyCJ1lF3GqubWNZYqRm7u4S5KEqPM6yDt4U2L2HuBGh9pgPA5TFPp0uYFO//Dk5Ocj6Pj1UX1MSs/cK9DeGaVwGfHB6givHWLBLB4lSSVeZsuXylyyal6YscW4q8BsrU2jnsdt2nPAdK3whe/jS8r/XXnc63OFyp3iV3EMEwcmCtQXxrWETgS83dXJrXAoaM1JVNbi0x4rnMJxaqveqwE4PA3swkd6VHTYR+inpwYolstBuGKWn9ybDRZqX6f4O0J1SQLnDJpBt9azC8gkzt6T9vfzMTbxjmhaFQeHQkG0HTemDLQZsbVBgg+JMDK+snImaO7/7SNaHyd3h8uj9SN+gZ7DazdOh37txxeSx2J2+rLt0L60PT1QI4qKE7WgkQi+FSq8KPyt+bXVm19p1gZuRPiQECPMcXNLRJO8A9yEkiRUU+0qyhpiaWnWoQybJbzxrP/B1qzgxHXxhKi56rEdsTBxfuI4wAjmMe/BGMSv2SFa8pyI1S 4QCUxY31 l3VC7rUom51J+djo/63ebk0WPAk7WCbGREU0KoUbiWbgnKSBWiTWHxHAdzXQtM2g9fDLL6v+Jy2fDbtZZU6CpOHbXWhhHL+qe5x7jiDzxaTT16WjupgRdUyd5EWxmDGpYjq8ssClVPcwToLTSjmFj9LmZcma/PHkGcUT/GV2b5MTwUyL43Jej+HzEuzR0cTOxTdjMHuyWLxD9gk4xu9fCd5KN8/4kddtEngDfIcGl4aEMOp9kGVjdBEzxgzOvVn1MNAabhmgySwVgq+5lK5d+D/QghWB7An/9Yn1pf8pqUGorxCVpW6c3JcTYpSVHIbsGap4J00G1Gm+SDaXHUHB1f5d1cObyvpJq7wTEVccywXXk4bmeoeZaid4oVXGm5E7XIebjv0tFIjvUhmIlOwLGUsz55RrSbIjKyeuBzQt5apGca9J29v089/xjYbbp8HYAG1ujpSLBH3OERVMxDbQD2ILecg== 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: On Tue, Mar 14, 2023 at 8:07=E2=80=AFPM Waiman Long wr= ote: > > On 3/13/23 17:08, 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)); > > } > > > > This makes it consistent with page_memcg(), and makes sure future > > users are getting the "correct" memcg for whatever page they pass in. > > I am interested to hear other folks' opinions here. > > > > The only other user today is print_page_owner_memcg(). I am not sure > > if it's doing the right thing by explicitly reading page->memcg_data, > > but it is already excluding pages that have page->memcg_data =3D=3D 0, > > which should be the case for tail pages. > > It is reading memcg_data directly to see if it is slab cache page. It is > currently skipping page that does not have memcg_data set. IIUC this skips tail pages, because they should always have page->memcg_data =3D=3D 0, even if they are charged to a memcg. To correctly get their memcg we should read it from the compound_head()/page_folio(). My 2c, we can check PageSlab() to print the extra message for slab pages, instead of reading memcg_data directly, which kinda breaks the abstraction created by the various helpers for reading a page memcg. Someone can easily change something in how memcg_data is interpreted in those helpers without realizing that page_owner is also reading it. > > Cheers, > Longman >