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 F1502ECAAD1 for ; Wed, 31 Aug 2022 17:55:57 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 764FC6B0071; Wed, 31 Aug 2022 13:55:57 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 6ECD96B0072; Wed, 31 Aug 2022 13:55:57 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 566EF8D0001; Wed, 31 Aug 2022 13:55:57 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0011.hostedemail.com [216.40.44.11]) by kanga.kvack.org (Postfix) with ESMTP id 402306B0071 for ; Wed, 31 Aug 2022 13:55:57 -0400 (EDT) Received: from smtpin27.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay07.hostedemail.com (Postfix) with ESMTP id 0D82F1609CF for ; Wed, 31 Aug 2022 17:55:57 +0000 (UTC) X-FDA: 79860641154.27.FC24875 Received: from mail-pf1-f181.google.com (mail-pf1-f181.google.com [209.85.210.181]) by imf29.hostedemail.com (Postfix) with ESMTP id C608F12006C for ; Wed, 31 Aug 2022 17:55:56 +0000 (UTC) Received: by mail-pf1-f181.google.com with SMTP id z187so15144086pfb.12 for ; Wed, 31 Aug 2022 10:55:56 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112; h=cc:to:subject:message-id:date:from:in-reply-to:references :mime-version:from:to:cc:subject:date; bh=vmx4gHJp3u58Y95b/+pQhpJVRJy7LPLrGYUILaqzP90=; b=cR6Hz+GMMYCP8ejaPhssEUK/Gx3B34EwyM3LRufVEgzzzrO5yjztpWB3eb476vbXap mT8BdcHIZg3Vf94818iIUNfNWKoHPsOU+gNxl7BHpIIcRQJ5RQW6jriNTwViEx7KgUW6 G45dwnVeLQqAJKCAO8booBUPOGUmS/V1/wgaeWJabK+dXammfkjTmWxCX/XooMXoMjbj 2dmF7skteetLhBWrBOGpEFSib0wakZxn4yd2T5NtZlQw1c08d+gH6tsgJmLkEJJB5fJQ bbXYuYBkexBNqhVsmU7xGqhovjourbT3BbYrbN2062xgPrGh7TrApRjQtiNoLiiFu7i2 qGiw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=cc:to:subject:message-id:date:from:in-reply-to:references :mime-version:x-gm-message-state:from:to:cc:subject:date; bh=vmx4gHJp3u58Y95b/+pQhpJVRJy7LPLrGYUILaqzP90=; b=c4rCJZqqOSrC5R2LnlE6etpaBI9onG3oxwXBNfXCQIuNQyWOnqGm1OobKqjTDC4H5/ j6GlDcUIiprvnQ0qMp4MAcKZrLW0SaSpnFMow9Zpvn+JksF4DNx5v2rD7l48yDvOsJva HCUoutlJA8V985sBpHkGQY74opHDbry5LrsGCIg9k9p1PbC/nRhtyBJQZQYEjgzbxKse jv/alpM8qt46MzeRQ5844tsc7DGLfDytjnV/c3uHnCSK34rL97cj1gGGNiuD4BjYAhsM e/iqFziPMl6RwjY8tPSY1byYfCr1BG/TUfWAJHOJz93pwOUIvIRmeNPcveVoehiEqYYJ sXqw== X-Gm-Message-State: ACgBeo1Lb/lqQMzuv7+ZjqbHZjaS8d8bXpIWDjungQj84N2tNOYL+JD9 Xlx6LamV/ZDEpcmoV+7nDTap6IxL+/vsryyC334= X-Google-Smtp-Source: AA6agR7/NHH+IR98XLUgrClivA8XFdyDFCxcjCDKLVj7jfAgRPUp7YKdkpzhs8wwSbn2yp6oifmnZc80rfMAGHkfMYc= X-Received: by 2002:aa7:9193:0:b0:536:62e6:1a84 with SMTP id x19-20020aa79193000000b0053662e61a84mr27241415pfa.20.1661968555798; Wed, 31 Aug 2022 10:55:55 -0700 (PDT) MIME-Version: 1.0 References: <20220831083024.37138-1-david@redhat.com> In-Reply-To: <20220831083024.37138-1-david@redhat.com> From: Yang Shi Date: Wed, 31 Aug 2022 10:55:43 -0700 Message-ID: Subject: Re: [PATCH v1] mm/ksm: update stale comment in write_protect_page() To: David Hildenbrand Cc: linux-kernel@vger.kernel.org, linux-mm@kvack.org, Andrew Morton , Jason Gunthorpe , John Hubbard , Andrea Arcangeli , Hugh Dickins , Peter Xu , "Kirill A. Shutemov" Content-Type: text/plain; charset="UTF-8" ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1661968556; a=rsa-sha256; cv=none; b=kQakTrstxcY0CBgcRCTbKr9GEn9OVO13dr5AO2zBQoQPMzWJ6LFhsSQeR7MwAeJTMrg3tM H/ki/YrELQye/5ytt7J+a4CDFyN4XkEo1bbMp2Rx6/ry85OCwmp0d4foJ/iPA0FVPgQ5nQ w4UepsXybH849WXaKfPZjAjIjnhrOPA= ARC-Authentication-Results: i=1; imf29.hostedemail.com; dkim=pass header.d=gmail.com header.s=20210112 header.b=cR6Hz+GM; dmarc=pass (policy=none) header.from=gmail.com; spf=pass (imf29.hostedemail.com: domain of shy828301@gmail.com designates 209.85.210.181 as permitted sender) smtp.mailfrom=shy828301@gmail.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1661968556; 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=vmx4gHJp3u58Y95b/+pQhpJVRJy7LPLrGYUILaqzP90=; b=wMBoBxILnQXqL0zjxjc+S7Lw+LmrNRnYRYKDeLdXrwmpI04NKPSB0LPLsQsGWofMZNElbj jEOZWrRqckz0+5zv+gx1pQi05Z6pUiqwbgaV9S8BPr9zTPDwc8oOzqpMr0kRmxW7LsjJ4X ENj9N7dB0ntYrJdce93k0bBpfQd4fWc= Authentication-Results: imf29.hostedemail.com; dkim=pass header.d=gmail.com header.s=20210112 header.b=cR6Hz+GM; dmarc=pass (policy=none) header.from=gmail.com; spf=pass (imf29.hostedemail.com: domain of shy828301@gmail.com designates 209.85.210.181 as permitted sender) smtp.mailfrom=shy828301@gmail.com X-Rspam-User: X-Rspamd-Server: rspam12 X-Stat-Signature: eyr711dgah3d96pxuktx4fiye6xtizo4 X-Rspamd-Queue-Id: C608F12006C X-HE-Tag: 1661968556-225320 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, Aug 31, 2022 at 1:30 AM David Hildenbrand wrote: > > The comment is stale, because a TLB flush is no longer sufficient and > required to synchronize against concurrent GUP-fast. This used to be true > in the past, whereby a TLB flush would have implied an IPI on architectures > that support GUP-fast, resulting in GUP-fast that disables local interrupts > from completing before completing the flush. Hmm... it seems there might be problem for THP collapse IIUC. THP collapse clears and flushes pmd before doing anything on pte and relies on interrupt disable of fast GUP to serialize against fast GUP. But if TLB flush is no longer sufficient, then we may run into the below race IIUC: CPU A CPU B THP collapse fast GUP gup_pmd_range() <-- see valid pmd gup_pte_range() <-- work on pte clear pmd and flush TLB __collapse_huge_page_isolate() isolate page <-- before GUP bump refcount pin the page __collapse_huge_page_copy() copy data to huge page clear pte (don't flush TLB) Install huge pmd for huge page return the obsolete page > > However, ever since general RCU GUP-fast was introduced in > commit 2667f50e8b81 ("mm: introduce a general RCU get_user_pages_fast()"), > this handling no longer applies in general. RCU primarily prevents page > tables from getting freed while they might still get walked by GUP-fast, > but we can race with GUP-fast after clearing the PTE and flushing the > TLB. > > Nowadays, we can see a refcount change from GUP-fast at any point in > time. However, GUP-fast detects concurrent PTE changes by looking up the > PTE, temporarily grabbing a reference, and dropping the reference again if > the PTE changed in the meantime. > > An explanation by Jason Gunthorpe regarding how existing memory barriers > should be fine to make sure that concurrent GUP-fast cannot succeed in > grabbing a reference with write permissions after we cleared the PTE and > flushed the TLB can be found at [1]. > > Note that clearing PageAnonExclusive via page_try_share_anon_rmap() > might still need some explicit memory barriers to not have any possible > races with RCU GUP-fast. > > [1] https://lkml.kernel.org/r/Yw5rwIUPm49XtqOB@nvidia.com > > Cc: Andrew Morton > Cc: Jason Gunthorpe > Cc: John Hubbard > Cc: Andrea Arcangeli > Cc: Hugh Dickins > Cc: Peter Xu > Signed-off-by: David Hildenbrand > --- > mm/ksm.c | 21 +++++++++------------ > 1 file changed, 9 insertions(+), 12 deletions(-) > > diff --git a/mm/ksm.c b/mm/ksm.c > index 42ab153335a2..e88291f63461 100644 > --- a/mm/ksm.c > +++ b/mm/ksm.c > @@ -1072,23 +1072,20 @@ static int write_protect_page(struct vm_area_struct *vma, struct page *page, > swapped = PageSwapCache(page); > flush_cache_page(vma, pvmw.address, page_to_pfn(page)); > /* > - * Ok this is tricky, when get_user_pages_fast() run it doesn't > - * take any lock, therefore the check that we are going to make > - * with the pagecount against the mapcount is racy and > - * O_DIRECT can happen right after the check. > - * So we clear the pte and flush the tlb before the check > - * this assure us that no O_DIRECT can happen after the check > - * or in the middle of the check. > - * > - * No need to notify as we are downgrading page table to read > - * only not changing it to point to a new page. > + * Especially if we're downgrading protection, make sure to > + * flush the TLB now. No need to notify as we are not changing > + * the PTE to point at a different page. > * > * See Documentation/mm/mmu_notifier.rst > */ > entry = ptep_clear_flush(vma, pvmw.address, pvmw.pte); > + > /* > - * Check that no O_DIRECT or similar I/O is in progress on the > - * page > + * Make sure that there are no unexpected references (e.g., > + * concurrent O_DIRECT). Note that while concurrent GUP-fast > + * could raise the refcount temporarily to grab a write > + * reference, it will observe the changed PTE and drop that > + * temporary reference again. > */ > if (page_mapcount(page) + 1 + swapped != page_count(page)) { > set_pte_at(mm, pvmw.address, pvmw.pte, entry); > -- > 2.37.1 > >