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 29170EB64D8 for ; Fri, 16 Jun 2023 06:37:47 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id A61C18E0003; Fri, 16 Jun 2023 02:37:46 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 9EAEF8E0001; Fri, 16 Jun 2023 02:37:46 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 88BCA8E0003; Fri, 16 Jun 2023 02:37:46 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0010.hostedemail.com [216.40.44.10]) by kanga.kvack.org (Postfix) with ESMTP id 76CFD8E0001 for ; Fri, 16 Jun 2023 02:37:46 -0400 (EDT) Received: from smtpin01.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay03.hostedemail.com (Postfix) with ESMTP id 38295A02E1 for ; Fri, 16 Jun 2023 06:37:46 +0000 (UTC) X-FDA: 80907655332.01.C4A71DC Received: from madras.collabora.co.uk (madras.collabora.co.uk [46.235.227.172]) by imf17.hostedemail.com (Postfix) with ESMTP id 4C5DC40009 for ; Fri, 16 Jun 2023 06:37:43 +0000 (UTC) Authentication-Results: imf17.hostedemail.com; dkim=pass header.d=collabora.com header.s=mail header.b=XHJdr0Nf; spf=pass (imf17.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=quarantine) header.from=collabora.com ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1686897463; a=rsa-sha256; cv=none; b=Eu5qinfEEW0m3tB1FJTt6LjWvwLG7hb43vkZq6cPoufeNG+2MVsN36Z+KGv2Ti6TKGH1sM F4+Wwb0pSIcULrkKqQeulPWMlCm7+b/z6JrL/pmMXGHOENjrRQwcOGvmgDlc8isGJTSYxV 5cCYp4r6nrF2XqW/3aBzaU+vcwg/cIc= ARC-Authentication-Results: i=1; imf17.hostedemail.com; dkim=pass header.d=collabora.com header.s=mail header.b=XHJdr0Nf; spf=pass (imf17.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=quarantine) header.from=collabora.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1686897463; 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=5ZX1ZlVgiCDjExMmKncmPT3Gkp8REdheMFY29a3kP08=; b=b0KH/so5lfV9C2a1KhCeNTTxanWBGRtvCZGimoar80h2jQCMhA6qTxELsHGPrO4Ud9roV9 Ak06dP7Dh8mUZpy92e7XG4pHYZyavzD9lG/pQZKu4BDpJprJB0oXL0+eG9R9H5Xy2Ju/E8 sDUwYa70lS/SAcpnovbxQQDf29dO6g4= Received: from [192.168.10.55] (unknown [119.155.33.163]) (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 3F4E36606F79; Fri, 16 Jun 2023 07:37:35 +0100 (BST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=collabora.com; s=mail; t=1686897461; bh=oi7kcW/MH0K5SRnKY57domar7n22lk4AWinBLcFzc2Y=; h=Date:Cc:Subject:To:References:From:In-Reply-To:From; b=XHJdr0Nfi3TmYUy9talcVsP2kNainfnuLZ+cQUlJSMvVBd5K2RzKq+QjL/iO+0erh rlieeQA5y2IgtBmWL9z4xmlnKABdf2l/Je6XNY8uM+jWVm+r85jk4Wg1c6O6eANm0F NPBThlYsIW9Vz+4qMTow9j1/jniZGGaokzKVJ85A63Fdj83FLl1n0gPP2cCNKUrRjH 5+4oUvLQ5pHETxdRdQleozuwKZly81uIUc33VKJKOKei+mOH1TZ1+W8CUTTblGuE8c MZ+3fh+2lqT+GgUKTOJM1jxgG36wqTAKpfUixUAU2dvIHAzL2y9nn8ODhljK1kdfre z3niSZM9oDFug== Message-ID: <41d3bdd4-cd69-0e15-cec4-720804bf3580@collabora.com> Date: Fri, 16 Jun 2023 11:37:31 +0500 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101 Thunderbird/102.11.0 Cc: Muhammad Usama Anjum , Peter Xu , David Hildenbrand , Andrew Morton , Andrei Vagin , Danylo Mocherniuk , Paul Gofman , Cyrill Gorcunov , Mike Rapoport , Nadav Amit , Alexander Viro , Shuah Khan , Christian Brauner , Yang Shi , Vlastimil Babka , "Liam R . Howlett" , Yun Zhou , Suren Baghdasaryan , Alex Sierra , Matthew Wilcox , Pasha Tatashin , Axel Rasmussen , "Gustavo A . R . Silva" , Dan Williams , linux-kernel@vger.kernel.org, linux-fsdevel@vger.kernel.org, linux-mm@kvack.org, linux-kselftest@vger.kernel.org, Greg KH , kernel@collabora.com Subject: Re: [PATCH v18 2/5] fs/proc/task_mmu: Implement IOCTL to get and optionally clear info about PTEs Content-Language: en-US To: =?UTF-8?B?TWljaGHFgiBNaXJvc8WCYXc=?= References: <20230613102905.2808371-1-usama.anjum@collabora.com> <20230613102905.2808371-3-usama.anjum@collabora.com> <0db01d90-09d6-08a4-bbb8-70670d3baa94@collabora.com> <34203acf-7270-7ade-a60e-ae0f729dcf70@collabora.com> <96b7cc00-d213-ad7d-1b48-b27f75b04d22@collabora.com> <43c96533-8009-e42f-721c-4b2d1e142f5d@collabora.com> From: Muhammad Usama Anjum In-Reply-To: Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit X-Rspamd-Server: rspam08 X-Rspamd-Queue-Id: 4C5DC40009 X-Stat-Signature: 75wdu5ofy443u855f5dekx7inyu6m8op X-Rspam-User: X-HE-Tag: 1686897463-351234 X-HE-Meta: U2FsdGVkX1+AYagyV/C7KmBrs5ra6MbTRH6iK44qDVQUOyyrs8rt8ZUP+O9Na87tAyNbaNiRbd5mjYtRJ4y1bh5cZyIpJLVgP6mMpLL9Q3cLgcvVM9FdaMDwPT8rPmVP2gHIU6iluHwJKXxBwJQNjvCMgb4NGxNWEuW4m7fFgQp6YutQbFaQONfQIPhpAMMd30X/mEW1AO7X8DvBhNQu2jxjzt42rhoP6LzRs/2PDq8y4fZImpMM5FcWi0FfpbBJthal5KySQ9gotpb7oNULxt6+skmGp1qi/y81e2X8LY0Dg1VS0OvZeuJe/KoL7qpRqidR8WEu6hKF7QQMqkFD421PFJz+fnHKVsWCLGqPRuSOGyCmpxHD6Pgavpz+qs0ogwVHnWLl3i9aLimu/We0YHE64mBy1FX/kNj1RRBc6VOP9UHVaOYY07xy9WXQpSWaN4gT6rH3a7v65UN1ta8FoFy+hqqQYXi2p0pWqFj7cZGkuungFKAFU7/F15Ex2uGm/OrJrIDQKzBGn3aKhNo/MbQ9HvvR3ypE21xUnJDK3az5M1euJgGHctJocMtl/sk/hkP9RXfeYpiUKrZYrkKs8uSt2mX2xR03K+BSkNbWek+oWRCLzw3PjVorp4V94nLqZ22hEuEU3TmcudYSifX0FGAdyhK2GKyUjr7JboambjpOQg6V43kWY8nG7M45VlxSIj+ShACjNWNQPAh2dpjIVZYsSlf/mnOuc4TJIBvN4e+REE9FdiXd84Ix+5GhDV7JoGrcUxMIc/6rxeIHs4pgWT5QxtbjJwvTVQa0v5EBTdtu2vfFX2Va07CDbKe8rYlDR8AuWk0oPnarLQWnZNkAD+A9s2goOpY+Nmx4PwZJq6VOTndN8Q/SuEhopLDtBFAkbiNxmecvKRCaZfj7i872hwtkKeQTcj8KllAmzSxCPxAw9ZbMXWPehPnzUQbFoudrtspyrXIrdVRQfUuDqpn kX/rnM5K Jd9vxhFGHVbH9BD8DhonXk7v/Zi6xD2lNjo3YaRNQQY2YysMLtxDwCRXsCcGm33mRfie9MG3Fl/MGbDAI9D9LtV6zOX97ODNcydZfHufSp4x3BdgL69foAdtkIrg6cYCsSRF0gafCd1Bgm3qgAw+D09FWfz5RVE1rkSrp59kthEu3NV7aScf68y11cAUi2TNjxu175sVufn85whc+guGUsBYkAxH+BdHxbkSu5LUK0afXdIuTd7E76+dgDXKIxFnLLWzVnoqRCuTl6QREkHRXYZmuqBiCfkxpcUgWFmIaI/OInMLilqlgzotcuiCYwmaPYrgCoqUlvc5mRJs= 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 6/16/23 1:00 AM, Michał Mirosław wrote: > On Thu, 15 Jun 2023 at 17:16, Muhammad Usama Anjum > wrote: >> >> Please review the v19. I hope to get your reviewed by tag soon. >> >> On 6/15/23 7:58 PM, Michał Mirosław wrote: >>> On Thu, 15 Jun 2023 at 16:52, Michał Mirosław wrote: >>>> On Thu, 15 Jun 2023 at 15:58, Muhammad Usama Anjum >>>> wrote: >>>>> I'll send next revision now. >>>>> On 6/14/23 11:00 PM, Michał Mirosław wrote: >>>>>> (A quick reply to answer open questions in case they help the next version.) >>>>>> >>>>>> On Wed, 14 Jun 2023 at 19:10, Muhammad Usama Anjum >>>>>> wrote: >>>>>>> On 6/14/23 8:14 PM, Michał Mirosław wrote: >>>>>>>> On Wed, 14 Jun 2023 at 15:46, Muhammad Usama Anjum >>>>>>>> wrote: >>>>>>>>> >>>>>>>>> On 6/14/23 3:36 AM, Michał Mirosław wrote: >>>>>>>>>> On Tue, 13 Jun 2023 at 12:29, Muhammad Usama Anjum >>>>>>>>>> wrote: >>>>>>>>>> For flags name: PM_REQUIRE_WRITE_ACCESS? >>>>>>>>>> Or Is it intended to be checked only if doing WP (as the current name >>>>>>>>>> suggests) and so it would be redundant as WP currently requires >>>>>>>>>> `p->required_mask = PAGE_IS_WRITTEN`? >>>>>>>>> This is intended to indicate that if userfaultfd is needed. If >>>>>>>>> PAGE_IS_WRITTEN is mentioned in any of mask, we need to check if >>>>>>>>> userfaultfd has been initialized for this memory. I'll rename to >>>>>>>>> PM_SCAN_REQUIRE_UFFD. >>>>>>>> >>>>>>>> Why do we need that check? Wouldn't `is_written = false` work for vmas >>>>>>>> not registered via uffd? >>>>>>> UFFD_FEATURE_WP_ASYNC and UNPOPULATED needs to be set on the memory region >>>>>>> for it to report correct written values on the memory region. Without UFFD >>>>>>> WP ASYNC and UNPOUPULATED defined on the memory, we consider UFFD_WP state >>>>>>> undefined. If user hasn't initialized memory with UFFD, he has no right to >>>>>>> set is_written = false. >>>>>> >>>>>> How about calculating `is_written = is_uffd_registered() && >>>>>> is_uffd_wp()`? This would enable a user to apply GET+WP for the whole >>>>>> address space of a process regardless of whether all of it is >>>>>> registered. >>>>> I wouldn't want to check if uffd is registered again and again. This is why >>>>> we are doing it only once every walk in pagemap_scan_test_walk(). >>>> >>>> There is no need to do the checks repeatedly. If I understand the code >>>> correctly, uffd registration is per-vma, so it can be communicated >>>> from test_walk to entry/hole callbacks via a field in >>>> pagemap_scan_private. >>> >>> Actually... this could be exposed as a page category for the filter >>> (e.g. PAGE_USES_UFFD_WP) and then you could just make the ioctl() to >>> work for your usecase without tracking the ranges at the userspace >>> side. >> I'm not sure about page category. ASAIK the current check isn't bad when we >> already mention in documentation that memory must be registered with UFFD >> WP before using write feature of the IOCTL. > > You could relax the (documentation) rule to be "WP works only on > ranges registeder via UFFD for ASYNC_WP". That way you allow people, > who don't read documentation to shoot their foot, They'll shoot their foot and have no idea why they are getting false results. Isn't it better that they get error and they go read the documentation and then register with UFFD first? I think, returning error is way better than not returning anything. > but don't block > people that know what they are doing from exploiting the nice feature > that they don't need to track all the WP-registered ranges calling the > ioctl() for each one and instead can just call it once for the whole > address space. > > Best Regards > Michał Mirosław -- BR, Muhammad Usama Anjum