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 11EC2C64ED6 for ; Wed, 1 Mar 2023 07:56:03 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 8F1BC6B0071; Wed, 1 Mar 2023 02:56:02 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id 8A1286B0072; Wed, 1 Mar 2023 02:56:02 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 768C46B0073; Wed, 1 Mar 2023 02:56:02 -0500 (EST) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0013.hostedemail.com [216.40.44.13]) by kanga.kvack.org (Postfix) with ESMTP id 63A466B0071 for ; Wed, 1 Mar 2023 02:56:02 -0500 (EST) Received: from smtpin15.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay09.hostedemail.com (Postfix) with ESMTP id 2B2D281386 for ; Wed, 1 Mar 2023 07:56:02 +0000 (UTC) X-FDA: 80519570964.15.32C4F43 Received: from madras.collabora.co.uk (madras.collabora.co.uk [46.235.227.172]) by imf09.hostedemail.com (Postfix) with ESMTP id 3203214001A for ; Wed, 1 Mar 2023 07:55:59 +0000 (UTC) Authentication-Results: imf09.hostedemail.com; dkim=pass header.d=collabora.com header.s=mail header.b=l69jCCiM; spf=pass (imf09.hostedemail.com: domain of usama.anjum@collabora.com designates 46.235.227.172 as permitted sender) smtp.mailfrom=usama.anjum@collabora.com; dmarc=pass (policy=reject) header.from=collabora.com ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1677657360; a=rsa-sha256; cv=none; b=LMc/okD+Sq5sVJknEke0ZiE/swSGM3kkiefyBCh60KKMYt6Ewom2ztlhnFaO/WEIM27ZNX lfouHBWupE/2lVDUYXiCOTGy+wdVmKrRDBXJyISUpLO/ambxgM/LzKQi6gxqBZ7s4Eeb4+ BfsV/12QliMc+Mds8AQUInU4/z2PXsM= ARC-Authentication-Results: i=1; imf09.hostedemail.com; dkim=pass header.d=collabora.com header.s=mail header.b=l69jCCiM; spf=pass (imf09.hostedemail.com: domain of usama.anjum@collabora.com designates 46.235.227.172 as permitted sender) smtp.mailfrom=usama.anjum@collabora.com; dmarc=pass (policy=reject) header.from=collabora.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1677657360; 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:content-transfer-encoding: in-reply-to:in-reply-to:references:references:dkim-signature; bh=a00NBpbRE/aTXQ5ABOY4QmF7a5dRmsk1Hj9/d7Xm4iA=; b=JWYgpJtmk/4J++KDETFLkZB1OuYxg16XTkDnfLM4VNiCnNqn67o4S1QE7/zUf7tRfYtUi6 4LFb8iDK847P45+R+My0iOzqD7knAlmvj/R/zOEwdlzdj68aOJLmKWh9/yHZ1CYhwye+P5 /6op3TzlgY0j/khJ3z2pChKb11hp4AM= Received: from [192.168.10.12] (unknown [39.45.217.110]) (using TLSv1.3 with cipher TLS_AES_128_GCM_SHA256 (128/128 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256) (No client certificate requested) (Authenticated sender: usama.anjum) by madras.collabora.co.uk (Postfix) with ESMTPSA id 25841660035B; Wed, 1 Mar 2023 07:55:55 +0000 (GMT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=collabora.com; s=mail; t=1677657358; bh=BbfNtQPnACGWLjxystJr489MF9PkPbOm0YLmC80+ESw=; h=Date:Cc:Subject:To:References:From:In-Reply-To:From; b=l69jCCiM9TtlG3RkbMTtF0qVkWMC8uM5lLRmIBcNKuoPhvk8yWNp4a8PXmKXJfI/r yOmHWmSG/SEclccVDdthT1uUw4OF3BXymSw+Hcue10t9yf+r68xN360Di2j3H2sirs zSP27VrdZuFc3ZofVedkTw8oZp+wcvE2QUyEx4n5OKftWHDEZRwvwoMrIuJyZGH/4Y Xu7h5U/DUS2Yk6uL85GVrPwxTryLCLdAm6LAQOx+LTV9bpZ5O/6DK6rsY+IVdz1sYb frdH2tBfH8EVp4Bd5F/M+19UosZsOaG2fwzlt0mhNpKnzLHR6Hh4OnLXBQh3kniyHE CoKvHAGm0hv4A== Message-ID: <640319be-ddb6-d74f-b731-eee5ceab3d01@collabora.com> Date: Wed, 1 Mar 2023 12:55:51 +0500 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101 Thunderbird/102.7.2 Cc: Muhammad Usama Anjum , linux-kernel@vger.kernel.org, linux-mm@kvack.org, Andrea Arcangeli , Andrew Morton , Mike Rapoport , Axel Rasmussen , Nadav Amit , David Hildenbrand , "kernel@collabora.com" Subject: Re: [PATCH v2] mm/uffd: UFFD_FEATURE_WP_UNPOPULATED To: Peter Xu References: <20230227230044.1596744-1-peterx@redhat.com> Content-Language: en-US From: Muhammad Usama Anjum In-Reply-To: Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit X-Rspam-User: X-Rspamd-Queue-Id: 3203214001A X-Rspamd-Server: rspam01 X-Stat-Signature: tehf9t51pqu3prhojc59rxfknp1ka9na X-HE-Tag: 1677657359-769581 X-HE-Meta: U2FsdGVkX18tyWK0YoymO4WdpPcp2nXtPCicXcCzbYbQd0qO98AjvyUf8HbwQ9YX7j2gO0GpuFT1eW5ABiGTEqWRMH5TNzhPcvnNIxZTxwSe3WUaWndvA33iqw4dvcEdZtaHSWkHFrbG5a/Ajj7hCUw/EbErZkmnQMd9mJwy+ObZeTNhK4t/k2yejmHIgc6Ae/RODfWG6CQocGy69AEwjh6a0CNuqHmTW5IQSRTSsaMf5y79tEQwN1qM5qiYT00Oik5uL8KcXvqnpCp79CiIqL+l63pCdzEUjd4RZuwiQyWvnrzw6Mu+RcpPLvIypXi4vbQNtT49hy+77PhZWZP7V0O/jUhtDBtYDOlZDesQupo1+LKaB0wqcpyoRb4WeFPWd+7Im4tu6h5sJ77nwWYOAgAXLEux0WzCwf7eUo6qI4syzLKFOYXOETDimK799nCxwcmPDRSjkozbNbQ41FoAhjBuV4f7wihQXRQTEe6K/5wJLZ+lx4bnImYt7HRyEdJdKzlOKRnXwRCjvNyl9x+xUSjTqHMM2oK1NO7wzrf7Pp9WKnunIRHGp+szAOAMW4PMT0xMN2hqEMgRpRViJK8Hj/8iIfTSCZ0H9M+DBRcZkoxIktBQXCUMlgp82EeHMn4V6cR8JRn9nWba25naYJicWtmNshtlf8rh5VOAgy9z5a47eeYkpn51S7GKJForbWr4wt1JUdodusCbJGEKWVe77KxvuPH0qfLIIqWA8tpwJOsaFq/4M07y2lfXgg2+fX3f1NqtDKm/mkpdWkE3GZd3qtZAjtOi9ALgbRiVDQRPTloPaGCUWbaKfzUdNwtDGfIH/n6TiUoVgHz/fJLkEESMEdNLDNW95MJQdd6rAB2VvKCoVN5ClawB00QX9Cp0mHNyAzegt9VYY1ow7YbQjGRPcyjcZZaooq0P0sIu8q9EaLeptdP43jOuvZt9Ecm2nrWxGEtzP2/RN9hinyFcwir zrIwl1Wv jPHnXOQWXXC79O0j53wQAbTI/qlWBo997xEAJaogLXYyP4zJs9QDJBG0F5II2UGZdEBJ10z9xmvTxdC7T7pKZeYqs/ZWUt7Yy3Jz5LE53mWTTIPUm57OMdlr/2U420wbh3ehqCktdxCN0pcAnydE3S1gBN+zP2VIcEJv62i2FE9BbIrZNF7z2qq3U4c0kKKu7awpqaNotsCiXUOVMvMjnVbwiapXj9HdQOv2MKyl6t3amgIF21UW+g9YJkLOVzE/64yxcS7D8n/vhh4c+ECdY/DW2dik1VT85z3mYw0VTpchWNiEtAwsWr9oOhPClCtzuyK5K3w9lXJh7Qqumv4K4tkgejbCuDO1UIGIdaX85XH2a/mZhIo7XV70kGFJmBRVgiKau/B4HB/Q+PHJn5tkW/snDPDYc0S9+grZPdZMBWIbPihU3j96QrgasOyLLX1epdKr0GT4cibQf1DmvwMgaBuxJ6mRizek+6b4vKEDH+Pd/QBlifRLJj8P1DjR9WxWs8g3PNqWaAKVpc0M= 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: Hi Peter, Finally I've found the bug. On 2/28/23 9:24 PM, Muhammad Usama Anjum wrote: > On 2/28/23 8:58 PM, Peter Xu wrote: >> On Tue, Feb 28, 2023 at 12:21:45PM +0500, Muhammad Usama Anjum wrote: >>> Hi Peter, >> >> Hi, Muhammad, >> >>> >>> Thank you so much for sending. >>> >>> On 2/28/23 5:36 AM, Peter Xu wrote: >>>> On Mon, Feb 27, 2023 at 06:00:44PM -0500, Peter Xu wrote: >>>>> This is a new feature that controls how uffd-wp handles none ptes. When >>>>> it's set, the kernel will handle anonymous memory the same way as file >>>>> memory, by allowing the user to wr-protect unpopulated ptes. >>>>> >>>>> File memories handles none ptes consistently by allowing wr-protecting of >>>>> none ptes because of the unawareness of page cache being exist or not. For >>>>> anonymous it was not as persistent because we used to assume that we don't >>>>> need protections on none ptes or known zero pages. >>>>> >>>>> One use case of such a feature bit was VM live snapshot, where if without >>>>> wr-protecting empty ptes the snapshot can contain random rubbish in the >>>>> holes of the anonymous memory, which can cause misbehave of the guest when >>>>> the guest OS assumes the pages should be all zeros. >>>>> >>>>> QEMU worked it around by pre-populate the section with reads to fill in >>>>> zero page entries before starting the whole snapshot process [1]. >>>>> >>>>> Recently there's another need raised on using userfaultfd wr-protect for >>>>> detecting dirty pages (to replace soft-dirty in some cases) [2]. In that >>>>> case if without being able to wr-protect none ptes by default, the dirty >>>>> info can get lost, since we cannot treat every none pte to be dirty (the >>>>> current design is identify a page dirty based on uffd-wp bit being cleared). >>>>> >>>>> In general, we want to be able to wr-protect empty ptes too even for >>>>> anonymous. >>>>> >>>>> This patch implements UFFD_FEATURE_WP_UNPOPULATED so that it'll make >>>>> uffd-wp handling on none ptes being consistent no matter what the memory >>>>> type is underneath. It doesn't have any impact on file memories so far >>>>> because we already have pte markers taking care of that. So it only >>>>> affects anonymous. >>>>> >>>>> The feature bit is by default off, so the old behavior will be maintained. >>>>> Sometimes it may be wanted because the wr-protect of none ptes will contain >>>>> overheads not only during UFFDIO_WRITEPROTECT (by applying pte markers to >>>>> anonymous), but also on creating the pgtables to store the pte markers. So >>>>> there's potentially less chance of using thp on the first fault for a none >>>>> pmd or larger than a pmd. >>>>> >>>>> The major implementation part is teaching the whole kernel to understand >>>>> pte markers even for anonymously mapped ranges, meanwhile allowing the >>>>> UFFDIO_WRITEPROTECT ioctl to apply pte markers for anonymous too when the >>>>> new feature bit is set. >>>>> >>>>> Note that even if the patch subject starts with mm/uffd, there're a few >>>>> small refactors to major mm path of handling anonymous page faults. But >>>>> they should be straightforward. >>>>> >>>>> So far, add a very light smoke test within the userfaultfd kselftest >>>>> pagemap unit test to make sure anon pte markers work. >>>>> >>>>> [1] https://lore.kernel.org/all/20210401092226.102804-4-andrey.gruzdev@virtuozzo.com/ >>>>> [1] https://lore.kernel.org/all/Y+v2HJ8+3i%2FKzDBu@x1n/ >>>>> >>>>> Signed-off-by: Peter Xu >>>>> --- >>>>> v1->v2: >>>>> - Use pte markers rather than populate zero pages when protect [David] >>>>> - Rename WP_ZEROPAGE to WP_UNPOPULATED [David] >>>> >>>> Some very initial performance numbers (I only ran in a VM but it should be >>>> similar, unit is "us") below as requested. The measurement is about time >>>> spent when wr-protecting 10G range of empty but mapped memory. It's done >>>> in a VM, assuming we'll get similar results on bare metal. >>>> >>>> Four test cases: >>>> >>>> - default UFFDIO_WP >>>> - pre-read the memory, then UFFDIO_WP (what QEMU does right now) >>>> - pre-fault using MADV_POPULATE_READ, then default UFFDIO_WP >>>> - UFFDIO_WP with WP_UNPOPULATED >>>> >>>> Results: >>>> >>>> Test DEFAULT: 2 >>>> Test PRE-READ: 3277099 (pre-fault 3253826) >>>> Test MADVISE: 2250361 (pre-fault 2226310) >>>> Test WP-UNPOPULATE: 20850 >>>> >>>> I'll add these information into the commit message when there's a new >>>> version. >>> I'm hitting a bug where I'm unable to write to the memory after adding this >>> patch and wp the memory. I'm hitting this case in your test and my tests as >>> well. Please apply the following diff to your test to reproduce on your end: >>> >>> --- uffd_wp_perf.c.orig 2023-02-28 12:09:38.971820791 +0500 >>> +++ uffd_wp_perf.c 2023-02-28 12:13:11.077827160 +0500 >>> @@ -114,6 +114,7 @@ >>> start1 = get_usec(); >>> } >>> wp_range(uffd, buffer, SIZE, true); >>> + buffer[0] = 'a'; While using WP_UNPOPULATED, we get stuck if newly allocated memory is read without initialization. This can be reproduced by either of the following statements: printf("%c", buffer[0]); buffer[0]++; This bug has start to appear on this patch. How are you handling reading newly allocated memory when WP_UNPOPULATED is defined? Running my pagemap_ioctl selftest as benchmark in a VM: without zeropage / wp_unpopulated (decide from pte_none() if page is dirty or not, buggy and wrong implementation, just for reference) 26.608 seconds with zeropage 39.203 seconds with wp_unpopulated 62.907 seconds 136% worse performance overall 60% worse performance of unpopulated than zeropage >>> if (start1 == 0) >>> printf("%"PRIu64"\n", get_usec() - start); >>> else >> >> This is expected, because the test didn't start any fault resolving thread, >> so the write will block until someone unprotects the page. > Ohh.. sorry. Wrong reproducer. > >> >> But it shouldn't happen to your use case if you applied both WP_UNPOPULATED >> & WP_ASYNC. > I'm using both WP_UNPOPULATED and ASYNC. The program gets stuck at right time: > > > 1..57 > ok 1 sanity_tests_sd wrong flag specified > ok 2 sanity_tests_sd wrong mask specified > ok 3 sanity_tests_sd wrong return mask specified > ok 4 sanity_tests_sd mixture of correct and wrong flag > ok 5 sanity_tests_sd PM_SCAN_OP_WP cannot be used without get > ok 6 sanity_tests_sd Clear area with larger vec size > ^C > Program received signal SIGINT, Interrupt. > 0x000000000040220c in sanity_tests_sd () at pagemap_ioctl.c:198 > 198 mem[i]++; > (gdb) bt > #0 0x000000000040220c in sanity_tests_sd () at pagemap_ioctl.c:198 > #1 0x0000000000404e14 in main () at pagemap_ioctl.c:846 > () > > /proc/$PID/stack is empty. Not sure why. I can see stack trace of other > applications, but not this one's. > > Let me send better reproducer for you. > >> >> Could you try "cat /proc/$PID/stack" to see where does your thread stuck >> at? >> > -- BR, Muhammad Usama Anjum