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 E65F8C001B0 for ; Fri, 11 Aug 2023 13:32:43 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id E86D86B0071; Fri, 11 Aug 2023 09:32:42 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id E0FDC6B0072; Fri, 11 Aug 2023 09:32:42 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id CB0756B0074; Fri, 11 Aug 2023 09:32:42 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0014.hostedemail.com [216.40.44.14]) by kanga.kvack.org (Postfix) with ESMTP id B73456B0071 for ; Fri, 11 Aug 2023 09:32:42 -0400 (EDT) Received: from smtpin07.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay03.hostedemail.com (Postfix) with ESMTP id 88DC9A11DF for ; Fri, 11 Aug 2023 13:32:42 +0000 (UTC) X-FDA: 81111913764.07.A2F6F10 Received: from rere.qmqm.pl (rere.qmqm.pl [91.227.64.183]) by imf17.hostedemail.com (Postfix) with ESMTP id 3A7F740020 for ; Fri, 11 Aug 2023 13:32:38 +0000 (UTC) Authentication-Results: imf17.hostedemail.com; dkim=pass header.d=rere.qmqm.pl header.s=1 header.b=LrLplw1V; spf=pass (imf17.hostedemail.com: domain of mirq-linux@rere.qmqm.pl designates 91.227.64.183 as permitted sender) smtp.mailfrom=mirq-linux@rere.qmqm.pl; dmarc=pass (policy=reject) header.from=rere.qmqm.pl ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1691760759; 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=ypEKTkLpCeJxqqHeAxtQ8Lkcxka0IkUbUa7Fv39L/lc=; b=O9Y+s2jvqxwmKpF7WtGATAy9JdikjT5uF7HwZV2VQLHY5PIaWbKsl33OFUw8E73edJg2MK BPxFzAuoqkcuywWphQS7me2Qo0Oua4xbT7pfUCqgFasF51Q6CkV029mmmqQ/fmY7Fwiueb Df9flnnvaaPmHTKrBMQ7/wfYEhSn8Tc= ARC-Authentication-Results: i=1; imf17.hostedemail.com; dkim=pass header.d=rere.qmqm.pl header.s=1 header.b=LrLplw1V; spf=pass (imf17.hostedemail.com: domain of mirq-linux@rere.qmqm.pl designates 91.227.64.183 as permitted sender) smtp.mailfrom=mirq-linux@rere.qmqm.pl; dmarc=pass (policy=reject) header.from=rere.qmqm.pl ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1691760759; a=rsa-sha256; cv=none; b=m+tW28yNojFP9wosFIg8fsIRaR8Xs2wh2vw2v33W69gtQTiME2klZdprjPiGdUo7UQEHyU 9QLKm7jWI/DkROqMnqktiyeIwxm03lrkcu05VcZNPbJGgqSwNaI7EuzOzNwLCm9REoir5o 5x9jl4yH3r6YgA9VODZo5Hq3H7b/65I= Received: from remote.user (localhost [127.0.0.1]) by rere.qmqm.pl (Postfix) with ESMTPSA id 4RMl8c53QwzH1; Fri, 11 Aug 2023 15:32:32 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=rere.qmqm.pl; s=1; t=1691760756; bh=NfrgfFGHCga9/EC5NbBTXDZZMQ+au5SsG2T+NouDd98=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=LrLplw1VKNapfSiTZM0ge43vU7/4YDPbdoM8upUMUpGUcvwmbVp2S1UWV1X9oowhK 2LW9XDZUrqRegyaDh1es58iNHcJhmITsYCWxHUS6avG+8V9flhoJREZaFx4Hz5Tr9e xHbAy866iRFY572fHAogvK8ZCPWs9vKxuxZF9i3C601NDP84vpWwJx2KEflpIQzAf3 2i4DDZ31Y2P5RnRZBmU5DwHDLg/COv2TbaCKYZsSI2hhLe5kHHMz2VDJ7xVuVfPthy E5ZR16V6YjH02Tl15rTTUWcotslLVJHCDb7ND4kC+kKsQyxri7RBlVIEb1EogZ/pBW P6IYc0Bt7r64w== X-Virus-Status: Clean X-Virus-Scanned: clamav-milter 0.103.8 at mail Date: Fri, 11 Aug 2023 15:32:31 +0200 From: =?iso-8859-2?Q?Micha=B3_Miros=B3aw?= To: Muhammad Usama Anjum Cc: =?iso-8859-2?Q?Micha=B3_Miros=B3aw?= , 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 v28 2/6] fs/proc/task_mmu: Implement IOCTL to get and optionally clear info about PTEs Message-ID: References: <20230809061603.1969154-1-usama.anjum@collabora.com> <20230809061603.1969154-3-usama.anjum@collabora.com> <97de19a3-bba2-9260-7741-cd5b6f4581e9@collabora.com> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: <97de19a3-bba2-9260-7741-cd5b6f4581e9@collabora.com> X-Rspamd-Queue-Id: 3A7F740020 X-Rspam-User: X-Stat-Signature: ambsqita9faf1eyatsd7re694uey87tc X-Rspamd-Server: rspam01 X-HE-Tag: 1691760758-134870 X-HE-Meta: U2FsdGVkX1/LZt31FVMTt4tl+itMW2/rGeGvOzP7LoLSpQHK8V6/QFRU45na7xFmq0qyVLTgdoQZAIEL5LjZtO1r+/HGe3iYDc8ZkybF0or1sqPu9i+KcS8aFGQoXTWeQSKhjhbpRJGQ4a56npZdNKtf05oTwZC9Gy+huuSTO/BHfBC26XkPhFlesfd4BvanAS9kXEZiQEENmVHTmyXJkO908zEIgwfXjK08HQwcv+eQYdSfann8Cz70DbEPOiYiZj3hdaMH8CD4wvb4zOx3Zmlq/z5gl1B+TlAu6pjE85BhyDrro+//JZUkM4GiGh8TdFa4kRrWkMMESw9/TEo7UoR+RymkIOZRL8WsGCFA25XH+1kBT4xdja0cT0y1+kiNMroWqqigkly0DIKNezshWTEFsXhjh8tFBr2lFrKbgqHPXqyf/7FGO6K136U6vqQD1fKyn5QwRPMJHfO6eGqv8vU+Y5bDdnWVAA6jZn/hzNEEV3jDpRbYnGjqQlTLqj5zQPDlV9nQPdp72OJjet3X70ojuC+43jZ9YWXwBQtjYKRPg7EpiA97w0mhD7vWhsuob2MXlS2BTJVPdPmZetlxNWCnQ3CqtmX9xAQdyS+Mndjm3RD8cx5KAoYrAx/pi1Wo336Lsrvb7AEuKphr2PBUvBtzCZ/SljUcu+YFbB6MrpKxI4yydv3RzkCxN9hcK+bYiugAXjw7SHwUoTznJg2M6VOR/SLwfqVRwPLJ2ORpOR9iT20pSUSkbdofikbTo96A+7HWlQ5OvxRB3rQvlkRzsEQj1f/bXvcz1EA/NrBuaYNazt6fHKy2RVBhmpDfWpCgAjiCk9NMGf4wG6U5qscvtrUWzLFnXKJgptWGWnYIuupi78W26/twH03yC2Sp4OebUJXE44lGuLuf7VkxY/ZgeeHkIEdm4EHsmDEycHTb5OYHejbYvPgLGpobbUw9Od0xOL8/Dk6qtr4pdQofAeh Kv2lsxZb BrLRR6G5Lvu8iGu0z4xA9k/XNYztJQWpn7faa9o44D9YVVGQDEDyOdiaXHpHPRbfOc34UIjBM3HPPmJCIT/9Y8adKOEyNH5dQvSRD8Hl8aYIyi13xrS7uX2MLIWn1LIp9LoahtOu1JOGmDAj4SukJsLGlC6rRNAIvI4NXM1FzLJtHmXV+wnXfX/N7YEsr9b03Ms3cGl0DZLInocWfp3dBjg6OqeGzTmbSsR7UeTQG9NNgNEJXRyQ7wk2utw1xydIHD9U6j5u5jSsCPzu9u7Y0tuGsn/Z1kkAEDavI+pRZO8bT5KIsRfaCFdLrg9JeU+p0nFVkRH29a+iG2TTcdjPP9GCCbw== 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 Fri, Aug 11, 2023 at 05:02:44PM +0500, Muhammad Usama Anjum wrote: > On 8/10/23 10:32 PM, Michał Mirosław wrote: > > On Wed, 9 Aug 2023 at 08:16, Muhammad Usama Anjum > > wrote: [...] > >> --- a/fs/proc/task_mmu.c > >> +++ b/fs/proc/task_mmu.c > > [...] > >> +#ifdef CONFIG_TRANSPARENT_HUGEPAGE > >> +static unsigned long pagemap_thp_category(pmd_t pmd) > >> +{ > >> + unsigned long categories = PAGE_IS_HUGE; > >> + > >> + if (pmd_present(pmd)) { > >> + categories |= PAGE_IS_PRESENT; > >> + if (!pmd_uffd_wp(pmd)) > >> + categories |= PAGE_IS_WRITTEN; > >> + if (is_zero_pfn(pmd_pfn(pmd))) > >> + categories |= PAGE_IS_PFNZERO; > >> + } else if (is_swap_pmd(pmd)) { > >> + categories |= PAGE_IS_SWAPPED; > >> + if (!pmd_swp_uffd_wp(pmd)) > >> + categories |= PAGE_IS_WRITTEN; > >> + } > >> + > >> + return categories; > >> +} > > I guess THPs can't be file-backed currently, but can we somehow mark > > this assumption so it can be easily found if the capability arrives? > Yeah, THPs cannot be file backed. Lets not care for some feature which may > not arrive in several years or eternity. Yes, it might not arrive. But please add at least a comment, so that it is clearly visible that lack if PAGE_IS_FILE here is intentional. > >> +#endif /* CONFIG_TRANSPARENT_HUGEPAGE */ > >> + > >> +#ifdef CONFIG_HUGETLB_PAGE > >> +static unsigned long pagemap_hugetlb_category(pte_t pte) > >> +{ > >> + unsigned long categories = PAGE_IS_HUGE; > >> + > >> + if (pte_present(pte)) { > >> + categories |= PAGE_IS_PRESENT; > >> + if (!huge_pte_uffd_wp(pte)) > >> + categories |= PAGE_IS_WRITTEN; > >> + if (!PageAnon(pte_page(pte))) > >> + categories |= PAGE_IS_FILE; > >> + if (is_zero_pfn(pte_pfn(pte))) > >> + categories |= PAGE_IS_PFNZERO; > >> + } else if (is_swap_pte(pte)) { > >> + categories |= PAGE_IS_SWAPPED; > >> + if (!pte_swp_uffd_wp_any(pte)) > >> + categories |= PAGE_IS_WRITTEN; > >> + } > > > > BTW, can a HugeTLB page be file-backed and swapped out? > Accourding to pagemap_hugetlb_range(), file-backed HugeTLB page cannot be > swapped. Here too a comment that leaving out this case is intentional would be useful. > > [...] > >> + walk_start = p.arg.start; > >> + for (; walk_start < p.arg.end; walk_start = p.arg.walk_end) { [...[ > >> + ret = mmap_read_lock_killable(mm); > >> + if (ret) > >> + break; > >> + ret = walk_page_range(mm, walk_start, p.arg.end, > >> + &pagemap_scan_ops, &p); > >> + mmap_read_unlock(mm); [...] > >> + if (ret != -ENOSPC || p.arg.vec_len - 1 == 0 || > >> + p.found_pages == p.arg.max_pages) > >> + break; > > > > The second condition is equivalent to `p.arg.vec_len == 1`, but why is > > that an ending condition? Isn't the last entry enough to gather one > > more range? (The walk could have returned -ENOSPC due to buffer full > > and after flushing it could continue with the last free entry.) > Now we are walking the entire range walk_page_range(). We don't break loop > when we get -ENOSPC as this error may only mean that the temporary buffer > is full. So we need check if max pages have been found or output buffer is > full or ret is 0 or any other error. When p.arg.vec_len = 1 is end > condition as the last entry is in cur. As we have walked over the entire > range, cur must be full after which the walk returned. > > So current condition is necessary. I've double checked it. I'll change it > to `p.arg.vec_len == 1`. If we have walked the whole range, then the loop will end anyway due to `walk_start < walk_end` not held in the `for()`'s condition. [...] > >> +/* > >> + * struct pm_scan_arg - Pagemap ioctl argument > >> + * @size: Size of the structure > >> + * @flags: Flags for the IOCTL > >> + * @start: Starting address of the region > >> + * @end: Ending address of the region > >> + * @walk_end Address where the scan stopped (written by kernel). > >> + * walk_end == end informs that the scan completed on entire range. > > > > Can we ensure this holds also for the tagged pointers? > No, we cannot. So this need explanation in the comment here. (Though I'd still like to know how the address tags are supposed to be used from someone that knows them.) Best Regards Michał Mirosław