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 C0300C25B10 for ; Mon, 6 May 2024 20:18:14 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 2692F6B0082; Mon, 6 May 2024 16:18:14 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 218F06B0089; Mon, 6 May 2024 16:18:14 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 107C36B0083; Mon, 6 May 2024 16:18:14 -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 E5ECE6B0089 for ; Mon, 6 May 2024 16:18:13 -0400 (EDT) Received: from smtpin19.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay04.hostedemail.com (Postfix) with ESMTP id 5FE121A0111 for ; Mon, 6 May 2024 20:18:13 +0000 (UTC) X-FDA: 82089082866.19.68713E3 Received: from mail-yw1-f202.google.com (mail-yw1-f202.google.com [209.85.128.202]) by imf01.hostedemail.com (Postfix) with ESMTP id 8D9A54001A for ; Mon, 6 May 2024 20:18:11 +0000 (UTC) Authentication-Results: imf01.hostedemail.com; dkim=pass header.d=google.com header.s=20230601 header.b=1GrUwL8a; dmarc=pass (policy=reject) header.from=google.com; spf=pass (imf01.hostedemail.com: domain of 3Ajs5ZgoKCJUNDHGNz6B325DD5A3.1DBA7CJM-BB9Kz19.DG5@flex--yosryahmed.bounces.google.com designates 209.85.128.202 as permitted sender) smtp.mailfrom=3Ajs5ZgoKCJUNDHGNz6B325DD5A3.1DBA7CJM-BB9Kz19.DG5@flex--yosryahmed.bounces.google.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1715026691; 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: in-reply-to:in-reply-to:references:references:dkim-signature; bh=JQwFOhdOI6jYmDLMArB0z5zEnwyrO6RqamuYEZi4wtk=; b=ZdbR1Cd6xLNLsfLHalSjylOFw5X8PToYi5gcuVAl+Xo21VaO4ACMPRF9lBY3skYDDTwsrF LOHb2RM2C7xtRCvlf4rpnzGLT1XlVtMvrZDKHYyx24stWQFwmQTEEZTS6/fK7qxa4KaI/C aF/jpppgzLGU7FzKBqNNz02B/DRB4pc= ARC-Authentication-Results: i=1; imf01.hostedemail.com; dkim=pass header.d=google.com header.s=20230601 header.b=1GrUwL8a; dmarc=pass (policy=reject) header.from=google.com; spf=pass (imf01.hostedemail.com: domain of 3Ajs5ZgoKCJUNDHGNz6B325DD5A3.1DBA7CJM-BB9Kz19.DG5@flex--yosryahmed.bounces.google.com designates 209.85.128.202 as permitted sender) smtp.mailfrom=3Ajs5ZgoKCJUNDHGNz6B325DD5A3.1DBA7CJM-BB9Kz19.DG5@flex--yosryahmed.bounces.google.com ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1715026691; a=rsa-sha256; cv=none; b=6DRvlsuMfHW4nKrZV8rGpRadx3+3mxuMAHHyHCXL2rvdPuCsk2uZFEAbf7UXJ5j2wt94iw D3ksRX/Uj2FBY9bSodcJ4nve+kHrDJ/9LCMdBJMEHeAKxnyZSx8pTqaNtxLFCpO5utIIg8 qQsVI12udQDgUlxYpvu1dzOgwJakxt0= Received: by mail-yw1-f202.google.com with SMTP id 00721157ae682-61df903af62so47775397b3.1 for ; Mon, 06 May 2024 13:18:11 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20230601; t=1715026690; x=1715631490; darn=kvack.org; h=cc:to:from:subject:message-id:references:mime-version:in-reply-to :date:from:to:cc:subject:date:message-id:reply-to; bh=JQwFOhdOI6jYmDLMArB0z5zEnwyrO6RqamuYEZi4wtk=; b=1GrUwL8aM6fHNUVpDVWgCdKamiAmqmFYQEoY2mN2ttRvkrDt6VOa9URMXUS9+4CVXr On8J8cJ5YfQKfBBfAAv41/h5/ycwWcSq3GbBAdcVFA6ZTLm8zMiCXEFuiprmfjnUUuvH J9wtkXAyb2xEsgJo5Tl6/bb4uldibet6fJ6nsGt7FbatdeG4ci0xwb/yPcd8f1gPwunf ewmKRLvxAQL17SE547S1pBct3ZsRCcpKWXwo4Nu3VHNBNrehXAOYdGDseuYv7dI9fw30 SrXQAC5c/4Rs2IlQVe2qZTCien/qsCmv94YHz+CYbcgG4jbURMGSdsymrx0lc61r1eW/ 0pSg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1715026690; x=1715631490; h=cc:to:from:subject:message-id:references:mime-version:in-reply-to :date:x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=JQwFOhdOI6jYmDLMArB0z5zEnwyrO6RqamuYEZi4wtk=; b=XfTy+Va8d+gHRXnQvCmz3U06BU8kq5Vz41zEpF2ifZWQ8i0p9HmmZ688OFKZojdlJw nZrsCMafhAUkcCvwvSeofF1FnYGVvaPpxZIRRWB7NEHNjAsTyef+isTJy0+qUyafhhhJ aTTjY3nsRlYtTrUAni+bT2HlbmHZb7fUhLovFG+uGJHHyzqGwrH7kyiURRL2wpTRA3H3 Ln7kAen4QmdK6k4++VdDxS+WOzjeFi0HaNLud59lrEGctdqQJ/GGa9OrCe/p3495AcyK iRtp3g0VvqM2LtUmmP3/nEaNzgCklVVCTu2RCb6W4TJIa8S4cEKh8LVbu/QGp8RvNGef gnTA== X-Forwarded-Encrypted: i=1; AJvYcCVXqyu1qV8NUQQiVllvsp/bLvVpUQqSgHgjpNbmrcqwXOu7R8Sk95t4XsGa/NOHHcqcYYOzLvVUgG4az0AofadHhpY= X-Gm-Message-State: AOJu0YyG0oByeHCmNDwfDeC6KCPqkEA5SrpFggqxneJlybKW5TiqjtnB 5bFeyzkPWkGpyI99UDrwFmDNjsT3vFzhu1i630wNM6okLnoUUn+BD4Ckf9cx3tP6IDjJPPjg9sK HR7huRZM4iTNBujC4Iw== X-Google-Smtp-Source: AGHT+IEvo342+Ng4YHDFjSk7GY34DyRjOBC+Ie5rEhFeaPOsQIdlHqK+1ZKnNgaFopd7uYTbNveJFBD4HB2z1C9/ X-Received: from yosry.c.googlers.com ([fda3:e722:ac3:cc00:20:ed76:c0a8:29b4]) (user=yosryahmed job=sendgmr) by 2002:a81:4f05:0:b0:61b:e75b:4806 with SMTP id d5-20020a814f05000000b0061be75b4806mr3029323ywb.1.1715026690581; Mon, 06 May 2024 13:18:10 -0700 (PDT) Date: Mon, 6 May 2024 20:18:08 +0000 In-Reply-To: Mime-Version: 1.0 References: <20240506192924.271999-1-yosryahmed@google.com> Message-ID: Subject: Re: [PATCH v2] mm: do not update memcg stats for NR_{FILE/SHMEM}_PMDMAPPED From: Yosry Ahmed To: David Hildenbrand Cc: Andrew Morton , Shakeel Butt , Johannes Weiner , Michal Hocko , Roman Gushchin , Muchun Song , linux-mm@kvack.org, cgroups@vger.kernel.org, linux-kernel@vger.kernel.org, syzbot+9319a4268a640e26b72b@syzkaller.appspotmail.com Content-Type: text/plain; charset="us-ascii" X-Rspamd-Server: rspam07 X-Rspamd-Queue-Id: 8D9A54001A X-Stat-Signature: eg19scqzcxc5i6dhrkxwr6xa4aj8wurm X-Rspam-User: X-HE-Tag: 1715026691-840692 X-HE-Meta: U2FsdGVkX1/izBWtjpIA2lNfs4oVwjb+EPOPdXVXQZtV/cRA9Q4Z4XLNTJflwq0E3OJmHH4idaum2bzSX5AnKpMutsEVw9dqmTVSjVWdfaHxie1XNMDUHtRoNBG9tm0gk1dKzj544wv72uGve8A1Uh7Qas0q9HbsG8zu0eUl0E4hO21mWLweNTrSy1EQVSxq57t4xjGdwHwJqHAvAiX7tuLGi/hfG4mgUMZfl77uBTfoFC/f/WqqXAmK6wahuxglbgxanVs33I0xGZqZGlkCZzytsumEytCbPqyj9dZOT9j4TAFOLnHZuQfHu22lDnO0oAGuPuG+R309O2OVASKiyX7IthwMWF2/lqKCK55CxWFCTAbwJIUWUJfzYhZUTkELq2l59Xt5/afvkpQAfaWdNRXWKJl+CnnOIXmh9qL3wyGaxZlOX+iluH+7DTnCKMb8TxXs14lAUvkv1I9c6Zz7WgtUpQ/gwGw7X0C2QVupQzW3f40OaPm4ilTyPOp8Gy3v4YHu5LrWgTVhnUU3jcafQRq1xZ3MxbBpY9KKysYlBsXtIDcKXVIF+jfCIYS+IdvAlMtol8yrhZHZV81x77YftmZ6AlYg5MHP/C+xkEPdU6QSbPEvYz1vktNNRqpM2pl9aHHEt+Xti64Twu+vAb1jfiTEtxwPspOgew5tufqZBunRt4TOVWR5lnmuNxJTYml5Db9GEzQATh1DCR4JMwCh1K5o1iKok0bhEHWzedUMeHBzWYydbBk76zuo7Vaxq1fauesKhwQwxC03GpvJqq1iQpvseYQ0knh26ZQWalnmTZAfz9HFId4rd4HoCjRvxLvzoG5W7uOEagKm/NVX+yfHbySo44SVpPZa0092R9DlN/7pctyeOxtW4YoXnKTAwv+pZdOOPEMu9NUekpVT/rFJiZ388jXv/+QHDsB06U4LhGbMyRMVJXz0+qbejSSzujSgzOfvnsFauEQvc/OSgHK 4XemoTpr EHe7sm07QiB7NOuWpF+XE3mQBD7TVluyz7aISqVi2y5lFfD/Cdh5ycm2zthR3eMe6e8ou87abtlECnfJoXsD2wt4G3ZC638n+nazt9T1oY/RtR7jpaqZsNybsgJF6kHa8Zte6y+H0mdhALjpS9Jjt6QAcAQYqiVfwVV2EwgVSkUN8pbOKj4guMXiOme/HMOpp5TA4jHFMHNEc5Kd53NOT88jzqaXibMwfFpplLtXNC6ZH/hofXfzdG+ncCBvhCH12AJdebogLT3HqS3UF9PJwDaGwr5cNuDlcnWilmj9Cuj1nxrMvyYQxmq315PzXbVJezADPic56TmR8U9lWsAZ6P68sBiTbPXVA7odQ5aTy/HgWhYIdYwYmHjEwwnX0CPsZOqkAbjb3Xb0pzazh8n6mQtgTQFLb1lLwvGo+JK0ettS4ddPBW98OEJPBiEiFpV2E35Dv+F5tAyX1jGphy59nplzkvEGhBKUrrJf0BDR01iVE8xLwd1pVWArjaUIjHuPUxeuKJZWiE9n+sjGuF5UEELjUkXoFvZ+Yl/XoV3LjZoWDtWYsMF2sMafnOqPBMBcMtBRiv6YskeiiyIjHTvAZwdS0+9rtlzz1WjgjZ3jHXbisUH3dwrnn4amcGPUYHCCiFQa9tcrYlGX4x08WLP+9YcbXmTnOv64jdrJbd7hY8/zPpo9DF+2p5AlVPIZHvRl7i7seHc5OLp3Xo5SJ4/+0/IAcuqv+G42zDf61Tt+HFVHyoFccIJxrDScrqpXfqdjJ1UfD2REuvA/zSzmfE7B1CxX83Q8RTb7vOETnUuqPYpdi0yM9pedQdzqlVsE84VDIsp7T/Zc53LR9gqEmgXI49Dzk2ttbdakkxvJEWvQaHnn898wPsaUMxf37gg+6enIWrm/oZENeNHMtwuPcQPnSLhFvXH7s6y2vKhCIcwWfrLBkknM= 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 Mon, May 06, 2024 at 09:50:10PM +0200, David Hildenbrand wrote: > On 06.05.24 21:29, Yosry Ahmed wrote: > > Previously, all NR_VM_EVENT_ITEMS stats were maintained per-memcg, > > although some of those fields are not exposed anywhere. Commit > > 14e0f6c957e39 ("memcg: reduce memory for the lruvec and memcg stats") > > changed this such that we only maintain the stats we actually expose > > per-memcg via a translation table. > > > > Additionally, commit 514462bbe927b ("memcg: warn for unexpected events > > and stats") added a warning if a per-memcg stat update is attempted for > > a stat that is not in the translation table. The warning started firing > > for the NR_{FILE/SHMEM}_PMDMAPPED stat updates in the rmap code. These > > stats are not maintained per-memcg, and hence are not in the translation > > table. > > > > Do not use __lruvec_stat_mod_folio() when updating NR_FILE_PMDMAPPED and > > NR_SHMEM_PMDMAPPED. Use __mod_node_page_state() instead, which updates > > the global per-node stats only. > > > > Reported-by: syzbot+9319a4268a640e26b72b@syzkaller.appspotmail.com > > Closes: https://lore.kernel.org/lkml/0000000000001b9d500617c8b23c@google.com > > Fixes: 514462bbe927 ("memcg: warn for unexpected events and stats") > > Acked-by: Shakeel Butt > > Signed-off-by: Yosry Ahmed > > --- > > mm/rmap.c | 15 +++++++++------ > > 1 file changed, 9 insertions(+), 6 deletions(-) > > > > diff --git a/mm/rmap.c b/mm/rmap.c > > index 12be4241474ab..ed7f820369864 100644 > > --- a/mm/rmap.c > > +++ b/mm/rmap.c > > @@ -1435,13 +1435,14 @@ static __always_inline void __folio_add_file_rmap(struct folio *folio, > > struct page *page, int nr_pages, struct vm_area_struct *vma, > > enum rmap_level level) > > { > > + pg_data_t *pgdat = folio_pgdat(folio); > > int nr, nr_pmdmapped = 0; > > VM_WARN_ON_FOLIO(folio_test_anon(folio), folio); > > nr = __folio_add_rmap(folio, page, nr_pages, level, &nr_pmdmapped); > > if (nr_pmdmapped) > > - __lruvec_stat_mod_folio(folio, folio_test_swapbacked(folio) ? > > + __mod_node_page_state(pgdat, folio_test_swapbacked(folio) ? > > NR_SHMEM_PMDMAPPED : NR_FILE_PMDMAPPED, nr_pmdmapped); > > if (nr) > > __lruvec_stat_mod_folio(folio, NR_FILE_MAPPED, nr); > > @@ -1493,6 +1494,7 @@ static __always_inline void __folio_remove_rmap(struct folio *folio, > > enum rmap_level level) > > { > > atomic_t *mapped = &folio->_nr_pages_mapped; > > + pg_data_t *pgdat = folio_pgdat(folio); > > int last, nr = 0, nr_pmdmapped = 0; > > bool partially_mapped = false; > > enum node_stat_item idx; > > @@ -1540,13 +1542,14 @@ static __always_inline void __folio_remove_rmap(struct folio *folio, > > } > > if (nr_pmdmapped) { > > + /* NR_{FILE/SHMEM}_PMDMAPPED are not maintained per-memcg */ > > if (folio_test_anon(folio)) > > - idx = NR_ANON_THPS; > > - else if (folio_test_swapbacked(folio)) > > - idx = NR_SHMEM_PMDMAPPED; > > + __lruvec_stat_mod_folio(folio, NR_ANON_THPS, -nr_pmdmapped); > > else > > - idx = NR_FILE_PMDMAPPED; > > - __lruvec_stat_mod_folio(folio, idx, -nr_pmdmapped); > > + __mod_node_page_state(pgdat, > > folio_pgdat(folio) should fit here easily. :) > > But I would actually suggest something like the following in mm/rmap.c I am not a big fan of this. Not because I don't like the abstraction, but because I think it doesn't go all the way. It abstracts a very certain case: updating nr_pmdmapped for file folios. I think if we opt for abstracting the stats updates in mm/rmap.c, we should go all the way with something like the following (probably split as two patches: refactoring then bug fix). WDYT about the below? diff --git a/mm/rmap.c b/mm/rmap.c index 12be4241474ab..70d6f6309da01 100644 --- a/mm/rmap.c +++ b/mm/rmap.c @@ -1269,6 +1269,28 @@ static void __page_check_anon_rmap(struct folio *folio, struct page *page, page); } +static void __foio_mod_stat(struct folio *folio, int nr, int nr_pmdmapped) +{ + int idx; + + if (nr) { + idx = folio_test_anon(folio) ? NR_ANON_MAPPED : NR_FILE_MAPPED; + __lruvec_stat_mod_folio(folio, idx, nr); + } + if (nr_pmdmapped) { + if (folio_test_anon(folio)) { + idx = NR_ANON_THPS; + __lruvec_stat_mod_folio(folio, idx, nr_pmdmapped); + } else { + /* NR_*_PMDMAPPED are not maintained per-memcg */ + idx = folio_test_swapbacked(folio) ? + NR_SHMEM_PMDMAPPED : NR_FILE_PMDMAPPED; + __mod_node_page_state(folio_pgdat(folio), idx, + nr_pmdmapped); + } + } +} + static __always_inline void __folio_add_anon_rmap(struct folio *folio, struct page *page, int nr_pages, struct vm_area_struct *vma, unsigned long address, rmap_t flags, enum rmap_level level) @@ -1276,10 +1298,6 @@ static __always_inline void __folio_add_anon_rmap(struct folio *folio, int i, nr, nr_pmdmapped = 0; nr = __folio_add_rmap(folio, page, nr_pages, level, &nr_pmdmapped); - if (nr_pmdmapped) - __lruvec_stat_mod_folio(folio, NR_ANON_THPS, nr_pmdmapped); - if (nr) - __lruvec_stat_mod_folio(folio, NR_ANON_MAPPED, nr); if (unlikely(!folio_test_anon(folio))) { VM_WARN_ON_FOLIO(!folio_test_locked(folio), folio); @@ -1297,6 +1315,8 @@ static __always_inline void __folio_add_anon_rmap(struct folio *folio, __page_check_anon_rmap(folio, page, vma, address); } + __folio_mod_stat(folio, nr, nr_pmdmapped); + if (flags & RMAP_EXCLUSIVE) { switch (level) { case RMAP_LEVEL_PTE: @@ -1393,6 +1413,7 @@ void folio_add_new_anon_rmap(struct folio *folio, struct vm_area_struct *vma, unsigned long address) { int nr = folio_nr_pages(folio); + int nr_pmdmapped = 0; VM_WARN_ON_FOLIO(folio_test_hugetlb(folio), folio); VM_BUG_ON_VMA(address < vma->vm_start || @@ -1425,10 +1446,10 @@ void folio_add_new_anon_rmap(struct folio *folio, struct vm_area_struct *vma, atomic_set(&folio->_large_mapcount, 0); atomic_set(&folio->_nr_pages_mapped, ENTIRELY_MAPPED); SetPageAnonExclusive(&folio->page); - __lruvec_stat_mod_folio(folio, NR_ANON_THPS, nr); + nr_pmdmapped = nr; } - __lruvec_stat_mod_folio(folio, NR_ANON_MAPPED, nr); + __folio_mod_stat(folio, nr, nr_pmdmapped); } static __always_inline void __folio_add_file_rmap(struct folio *folio, @@ -1440,11 +1461,7 @@ static __always_inline void __folio_add_file_rmap(struct folio *folio, VM_WARN_ON_FOLIO(folio_test_anon(folio), folio); nr = __folio_add_rmap(folio, page, nr_pages, level, &nr_pmdmapped); - if (nr_pmdmapped) - __lruvec_stat_mod_folio(folio, folio_test_swapbacked(folio) ? - NR_SHMEM_PMDMAPPED : NR_FILE_PMDMAPPED, nr_pmdmapped); - if (nr) - __lruvec_stat_mod_folio(folio, NR_FILE_MAPPED, nr); + __folio_mod_stat(folio, nr, nr_pmdmapped); /* See comments in folio_add_anon_rmap_*() */ if (!folio_test_large(folio)) @@ -1539,19 +1556,7 @@ static __always_inline void __folio_remove_rmap(struct folio *folio, break; } - if (nr_pmdmapped) { - if (folio_test_anon(folio)) - idx = NR_ANON_THPS; - else if (folio_test_swapbacked(folio)) - idx = NR_SHMEM_PMDMAPPED; - else - idx = NR_FILE_PMDMAPPED; - __lruvec_stat_mod_folio(folio, idx, -nr_pmdmapped); - } if (nr) { - idx = folio_test_anon(folio) ? NR_ANON_MAPPED : NR_FILE_MAPPED; - __lruvec_stat_mod_folio(folio, idx, -nr); - /* * Queue anon large folio for deferred split if at least one * page of the folio is unmapped and at least one page @@ -1563,6 +1568,7 @@ static __always_inline void __folio_remove_rmap(struct folio *folio, list_empty(&folio->_deferred_list)) deferred_split_folio(folio); } + __foio_mod_stat(folio, nr, nr_pmdmapped); /* * It would be tidy to reset folio_test_anon mapping when fully > > static void __folio_mod_node_file_state(folio, int nr_pages) > { > enum node_stat_item idx = NR_FILE_PMDMAPPED; > > if (folio_test_swapbacked(folio)) > idx = NR_SHMEM_PMDMAPPED; > > __mod_node_page_state(folio_pgdat(folio), idx, nr_pages); > } > > And then simply calling here > > __folio_mod_node_file_state(folio, -nr_pmdmapped); > > And likewise in __folio_add_file_rmap() > > > ... will be cleaner. > > In any case > > Acked-by: David Hildenbrand > > -- > Cheers, > > David / dhildenb >