Received: by 2002:a05:6358:3188:b0:123:57c1:9b43 with SMTP id q8csp8895270rwd; Tue, 20 Jun 2023 23:42:33 -0700 (PDT) X-Google-Smtp-Source: ACHHUZ6oyPt5C81IJh+MIImfANNOG4n98e9i+/4rVslbxjHMPrcHjfixIaDNqzRB1Pv0lN04faBn X-Received: by 2002:a05:6a20:431b:b0:122:7709:5867 with SMTP id h27-20020a056a20431b00b0012277095867mr4185921pzk.59.1687329753707; Tue, 20 Jun 2023 23:42:33 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1687329753; cv=none; d=google.com; s=arc-20160816; b=tLuYqkT3kBirov35ruZe85l7yngyvRMAVFSFbg96j9xYzTc1jUm4GWKB/V4ObVicUF QHbsBRsacm3HvS+gVWSGe4PIFdEf/lD5nrmTVGwdb+h4GYPPssvL9jJatalFBD5HmWGQ /aBimqnoe9XjaW+jW+9f4oba3WvrR0HDfEyMj1y0iSURAMDAHk06VpMm82PrmcJZ0py5 B9lTPQkKCC+rD/3Cr5KRf4np6DFxFg2FfLyDFOmp294uGzvV8Je6jNmX1EZpn0uQzwqz UNGb34eExBY+EethEht9GUpDwroTM3C3xQQL2XUEB1+y2vwkNzSFKXlHLLd2s9LZEVvH 8pXg== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:content-transfer-encoding:in-reply-to:from :references:to:content-language:subject:cc:user-agent:mime-version :date:message-id:dkim-signature; bh=WLHG8vgDSyYE/FU/nhW6BCGjOaX5KO2lry/qM6ipaOM=; b=0ehhgYmFbldSBqMInuuHm3FmPbP55DSNBEmUzdEtk20MSDWffm0r1obOdzSirnQuS5 1yin74/iiSuZiVBwyB2cYY+/1Ep/1CEacjN9u5B1IeB60+8Iqn7+H5GaDSSvs8DpF3T/ TuKZAaYfB7VNK3/taMIzagFItJC3YIXfuG4+GLwydjUOAehju1a9BtOAoXKXZHjs2lG0 RLq7oyZ5+XWnnF++xJUKlbPqiiUktNXlqXwMb3nO/E0GhKpG5QsDJxS05wKl01Zp5z4H SZVdswsLu18DOWWVkcdHDhW4PGEC1y0zaW8dRHbNk5U8HBgIgnMSK4r28udc43nENoW7 i9lg== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@collabora.com header.s=mail header.b="U/8Yn5SX"; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=QUARANTINE sp=QUARANTINE dis=NONE) header.from=collabora.com Return-Path: Received: from out1.vger.email (out1.vger.email. [2620:137:e000::1:20]) by mx.google.com with ESMTP id z10-20020a170902d54a00b001b074fbdce3si3702570plf.478.2023.06.20.23.42.19; Tue, 20 Jun 2023 23:42:33 -0700 (PDT) Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20; Authentication-Results: mx.google.com; dkim=pass header.i=@collabora.com header.s=mail header.b="U/8Yn5SX"; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=QUARANTINE sp=QUARANTINE dis=NONE) header.from=collabora.com Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S230464AbjFUGg2 (ORCPT + 99 others); Wed, 21 Jun 2023 02:36:28 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:34662 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S230426AbjFUGf7 (ORCPT ); Wed, 21 Jun 2023 02:35:59 -0400 Received: from madras.collabora.co.uk (madras.collabora.co.uk [IPv6:2a00:1098:0:82:1000:25:2eeb:e5ab]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 30FEF1FDF; Tue, 20 Jun 2023 23:35:13 -0700 (PDT) Received: from [192.168.10.54] (unknown [119.155.63.248]) (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 245326606F53; Wed, 21 Jun 2023 07:34:58 +0100 (BST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=collabora.com; s=mail; t=1687329305; bh=AKyfB9/vduPdB/DpupJOlpdlwnxUH6NDiUXGlXhNUl0=; h=Date:Cc:Subject:To:References:From:In-Reply-To:From; b=U/8Yn5SXoAnIaXTwmxFghO7ui1+pgpcevJNcaOI6geU7P90B6OHAadPbQQN2qFt/l WKaeqQI62on8+oqUAK/d4ZezLz68NGAQA6ao9RwListEtu8AvJjokPiv1CEX3HuX9k YGcMi3Z7O3oyn7X4n/KqBdYVFgAZGX1DZlXWspjn+Il9L0+iMLEDJCyYvWYExISxcz jOYCguyayMBSSO7sUTd/+xsglCzjgj5ZUHa1c07GW+9wxrdKpg25AWpBKEkbzHrf+2 q0mCp3gmEoUfQRTLl3/XgQKZ+kDOQ9pJtpNMbPziYjj7Z8GEmKCHfXESxy+02N+Z64 RTX1EmMsOf01Q== Message-ID: <1c1beeda-ceed-fdab-bbf5-1881e0a8b102@collabora.com> Date: Wed, 21 Jun 2023 11:34:54 +0500 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101 Thunderbird/102.12.0 Cc: Muhammad Usama Anjum , Peter Xu , David Hildenbrand , Andrew Morton , =?UTF-8?B?TWljaGHFgiBNaXJvc8WC?= =?UTF-8?Q?aw?= , 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 v19 2/5] fs/proc/task_mmu: Implement IOCTL to get and optionally clear info about PTEs Content-Language: en-US To: Andrei Vagin References: <20230615141144.665148-1-usama.anjum@collabora.com> <20230615141144.665148-3-usama.anjum@collabora.com> From: Muhammad Usama Anjum In-Reply-To: Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit X-Spam-Status: No, score=-2.2 required=5.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,NICE_REPLY_A,SPF_HELO_NONE, SPF_PASS,T_SCC_BODY_TEXT_LINE,URIBL_BLOCKED autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on lindbergh.monkeyblade.net Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Hi, Thank you for your review. On 6/20/23 11:03 PM, Andrei Vagin wrote: ... >> +struct pagemap_scan_private { >> + struct page_region *vec_buf, cur_buf; >> + unsigned long long vec_buf_len, vec_buf_index, max_pages, found_pages, flags; > > should it be just unsigned long? These internal values are storing data coming from user in struct pm_scan_arg in which all variables are 64 bit(__u64) explicitly. This is why we have unsigned long long here. It is absolutely necessary. > >> + unsigned long long required_mask, anyof_mask, excluded_mask, return_mask; >> +}; >> + >> +static inline bool is_pte_uffd_wp(pte_t pte) >> +{ >> + return (pte_present(pte) && pte_uffd_wp(pte)) || >> + pte_swp_uffd_wp_any(pte); >> +} >> + >> +static inline void make_uffd_wp_pte(struct vm_area_struct *vma, >> + unsigned long addr, pte_t *pte) >> +{ >> + pte_t ptent = ptep_get(pte); >> + >> + if (pte_present(ptent)) { >> + pte_t old_pte; >> + >> + old_pte = ptep_modify_prot_start(vma, addr, pte); >> + ptent = pte_mkuffd_wp(ptent); >> + ptep_modify_prot_commit(vma, addr, pte, old_pte, ptent); >> + } else if (is_swap_pte(ptent)) { >> + ptent = pte_swp_mkuffd_wp(ptent); >> + set_pte_at(vma->vm_mm, addr, pte, ptent); >> + } else { >> + set_pte_at(vma->vm_mm, addr, pte, >> + make_pte_marker(PTE_MARKER_UFFD_WP)); >> + } >> +} >> + >> +#ifdef CONFIG_TRANSPARENT_HUGEPAGE >> +static inline bool is_pmd_uffd_wp(pmd_t pmd) >> +{ >> + return (pmd_present(pmd) && pmd_uffd_wp(pmd)) || >> + (is_swap_pmd(pmd) && pmd_swp_uffd_wp(pmd)); >> +} >> + >> +static inline void make_uffd_wp_pmd(struct vm_area_struct *vma, >> + unsigned long addr, pmd_t *pmdp) >> +{ >> + pmd_t old, pmd = *pmdp; >> + >> + if (pmd_present(pmd)) { >> + old = pmdp_invalidate_ad(vma, addr, pmdp); >> + pmd = pmd_mkuffd_wp(old); >> + set_pmd_at(vma->vm_mm, addr, pmdp, pmd); >> + } else if (is_migration_entry(pmd_to_swp_entry(pmd))) { >> + pmd = pmd_swp_mkuffd_wp(pmd); >> + set_pmd_at(vma->vm_mm, addr, pmdp, pmd); >> + } >> +} >> +#endif >> + >> +#ifdef CONFIG_HUGETLB_PAGE >> +static inline bool is_huge_pte_uffd_wp(pte_t pte) >> +{ >> + return (pte_present(pte) && huge_pte_uffd_wp(pte)) || >> + pte_swp_uffd_wp_any(pte); >> +} >> + >> +static inline void make_uffd_wp_huge_pte(struct vm_area_struct *vma, >> + unsigned long addr, pte_t *ptep, >> + pte_t ptent) >> +{ >> + if (is_hugetlb_entry_hwpoisoned(ptent) || is_pte_marker(ptent)) >> + return; >> + >> + if (is_hugetlb_entry_migration(ptent)) >> + set_huge_pte_at(vma->vm_mm, addr, ptep, >> + pte_swp_mkuffd_wp(ptent)); >> + else if (!huge_pte_none(ptent)) >> + huge_ptep_modify_prot_commit(vma, addr, ptep, ptent, >> + huge_pte_mkuffd_wp(ptent)); >> + else >> + set_huge_pte_at(vma->vm_mm, addr, ptep, >> + make_pte_marker(PTE_MARKER_UFFD_WP)); >> +} >> +#endif >> + >> +static int pagemap_scan_test_walk(unsigned long start, unsigned long end, >> + struct mm_walk *walk) >> +{ >> + struct pagemap_scan_private *p = walk->private; >> + struct vm_area_struct *vma = walk->vma; >> + >> + if ((p->flags & PM_SCAN_REQUIRE_UFFD) && (!userfaultfd_wp_async(vma) || >> + !userfaultfd_wp_use_markers(vma))) >> + return -EPERM; >> + >> + if (vma->vm_flags & VM_PFNMAP) >> + return 1; >> + >> + return 0; >> +} >> + >> +static int pagemap_scan_output(unsigned long bitmap, >> + struct pagemap_scan_private *p, >> + unsigned long addr, unsigned int n_pages) >> +{ >> + struct page_region *cur_buf = &p->cur_buf; >> + >> + if (!n_pages) >> + return -EINVAL; >> + >> + bitmap &= p->return_mask; >> + >> + if (cur_buf->flags == bitmap && >> + cur_buf->start + cur_buf->len * PAGE_SIZE == addr) { >> + cur_buf->len += n_pages; >> + p->found_pages += n_pages; >> + } else { >> + if (cur_buf->len) { > > I would add a comment that vec_buf_len has been decremented by one for > cur_buf. > >> + if (p->vec_buf_index >= p->vec_buf_len) >> + return PM_SCAN_BUFFER_FULL; >> + >> + memcpy(&p->vec_buf[p->vec_buf_index], cur_buf, >> + sizeof(*p->vec_buf)); >> + p->vec_buf_index++; >> + } >> + >> + cur_buf->start = addr; >> + cur_buf->len = n_pages; >> + cur_buf->flags = bitmap; >> + p->found_pages += n_pages; >> + } >> + >> + if (p->found_pages == p->max_pages) >> + return PM_SCAN_FOUND_MAX_PAGES; >> + >> + return 0; >> +} >> + >> +static bool pagemap_scan_is_interesting_page(unsigned long bitmap, >> + struct pagemap_scan_private *p) >> +{ >> + if ((p->required_mask & bitmap) != p->required_mask) >> + return false; >> + if (p->anyof_mask && !(p->anyof_mask & bitmap)) >> + return false; >> + if (p->excluded_mask & bitmap) >> + return false; >> + >> + return true; >> +} >> + >> +static int pagemap_scan_pmd_entry(pmd_t *pmd, unsigned long start, >> + unsigned long end, struct mm_walk *walk) >> +{ >> + bool is_written, flush = false, is_interesting = true; >> + struct pagemap_scan_private *p = walk->private; >> + struct vm_area_struct *vma = walk->vma; >> + unsigned long bitmap, addr = end; >> + pte_t *pte, *orig_pte, ptent; >> + spinlock_t *ptl; >> + int ret = 0; >> + >> + arch_enter_lazy_mmu_mode(); >> + >> +#ifdef CONFIG_TRANSPARENT_HUGEPAGE >> + ptl = pmd_trans_huge_lock(pmd, vma); >> + if (ptl) { >> + unsigned long n_pages = (end - start)/PAGE_SIZE; >> + >> + if (p->max_pages && n_pages > p->max_pages - p->found_pages) >> + n_pages = p->max_pages - p->found_pages; >> + >> + is_written = !is_pmd_uffd_wp(*pmd); >> + >> + /* >> + * Break huge page into small pages if the WP operation need to >> + * be performed is on a portion of the huge page. >> + */ >> + if (is_written && IS_PM_SCAN_WP(p->flags) && >> + n_pages < HPAGE_SIZE/PAGE_SIZE) { > > It might be worth stopping rather than splitting the huge page. Sometimes normal pages can be grouped to become THP by the kernel depending upon the configurations. We don't want to reject ioctl just because this folding operation happened. We don't want to stop rather spliting is the best option and it does work fine. > >> + spin_unlock(ptl); >> + >> + split_huge_pmd(vma, pmd, start); >> + goto process_smaller_pages; >> + } >> + >> + bitmap = PM_SCAN_FLAGS(is_written, (bool)vma->vm_file, >> + pmd_present(*pmd), is_swap_pmd(*pmd)); >> + >> + if (IS_PM_SCAN_GET(p->flags)) { >> + is_interesting = pagemap_scan_is_interesting_page(bitmap, p); >> + if (is_interesting) >> + ret = pagemap_scan_output(bitmap, p, start, n_pages); >> + } >> + >> + if (IS_PM_SCAN_WP(p->flags) && is_written && is_interesting && >> + ret >= 0) { >> + make_uffd_wp_pmd(vma, start, pmd); >> + flush_tlb_range(vma, start, end); >> + } >> + >> + spin_unlock(ptl); >> + >> + arch_leave_lazy_mmu_mode(); >> + return ret; >> + } >> + >> +process_smaller_pages: >> +#endif >> + >> + orig_pte = pte = pte_offset_map_lock(vma->vm_mm, pmd, start, &ptl); >> + if (!pte) { >> + walk->action = ACTION_AGAIN; >> + return 0; >> + } >> + >> + for (addr = start; addr < end && !ret; pte++, addr += PAGE_SIZE) { >> + ptent = ptep_get(pte); >> + is_written = !is_pte_uffd_wp(ptent); >> + >> + bitmap = PM_SCAN_FLAGS(is_written, (bool)vma->vm_file, >> + pte_present(ptent), is_swap_pte(ptent)); >> + >> + if (IS_PM_SCAN_GET(p->flags)) { >> + is_interesting = pagemap_scan_is_interesting_page(bitmap, p); >> + if (is_interesting) >> + ret = pagemap_scan_output(bitmap, p, addr, 1); >> + } >> + >> + if (IS_PM_SCAN_WP(p->flags) && is_written && is_interesting && >> + ret >= 0) { >> + make_uffd_wp_pte(vma, addr, pte); >> + flush = true; >> + } >> + } >> + >> + if (flush) >> + flush_tlb_range(vma, start, addr); >> + >> + pte_unmap_unlock(orig_pte, ptl); >> + arch_leave_lazy_mmu_mode(); >> + >> + cond_resched(); >> + return ret; >> +} >> + >> +#ifdef CONFIG_HUGETLB_PAGE >> +static int pagemap_scan_hugetlb_entry(pte_t *ptep, unsigned long hmask, >> + unsigned long start, unsigned long end, >> + struct mm_walk *walk) >> +{ >> + unsigned long n_pages = (end - start)/PAGE_SIZE; >> + struct pagemap_scan_private *p = walk->private; >> + struct vm_area_struct *vma = walk->vma; >> + bool is_written, is_interesting = true; >> + struct hstate *h = hstate_vma(vma); >> + unsigned long bitmap; >> + spinlock_t *ptl; >> + int ret = 0; >> + pte_t ptent; >> + >> + if (IS_PM_SCAN_WP(p->flags) && n_pages < HPAGE_SIZE/PAGE_SIZE) >> + return -EINVAL; >> + >> + if (p->max_pages && n_pages > p->max_pages - p->found_pages) >> + n_pages = p->max_pages - p->found_pages; >> + >> + if (IS_PM_SCAN_WP(p->flags)) { >> + i_mmap_lock_write(vma->vm_file->f_mapping); >> + ptl = huge_pte_lock(h, vma->vm_mm, ptep); >> + } >> + >> + ptent = huge_ptep_get(ptep); >> + is_written = !is_huge_pte_uffd_wp(ptent); >> + >> + /* >> + * Partial hugetlb page clear isn't supported >> + */ >> + if (is_written && IS_PM_SCAN_WP(p->flags) && >> + n_pages < HPAGE_SIZE/PAGE_SIZE) { > > n_pages depends on found_pages that is incremented in > pagemap_scan_output. Should we do this check right before calling > pagemap_scan_output? This condition is acting like a check before pagemap_scan_output() already. n_pages is telling us that how many empty pages are left to find out. But if by making n_pages less, we cannot cover the whole hugetlb, we should just return error as we don't split the hugetlb. > >> + ret = -ENOSPC; > > should it be PM_SCAN_FOUND_MAX_PAGES? Otherwise, it fails the ioctl even > if it has handled some pages already. It is a double edge sword. If we don't return error, user will never know that he needs to specify more max_pages or his output buffer is small and not coverig the entire range. The real purpose here that user gets aware that he needs to specify full hugetlb range and found pages should cover the entire range as well. > >> + goto unlock_and_return; >> + } >> + >> + bitmap = PM_SCAN_FLAGS(is_written, (bool)vma->vm_file, >> + pte_present(ptent), is_swap_pte(ptent)); >> + >> + if (IS_PM_SCAN_GET(p->flags)) { >> + is_interesting = pagemap_scan_is_interesting_page(bitmap, p); >> + if (is_interesting) >> + ret = pagemap_scan_output(bitmap, p, start, n_pages); >> + } >> + >> + if (IS_PM_SCAN_WP(p->flags) && is_written && is_interesting && >> + ret >= 0) { >> + make_uffd_wp_huge_pte(vma, start, ptep, ptent); >> + flush_hugetlb_tlb_range(vma, start, end); >> + } >> + >> +unlock_and_return: >> + if (IS_PM_SCAN_WP(p->flags)) { >> + spin_unlock(ptl); >> + i_mmap_unlock_write(vma->vm_file->f_mapping); >> + } >> + >> + return ret; >> +} >> +#else >> +#define pagemap_scan_hugetlb_entry NULL >> +#endif >> + >> +static int pagemap_scan_pte_hole(unsigned long addr, unsigned long end, >> + int depth, struct mm_walk *walk) >> +{ >> + unsigned long n_pages = (end - addr)/PAGE_SIZE; >> + struct pagemap_scan_private *p = walk->private; >> + struct vm_area_struct *vma = walk->vma; >> + int ret = 0; >> + >> + if (!vma || !IS_PM_SCAN_GET(p->flags)) >> + return 0; >> + >> + if (p->max_pages && n_pages > p->max_pages - p->found_pages) >> + n_pages = p->max_pages - p->found_pages; >> + >> + ret = pagemap_scan_output(PM_SCAN_FLAGS(false, (bool)vma->vm_file, >> + false, false), p, addr, n_pages); >> + >> + return ret; >> +} >> + >> +static const struct mm_walk_ops pagemap_scan_ops = { >> + .test_walk = pagemap_scan_test_walk, >> + .pmd_entry = pagemap_scan_pmd_entry, >> + .pte_hole = pagemap_scan_pte_hole, >> + .hugetlb_entry = pagemap_scan_hugetlb_entry, >> +}; >> + >> +static int pagemap_scan_args_valid(struct pm_scan_arg *arg, unsigned long start, >> + struct page_region __user *vec) >> +{ >> + /* Detect illegal size, flags, len and masks */ >> + if (arg->size != sizeof(struct pm_scan_arg)) >> + return -EINVAL; >> + if (!arg->flags) >> + return -EINVAL; >> + if (arg->flags & ~PM_SCAN_OPS) >> + return -EINVAL; >> + if (!arg->len) >> + return -EINVAL; >> + if ((arg->required_mask | arg->anyof_mask | arg->excluded_mask | >> + arg->return_mask) & ~PM_SCAN_BITS_ALL) >> + return -EINVAL; >> + if (!arg->required_mask && !arg->anyof_mask && >> + !arg->excluded_mask) >> + return -EINVAL; >> + if (!arg->return_mask) >> + return -EINVAL; >> + >> + /* Validate memory range */ >> + if (!IS_ALIGNED(start, PAGE_SIZE)) >> + return -EINVAL; >> + if (!access_ok((void __user *)start, arg->len)) >> + return -EFAULT; >> + >> + if (IS_PM_SCAN_GET(arg->flags)) { >> + if (!arg->vec) >> + return -EINVAL; >> + if (arg->vec_len == 0) >> + return -EINVAL; >> + if (!access_ok((void __user *)arg->vec, >> + arg->vec_len * sizeof(struct page_region))) >> + return -EFAULT; >> + } >> + >> + if (IS_PM_SCAN_WP(arg->flags) && !IS_PM_SCAN_GET(arg->flags) && >> + arg->max_pages) >> + return -EINVAL; >> + >> + return 0; >> +} >> + >> +static long do_pagemap_scan(struct mm_struct *mm, unsigned long __arg) >> +{ >> + struct pm_scan_arg __user *uarg = (struct pm_scan_arg __user *)__arg; >> + unsigned long long start, end, walk_start, walk_end; >> + unsigned long long empty_slots, vec_index = 0; >> + struct mmu_notifier_range range; >> + struct page_region __user *vec; >> + struct pagemap_scan_private p; >> + struct pm_scan_arg arg; >> + int ret = 0; >> + >> + if (copy_from_user(&arg, uarg, sizeof(arg))) >> + return -EFAULT; >> + >> + start = untagged_addr((unsigned long)arg.start); >> + vec = (struct page_region *)untagged_addr((unsigned long)arg.vec); >> + >> + ret = pagemap_scan_args_valid(&arg, start, vec); >> + if (ret) >> + return ret; >> + >> + end = start + arg.len; >> + p.max_pages = arg.max_pages; >> + p.found_pages = 0; >> + p.required_mask = arg.required_mask; >> + p.anyof_mask = arg.anyof_mask; >> + p.excluded_mask = arg.excluded_mask; >> + p.return_mask = arg.return_mask; >> + p.flags = arg.flags; >> + p.flags |= ((p.required_mask | p.anyof_mask | p.excluded_mask) & >> + PAGE_IS_WRITTEN) ? PM_SCAN_REQUIRE_UFFD : 0; >> + p.cur_buf.start = p.cur_buf.len = p.cur_buf.flags = 0; >> + p.vec_buf = NULL; >> + p.vec_buf_len = PAGEMAP_WALK_SIZE >> PAGE_SHIFT; >> + >> + /* >> + * Allocate smaller buffer to get output from inside the page walk >> + * functions and walk page range in PAGEMAP_WALK_SIZE size chunks. As >> + * we want to return output to user in compact form where no two >> + * consecutive regions should be continuous and have the same flags. >> + * So store the latest element in p.cur_buf between different walks and >> + * store the p.cur_buf at the end of the walk to the user buffer. >> + */ >> + if (IS_PM_SCAN_GET(p.flags)) { >> + p.vec_buf = kmalloc_array(p.vec_buf_len, sizeof(*p.vec_buf), >> + GFP_KERNEL); >> + if (!p.vec_buf) >> + return -ENOMEM; >> + } >> + >> + if (IS_PM_SCAN_WP(p.flags)) { >> + mmu_notifier_range_init(&range, MMU_NOTIFY_PROTECTION_VMA, 0, >> + mm, start, end); >> + mmu_notifier_invalidate_range_start(&range); >> + } >> + >> + walk_start = walk_end = start; >> + while (walk_end < end && !ret) { > > How long can it take to run this loop? Should we interrupt it if a > signal has been queued? I'm following mincore and pagemap_read here. There is no such thing there. IOCTL is performing what user has requested. If the execution time is a concern, user should have called the IOCTL on shorter address range. > >> + if (IS_PM_SCAN_GET(p.flags)) { >> + p.vec_buf_index = 0; >> + >> + /* >> + * All data is copied to cur_buf first. When more data >> + * is found, we push cur_buf to vec_buf and copy new >> + * data to cur_buf. Subtract 1 from length as the >> + * index of cur_buf isn't counted in length. >> + */ >> + empty_slots = arg.vec_len - vec_index; >> + p.vec_buf_len = min(p.vec_buf_len, empty_slots - 1); >> + } >> + >> + walk_end = (walk_start + PAGEMAP_WALK_SIZE) & PAGEMAP_WALK_MASK; >> + if (walk_end > end) >> + walk_end = end; >> + >> + ret = mmap_read_lock_killable(mm); >> + if (ret) >> + goto free_data; > > This will fail the ioctl, but it isn't acceptable if any pages has been > handled. I'm following pagemap_read() here. > > >> + ret = walk_page_range(mm, walk_start, walk_end, >> + &pagemap_scan_ops, &p); >> + mmap_read_unlock(mm); >> + > > ret can be ENOSPC returned from pagemap_scan_pmd_entry, but we should > not return this error if any pages has been handled. ENOSPC isn't being returned from pagemap_scan_pmd_entry now. > >> + if (ret && ret != PM_SCAN_BUFFER_FULL && >> + ret != PM_SCAN_FOUND_MAX_PAGES) >> + goto free_data; >> + >> + walk_start = walk_end; >> + if (IS_PM_SCAN_GET(p.flags) && p.vec_buf_index) { >> + if (copy_to_user(&vec[vec_index], p.vec_buf, >> + p.vec_buf_index * sizeof(*p.vec_buf))) { >> + /* >> + * Return error even though the OP succeeded >> + */ >> + ret = -EFAULT; >> + goto free_data; >> + } >> + vec_index += p.vec_buf_index; >> + } >> + } >> + >> + if (p.cur_buf.len) { >> + if (copy_to_user(&vec[vec_index], &p.cur_buf, sizeof(p.cur_buf))) { >> + ret = -EFAULT; >> + goto free_data; >> + } >> + vec_index++; >> + } >> + >> + ret = vec_index; >> + >> +free_data: >> + if (IS_PM_SCAN_WP(p.flags)) >> + mmu_notifier_invalidate_range_end(&range); >> + >> + kfree(p.vec_buf); >> + return ret; >> +} >> + >> +static long do_pagemap_cmd(struct file *file, unsigned int cmd, >> + unsigned long arg) >> +{ >> + struct mm_struct *mm = file->private_data; >> + >> + switch (cmd) { >> + case PAGEMAP_SCAN: >> + return do_pagemap_scan(mm, arg); >> + >> + default: >> + return -EINVAL; >> + } >> +} >> + >> const struct file_operations proc_pagemap_operations = { >> .llseek = mem_lseek, /* borrow this */ >> .read = pagemap_read, >> .open = pagemap_open, >> .release = pagemap_release, >> + .unlocked_ioctl = do_pagemap_cmd, >> + .compat_ioctl = do_pagemap_cmd, >> }; >> #endif /* CONFIG_PROC_PAGE_MONITOR */ >> >> diff --git a/include/linux/hugetlb.h b/include/linux/hugetlb.h >> index beb7c63d2871..a6e773c3e2b4 100644 >> --- a/include/linux/hugetlb.h >> +++ b/include/linux/hugetlb.h >> @@ -261,6 +261,7 @@ long hugetlb_change_protection(struct vm_area_struct *vma, >> unsigned long cp_flags); >> >> bool is_hugetlb_entry_migration(pte_t pte); >> +bool is_hugetlb_entry_hwpoisoned(pte_t pte); >> void hugetlb_unshare_all_pmds(struct vm_area_struct *vma); >> >> #else /* !CONFIG_HUGETLB_PAGE */ >> diff --git a/include/uapi/linux/fs.h b/include/uapi/linux/fs.h >> index b7b56871029c..a9fb44db84a3 100644 >> --- a/include/uapi/linux/fs.h >> +++ b/include/uapi/linux/fs.h >> @@ -305,4 +305,57 @@ typedef int __bitwise __kernel_rwf_t; >> #define RWF_SUPPORTED (RWF_HIPRI | RWF_DSYNC | RWF_SYNC | RWF_NOWAIT |\ >> RWF_APPEND) >> >> +/* Pagemap ioctl */ >> +#define PAGEMAP_SCAN _IOWR('f', 16, struct pm_scan_arg) >> + >> +/* Bits are set in flags of the page_region and masks in pm_scan_args */ >> +#define PAGE_IS_WRITTEN (1 << 0) >> +#define PAGE_IS_FILE (1 << 1) >> +#define PAGE_IS_PRESENT (1 << 2) >> +#define PAGE_IS_SWAPPED (1 << 3) >> + >> +/* >> + * struct page_region - Page region with flags >> + * @start: Start of the region >> + * @len: Length of the region in pages >> + * @bitmap: Bits sets for the region >> + */ >> +struct page_region { >> + __u64 start; >> + __u64 len; >> + __u64 flags; >> +}; >> + >> +/* >> + * struct pm_scan_arg - Pagemap ioctl argument >> + * @size: Size of the structure >> + * @flags: Flags for the IOCTL >> + * @start: Starting address of the region >> + * @len: Length of the region (All the pages in this length are included) >> + * @vec: Address of page_region struct array for output >> + * @vec_len: Length of the page_region struct array >> + * @max_pages: Optional max return pages >> + * @required_mask: Required mask - All of these bits have to be set in the PTE >> + * @anyof_mask: Any mask - Any of these bits are set in the PTE >> + * @excluded_mask: Exclude mask - None of these bits are set in the PTE >> + * @return_mask: Bits that are to be reported in page_region >> + */ >> +struct pm_scan_arg { >> + __u64 size; >> + __u64 flags; >> + __u64 start; >> + __u64 len; >> + __u64 vec; >> + __u64 vec_len; >> + __u64 max_pages; >> + __u64 required_mask; >> + __u64 anyof_mask; >> + __u64 excluded_mask; >> + __u64 return_mask; >> +}; >> + >> +/* Supported flags */ >> +#define PM_SCAN_OP_GET (1 << 0) >> +#define PM_SCAN_OP_WP (1 << 1) >> + >> #endif /* _UAPI_LINUX_FS_H */ >> diff --git a/mm/hugetlb.c b/mm/hugetlb.c >> index 0db13167b1ee..7e60f0f3fd03 100644 >> --- a/mm/hugetlb.c >> +++ b/mm/hugetlb.c >> @@ -4980,7 +4980,7 @@ bool is_hugetlb_entry_migration(pte_t pte) >> return false; >> } >> >> -static bool is_hugetlb_entry_hwpoisoned(pte_t pte) >> +bool is_hugetlb_entry_hwpoisoned(pte_t pte) >> { >> swp_entry_t swp; >> >> -- >> 2.39.2 >> -- BR, Muhammad Usama Anjum