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=-23.3 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 DFE27C433F5 for ; Wed, 22 Sep 2021 20:50:21 +0000 (UTC) Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by mail.kernel.org (Postfix) with ESMTP id 5A2BA6112F for ; Wed, 22 Sep 2021 20:50:21 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.4.1 mail.kernel.org 5A2BA6112F Authentication-Results: mail.kernel.org; dmarc=fail (p=reject dis=none) header.from=google.com Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=kvack.org Received: by kanga.kvack.org (Postfix) id D4AD4900003; Wed, 22 Sep 2021 16:50:20 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id CFD41900002; Wed, 22 Sep 2021 16:50:20 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id B9C56900003; Wed, 22 Sep 2021 16:50:20 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from forelay.hostedemail.com (smtprelay0163.hostedemail.com [216.40.44.163]) by kanga.kvack.org (Postfix) with ESMTP id AB201900002 for ; Wed, 22 Sep 2021 16:50:20 -0400 (EDT) Received: from smtpin39.hostedemail.com (10.5.19.251.rfc1918.com [10.5.19.251]) by forelay04.hostedemail.com (Postfix) with ESMTP id 5EE6632330 for ; Wed, 22 Sep 2021 20:50:20 +0000 (UTC) X-FDA: 78616402200.39.6B4403F Received: from mail-io1-f51.google.com (mail-io1-f51.google.com [209.85.166.51]) by imf28.hostedemail.com (Postfix) with ESMTP id 111E4900009A for ; Wed, 22 Sep 2021 20:50:19 +0000 (UTC) Received: by mail-io1-f51.google.com with SMTP id d18so5126151iof.13 for ; Wed, 22 Sep 2021 13:50:19 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20210112; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=MFMm6NiEdpDi099+JSn7qlXvpS8yt1RYEqmBc09i4cg=; b=iaStf23wLKtouI1H2CJCz2IVvLZdEQlSPxGZ11XDdpu7iMTPqk2j4tfSeFGBXY4pUj c5uS1pY1B3JpebdVIt+9ea5/J2fFDcqB4CVZX8nwPoQbFhV0rBt0iJ+uQ/p6+hSw1qOc xeYIU0J2DRUmawVjoEml10JTnobstgDrGFVVrndO4w2yKYkJ+XJ5H/l2wEJOgWvy5LZg CirTG3OmtI+nD0V1I5QLk972KwxYLmDsnn0d0EjgbVaxr013VsQv2bTPbaFZNVPctyJM cNKkzcDt/6N5ZHTKX2PYU42M5aEpt/Es6co/v3fRDijALtAaNDrcIROuJkpYbuqQez8H j6tA== 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=MFMm6NiEdpDi099+JSn7qlXvpS8yt1RYEqmBc09i4cg=; b=QZJI/1OGw4bBjxRTVKEfVDgv5yFjzyDw3oeCG0VmKAmnhvu1TcNf99itjz7wGO0W7g c5phfI3oWqM+c5ojU0ya71PXYAwgS0d7aLCE+K04W9gHsNyfSmG4EdTt8DYy7zQ4gno3 5LPJC132wyRJ6WEGE0kxlcPgPwN/5+/HSUbRKlWkIluDMv4cn5cyS0vdeia9IK6SQPhA YgcaGWBnnh0MIkvvh4el9fLbuSJdvRppUZbURFCV3Nj/K45eZ3aV1Xzh6WNheGaS/4hn MQrCUMc5DuLd8lqB+ZWy9HJJmMWK+eFOhkIj/WyyKvoqTvhZZh0iWh3Opcvd0PKSmPZP /Zsw== X-Gm-Message-State: AOAM533DMbZDIuvTjACYJVdIV4Bx40BQLuuuUgoyKjUaz2nWu7xicMAp mWGJWTh3Oc9pqyuUGFD11iWVOv39TbBmVkCeIiTwPg== X-Google-Smtp-Source: ABdhPJwFrYtiaDMP+aZPQ192rNXUYiDjQGtASWTxtXyZhFXBW+hdvV5CdXj4RyKwr/dT0+z9CMuBSREkadjzFMxl/eY= X-Received: by 2002:a5e:9810:: with SMTP id s16mr820110ioj.171.1632343819263; Wed, 22 Sep 2021 13:50:19 -0700 (PDT) MIME-Version: 1.0 References: <20210922175156.130228-1-peterx@redhat.com> In-Reply-To: <20210922175156.130228-1-peterx@redhat.com> From: Axel Rasmussen Date: Wed, 22 Sep 2021 13:49:42 -0700 Message-ID: Subject: Re: [PATCH] mm/khugepaged: Detecting uffd-wp vma more efficiently To: Peter Xu Cc: LKML , Linux MM , Andrew Morton , Andrea Arcangeli , Hugh Dickins , Nadav Amit Content-Type: text/plain; charset="UTF-8" X-Rspamd-Server: rspam05 X-Rspamd-Queue-Id: 111E4900009A X-Stat-Signature: tnt1gmgnidabyfzyrma8xmk3usz1oata Authentication-Results: imf28.hostedemail.com; dkim=pass header.d=google.com header.s=20210112 header.b=iaStf23w; spf=pass (imf28.hostedemail.com: domain of axelrasmussen@google.com designates 209.85.166.51 as permitted sender) smtp.mailfrom=axelrasmussen@google.com; dmarc=pass (policy=reject) header.from=google.com X-HE-Tag: 1632343819-127945 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, Sep 22, 2021 at 10:52 AM Peter Xu wrote: > > We forbid merging thps for uffd-wp enabled regions, by breaking the khugepaged > scanning right after we detected a uffd-wp armed pte (either present, or swap). > > It works, but it's less efficient, because those ptes only exist for VM_UFFD_WP > enabled VMAs. Checking against the vma flag would be more efficient, and good > enough. To be explicit, we could still be able to merge some thps for > VM_UFFD_WP regions before this patch as long as they have zero uffd-wp armed > ptes, however that's not a major target for thp collapse anyways. > > This mostly reverts commit e1e267c7928fe387e5e1cffeafb0de2d0473663a, but > instead we do the same check at vma level, so it's not a bugfix. > > This also paves the way for file-backed uffd-wp support, as the VM_UFFD_WP flag > will work for file-backed too. > > After this patch, the error for khugepaged for these regions will switch from > SCAN_PTE_UFFD_WP to SCAN_VMA_CHECK. > > Since uffd minor mode should not allow thp as well, do the same thing for minor > mode to stop early on trying to collapse pages in khugepaged. > > Cc: Andrea Arcangeli > Cc: Axel Rasmussen > Cc: Hugh Dickins > Cc: Nadav Amit > Signed-off-by: Peter Xu > --- > > Axel: as I asked in the other thread, please help check whether minor mode will > work properly with shmem thp enabled. If not, I feel like this patch could be > part of that effort at last, but it's also possible that I missed something. Sorry for missing the other thread. Unfortunately, I think shmem THP *doesn't* really work with minor faults, and what's worse, just checking the VMA flag isn't enough. First, let me note the guarantee UFFD minor faults are trying to provide: for a given mapping, any minor fault (that is, pte_none() but a page is present in the page cache) must result in a minor userfault event. Furthermore, the only way the fault may be resolved (i.e., a PTE installed) is via a UFFDIO_CONTINUE ioctl from userspace. A typical use case for minor faults is, we have two mappings (i.e., two VMAs), both pointing to the same underlying physical memory. It's typical for both to have MAP_SHARED. It's typical for one of these mappings to be fully faulted in (i.e., all of its PTEs exist), while the other one has some missing PTEs. The problem is, khugepaged might scan *either* of the two mappings. Say it picks the fully-faulted VMA: even if we set khugepaged_max_ptes_none to zero, it will still go ahead and collapse these pages - because *this* VMA has no missing PTEs. Why is this a problem? When we collapse, we install a PMD, for *all* VMAs which reference these pages. In other words, we might install PTEs for the other, minor-fault-registered mapping, and therefore userfaults will never trigger for some of those regions, even though userspace never UFFDIO_CONTINUE-ed them. I *think* the right place to check for this and solve it is in retract_page_tables(), and I have a patch which does this. I've been hesitant to send it though, as due to a lack of time and the complexity involved I haven't been able to write a clear reproducer program, which my patch clearly fixes. :/ > > Signed-off-by: Peter Xu > --- > include/trace/events/huge_memory.h | 1 - > mm/khugepaged.c | 26 +++----------------------- > 2 files changed, 3 insertions(+), 24 deletions(-) > > diff --git a/include/trace/events/huge_memory.h b/include/trace/events/huge_memory.h > index 4fdb14a81108..53532f5925c3 100644 > --- a/include/trace/events/huge_memory.h > +++ b/include/trace/events/huge_memory.h > @@ -15,7 +15,6 @@ > EM( SCAN_EXCEED_SWAP_PTE, "exceed_swap_pte") \ > EM( SCAN_EXCEED_SHARED_PTE, "exceed_shared_pte") \ > EM( SCAN_PTE_NON_PRESENT, "pte_non_present") \ > - EM( SCAN_PTE_UFFD_WP, "pte_uffd_wp") \ > EM( SCAN_PAGE_RO, "no_writable_page") \ > EM( SCAN_LACK_REFERENCED_PAGE, "lack_referenced_page") \ > EM( SCAN_PAGE_NULL, "page_null") \ > diff --git a/mm/khugepaged.c b/mm/khugepaged.c > index 045cc579f724..3afe66d48db0 100644 > --- a/mm/khugepaged.c > +++ b/mm/khugepaged.c > @@ -31,7 +31,6 @@ enum scan_result { > SCAN_EXCEED_SWAP_PTE, > SCAN_EXCEED_SHARED_PTE, > SCAN_PTE_NON_PRESENT, > - SCAN_PTE_UFFD_WP, > SCAN_PAGE_RO, > SCAN_LACK_REFERENCED_PAGE, > SCAN_PAGE_NULL, > @@ -467,6 +466,9 @@ static bool hugepage_vma_check(struct vm_area_struct *vma, > return false; > if (vma_is_temporary_stack(vma)) > return false; > + /* Don't allow thp merging for wp/minor enabled uffd regions */ > + if (userfaultfd_wp(vma) || userfaultfd_minor(vma)) > + return false; > return !(vm_flags & VM_NO_KHUGEPAGED); > } > > @@ -1246,15 +1248,6 @@ static int khugepaged_scan_pmd(struct mm_struct *mm, > pte_t pteval = *_pte; > if (is_swap_pte(pteval)) { > if (++unmapped <= khugepaged_max_ptes_swap) { > - /* > - * Always be strict with uffd-wp > - * enabled swap entries. Please see > - * comment below for pte_uffd_wp(). > - */ > - if (pte_swp_uffd_wp(pteval)) { > - result = SCAN_PTE_UFFD_WP; > - goto out_unmap; > - } > continue; > } else { > result = SCAN_EXCEED_SWAP_PTE; > @@ -1270,19 +1263,6 @@ static int khugepaged_scan_pmd(struct mm_struct *mm, > goto out_unmap; > } > } > - if (pte_uffd_wp(pteval)) { > - /* > - * Don't collapse the page if any of the small > - * PTEs are armed with uffd write protection. > - * Here we can also mark the new huge pmd as > - * write protected if any of the small ones is > - * marked but that could bring unknown > - * userfault messages that falls outside of > - * the registered range. So, just be simple. > - */ > - result = SCAN_PTE_UFFD_WP; > - goto out_unmap; > - } > if (pte_write(pteval)) > writable = true; > > -- > 2.31.1 >