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=-12.8 required=3.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,FREEMAIL_FORGED_FROMDOMAIN,FREEMAIL_FROM, HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_CR_TRAILER,INCLUDES_PATCH, MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS 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 1BB17C48BE5 for ; Tue, 15 Jun 2021 23:11:01 +0000 (UTC) Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by mail.kernel.org (Postfix) with ESMTP id A9C7361245 for ; Tue, 15 Jun 2021 23:11:00 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org A9C7361245 Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=gmail.com Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=owner-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix) id 2284F6B006C; Tue, 15 Jun 2021 19:11:00 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 1FE216B006E; Tue, 15 Jun 2021 19:11:00 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 09F296B0070; Tue, 15 Jun 2021 19:11:00 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from forelay.hostedemail.com (smtprelay0196.hostedemail.com [216.40.44.196]) by kanga.kvack.org (Postfix) with ESMTP id C9E626B006C for ; Tue, 15 Jun 2021 19:10:59 -0400 (EDT) Received: from smtpin18.hostedemail.com (10.5.19.251.rfc1918.com [10.5.19.251]) by forelay02.hostedemail.com (Postfix) with ESMTP id 651F4D200 for ; Tue, 15 Jun 2021 23:10:59 +0000 (UTC) X-FDA: 78257505438.18.CEB591B Received: from mail-ej1-f54.google.com (mail-ej1-f54.google.com [209.85.218.54]) by imf22.hostedemail.com (Postfix) with ESMTP id E59DAC00CBE2 for ; Tue, 15 Jun 2021 23:10:48 +0000 (UTC) Received: by mail-ej1-f54.google.com with SMTP id gt18so416969ejc.11 for ; Tue, 15 Jun 2021 16:10:58 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=AOuKGzIskjJRbFedWFW/C+I5PYhV95WPLhId+SvqihU=; b=Me2pvvkHKVtdhQPvq+vIdIjnZAeubKot8b/msdA4OqCL1eyPpVYMJhSJuwehR5FBtp zGQpbV5sNiw7tpr5PC9fuj0INw/rDTiZOnbpOKe3rN4J+7xRnaMpUSlp0lw7nzBjIzof XR9ZGP6GO2VbPNBm0LzcqY69dN2IVr5yvQpPs/ChBWQthPwRm9dRLowiuYIur5sPJAI5 su4rRk+RoRx+/IFezW+YNI4eF/ydVlTryAMjvre8g1ginl+2jCSfGopyI4safI3wohhG 1c2BnEG2yu4pdwJ0xZS5g4VM2eazl8SxMDP7YHBqqsHmC/iqzg9udNztQWaLc28Y8imH c+5w== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=AOuKGzIskjJRbFedWFW/C+I5PYhV95WPLhId+SvqihU=; b=NdhNWV4ioYTj9vZpplbe4oPK0gn0Wb9/yFVbU8i1LIiByK0+Ww9pTKKSsTk7t9J4ti 2m/YVrMDsaRmOYuTGXtrKf60JVoLOXWDHMkkq+PLechP234sXnAmik8SwP7+BGBYo6cT yPyy393ASFr7Eb/ZzLf8DLusaRB6sufVG2Zno4jrmGAubE9ZjcMvCxP58ZopNB6uSiiM pxXS+zQk40ievWjUiaqSHllosIz3Bt8gFGeu6aBuJrcjWAAjT/QLrmewaKN84l0D79mY Uhgp9hRqYICx2qdPU9NJbGABRxXGuhq0zN3qu7g2aIa0O9iw4Vtpf1IddY2lAQRWg0Qh fMFw== X-Gm-Message-State: AOAM531jsvCcm7Hy0up8hv7vcD5WZzl9kUoZ0T/Uo2V3KPVpDB/xJgjS NocCL1ex1B36ktl97bmP/5B8sM/5zo/gEXDEQtk= X-Google-Smtp-Source: ABdhPJzHX1BNpwnfCqqm59hp1H0X6TtuHe5B6/m0KBULTJlIVinWp8ZuuFM8L436LMgFXdlox6fVRlHjxCbLiYTB1uA= X-Received: by 2002:a17:907:9d1:: with SMTP id bx17mr1956080ejc.238.1623798657801; Tue, 15 Jun 2021 16:10:57 -0700 (PDT) MIME-Version: 1.0 References: <20210615012014.1100672-1-jannh@google.com> <50d828d1-2ce6-21b4-0e27-fb15daa77561@nvidia.com> In-Reply-To: From: Yang Shi Date: Tue, 15 Jun 2021 16:10:46 -0700 Message-ID: Subject: Re: [PATCH v2] mm/gup: fix try_grab_compound_head() race with split_huge_page() To: Jann Horn Cc: John Hubbard , Matthew Wilcox , Andrew Morton , Linux-MM , kernel list , "Kirill A . Shutemov" , Jan Kara , stable Content-Type: text/plain; charset="UTF-8" X-Rspamd-Server: rspam05 X-Rspamd-Queue-Id: E59DAC00CBE2 X-Stat-Signature: 4nwkeditym9dgigp5nnu61pd11qjqztw Authentication-Results: imf22.hostedemail.com; dkim=pass header.d=gmail.com header.s=20161025 header.b=Me2pvvkH; dmarc=pass (policy=none) header.from=gmail.com; spf=pass (imf22.hostedemail.com: domain of shy828301@gmail.com designates 209.85.218.54 as permitted sender) smtp.mailfrom=shy828301@gmail.com X-HE-Tag: 1623798648-963328 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, Jun 15, 2021 at 5:10 AM Jann Horn wrote: > > On Tue, Jun 15, 2021 at 8:37 AM John Hubbard wrote: > > On 6/14/21 6:20 PM, Jann Horn wrote: > > > try_grab_compound_head() is used to grab a reference to a page from > > > get_user_pages_fast(), which is only protected against concurrent > > > freeing of page tables (via local_irq_save()), but not against > > > concurrent TLB flushes, freeing of data pages, or splitting of compound > > > pages. > [...] > > Reviewed-by: John Hubbard > > Thanks! > > [...] > > > @@ -55,8 +72,23 @@ static inline struct page *try_get_compound_head(struct page *page, int refs) > > > if (WARN_ON_ONCE(page_ref_count(head) < 0)) > > > return NULL; > > > if (unlikely(!page_cache_add_speculative(head, refs))) > > > return NULL; > > > + > > > + /* > > > + * At this point we have a stable reference to the head page; but it > > > + * could be that between the compound_head() lookup and the refcount > > > + * increment, the compound page was split, in which case we'd end up > > > + * holding a reference on a page that has nothing to do with the page > > > + * we were given anymore. > > > + * So now that the head page is stable, recheck that the pages still > > > + * belong together. > > > + */ > > > + if (unlikely(compound_head(page) != head)) { > > > > I was just wondering about what all could happen here. Such as: page gets split, > > reallocated into a different-sized compound page, one that still has page pointing > > to head. I think that's OK, because we don't look at or change other huge page > > fields. > > > > But I thought I'd mention the idea in case anyone else has any clever ideas about > > how this simple check might be insufficient here. It seems fine to me, but I > > routinely lack enough imagination about concurrent operations. :) > > Hmmm... I think the scariest aspect here is probably the interaction > with concurrent allocation of a compound page on architectures with > store-store reordering (like ARM). *If* the page allocator handled > compound pages with lockless, non-atomic percpu freelists, I think it > might be possible that the zeroing of tail_page->compound_head in > put_page() could be reordered after the page has been freed, > reallocated and set to refcount 1 again? > > That shouldn't be possible at the moment, but it is still a bit scary. It might be possible after Mel's "mm/page_alloc: Allow high-order pages to be stored on the per-cpu lists" patch (https://patchwork.kernel.org/project/linux-mm/patch/20210611135753.GC30378@techsingularity.net/). > > > I think the lockless page cache code also has to deal with somewhat > similar ordering concerns when it uses page_cache_get_speculative(), > e.g. in mapping_get_entry() - first it looks up a page pointer with > xas_load(), and any access to the page later on would be a _dependent > load_, but if the page then gets freed, reallocated, and inserted into > the page cache again before the refcount increment and the re-check > using xas_reload(), then there would be no data dependency from > xas_reload() to the following use of the page... >