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 3E518C433F5 for ; Thu, 3 Mar 2022 20:08:19 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id A7E248D0002; Thu, 3 Mar 2022 15:08:18 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id A2D148D0001; Thu, 3 Mar 2022 15:08:18 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 91BE88D0002; Thu, 3 Mar 2022 15:08:18 -0500 (EST) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (relay.hostedemail.com [64.99.140.26]) by kanga.kvack.org (Postfix) with ESMTP id 7F9B98D0001 for ; Thu, 3 Mar 2022 15:08:18 -0500 (EST) Received: from smtpin05.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay01.hostedemail.com (Postfix) with ESMTP id 4914C60376 for ; Thu, 3 Mar 2022 20:08:18 +0000 (UTC) X-FDA: 79204161876.05.7C185DE Received: from mail-ed1-f44.google.com (mail-ed1-f44.google.com [209.85.208.44]) by imf16.hostedemail.com (Postfix) with ESMTP id CA76218000C for ; Thu, 3 Mar 2022 20:08:17 +0000 (UTC) Received: by mail-ed1-f44.google.com with SMTP id q17so8074230edd.4 for ; Thu, 03 Mar 2022 12:08:17 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=FWhW+YEtKAt25AZygKvKKjc7akmRR9ol+ClTeEq3Lbo=; b=IhaK1MCdFibrxiiYKMWtxOQzMJqlKexiy9nkBZphPO+tGMURXU9zv4kgOB1YX/UDOu qpzrrPE9/7Is64Tjw6dlXCGMAF6f6T9iLHFyT9wOu9pCFt4d2pifgRFSUvgD4rCbKnzo /YFQZuFmiGSVbJn09/1DxaNZ6h8T3uVRsAzNegiGma6nyvPuu2BGdxdcyAGMqitMzE1t RaONLcn5rEW1y3b05XCvt9G4f5/zFG0Bic1zEN+ANymJXXPLNFwlNSSkH6DnPrdTCEp4 dQ51ai1gVWIV9AXbmYGEKdQAsX+kKsXXekH8UN9lIWCnueTyFpPa35IB3T2EgGbxZRTp XiTQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=FWhW+YEtKAt25AZygKvKKjc7akmRR9ol+ClTeEq3Lbo=; b=SBSQx6kX8kUp6UrcidrVZoobMU5S3qsmwef5vajYFfO5VjgvUQl2IRPVPYULCyuSvJ D0SaoY5nftK/3vGeaL/37VZGoPpJso7Z2rY8Y0wvDW5JvXYJuGQQ1lzSfhIF+vn9xrlG IBaNpkjjNHOMEcluY7vpXTRu7rTzovWLmLYPMJMj1I5T1gcRnJc+P4qQf12A8957OHJY CbMoV3956au8xeZ5rXKEUzt+z1RmPs4zyEcdbNLbQyp0nH6MpD0Bp0gUtV2XLJ/ibjTm p196q8tU472pq0YXljCmh/2J1mq02QjAJtZIyWVzDrDaJuJCRBOm7HvfER3YlUeBhraF Ro+A== X-Gm-Message-State: AOAM532jMXwVAFHULdCu0gMQv2rsmhTw25tcO9vNPYjUktQm5CtwHRLb F5BjdNbkHFqnmQDzBuWsld/yvL1o/mxh8V6i5pk= X-Google-Smtp-Source: ABdhPJyWpNmagaVrWTlH/Le2wXtM87/a1EcwAsrEEtw1oF/pyNGsCesh8qEHkpkjr210cmrZhee+V8LWzy9t2BTw9ls= X-Received: by 2002:aa7:d78e:0:b0:415:d787:6226 with SMTP id s14-20020aa7d78e000000b00415d7876226mr5146631edq.121.1646338096463; Thu, 03 Mar 2022 12:08:16 -0800 (PST) MIME-Version: 1.0 References: In-Reply-To: From: Yang Shi Date: Thu, 3 Mar 2022 12:08:04 -0800 Message-ID: Subject: Re: [PATCH mmotm] mm/thp: fix NR_FILE_MAPPED accounting in page_*_file_rmap() To: Hugh Dickins Cc: Andrew Morton , "Kirill A. Shutemov" , Linux Kernel Mailing List , Linux MM Content-Type: text/plain; charset="UTF-8" X-Rspamd-Queue-Id: CA76218000C X-Stat-Signature: k33koy6paim6rbq7smycqgnkgaohmmk1 X-Rspam-User: Authentication-Results: imf16.hostedemail.com; dkim=pass header.d=gmail.com header.s=20210112 header.b=IhaK1MCd; dmarc=pass (policy=none) header.from=gmail.com; spf=pass (imf16.hostedemail.com: domain of shy828301@gmail.com designates 209.85.208.44 as permitted sender) smtp.mailfrom=shy828301@gmail.com X-Rspamd-Server: rspam03 X-HE-Tag: 1646338097-584499 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 Wed, Mar 2, 2022 at 5:46 PM Hugh Dickins wrote: > > NR_FILE_MAPPED accounting in mm/rmap.c (for /proc/meminfo "Mapped" and > /proc/vmstat "nr_mapped" and the memcg's memory.stat "mapped_file") is > slightly flawed for file or shmem huge pages. > > It is well thought out, and looks convincing, but there's a racy case > when the careful counting in page_remove_file_rmap() (without page lock) > gets discarded. So that in a workload like two "make -j20" kernel builds > under memory pressure, with cc1 on hugepage text, "Mapped" can easily > grow by a spurious 5MB or more on each iteration, ending up implausibly > bigger than most other numbers in /proc/meminfo. And, hypothetically, > might grow to the point of seriously interfering in mm/vmscan.c's > heuristics, which do take NR_FILE_MAPPED into some consideration. > > Fixed by moving the __mod_lruvec_page_state() down to where it will not > be missed before return (and I've grown a bit tired of that oft-repeated > but-not-everywhere comment on the __ness: it gets lost in the move here). > > Does page_add_file_rmap() need the same change? I suspect not, because > page lock is held in all relevant cases, and its skipping case looks safe; > but it's much easier to be sure, if we do make the same change. > > Fixes: dd78fedde4b9 ("rmap: support file thp") > Signed-off-by: Hugh Dickins Reviewed-by: Yang Shi > --- > If this were thought serious enough to backport (I don't feel strongly, > but it is something I keep in my own trees), it needs a little more care > near "out", because the mm/munlock series has removed some action there. > > mm/rmap.c | 31 ++++++++++++++----------------- > 1 file changed, 14 insertions(+), 17 deletions(-) > > --- a/mm/rmap.c > +++ b/mm/rmap.c > @@ -1238,14 +1238,14 @@ void page_add_new_anon_rmap(struct page *page, > void page_add_file_rmap(struct page *page, > struct vm_area_struct *vma, bool compound) > { > - int i, nr = 1; > + int i, nr = 0; > > VM_BUG_ON_PAGE(compound && !PageTransHuge(page), page); > lock_page_memcg(page); > if (compound && PageTransHuge(page)) { > int nr_pages = thp_nr_pages(page); > > - for (i = 0, nr = 0; i < nr_pages; i++) { > + for (i = 0; i < nr_pages; i++) { > if (atomic_inc_and_test(&page[i]._mapcount)) > nr++; > } > @@ -1262,11 +1262,12 @@ void page_add_file_rmap(struct page *page, > VM_WARN_ON_ONCE(!PageLocked(page)); > SetPageDoubleMap(compound_head(page)); > } > - if (!atomic_inc_and_test(&page->_mapcount)) > - goto out; > + if (atomic_inc_and_test(&page->_mapcount)) > + nr++; > } > - __mod_lruvec_page_state(page, NR_FILE_MAPPED, nr); > out: > + if (nr) > + __mod_lruvec_page_state(page, NR_FILE_MAPPED, nr); > unlock_page_memcg(page); > > mlock_vma_page(page, vma, compound); > @@ -1274,7 +1275,7 @@ void page_add_file_rmap(struct page *page, > > static void page_remove_file_rmap(struct page *page, bool compound) > { > - int i, nr = 1; > + int i, nr = 0; > > VM_BUG_ON_PAGE(compound && !PageHead(page), page); > > @@ -1289,12 +1290,12 @@ static void page_remove_file_rmap(struct page *page, bool compound) > if (compound && PageTransHuge(page)) { > int nr_pages = thp_nr_pages(page); > > - for (i = 0, nr = 0; i < nr_pages; i++) { > + for (i = 0; i < nr_pages; i++) { > if (atomic_add_negative(-1, &page[i]._mapcount)) > nr++; > } > if (!atomic_add_negative(-1, compound_mapcount_ptr(page))) > - return; > + goto out; > if (PageSwapBacked(page)) > __mod_lruvec_page_state(page, NR_SHMEM_PMDMAPPED, > -nr_pages); > @@ -1302,16 +1303,12 @@ static void page_remove_file_rmap(struct page *page, bool compound) > __mod_lruvec_page_state(page, NR_FILE_PMDMAPPED, > -nr_pages); > } else { > - if (!atomic_add_negative(-1, &page->_mapcount)) > - return; > + if (atomic_add_negative(-1, &page->_mapcount)) > + nr++; > } > - > - /* > - * We use the irq-unsafe __{inc|mod}_lruvec_page_state because > - * these counters are not modified in interrupt context, and > - * pte lock(a spinlock) is held, which implies preemption disabled. > - */ > - __mod_lruvec_page_state(page, NR_FILE_MAPPED, -nr); > +out: > + if (nr) > + __mod_lruvec_page_state(page, NR_FILE_MAPPED, -nr); > } > > static void page_remove_anon_compound_rmap(struct page *page)