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 X-Spam-Level: X-Spam-Status: No, score=-18.9 required=3.0 tests=BAYES_00,DKIMWL_WL_MED, DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS, INCLUDES_PATCH,MAILING_LIST_MULTI,SIGNED_OFF_BY,SPF_HELO_NONE,SPF_PASS, URIBL_BLOCKED,USER_AGENT_SANE_1,USER_IN_DEF_DKIM_WL autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 28858C388F9 for ; Wed, 11 Nov 2020 07:28:14 +0000 (UTC) Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by mail.kernel.org (Postfix) with ESMTP id 69BF820795 for ; Wed, 11 Nov 2020 07:28:12 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (2048-bit key) header.d=google.com header.i=@google.com header.b="Gf4tMT9i" DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 69BF820795 Authentication-Results: mail.kernel.org; dmarc=fail (p=reject dis=none) header.from=google.com Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=owner-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix) id 8027F6B0036; Wed, 11 Nov 2020 02:28:12 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id 7907F6B005D; Wed, 11 Nov 2020 02:28:12 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 652D86B0068; Wed, 11 Nov 2020 02:28:12 -0500 (EST) X-Delivered-To: linux-mm@kvack.org Received: from forelay.hostedemail.com (smtprelay0223.hostedemail.com [216.40.44.223]) by kanga.kvack.org (Postfix) with ESMTP id 352F56B0036 for ; Wed, 11 Nov 2020 02:28:12 -0500 (EST) Received: from smtpin21.hostedemail.com (10.5.19.251.rfc1918.com [10.5.19.251]) by forelay02.hostedemail.com (Postfix) with ESMTP id CA171362C for ; Wed, 11 Nov 2020 07:28:11 +0000 (UTC) X-FDA: 77471308782.21.corn33_530c228272fb Received: from filter.hostedemail.com (10.5.16.251.rfc1918.com [10.5.16.251]) by smtpin21.hostedemail.com (Postfix) with ESMTP id B09BF180442C0 for ; Wed, 11 Nov 2020 07:28:11 +0000 (UTC) X-HE-Tag: corn33_530c228272fb X-Filterd-Recvd-Size: 10454 Received: from mail-oo1-f66.google.com (mail-oo1-f66.google.com [209.85.161.66]) by imf48.hostedemail.com (Postfix) with ESMTP for ; Wed, 11 Nov 2020 07:28:11 +0000 (UTC) Received: by mail-oo1-f66.google.com with SMTP id n2so212649ooo.8 for ; Tue, 10 Nov 2020 23:28:11 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20161025; h=date:from:to:cc:subject:in-reply-to:message-id:references :user-agent:mime-version; bh=DkcV1x7MgfBS236GtgzpXoMpSPJBXCFSqYnEz2asS8Y=; b=Gf4tMT9irso5W1qAZaQwEIQlLc3fBbqGXy95E/Xm26rt6qFj3o/whHQ2xSvngDlqYF +vJUBG9fAZrNuVlOA2/pgxsCcs+9epBcBrAWtNFVQKW2WZk8qjMPoyBWWBuqh7OCHJjt dUAHkYVjqsx9e9QbCBr8oznfG0H/DVtWt8iwN8Xrq1XUdCjDyrqOPb3m3o/Vco87M76t 3+5Ka0GhgcEy1jngP+hKdR+QfAVxpZSSyL+K/aL/gmDpirv0lu7X6Bo+RoKAAycLxRXL 5Rdkwa4r3TqZyyWgSD+i5GkOhkQlSKG5Ib1aweIyfEM7O5r+WQkKzqz0U8SgnKu0G15/ fWXA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:date:from:to:cc:subject:in-reply-to:message-id :references:user-agent:mime-version; bh=DkcV1x7MgfBS236GtgzpXoMpSPJBXCFSqYnEz2asS8Y=; b=PEgH3s1fl3kO1t7lTqaQRTctlJFKDwLRee0S+VqiayLA054TMTu4GvsnwAHyXFT8VS 7Ryh6+J+iIBIh7rNpG+NK+jUTl0qcnuIbQtf/DaX7NX5lGiTA/55g9RD7cxeQHMDiLvn RSrJckU3EkKqAeSjj4bweGkkoHl75uc4XWXYiOioeLY/gvFxqe7bhFkNHje43cxrNrAV Fm+Rywi3EJ8mPVqcqPudbtpoGDmjifgPsSMkZD2Jb8WgFxurcCzCjRcWRZVl/ZttNay4 OFdMjzq9X4tcjwPRM2mayRaZ6wrGo2bjzyQAchijtDvaNbNbvPnndZ1kLxxwJSO/861U Lp/A== X-Gm-Message-State: AOAM531fQE3mXqUyxUpGWQigpDIDXxZqVy0XyOAzzrEkn1dimSz3pN/3 zZPaNy23sC9cCxZlavny7eBVmw== X-Google-Smtp-Source: ABdhPJzL1ToOpmmFpblyXPcpRbSl5YEfquIJpnotyhSxXxlp+lwCXytEgF+mxcgvDW5vfPNiX4gDnA== X-Received: by 2002:a4a:b3c5:: with SMTP id q5mr1122260ooo.60.1605079690270; Tue, 10 Nov 2020 23:28:10 -0800 (PST) Received: from eggly.attlocal.net (172-10-233-147.lightspeed.sntcca.sbcglobal.net. [172.10.233.147]) by smtp.gmail.com with ESMTPSA id w14sm378030oou.16.2020.11.10.23.28.07 (version=TLS1 cipher=ECDHE-ECDSA-AES128-SHA bits=128/128); Tue, 10 Nov 2020 23:28:09 -0800 (PST) Date: Tue, 10 Nov 2020 23:27:54 -0800 (PST) From: Hugh Dickins X-X-Sender: hugh@eggly.anvils To: Johannes Weiner cc: Alex Shi , akpm@linux-foundation.org, mgorman@techsingularity.net, tj@kernel.org, hughd@google.com, khlebnikov@yandex-team.ru, daniel.m.jordan@oracle.com, willy@infradead.org, lkp@intel.com, linux-mm@kvack.org, linux-kernel@vger.kernel.org, cgroups@vger.kernel.org, shakeelb@google.com, iamjoonsoo.kim@lge.com, richard.weiyang@gmail.com, kirill@shutemov.name, alexander.duyck@gmail.com, rong.a.chen@intel.com, mhocko@suse.com, vdavydov.dev@gmail.com, shy828301@gmail.com, Vlastimil Babka , Minchan Kim Subject: Re: [PATCH v20 08/20] mm: page_idle_get_page() does not need lru_lock In-Reply-To: <20201102144110.GB724984@cmpxchg.org> Message-ID: References: <1603968305-8026-1-git-send-email-alex.shi@linux.alibaba.com> <1603968305-8026-9-git-send-email-alex.shi@linux.alibaba.com> <20201102144110.GB724984@cmpxchg.org> User-Agent: Alpine 2.11 (LSU 23 2013-08-11) MIME-Version: 1.0 Content-Type: TEXT/PLAIN; charset=US-ASCII 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 Mon, 2 Nov 2020, Johannes Weiner wrote: > On Thu, Oct 29, 2020 at 06:44:53PM +0800, Alex Shi wrote: > > From: Hugh Dickins > > > > It is necessary for page_idle_get_page() to recheck PageLRU() after > > get_page_unless_zero(), but holding lru_lock around that serves no > > useful purpose, and adds to lru_lock contention: delete it. > > > > See https://lore.kernel.org/lkml/20150504031722.GA2768@blaptop for the > > discussion that led to lru_lock there; but __page_set_anon_rmap() now > > uses WRITE_ONCE(), > > That doesn't seem to be the case in Linus's or Andrew's tree. Am I > missing a dependent patch series? Sorry, I was out of action, then slower than ever, for a while. Many thanks for calling out my falsehood there, Johannes. What led me to write that? It has baffled me, but at last I see: this patch to page_idle_get_page() was 0002 in my lru_lock patchset against v5.3 last year, and 0001 was the patch which made it true. Then when I checked against mainline, I must have got confused by the similar WRITE_ONCE in page_move_anon_rmap(). Appended below, but not rediffed, and let's not hold up Alex's set for the rest of it: it is all theoretical until the kernel gets to be built with a suitably malicious compiler; but I'll follow up with a fresh version of the below after his set is safely in. >From a1abcbc2aac70c6ba47b8991992bb85b86b4a160 Mon Sep 17 00:00:00 2001 From: Hugh Dickins Date: Thu, 22 Aug 2019 15:49:44 -0700 Subject: [PATCH 1/9] mm: more WRITE_ONCE and READ_ONCE on page->mapping v4.2 commit 414e2fb8ce5a ("rmap: fix theoretical race between do_wp_page and shrink_active_list") added a WRITE_ONCE() where page_move_anon_rmap() composes page->mapping from anon_vma pointer and PAGE_MAPPING_ANON. Now do the same where __page_set_anon_rmap() does the same, and where compaction.c applies PAGE_MAPPING_MOVABLE, and ksm.c PAGE_MAPPING_KSM. rmap.c already uses READ_ONCE(page->mapping), but util.c should too: add READ_ONCE() in page_rmapping(), page_anon_vma() and page_mapping(). Delete the then unused helper __page_rmapping(). I doubt that this commit fixes anything, but it's harmless and unintrusive, and makes reasoning about page mapping flags easier. What if a compiler implements "page->mapping = mapping" in other places by, say, first assigning the odd bits of mapping, then adding in the even bits? Then we shall not build the kernel with such a compiler. Signed-off-by: Hugh Dickins Cc: Vladimir Davydov Cc: Vlastimil Babka Cc: Minchan Kim Cc: Yu Zhao Cc: Alex Shi --- mm/compaction.c | 7 ++++--- mm/ksm.c | 2 +- mm/rmap.c | 7 ++++++- mm/util.c | 24 ++++++++++-------------- 4 files changed, 21 insertions(+), 19 deletions(-) diff --git a/mm/compaction.c b/mm/compaction.c index 952dc2fb24e5..c405f4362624 100644 --- a/mm/compaction.c +++ b/mm/compaction.c @@ -113,7 +113,8 @@ void __SetPageMovable(struct page *page, struct address_space *mapping) { VM_BUG_ON_PAGE(!PageLocked(page), page); VM_BUG_ON_PAGE((unsigned long)mapping & PAGE_MAPPING_MOVABLE, page); - page->mapping = (void *)((unsigned long)mapping | PAGE_MAPPING_MOVABLE); + WRITE_ONCE(page->mapping, + (unsigned long)mapping | PAGE_MAPPING_MOVABLE); } EXPORT_SYMBOL(__SetPageMovable); @@ -126,8 +127,8 @@ void __ClearPageMovable(struct page *page) * flag so that VM can catch up released page by driver after isolation. * With it, VM migration doesn't try to put it back. */ - page->mapping = (void *)((unsigned long)page->mapping & - PAGE_MAPPING_MOVABLE); + WRITE_ONCE(page->mapping, + (unsigned long)page->mapping & PAGE_MAPPING_MOVABLE); } EXPORT_SYMBOL(__ClearPageMovable); diff --git a/mm/ksm.c b/mm/ksm.c index 3dc4346411e4..426b6a40ea41 100644 --- a/mm/ksm.c +++ b/mm/ksm.c @@ -865,7 +865,7 @@ static inline struct stable_node *page_stable_node(struct page *page) static inline void set_page_stable_node(struct page *page, struct stable_node *stable_node) { - page->mapping = (void *)((unsigned long)stable_node | PAGE_MAPPING_KSM); + WRITE_ONCE(page->mapping, (unsigned long)stable_node | PAGE_MAPPING_KSM); } #ifdef CONFIG_SYSFS diff --git a/mm/rmap.c b/mm/rmap.c index 003377e24232..9480df437edc 100644 --- a/mm/rmap.c +++ b/mm/rmap.c @@ -1044,7 +1044,12 @@ static void __page_set_anon_rmap(struct page *page, anon_vma = anon_vma->root; anon_vma = (void *) anon_vma + PAGE_MAPPING_ANON; - page->mapping = (struct address_space *) anon_vma; + /* + * Ensure that anon_vma and the PAGE_MAPPING_ANON bit are written + * simultaneously, so a concurrent reader (eg page_referenced()'s + * PageAnon()) will not see one without the other. + */ + WRITE_ONCE(page->mapping, (struct address_space *) anon_vma); page->index = linear_page_index(vma, address); } diff --git a/mm/util.c b/mm/util.c index e6351a80f248..09b9fcbedac3 100644 --- a/mm/util.c +++ b/mm/util.c @@ -489,21 +489,14 @@ void kvfree(const void *addr) } EXPORT_SYMBOL(kvfree); -static inline void *__page_rmapping(struct page *page) -{ - unsigned long mapping; - - mapping = (unsigned long)page->mapping; - mapping &= ~PAGE_MAPPING_FLAGS; - - return (void *)mapping; -} - /* Neutral page->mapping pointer to address_space or anon_vma or other */ void *page_rmapping(struct page *page) { + unsigned long mapping; + page = compound_head(page); - return __page_rmapping(page); + mapping = (unsigned long)READ_ONCE(page->mapping); + return (void *)(mapping & ~PAGE_MAPPING_FLAGS); } /* @@ -534,10 +527,11 @@ struct anon_vma *page_anon_vma(struct page *page) unsigned long mapping; page = compound_head(page); - mapping = (unsigned long)page->mapping; + mapping = (unsigned long)READ_ONCE(page->mapping); + /* Return NULL if file or PageMovable or PageKsm */ if ((mapping & PAGE_MAPPING_FLAGS) != PAGE_MAPPING_ANON) return NULL; - return __page_rmapping(page); + return (struct anon_vma *)(mapping & ~PAGE_MAPPING_FLAGS); } struct address_space *page_mapping(struct page *page) @@ -557,10 +551,12 @@ struct address_space *page_mapping(struct page *page) return swap_address_space(entry); } - mapping = page->mapping; + mapping = READ_ONCE(page->mapping); + /* Return NULL if PageAnon (including PageKsm) */ if ((unsigned long)mapping & PAGE_MAPPING_ANON) return NULL; + /* Return struct address_space pointer if file or PageMovable */ return (void *)((unsigned long)mapping & ~PAGE_MAPPING_FLAGS); } EXPORT_SYMBOL(page_mapping); -- 2.23.0.187.g17f5b7556c-goog