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=-24.0 required=3.0 tests=BAYES_00,DKIMWL_WL_MED, DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS, INCLUDES_CR_TRAILER,INCLUDES_PATCH,MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS, 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 8EC07C07E96 for ; Fri, 9 Jul 2021 02:50:44 +0000 (UTC) Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by mail.kernel.org (Postfix) with ESMTP id 0502561417 for ; Fri, 9 Jul 2021 02:50:43 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 0502561417 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 7E7706B006C; Thu, 8 Jul 2021 22:50:43 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 797526B0070; Thu, 8 Jul 2021 22:50:43 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 638156B0071; Thu, 8 Jul 2021 22:50:43 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from forelay.hostedemail.com (smtprelay0093.hostedemail.com [216.40.44.93]) by kanga.kvack.org (Postfix) with ESMTP id 343DF6B006C for ; Thu, 8 Jul 2021 22:50:43 -0400 (EDT) Received: from smtpin16.hostedemail.com (10.5.19.251.rfc1918.com [10.5.19.251]) by forelay03.hostedemail.com (Postfix) with ESMTP id 70B3B819131D for ; Fri, 9 Jul 2021 02:50:42 +0000 (UTC) X-FDA: 78341521524.16.513315C Received: from mail-ot1-f52.google.com (mail-ot1-f52.google.com [209.85.210.52]) by imf08.hostedemail.com (Postfix) with ESMTP id 27DE130000A9 for ; Fri, 9 Jul 2021 02:50:42 +0000 (UTC) Received: by mail-ot1-f52.google.com with SMTP id w15-20020a056830144fb02904af2a0d96f3so7973137otp.6 for ; Thu, 08 Jul 2021 19:50:41 -0700 (PDT) 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 :mime-version; bh=MwaOWtEym8nCXwyHmi2S0bKX8uQfDz4h1Y9F3kJtM7g=; b=LItYAZCA/tB6YBa8MmeXx/skxiw8M9mhyu5+BVO5Xd9D1QpUqbybHYRGMQftQmt40X SAgI8J7jCyr7ksOybF144mdyQhEkYnZD54yTQ3qZkcDmgjXzZ8ookpaZSWmHITxU4fDB opKkBLCVOyy8RFdfkqvxslOvnaNJXFMJgCOMFlnFT1qEIkB7RjQLHmBbTZk/m4fUPM5/ /xlg20rBkyodyjJv3CjuRbPAqH5TjmwX71xj7khcv9ixRW6APaxUEubUajhZ6+emlHJH ie7eSL9l2b5Z0ooiYn5X/VwbMICmU/pHSEkRBBXCAuTN5FsU50J7nLDwMH0hSH98yGXA DD5A== 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:mime-version; bh=MwaOWtEym8nCXwyHmi2S0bKX8uQfDz4h1Y9F3kJtM7g=; b=X6/h+XaMBPDqMhuD26eAXmSz5XRL8eNQrbK2elWClTDQQANRPfWDxowbENjEAg2apM oEyEvLBQdF0prCYeCvYmzSda70oepOxdoT60as9Ap1LReBQEETN3tDgRXmInkbhv6lh5 s+L5ZvrWsQ99kSx4wTqlLt4D2Ver8yplz/KZn9xbP0wlXBeircRoBJx5YVRuccEcBUeB etDHomYo2nVzthis2oyWyldxQsEe7eqWk8N88hx9OXk3WQWLgUK7A94VVjQAB8yrjj0k 2YRoTPXiZnyeAK7u0aXzM0j4T6zLdTR06Luy3jFJQtxJVys/sn47rP7oF8fikxFIPzZr bvjQ== X-Gm-Message-State: AOAM531J0snUzHiVZFylLfh/XHIJKZdZhdYAepggOKLRmdenH+X0VO0I v3PXaopq5eVpK35Rs8rVYzRXBQ== X-Google-Smtp-Source: ABdhPJzCBsJc9yDbXcCOLkUkY0CuiN56cSI554/8flO/vr93Ml88UfxmeKHuH5qM4hLb+4w++sOC/Q== X-Received: by 2002:a05:6830:409d:: with SMTP id x29mr18170259ott.189.1625799041155; Thu, 08 Jul 2021 19:50:41 -0700 (PDT) Received: from ripple.attlocal.net (172-10-233-147.lightspeed.sntcca.sbcglobal.net. [172.10.233.147]) by smtp.gmail.com with ESMTPSA id z5sm938657oib.14.2021.07.08.19.50.39 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 08 Jul 2021 19:50:40 -0700 (PDT) Date: Thu, 8 Jul 2021 19:50:26 -0700 (PDT) From: Hugh Dickins X-X-Sender: hugh@ripple.attlocal.net To: "Kirill A. Shutemov" cc: Hugh Dickins , Andrew Morton , Alistair Popple , Jason Gunthorpe , Ralph Campbell , Christoph Hellwig , Yang Shi , "Kirill A. Shutemov" , Shakeel Butt , linux-kernel@vger.kernel.org, linux-mm@kvack.org Subject: Re: [PATCH 2/4] mm/rmap: fix old bug: munlocking THP missed other mlocks In-Reply-To: <20210708135811.775drqgwkwc76vcb@box.shutemov.name> Message-ID: <6c4d46aa-4d73-76a7-bcce-a09024768f63@google.com> References: <563ce5b2-7a44-5b4d-1dfd-59a0e65932a9@google.com> <20210708135811.775drqgwkwc76vcb@box.shutemov.name> MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Authentication-Results: imf08.hostedemail.com; dkim=pass header.d=google.com header.s=20161025 header.b=LItYAZCA; dmarc=pass (policy=reject) header.from=google.com; spf=pass (imf08.hostedemail.com: domain of hughd@google.com designates 209.85.210.52 as permitted sender) smtp.mailfrom=hughd@google.com X-Rspamd-Server: rspam06 X-Rspamd-Queue-Id: 27DE130000A9 X-Stat-Signature: o71965hkt3e94g8tukzzg8qaf1m1o5op X-HE-Tag: 1625799042-512529 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 Thu, 8 Jul 2021, Kirill A. Shutemov wrote: > On Wed, Jul 07, 2021 at 01:08:53PM -0700, Hugh Dickins wrote: > > The kernel recovers in due course from missing Mlocked pages: but there > > was no point in calling page_mlock() (formerly known as try_to_munlock()) > > on a THP, because nothing got done even when it was found to be mapped in > > another VM_LOCKED vma. > > > > It's true that we need to be careful: Mlocked accounting of pte-mapped > > THPs is too difficult (so consistently avoided); but Mlocked accounting > > of only-pmd-mapped THPs is supposed to work, even when multiple mappings > > are mlocked and munlocked or munmapped. Refine the tests. > > Well, that's true that it should be fine to mlock only-pmd-mapped THPs, > but the refined check doesn't gurantee that the page is not mapped with > PTEs. !PageDoubleMap(page) only guarantees that the page in not mapped > with both PMDs and PTEs at the same time. For anon pages, we clear the > flag when the last PMD mapping is gone and only PTEs left. > > Do I miss some detail here? Maybe we exclude anon pages here somehow? > I don't see it. Yes, you're right, Kirill: thanks a lot for catching that. PageDoubleMap: certainly not my favourite page flag! And now that I've seen follow_trans_huge_pmd(), its comments, and its goto skip_mlock on a PageAnon with compound_mapcount != 1, the right fix for page_mlock() seems to be to skip over Anon THP altogether. Here's a v2 of just this patch (others remain good): what do you think? [PATCH v2 2/4] mm/rmap: fix old bug: munlocking THP missed other mlocks The kernel recovers in due course from missing Mlocked pages: but there was no point in calling page_mlock() (formerly known as try_to_munlock()) on a THP, because nothing got done even when it was found to be mapped in another VM_LOCKED vma. It's true that we need to be careful: Mlocked accounting of pte-mapped THPs is too difficult (so consistently avoided); but Mlocked accounting of only-pmd-mapped file THPs is supposed to work, even when multiple mappings are mlocked and munlocked or munmapped. Refine the tests. Many thanks to Kirill for reminding that PageDoubleMap cannot be relied on to warn of pte mappings in the Anon THP case; and a scan of subpages does not seem appropriate here. Note how follow_trans_huge_pmd() does not even mark an Anon THP as mlocked when compound_mapcount != 1: multiple mlocking of Anon THP is avoided, so simply return from page_mlock() in this case. I said the kernel recovers: but would page reclaim be likely to split THP before rediscovering that it's VM_LOCKED? Apparently so. I have worked on a fix for that, but it's a different issue, and not something to rush. Whereas page_mlock_one() could not be reviewed without fixing this first. Fixes: 9a73f61bdb8a ("thp, mlock: do not mlock PTE-mapped file huge pages") Signed-off-by: Hugh Dickins --- mm/rmap.c | 42 +++++++++++++++++++++++++----------------- 1 file changed, 25 insertions(+), 17 deletions(-) diff --git a/mm/rmap.c b/mm/rmap.c index 746013e282c3..f1d4edf9c696 100644 --- a/mm/rmap.c +++ b/mm/rmap.c @@ -1440,20 +1440,20 @@ static bool try_to_unmap_one(struct page *page, struct vm_area_struct *vma, /* * If the page is mlock()d, we cannot swap it out. */ - if (!(flags & TTU_IGNORE_MLOCK)) { - if (vma->vm_flags & VM_LOCKED) { - /* PTE-mapped THP are never mlocked */ - if (!PageTransCompound(page)) { - /* - * Holding pte lock, we do *not* need - * mmap_lock here - */ - mlock_vma_page(page); - } - ret = false; - page_vma_mapped_walk_done(&pvmw); - break; - } + if (!(flags & TTU_IGNORE_MLOCK) && + (vma->vm_flags & VM_LOCKED)) { + /* + * PTE-mapped THP are never marked as mlocked: so do + * not set it on a DoubleMap THP, nor on an Anon THP + * (which may still be PTE-mapped after DoubleMap was + * cleared). But stop unmapping even in those cases. + */ + if (!PageTransCompound(page) || (PageHead(page) && + !PageDoubleMap(page) && !PageAnon(page))) + mlock_vma_page(page); + page_vma_mapped_walk_done(&pvmw); + ret = false; + break; } /* Unexpected PMD-mapped THP? */ @@ -1984,9 +1984,13 @@ static bool page_mlock_one(struct page *page, struct vm_area_struct *vma, * munlock_vma_pages_range(). */ if (vma->vm_flags & VM_LOCKED) { - /* PTE-mapped THP are never mlocked */ - if (!PageTransCompound(page)) - mlock_vma_page(page); + /* + * PTE-mapped THP are never marked as mlocked; but + * this function is never called on a DoubleMap THP, + * nor on an Anon THP (which may still be PTE-mapped + * after DoubleMap was cleared). + */ + mlock_vma_page(page); page_vma_mapped_walk_done(&pvmw); } @@ -2020,6 +2024,10 @@ void page_mlock(struct page *page) VM_BUG_ON_PAGE(!PageLocked(page) || PageLRU(page), page); VM_BUG_ON_PAGE(PageCompound(page) && PageDoubleMap(page), page); + /* Anon THP are only marked as mlocked when singly mapped */ + if (PageTransCompound(page) && PageAnon(page)) + return; + rmap_walk(page, &rwc); } -- 2.26.2