*Changes in v12* - Update and other memory types to UFFD_FEATURE_WP_ASYNC - Rebaase on top of next-20230406 - Review updates
*Changes in v11* - Rebase on top of next-20230307 - Base patches on UFFD_FEATURE_WP_UNPOPULATED - Do a lot of cosmetic changes and review updates - Remove ENGAGE_WP + !GET operation as it can be performed with UFFDIO_WRITEPROTECT
*Changes in v10* - Add specific condition to return error if hugetlb is used with wp async - Move changes in tools/include/uapi/linux/fs.h to separate patch - Add documentation
*Changes in v9:* - Correct fault resolution for userfaultfd wp async - Fix build warnings and errors which were happening on some configs - Simplify pagemap ioctl's code
*Changes in v8:* - Update uffd async wp implementation - Improve PAGEMAP_IOCTL implementation
*Changes in v7:* - Add uffd wp async - Update the IOCTL to use uffd under the hood instead of soft-dirty flags
*Motivation* The real motivation for adding PAGEMAP_SCAN IOCTL is to emulate Windows GetWriteWatch() syscall [1]. The GetWriteWatch{} retrieves the addresses of the pages that are written to in a region of virtual memory.
This syscall is used in Windows applications and games etc. This syscall is being emulated in pretty slow manner in userspace. Our purpose is to enhance the kernel such that we translate it efficiently in a better way. Currently some out of tree hack patches are being used to efficiently emulate it in some kernels. We intend to replace those with these patches. So the whole gaming on Linux can effectively get benefit from this. It means there would be tons of users of this code.
CRIU use case [2] was mentioned by Andrei and Danylo:
Use cases for migrating sparse VMAs are binaries sanitized with ASAN, MSAN or TSAN [3]. All of these sanitizers produce sparse mappings of shadow memory [4]. Being able to migrate such binaries allows to highly reduce the amount of work needed to identify and fix post-migration crashes, which happen constantly.
Andrei's defines the following uses of this code: * it is more granular and allows us to track changed pages more effectively. The current interface can clear dirty bits for the entire process only. In addition, reading info about pages is a separate operation. It means we must freeze the process to read information about all its pages, reset dirty bits, only then we can start dumping pages. The information about pages becomes more and more outdated, while we are processing pages. The new interface solves both these downsides. First, it allows us to read pte bits and clear the soft-dirty bit atomically. It means that CRIU will not need to freeze processes to pre-dump their memory. Second, it clears soft-dirty bits for a specified region of memory. It means CRIU will have actual info about pages to the moment of dumping them. * The new interface has to be much faster because basic page filtering is happening in the kernel. With the old interface, we have to read pagemap for each page.
*Implementation Evolution (Short Summary)* From the definition of GetWriteWatch(), we feel like kernel's soft-dirty feature can be used under the hood with some additions like: * reset soft-dirty flag for only a specific region of memory instead of clearing the flag for the entire process * get and clear soft-dirty flag for a specific region atomically
So we decided to use ioctl on pagemap file to read or/and reset soft-dirty flag. But using soft-dirty flag, sometimes we get extra pages which weren't even written. They had become soft-dirty because of VMA merging and VM_SOFTDIRTY flag. This breaks the definition of GetWriteWatch(). We were able to by-pass this short coming by ignoring VM_SOFTDIRTY until David reported that mprotect etc messes up the soft-dirty flag while ignoring VM_SOFTDIRTY [5]. This wasn't happening until [6] got introduced. We discussed if we can revert these patches. But we could not reach to any conclusion. So at this point, I made couple of tries to solve this whole VM_SOFTDIRTY issue by correcting the soft-dirty implementation: * [7] Correct the bug fixed wrongly back in 2014. It had potential to cause regression. We left it behind. * [8] Keep a list of soft-dirty part of a VMA across splits and merges. I got the reply don't increase the size of the VMA by 8 bytes.
At this point, we left soft-dirty considering it is too much delicate and userfaultfd [9] seemed like the only way forward. From there onward, we have been basing soft-dirty emulation on userfaultfd wp feature where kernel resolves the faults itself when WP_ASYNC feature is used. It was straight forward to add WP_ASYNC feature in userfautlfd. Now we get only those pages dirty or written-to which are really written in reality. (PS There is another WP_UNPOPULATED userfautfd feature is required which is needed to avoid pre-faulting memory before write-protecting [9].)
All the different masks were added on the request of CRIU devs to create interface more generic and better.
[1] https://learn.microsoft.com/en-us/windows/win32/api/memoryapi/nf-memoryapi-g... [2] https://lore.kernel.org/all/20221014134802.1361436-1-mdanylo@google.com [3] https://github.com/google/sanitizers [4] https://github.com/google/sanitizers/wiki/AddressSanitizerAlgorithm#64-bit [5] https://lore.kernel.org/all/bfcae708-db21-04b4-0bbe-712badd03071@redhat.com [6] https://lore.kernel.org/all/20220725142048.30450-1-peterx@redhat.com/ [7] https://lore.kernel.org/all/20221122115007.2787017-1-usama.anjum@collabora.c... [8] https://lore.kernel.org/all/20221220162606.1595355-1-usama.anjum@collabora.c... [9] https://lore.kernel.org/all/20230306213925.617814-1-peterx@redhat.com [10] https://lore.kernel.org/all/20230125144529.1630917-1-mdanylo@google.com
* Original Cover letter from v8* Hello,
Note: Soft-dirty pages and pages which have been written-to are synonyms. As kernel already has soft-dirty feature inside which we have given up to use, we are using written-to terminology while using UFFD async WP under the hood.
This IOCTL, PAGEMAP_SCAN on pagemap file can be used to get and/or clear the info about page table entries. The following operations are supported in this ioctl: - Get the information if the pages have been written-to (PAGE_IS_WRITTEN), file mapped (PAGE_IS_FILE), present (PAGE_IS_PRESENT) or swapped (PAGE_IS_SWAPPED). - Write-protect the pages (PAGEMAP_WP_ENGAGE) to start finding which pages have been written-to. - Find pages which have been written-to and write protect the pages (atomic PAGE_IS_WRITTEN + PAGEMAP_WP_ENGAGE)
It is possible to find and clear soft-dirty pages entirely in userspace. But it isn't efficient: - The mprotect and SIGSEGV handler for bookkeeping - The userfaultfd wp (synchronous) with the handler for bookkeeping
Some benchmarks can be seen here[1]. This series adds features that weren't present earlier: - There is no atomic get soft-dirty/Written-to status and clear present in the kernel. - The pages which have been written-to can not be found in accurate way. (Kernel's soft-dirty PTE bit + sof_dirty VMA bit shows more soft-dirty pages than there actually are.)
Historically, soft-dirty PTE bit tracking has been used in the CRIU project. The procfs interface is enough for finding the soft-dirty bit status and clearing the soft-dirty bit of all the pages of a process. We have the use case where we need to track the soft-dirty PTE bit for only specific pages on-demand. We need this tracking and clear mechanism of a region of memory while the process is running to emulate the getWriteWatch() syscall of Windows.
*(Moved to using UFFD instead of soft-dirtyi feature to find pages which have been written-to from v7 patch series)*: Stop using the soft-dirty flags for finding which pages have been written to. It is too delicate and wrong as it shows more soft-dirty pages than the actual soft-dirty pages. There is no interest in correcting it [2][3] as this is how the feature was written years ago. It shouldn't be updated to changed behaviour. Peter Xu has suggested using the async version of the UFFD WP [4] as it is based inherently on the PTEs.
So in this patch series, I've added a new mode to the UFFD which is asynchronous version of the write protect. When this variant of the UFFD WP is used, the page faults are resolved automatically by the kernel. The pages which have been written-to can be found by reading pagemap file (!PM_UFFD_WP). This feature can be used successfully to find which pages have been written to from the time the pages were write protected. This works just like the soft-dirty flag without showing any extra pages which aren't soft-dirty in reality.
The information related to pages if the page is file mapped, present and swapped is required for the CRIU project [5][6]. The addition of the required mask, any mask, excluded mask and return masks are also required for the CRIU project [5].
The IOCTL returns the addresses of the pages which match the specific masks. The page addresses are returned in struct page_region in a compact form. The max_pages is needed to support a use case where user only wants to get a specific number of pages. So there is no need to find all the pages of interest in the range when max_pages is specified. The IOCTL returns when the maximum number of the pages are found. The max_pages is optional. If max_pages is specified, it must be equal or greater than the vec_size. This restriction is needed to handle worse case when one page_region only contains info of one page and it cannot be compacted. This is needed to emulate the Windows getWriteWatch() syscall.
The patch series include the detailed selftest which can be used as an example for the uffd async wp test and PAGEMAP_IOCTL. It shows the interface usages as well.
[1] https://lore.kernel.org/lkml/54d4c322-cd6e-eefd-b161-2af2b56aae24@collabora.... [2] https://lore.kernel.org/all/20221220162606.1595355-1-usama.anjum@collabora.c... [3] https://lore.kernel.org/all/20221122115007.2787017-1-usama.anjum@collabora.c... [4] https://lore.kernel.org/all/Y6Hc2d+7eTKs7AiH@x1n [5] https://lore.kernel.org/all/YyiDg79flhWoMDZB@gmail.com/ [6] https://lore.kernel.org/all/20221014134802.1361436-1-mdanylo@google.com/
Regards, Muhammad Usama Anjum
Muhammad Usama Anjum (4): fs/proc/task_mmu: Implement IOCTL to get and optionally clear info about PTEs tools headers UAPI: Update linux/fs.h with the kernel sources mm/pagemap: add documentation of PAGEMAP_SCAN IOCTL selftests: mm: add pagemap ioctl tests
Peter Xu (1): userfaultfd: UFFD_FEATURE_WP_ASYNC
Documentation/admin-guide/mm/pagemap.rst | 56 + Documentation/admin-guide/mm/userfaultfd.rst | 35 + fs/proc/task_mmu.c | 426 ++++++ fs/userfaultfd.c | 26 +- include/linux/userfaultfd_k.h | 29 +- include/uapi/linux/fs.h | 53 + include/uapi/linux/userfaultfd.h | 9 +- mm/hugetlb.c | 32 +- mm/memory.c | 27 +- tools/include/uapi/linux/fs.h | 53 + tools/testing/selftests/mm/.gitignore | 1 + tools/testing/selftests/mm/Makefile | 4 +- tools/testing/selftests/mm/config | 1 + tools/testing/selftests/mm/pagemap_ioctl.c | 1301 ++++++++++++++++++ tools/testing/selftests/mm/run_vmtests.sh | 4 + 15 files changed, 2034 insertions(+), 23 deletions(-) create mode 100644 tools/testing/selftests/mm/pagemap_ioctl.c mode change 100644 => 100755 tools/testing/selftests/mm/run_vmtests.sh
From: Peter Xu peterx@redhat.com
This patch adds a new userfaultfd-wp feature UFFD_FEATURE_WP_ASYNC, that allows userfaultfd wr-protect faults to be resolved by the kernel directly.
It can be used like a high accuracy version of soft-dirty, without vma modifications during tracking, and also with ranged support by default rather than for a whole mm when reset the protections due to existence of ioctl(UFFDIO_WRITEPROTECT).
Several goals of such a dirty tracking interface:
1. All types of memory should be supported and tracable. This is nature for soft-dirty but should mention when the context is userfaultfd, because it used to only support anon/shmem/hugetlb. The problem is for a dirty tracking purpose these three types may not be enough, and it's legal to track anything e.g. any page cache writes from mmap.
2. Protections can be applied to partial of a memory range, without vma split/merge fuss. The hope is that the tracking itself should not affect any vma layout change. It also helps when reset happens because the reset will not need mmap write lock which can block the tracee.
3. Accuracy needs to be maintained. This means we need pte markers to work on any type of VMA.
One could question that, the whole concept of async dirty tracking is not really close to fundamentally what userfaultfd used to be: it's not "a fault to be serviced by userspace" anymore. However, using userfaultfd-wp here as a framework is convenient for us in at least:
1. VM_UFFD_WP vma flag, which has a very good name to suite something like this, so we don't need VM_YET_ANOTHER_SOFT_DIRTY. Just use a new feature bit to identify from a sync version of uffd-wp registration.
2. PTE markers logic can be leveraged across the whole kernel to maintain the uffd-wp bit as long as an arch supports, this also applies to this case where uffd-wp bit will be a hint to dirty information and it will not go lost easily (e.g. when some page cache ptes got zapped).
3. Reuse ioctl(UFFDIO_WRITEPROTECT) interface for either starting or resetting a range of memory, while there's no counterpart in the old soft-dirty world, hence if this is wanted in a new design we'll need a new interface otherwise.
We can somehow understand that commonality because uffd-wp was fundamentally a similar idea of write-protecting pages just like soft-dirty.
This implementation allows WP_ASYNC to imply WP_UNPOPULATED, because so far WP_ASYNC seems to not usable if without WP_UNPOPULATE. This also gives us chance to modify impl of WP_ASYNC just in case it could be not depending on WP_UNPOPULATED anymore in the future kernels. It's also fine to imply that because both features will rely on PTE_MARKER_UFFD_WP config option, so they'll show up together (or both missing) in an UFFDIO_API probe.
vma_can_userfault() now allows any VMA if the userfaultfd registration is only about async uffd-wp. So we can track dirty for all kinds of memory including generic file systems (like XFS, EXT4 or BTRFS).
One trick worth mention in do_wp_page() is that we need to manually update vmf->orig_pte here because it can be used later with a pte_same() check - this path always has FAULT_FLAG_ORIG_PTE_VALID set in the flags.
The major defect of this approach of dirty tracking is we need to populate the pgtables when tracking starts. Soft-dirty doesn't do it like that. It's unwanted in the case where the range of memory to track is huge and unpopulated (e.g., tracking updates on a 10G file with mmap() on top, without having any page cache installed yet). One way to improve this is to allow pte markers exist for larger than PTE level for PMD+. That will not change the interface if to implemented, so we can leave that for later.
Co-developed-by: Muhammad Usama Anjum usama.anjum@collabora.com Signed-off-by: Muhammad Usama Anjum usama.anjum@collabora.com Signed-off-by: Peter Xu peterx@redhat.com --- Changes in v12: - Peter added the hugetlb support and revamped some other implementation - Transferred the authorship to Peter - Merge documentation to this patch
Changes in v11: - Fix return code in userfaultfd_register() and minor changes here and there - Rebase on top of next-20230307 - Base patches on UFFD_FEATURE_WP_UNPOPULATED https://lore.kernel.org/all/20230306213925.617814-1-peterx@redhat.com - UFFD_FEATURE_WP_ASYNC depends on UFFD_FEATURE_WP_UNPOPULATED to work (correctly)
Changes in v10: - Build fix - Update comments and add error condition to return error from uffd register if hugetlb pages are present when wp async flag is set
Changes in v9: - Correct the fault resolution with code contributed by Peter
Changes in v7: - Remove UFFDIO_WRITEPROTECT_MODE_ASYNC_WP and add UFFD_FEATURE_WP_ASYNC - Handle automatic page fault resolution in better way (thanks to Peter) --- Documentation/admin-guide/mm/userfaultfd.rst | 35 ++++++++++++++++++++ fs/userfaultfd.c | 26 ++++++++++++--- include/linux/userfaultfd_k.h | 21 +++++++++++- include/uapi/linux/userfaultfd.h | 9 ++++- mm/hugetlb.c | 32 ++++++++++-------- mm/memory.c | 27 +++++++++++++-- 6 files changed, 128 insertions(+), 22 deletions(-)
diff --git a/Documentation/admin-guide/mm/userfaultfd.rst b/Documentation/admin-guide/mm/userfaultfd.rst index 7c304e432205..4b7f43fbbe18 100644 --- a/Documentation/admin-guide/mm/userfaultfd.rst +++ b/Documentation/admin-guide/mm/userfaultfd.rst @@ -244,6 +244,41 @@ write-protected (so future writes will also result in a WP fault). These ioctls support a mode flag (``UFFDIO_COPY_MODE_WP`` or ``UFFDIO_CONTINUE_MODE_WP`` respectively) to configure the mapping this way.
+If the userfaultfd context has ``UFFD_FEATURE_WP_ASYNC`` feature bit set, +any vma registered with write-protection will work in async mode rather +than the default sync mode. + +In async mode, there will be no message generated when a write operation +happens, meanwhile the write-protection will be resolved automatically by +the kernel. It can be seen as a more accurate version of soft-dirty +tracking and it can be different in a few ways: + + - The dirty result will not be affected by vma changes (e.g. vma + merging) because the dirty is only tracked by the pte. + + - It supports range operations by default, so one can enable tracking on + any range of memory as long as page aligned. + + - Dirty information will not get lost if the pte was zapped due to + various reasons (e.g. during split of a shmem transparent huge page). + + - Due to a reverted meaning of soft-dirty (page clean when uffd-wp bit + set; dirty when uffd-wp bit cleared), it has different semantics on + some of the memory operations. For example: ``MADV_DONTNEED`` on + anonymous (or ``MADV_REMOVE`` on a file mapping) will be treated as + dirtying of memory by dropping uffd-wp bit during the procedure. + +The user app can collect the "written/dirty" status by looking up the +uffd-wp bit for the pages being interested in /proc/pagemap. + +The page will not be under track of uffd-wp async mode until the page is +explicitly write-protected by ``ioctl(UFFDIO_WRITEPROTECT)`` with the mode +flag ``UFFDIO_WRITEPROTECT_MODE_WP`` set. Trying to resolve a page fault +that was tracked by async mode userfaultfd-wp is invalid. + +When userfaultfd-wp async mode is used alone, it can be applied to all +kinds of memory. + QEMU/KVM ========
diff --git a/fs/userfaultfd.c b/fs/userfaultfd.c index 359c8d1e590d..6c34a84fa12b 100644 --- a/fs/userfaultfd.c +++ b/fs/userfaultfd.c @@ -123,6 +123,11 @@ static bool userfaultfd_is_initialized(struct userfaultfd_ctx *ctx) return ctx->features & UFFD_FEATURE_INITIALIZED; }
+static bool userfaultfd_wp_async_ctx(struct userfaultfd_ctx *ctx) +{ + return ctx && (ctx->features & UFFD_FEATURE_WP_ASYNC); +} + /* * Whether WP_UNPOPULATED is enabled on the uffd context. It is only * meaningful when userfaultfd_wp()==true on the vma and when it's @@ -1332,6 +1337,7 @@ static int userfaultfd_register(struct userfaultfd_ctx *ctx, bool basic_ioctls; unsigned long start, end, vma_end; struct vma_iterator vmi; + bool wp_async = userfaultfd_wp_async_ctx(ctx);
user_uffdio_register = (struct uffdio_register __user *) arg;
@@ -1405,7 +1411,7 @@ static int userfaultfd_register(struct userfaultfd_ctx *ctx,
/* check not compatible vmas */ ret = -EINVAL; - if (!vma_can_userfault(cur, vm_flags)) + if (!vma_can_userfault(cur, vm_flags, wp_async)) goto out_unlock;
/* @@ -1464,7 +1470,7 @@ static int userfaultfd_register(struct userfaultfd_ctx *ctx, for_each_vma_range(vmi, vma, end) { cond_resched();
- BUG_ON(!vma_can_userfault(vma, vm_flags)); + BUG_ON(!vma_can_userfault(vma, vm_flags, wp_async)); BUG_ON(vma->vm_userfaultfd_ctx.ctx && vma->vm_userfaultfd_ctx.ctx != ctx); WARN_ON(!(vma->vm_flags & VM_MAYWRITE)); @@ -1563,6 +1569,7 @@ static int userfaultfd_unregister(struct userfaultfd_ctx *ctx, unsigned long start, end, vma_end; const void __user *buf = (void __user *)arg; struct vma_iterator vmi; + bool wp_async = userfaultfd_wp_async_ctx(ctx);
ret = -EFAULT; if (copy_from_user(&uffdio_unregister, buf, sizeof(uffdio_unregister))) @@ -1616,7 +1623,7 @@ static int userfaultfd_unregister(struct userfaultfd_ctx *ctx, * provides for more strict behavior to notice * unregistration errors. */ - if (!vma_can_userfault(cur, cur->vm_flags)) + if (!vma_can_userfault(cur, cur->vm_flags, wp_async)) goto out_unlock;
found = true; @@ -1629,7 +1636,7 @@ static int userfaultfd_unregister(struct userfaultfd_ctx *ctx, for_each_vma_range(vmi, vma, end) { cond_resched();
- BUG_ON(!vma_can_userfault(vma, vma->vm_flags)); + BUG_ON(!vma_can_userfault(vma, vma->vm_flags, wp_async));
/* * Nothing to do: this vma is already registered into this @@ -1966,6 +1973,11 @@ static int userfaultfd_continue(struct userfaultfd_ctx *ctx, unsigned long arg) return ret; }
+bool userfaultfd_wp_async(struct vm_area_struct *vma) +{ + return userfaultfd_wp_async_ctx(vma->vm_userfaultfd_ctx.ctx); +} + static inline unsigned int uffd_ctx_features(__u64 user_features) { /* @@ -1997,6 +2009,11 @@ static int userfaultfd_api(struct userfaultfd_ctx *ctx, ret = -EPERM; if ((features & UFFD_FEATURE_EVENT_FORK) && !capable(CAP_SYS_PTRACE)) goto err_out; + + /* WP_ASYNC relies on WP_UNPOPULATED, choose it unconditionally */ + if (features & UFFD_FEATURE_WP_ASYNC) + features |= UFFD_FEATURE_WP_UNPOPULATED; + /* report all available features and ioctls to userland */ uffdio_api.features = UFFD_API_FEATURES; #ifndef CONFIG_HAVE_ARCH_USERFAULTFD_MINOR @@ -2009,6 +2026,7 @@ static int userfaultfd_api(struct userfaultfd_ctx *ctx, #ifndef CONFIG_PTE_MARKER_UFFD_WP uffdio_api.features &= ~UFFD_FEATURE_WP_HUGETLBFS_SHMEM; uffdio_api.features &= ~UFFD_FEATURE_WP_UNPOPULATED; + uffdio_api.features &= ~UFFD_FEATURE_WP_ASYNC; #endif uffdio_api.ioctls = UFFD_API_IOCTLS; ret = -EFAULT; diff --git a/include/linux/userfaultfd_k.h b/include/linux/userfaultfd_k.h index d78b01524349..ce4f9d18de3e 100644 --- a/include/linux/userfaultfd_k.h +++ b/include/linux/userfaultfd_k.h @@ -157,11 +157,22 @@ static inline bool userfaultfd_armed(struct vm_area_struct *vma) }
static inline bool vma_can_userfault(struct vm_area_struct *vma, - unsigned long vm_flags) + unsigned long vm_flags, + bool wp_async) { + vm_flags &= __VM_UFFD_FLAGS; + if ((vm_flags & VM_UFFD_MINOR) && (!is_vm_hugetlb_page(vma) && !vma_is_shmem(vma))) return false; + + /* + * If wp async enabled, and WP is the only mode enabled, allow any + * memory type. + */ + if (wp_async && (vm_flags == VM_UFFD_WP)) + return true; + #ifndef CONFIG_PTE_MARKER_UFFD_WP /* * If user requested uffd-wp but not enabled pte markers for @@ -171,6 +182,8 @@ static inline bool vma_can_userfault(struct vm_area_struct *vma, if ((vm_flags & VM_UFFD_WP) && !vma_is_anonymous(vma)) return false; #endif + + /* By default, allow any of anon|shmem|hugetlb */ return vma_is_anonymous(vma) || is_vm_hugetlb_page(vma) || vma_is_shmem(vma); } @@ -193,6 +206,7 @@ extern int userfaultfd_unmap_prep(struct mm_struct *mm, unsigned long start, extern void userfaultfd_unmap_complete(struct mm_struct *mm, struct list_head *uf); extern bool userfaultfd_wp_unpopulated(struct vm_area_struct *vma); +extern bool userfaultfd_wp_async(struct vm_area_struct *vma);
#else /* CONFIG_USERFAULTFD */
@@ -293,6 +307,11 @@ static inline bool userfaultfd_wp_unpopulated(struct vm_area_struct *vma) return false; }
+static inline bool userfaultfd_wp_async(struct vm_area_struct *vma) +{ + return false; +} + #endif /* CONFIG_USERFAULTFD */
static inline bool userfaultfd_wp_use_markers(struct vm_area_struct *vma) diff --git a/include/uapi/linux/userfaultfd.h b/include/uapi/linux/userfaultfd.h index 66dd4cd277bd..cfb87a112a9f 100644 --- a/include/uapi/linux/userfaultfd.h +++ b/include/uapi/linux/userfaultfd.h @@ -39,7 +39,8 @@ UFFD_FEATURE_MINOR_SHMEM | \ UFFD_FEATURE_EXACT_ADDRESS | \ UFFD_FEATURE_WP_HUGETLBFS_SHMEM | \ - UFFD_FEATURE_WP_UNPOPULATED) + UFFD_FEATURE_WP_UNPOPULATED | \ + UFFD_FEATURE_WP_ASYNC) #define UFFD_API_IOCTLS \ ((__u64)1 << _UFFDIO_REGISTER | \ (__u64)1 << _UFFDIO_UNREGISTER | \ @@ -210,6 +211,11 @@ struct uffdio_api { * (i.e. empty ptes). This will be the default behavior for shmem * & hugetlbfs, so this flag only affects anonymous memory behavior * when userfault write-protection mode is registered. + * + * UFFD_FEATURE_WP_ASYNC indicates that userfaultfd write-protection + * asynchronous mode is supported in which the write fault is + * automatically resolved and write-protection is un-set. + * It implies UFFD_FEATURE_WP_UNPOPULATED. */ #define UFFD_FEATURE_PAGEFAULT_FLAG_WP (1<<0) #define UFFD_FEATURE_EVENT_FORK (1<<1) @@ -225,6 +231,7 @@ struct uffdio_api { #define UFFD_FEATURE_EXACT_ADDRESS (1<<11) #define UFFD_FEATURE_WP_HUGETLBFS_SHMEM (1<<12) #define UFFD_FEATURE_WP_UNPOPULATED (1<<13) +#define UFFD_FEATURE_WP_ASYNC (1<<14) __u64 features;
__u64 ioctls; diff --git a/mm/hugetlb.c b/mm/hugetlb.c index efc443a906fa..daf13a09591a 100644 --- a/mm/hugetlb.c +++ b/mm/hugetlb.c @@ -6142,21 +6142,27 @@ vm_fault_t hugetlb_fault(struct mm_struct *mm, struct vm_area_struct *vma, /* Handle userfault-wp first, before trying to lock more pages */ if (userfaultfd_wp(vma) && huge_pte_uffd_wp(huge_ptep_get(ptep)) && (flags & FAULT_FLAG_WRITE) && !huge_pte_write(entry)) { - struct vm_fault vmf = { - .vma = vma, - .address = haddr, - .real_address = address, - .flags = flags, - }; + if (!userfaultfd_wp_async(vma)) { + struct vm_fault vmf = { + .vma = vma, + .address = haddr, + .real_address = address, + .flags = flags, + };
- spin_unlock(ptl); - if (pagecache_folio) { - folio_unlock(pagecache_folio); - folio_put(pagecache_folio); + spin_unlock(ptl); + if (pagecache_folio) { + folio_unlock(pagecache_folio); + folio_put(pagecache_folio); + } + hugetlb_vma_unlock_read(vma); + mutex_unlock(&hugetlb_fault_mutex_table[hash]); + return handle_userfault(&vmf, VM_UFFD_WP); } - hugetlb_vma_unlock_read(vma); - mutex_unlock(&hugetlb_fault_mutex_table[hash]); - return handle_userfault(&vmf, VM_UFFD_WP); + + entry = huge_pte_clear_uffd_wp(entry); + set_huge_pte_at(mm, haddr, ptep, entry); + /* Fallthrough to CoW */ }
/* diff --git a/mm/memory.c b/mm/memory.c index 42dd1ab5e4e6..7433cf6d63ea 100644 --- a/mm/memory.c +++ b/mm/memory.c @@ -3328,11 +3328,28 @@ static vm_fault_t do_wp_page(struct vm_fault *vmf) const bool unshare = vmf->flags & FAULT_FLAG_UNSHARE; struct vm_area_struct *vma = vmf->vma; struct folio *folio = NULL; + pte_t pte;
if (likely(!unshare)) { if (userfaultfd_pte_wp(vma, *vmf->pte)) { - pte_unmap_unlock(vmf->pte, vmf->ptl); - return handle_userfault(vmf, VM_UFFD_WP); + if (!userfaultfd_wp_async(vma)) { + pte_unmap_unlock(vmf->pte, vmf->ptl); + return handle_userfault(vmf, VM_UFFD_WP); + } + + /* + * Nothing needed (cache flush, TLB invalidations, + * etc.) because we're only removing the uffd-wp bit, + * which is completely invisible to the user. + */ + pte = pte_clear_uffd_wp(*vmf->pte); + + set_pte_at(vma->vm_mm, vmf->address, vmf->pte, pte); + /* + * Update this to be prepared for following up CoW + * handling + */ + vmf->orig_pte = pte; }
/* @@ -4821,8 +4838,11 @@ static inline vm_fault_t wp_huge_pmd(struct vm_fault *vmf)
if (vma_is_anonymous(vmf->vma)) { if (likely(!unshare) && - userfaultfd_huge_pmd_wp(vmf->vma, vmf->orig_pmd)) + userfaultfd_huge_pmd_wp(vmf->vma, vmf->orig_pmd)) { + if (userfaultfd_wp_async(vmf->vma)) + goto split; return handle_userfault(vmf, VM_UFFD_WP); + } return do_huge_pmd_wp_page(vmf); }
@@ -4834,6 +4854,7 @@ static inline vm_fault_t wp_huge_pmd(struct vm_fault *vmf) } }
+split: /* COW or write-notify handled on pte level: split pmd. */ __split_huge_pmd(vmf->vma, vmf->pmd, vmf->address, false, NULL);
This IOCTL, PAGEMAP_SCAN on pagemap file can be used to get and/or clear the info about page table entries. The following operations are supported in this ioctl: - Get the information if the pages have been written-to (PAGE_IS_WRITTEN), file mapped (PAGE_IS_FILE), present (PAGE_IS_PRESENT) or swapped (PAGE_IS_SWAPPED). - Find pages which have been written-to and write protect the pages (atomic PAGE_IS_WRITTEN + PAGEMAP_WP_ENGAGE)
This IOCTL can be extended to get information about more PTE bits.
Signed-off-by: Muhammad Usama Anjum usama.anjum@collabora.com --- Changes in v12: - Add hugetlb support to cover all memory types - Merge "userfaultfd: Define dummy uffd_wp_range()" with this patch - Review updates to the code
Changes in v11: - Find written pages in a better way - Fix a corner case (thanks Paul) - Improve the code/comments - remove ENGAGE_WP + ! GET operation - shorten the commit message in favour of moving documentation to pagemap.rst
Changes in v10: - move changes in tools/include/uapi/linux/fs.h to separate patch - update commit message
Change in v8: - Correct is_pte_uffd_wp() - Improve readability and error checks - Remove some un-needed code
Changes in v7: - Rebase on top of latest next - Fix some corner cases - Base soft-dirty on the uffd wp async - Update the terminologies - Optimize the memory usage inside the ioctl
Changes in v6: - Rename variables and update comments - Make IOCTL independent of soft_dirty config - Change masks and bitmap type to _u64 - Improve code quality
Changes in v5: - Remove tlb flushing even for clear operation
Changes in v4: - Update the interface and implementation
Changes in v3: - Tighten the user-kernel interface by using explicit types and add more error checking
Changes in v2: - Convert the interface from syscall to ioctl - Remove pidfd support as it doesn't make sense in ioctl --- fs/proc/task_mmu.c | 426 ++++++++++++++++++++++++++++++++++ include/linux/userfaultfd_k.h | 8 + include/uapi/linux/fs.h | 53 +++++ 3 files changed, 487 insertions(+)
diff --git a/fs/proc/task_mmu.c b/fs/proc/task_mmu.c index 6134f0df4164..36531b04bcd6 100644 --- a/fs/proc/task_mmu.c +++ b/fs/proc/task_mmu.c @@ -19,6 +19,7 @@ #include <linux/shmem_fs.h> #include <linux/uaccess.h> #include <linux/pkeys.h> +#include <linux/minmax.h>
#include <asm/elf.h> #include <asm/tlb.h> @@ -1135,6 +1136,26 @@ static inline void clear_soft_dirty(struct vm_area_struct *vma, } #endif
+static inline bool is_pte_uffd_wp(pte_t pte) +{ + return ((pte_present(pte) && pte_uffd_wp(pte)) || + pte_swp_uffd_wp_any(pte)); +} + +#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)); +} +#endif + +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))); +} + #if defined(CONFIG_MEM_SOFT_DIRTY) && defined(CONFIG_TRANSPARENT_HUGEPAGE) static inline void clear_soft_dirty_pmd(struct vm_area_struct *vma, unsigned long addr, pmd_t *pmdp) @@ -1768,11 +1789,416 @@ static int pagemap_release(struct inode *inode, struct file *file) return 0; }
+#define PM_SCAN_FOUND_MAX_PAGES (1) +#define PM_SCAN_BITS_ALL (PAGE_IS_WRITTEN | PAGE_IS_FILE | \ + PAGE_IS_PRESENT | PAGE_IS_SWAPPED) +#define PM_SCAN_OPS (PM_SCAN_OP_GET | PM_SCAN_OP_WP) +#define PM_SCAN_OP_IS_WP(a) (a->flags & PM_SCAN_OP_WP) +#define PM_SCAN_BITMAP(wt, file, present, swap) \ + (wt | file << 1 | present << 2 | swap << 3) + +struct pagemap_scan_private { + struct page_region *vec; + struct page_region cur; + unsigned long vec_len, vec_index; + unsigned int max_pages, found_pages, flags; + unsigned long required_mask, anyof_mask, excluded_mask, return_mask; +}; + +static inline bool pagemap_scan_is_written_set(struct pagemap_scan_private *p) +{ + return ((p->required_mask | p->anyof_mask | p->excluded_mask) & + PAGE_IS_WRITTEN); +} + +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 (pagemap_scan_is_written_set(p) && (!userfaultfd_wp(vma) || + !userfaultfd_wp_async(vma))) + return -EPERM; + + if (vma->vm_flags & VM_PFNMAP) + return 1; + + return 0; +} + +static int pagemap_scan_output(bool wt, bool file, bool pres, bool swap, + struct pagemap_scan_private *p, + unsigned long addr, unsigned int n_pages) +{ + unsigned long bitmap = PM_SCAN_BITMAP(wt, file, pres, swap); + struct page_region *cur = &p->cur; + + if (!n_pages) + return -EINVAL; + + if ((p->required_mask & bitmap) != p->required_mask) + return 0; + if (p->anyof_mask && !(p->anyof_mask & bitmap)) + return 0; + if (p->excluded_mask & bitmap) + return 0; + + bitmap &= p->return_mask; + if (!bitmap) + return 0; + + if ((cur->start + cur->len * PAGE_SIZE == addr) && + (cur->bitmap == bitmap)) { + cur->len += n_pages; + p->found_pages += n_pages; + + if (p->max_pages && (p->found_pages == p->max_pages)) + return PM_SCAN_FOUND_MAX_PAGES; + + return 0; + } + + if (!p->vec_index || ((p->vec_index + 1) < p->vec_len)) { + + if (cur->len) { + memcpy(&p->vec[p->vec_index], cur, + sizeof(struct page_region)); + p->vec_index++; + } + cur->start = addr; + cur->len = n_pages; + cur->bitmap = bitmap; + p->found_pages += n_pages; + + if (p->max_pages && (p->found_pages == p->max_pages)) + return PM_SCAN_FOUND_MAX_PAGES; + + return 0; + } + + return -ENOSPC; +} + +static inline int pagemap_scan_deposit(struct pagemap_scan_private *p, + struct page_region __user *vec, + unsigned long *vec_index) +{ + struct page_region *cur = &p->cur; + + if (!cur->len) + return 0; + + if (copy_to_user(&vec[*vec_index], cur, sizeof(struct page_region))) + return -EFAULT; + + p->vec_index++; + (*vec_index)++; + + return 0; +} + +static int pagemap_scan_pmd_entry(pmd_t *pmd, unsigned long start, + unsigned long end, struct mm_walk *walk) +{ + struct pagemap_scan_private *p = walk->private; + bool is_written, is_file, is_present, is_swap; + struct vm_area_struct *vma = walk->vma; + unsigned long addr = end; + spinlock_t *ptl; + int ret = 0; + pte_t *pte; + +#ifdef CONFIG_TRANSPARENT_HUGEPAGE + ptl = pmd_trans_huge_lock(pmd, vma); + if (ptl) { + unsigned long n_pages = (end - start)/PAGE_SIZE; + + is_written = !is_pmd_uffd_wp(*pmd); + is_file = vma->vm_file; + is_present = pmd_present(*pmd); + is_swap = is_swap_pmd(*pmd); + + spin_unlock(ptl); + + /* + * Break huge page into small pages if the WP operation need to + * be performed is on a portion of the huge page or if max_pages + * pages limit would exceed. + */ + if (is_written && PM_SCAN_OP_IS_WP(p) && + ((end - start < HPAGE_SIZE) || + (p->max_pages && + (p->max_pages - p->found_pages) < n_pages))) { + + split_huge_pmd(vma, pmd, start); + goto process_smaller_pages; + } + + if (p->max_pages && + p->found_pages + n_pages > p->max_pages) + n_pages = p->max_pages - p->found_pages; + + ret = pagemap_scan_output(is_written, is_file, is_present, + is_swap, p, start, n_pages); + if (ret < 0) + return ret; + + if (is_written && PM_SCAN_OP_IS_WP(p) && + uffd_wp_range(vma, start, HPAGE_SIZE, true) < 0) + ret = -EINVAL; + + return ret; + } +process_smaller_pages: + if (pmd_trans_unstable(pmd)) + return 0; +#endif + + for (addr = start; addr < end && !ret; pte++, addr += PAGE_SIZE) { + pte = pte_offset_map_lock(vma->vm_mm, pmd, addr, &ptl); + + is_written = !is_pte_uffd_wp(*pte); + is_file = vma->vm_file; + is_present = pte_present(*pte); + is_swap = is_swap_pte(*pte); + + pte_unmap_unlock(pte, ptl); + + ret = pagemap_scan_output(is_written, is_file, is_present, + is_swap, p, addr, 1); + if (ret < 0) + return ret; + + if (is_written && PM_SCAN_OP_IS_WP(p) && + uffd_wp_range(vma, addr, PAGE_SIZE, true) < 0) + return -EINVAL; + } + + 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) +{ + struct pagemap_scan_private *p = walk->private; + bool is_written; + struct vm_area_struct *vma = walk->vma; + unsigned long n_pages = (end - start)/PAGE_SIZE; + pte_t pte; + int ret; + + pte = huge_ptep_get(ptep); + is_written = !is_huge_pte_uffd_wp(pte); + + /* + * Partial hugetlb page clear isn't supported + */ + if (is_written && PM_SCAN_OP_IS_WP(p) && + ((end - start < HPAGE_SIZE) || + (p->max_pages && + (p->max_pages - p->found_pages) < n_pages))) + return -EPERM; + + if (p->max_pages && + p->found_pages + n_pages > p->max_pages) + n_pages = p->max_pages - p->found_pages; + + ret = pagemap_scan_output(is_written, vma->vm_file, pte_present(pte), + is_swap_pte(pte), p, start, n_pages); + if (ret < 0) + return ret; + + if (is_written && PM_SCAN_OP_IS_WP(p) && + uffd_wp_range(vma, start, HPAGE_SIZE, true) < 0) + ret = -EINVAL; + + 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) +{ + struct pagemap_scan_private *p = walk->private; + struct vm_area_struct *vma = walk->vma; + unsigned long n_pages; + int ret = 0; + + if (!vma) + return 0; + + n_pages = (end - addr)/PAGE_SIZE; + if (p->max_pages && p->found_pages + n_pages > p->max_pages) + n_pages = p->max_pages - p->found_pages; + + ret = pagemap_scan_output(false, 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 & ~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 ranges */ + if (!(arg->flags & PM_SCAN_OP_GET)) + return -EINVAL; + if (!arg->vec) + return -EINVAL; + if (arg->vec_len == 0) + return -EINVAL; + + if (!IS_ALIGNED(start, PAGE_SIZE)) + return -EINVAL; + if (!access_ok((void __user *)start, arg->len)) + return -EFAULT; + + if (!PM_SCAN_OP_IS_WP(arg)) + return 0; + + if ((arg->required_mask | arg->anyof_mask | arg->excluded_mask) & + ~PAGE_IS_WRITTEN) + return -EINVAL; + + return 0; +} + +static long do_pagemap_cmd(struct mm_struct *mm, + struct pm_scan_arg __user *uarg) +{ + unsigned long start, end, walk_start, walk_end; + unsigned long empty_slots, vec_index = 0; + 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 = (unsigned long)untagged_addr(arg.start); + vec = (struct page_region *)(unsigned long)untagged_addr(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.flags = arg.flags; + 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.cur.len = 0; + p.cur.start = 0; + p.vec = NULL; + p.vec_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 between different walks and + * store the p.cur at the end of the walk to the user buffer. + */ + p.vec = kmalloc_array(p.vec_len, sizeof(struct page_region), + GFP_KERNEL); + if (!p.vec) + return -ENOMEM; + + walk_start = walk_end = start; + while (walk_end < end && !ret) { + p.vec_index = 0; + + empty_slots = arg.vec_len - vec_index; + p.vec_len = min(p.vec_len, empty_slots); + + walk_end = (walk_start + PAGEMAP_WALK_SIZE) & PAGEMAP_WALK_MASK; + if (walk_end > end) + walk_end = end; + + mmap_read_lock(mm); + ret = walk_page_range(mm, walk_start, walk_end, + &pagemap_scan_ops, &p); + mmap_read_unlock(mm); + + if (ret && ret != -ENOSPC && ret != PM_SCAN_FOUND_MAX_PAGES) + goto free_data; + + walk_start = walk_end; + if (p.vec_index) { + if (copy_to_user(&vec[vec_index], p.vec, + p.vec_index * + sizeof(struct page_region))) { + ret = -EFAULT; + goto free_data; + } + vec_index += p.vec_index; + } + } + ret = pagemap_scan_deposit(&p, vec, &vec_index); + if (!ret) + ret = vec_index; +free_data: + kfree(p.vec); + + return ret; +} + +static long pagemap_scan_ioctl(struct file *file, unsigned int cmd, + unsigned long arg) +{ + struct pm_scan_arg __user *uarg = (struct pm_scan_arg __user *)arg; + struct mm_struct *mm = file->private_data; + + switch (cmd) { + case PAGEMAP_SCAN: + return do_pagemap_cmd(mm, uarg); + + 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 = pagemap_scan_ioctl, + .compat_ioctl = pagemap_scan_ioctl, }; #endif /* CONFIG_PROC_PAGE_MONITOR */
diff --git a/include/linux/userfaultfd_k.h b/include/linux/userfaultfd_k.h index ce4f9d18de3e..325672cd6d6f 100644 --- a/include/linux/userfaultfd_k.h +++ b/include/linux/userfaultfd_k.h @@ -210,6 +210,14 @@ extern bool userfaultfd_wp_async(struct vm_area_struct *vma);
#else /* CONFIG_USERFAULTFD */
+static inline long uffd_wp_range(struct mm_struct *dst_mm, + struct vm_area_struct *vma, + unsigned long start, unsigned long len, + bool enable_wp) +{ + return 0; +} + /* mm helpers */ static inline vm_fault_t handle_userfault(struct vm_fault *vmf, unsigned long reason) diff --git a/include/uapi/linux/fs.h b/include/uapi/linux/fs.h index b7b56871029c..47879c38ce2f 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 the bitmap 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 bitmap 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 bitmap; +}; + +/* + * 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 */
Hi Muhammad,
kernel test robot noticed the following build errors:
[auto build test ERROR on akpm-mm/mm-everything] [also build test ERROR on next-20230406] [cannot apply to linus/master v6.3-rc5] [If your patch is applied to the wrong git tree, kindly drop us a note. And when submitting patch, we suggest to use '--base' as documented in https://git-scm.com/docs/git-format-patch#_base_tree_information]
url: https://github.com/intel-lab-lkp/linux/commits/Muhammad-Usama-Anjum/userfaul... base: https://git.kernel.org/pub/scm/linux/kernel/git/akpm/mm.git mm-everything patch link: https://lore.kernel.org/r/20230406074005.1784728-3-usama.anjum%40collabora.c... patch subject: [PATCH v12 2/5] fs/proc/task_mmu: Implement IOCTL to get and optionally clear info about PTEs config: arc-randconfig-r023-20230405 (https://download.01.org/0day-ci/archive/20230406/202304061914.N1Hmx12N-lkp@i...) compiler: arceb-elf-gcc (GCC) 12.1.0 reproduce (this is a W=1 build): wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross chmod +x ~/bin/make.cross # https://github.com/intel-lab-lkp/linux/commit/f13abb36f64c77913509da8ca15751... git remote add linux-review https://github.com/intel-lab-lkp/linux git fetch --no-tags linux-review Muhammad-Usama-Anjum/userfaultfd-UFFD_FEATURE_WP_ASYNC/20230406-154314 git checkout f13abb36f64c77913509da8ca157512d2fb9f031 # save the config file mkdir build_dir && cp config build_dir/.config COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0 make.cross W=1 O=build_dir ARCH=arc olddefconfig COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0 make.cross W=1 O=build_dir ARCH=arc SHELL=/bin/bash fs/proc/
If you fix the issue, kindly add following tag where applicable | Reported-by: kernel test robot lkp@intel.com | Link: https://lore.kernel.org/oe-kbuild-all/202304061914.N1Hmx12N-lkp@intel.com/
All error/warnings (new ones prefixed by >>):
fs/proc/task_mmu.c: In function 'pagemap_scan_pmd_entry':
fs/proc/task_mmu.c:1921:37: error: 'HPAGE_SIZE' undeclared (first use in this function); did you mean 'PAGE_SIZE'?
1921 | ((end - start < HPAGE_SIZE) || | ^~~~~~~~~~ | PAGE_SIZE fs/proc/task_mmu.c:1921:37: note: each undeclared identifier is reported only once for each function it appears in
fs/proc/task_mmu.c:1939:35: error: passing argument 1 of 'uffd_wp_range' from incompatible pointer type [-Werror=incompatible-pointer-types]
1939 | uffd_wp_range(vma, start, HPAGE_SIZE, true) < 0) | ^~~ | | | struct vm_area_struct * In file included from include/linux/mm_inline.h:9, from fs/proc/task_mmu.c:3: include/linux/userfaultfd_k.h:215:52: note: expected 'struct mm_struct *' but argument is of type 'struct vm_area_struct *' 215 | static inline long uffd_wp_range(struct mm_struct *dst_mm, | ~~~~~~~~~~~~~~~~~~^~~~~~
fs/proc/task_mmu.c:1939:40: warning: passing argument 2 of 'uffd_wp_range' makes pointer from integer without a cast [-Wint-conversion]
1939 | uffd_wp_range(vma, start, HPAGE_SIZE, true) < 0) | ^~~~~ | | | long unsigned int include/linux/userfaultfd_k.h:216:57: note: expected 'struct vm_area_struct *' but argument is of type 'long unsigned int' 216 | struct vm_area_struct *vma, | ~~~~~~~~~~~~~~~~~~~~~~~^~~
fs/proc/task_mmu.c:1939:21: error: too few arguments to function 'uffd_wp_range'
1939 | uffd_wp_range(vma, start, HPAGE_SIZE, true) < 0) | ^~~~~~~~~~~~~ include/linux/userfaultfd_k.h:215:20: note: declared here 215 | static inline long uffd_wp_range(struct mm_struct *dst_mm, | ^~~~~~~~~~~~~ fs/proc/task_mmu.c:1965:35: error: passing argument 1 of 'uffd_wp_range' from incompatible pointer type [-Werror=incompatible-pointer-types] 1965 | uffd_wp_range(vma, addr, PAGE_SIZE, true) < 0) | ^~~ | | | struct vm_area_struct * include/linux/userfaultfd_k.h:215:52: note: expected 'struct mm_struct *' but argument is of type 'struct vm_area_struct *' 215 | static inline long uffd_wp_range(struct mm_struct *dst_mm, | ~~~~~~~~~~~~~~~~~~^~~~~~ fs/proc/task_mmu.c:1965:40: warning: passing argument 2 of 'uffd_wp_range' makes pointer from integer without a cast [-Wint-conversion] 1965 | uffd_wp_range(vma, addr, PAGE_SIZE, true) < 0) | ^~~~ | | | long unsigned int include/linux/userfaultfd_k.h:216:57: note: expected 'struct vm_area_struct *' but argument is of type 'long unsigned int' 216 | struct vm_area_struct *vma, | ~~~~~~~~~~~~~~~~~~~~~~~^~~ fs/proc/task_mmu.c:1965:21: error: too few arguments to function 'uffd_wp_range' 1965 | uffd_wp_range(vma, addr, PAGE_SIZE, true) < 0) | ^~~~~~~~~~~~~ include/linux/userfaultfd_k.h:215:20: note: declared here 215 | static inline long uffd_wp_range(struct mm_struct *dst_mm, | ^~~~~~~~~~~~~ cc1: some warnings being treated as errors
vim +1921 fs/proc/task_mmu.c
1891 1892 static int pagemap_scan_pmd_entry(pmd_t *pmd, unsigned long start, 1893 unsigned long end, struct mm_walk *walk) 1894 { 1895 struct pagemap_scan_private *p = walk->private; 1896 bool is_written, is_file, is_present, is_swap; 1897 struct vm_area_struct *vma = walk->vma; 1898 unsigned long addr = end; 1899 spinlock_t *ptl; 1900 int ret = 0; 1901 pte_t *pte; 1902 1903 #ifdef CONFIG_TRANSPARENT_HUGEPAGE 1904 ptl = pmd_trans_huge_lock(pmd, vma); 1905 if (ptl) { 1906 unsigned long n_pages = (end - start)/PAGE_SIZE; 1907 1908 is_written = !is_pmd_uffd_wp(*pmd); 1909 is_file = vma->vm_file; 1910 is_present = pmd_present(*pmd); 1911 is_swap = is_swap_pmd(*pmd); 1912 1913 spin_unlock(ptl); 1914 1915 /* 1916 * Break huge page into small pages if the WP operation need to 1917 * be performed is on a portion of the huge page or if max_pages 1918 * pages limit would exceed. 1919 */ 1920 if (is_written && PM_SCAN_OP_IS_WP(p) &&
1921 ((end - start < HPAGE_SIZE) ||
1922 (p->max_pages && 1923 (p->max_pages - p->found_pages) < n_pages))) { 1924 1925 split_huge_pmd(vma, pmd, start); 1926 goto process_smaller_pages; 1927 } 1928 1929 if (p->max_pages && 1930 p->found_pages + n_pages > p->max_pages) 1931 n_pages = p->max_pages - p->found_pages; 1932 1933 ret = pagemap_scan_output(is_written, is_file, is_present, 1934 is_swap, p, start, n_pages); 1935 if (ret < 0) 1936 return ret; 1937 1938 if (is_written && PM_SCAN_OP_IS_WP(p) &&
1939 uffd_wp_range(vma, start, HPAGE_SIZE, true) < 0)
1940 ret = -EINVAL; 1941 1942 return ret; 1943 } 1944 process_smaller_pages: 1945 if (pmd_trans_unstable(pmd)) 1946 return 0; 1947 #endif 1948 1949 for (addr = start; addr < end && !ret; pte++, addr += PAGE_SIZE) { 1950 pte = pte_offset_map_lock(vma->vm_mm, pmd, addr, &ptl); 1951 1952 is_written = !is_pte_uffd_wp(*pte); 1953 is_file = vma->vm_file; 1954 is_present = pte_present(*pte); 1955 is_swap = is_swap_pte(*pte); 1956 1957 pte_unmap_unlock(pte, ptl); 1958 1959 ret = pagemap_scan_output(is_written, is_file, is_present, 1960 is_swap, p, addr, 1); 1961 if (ret < 0) 1962 return ret; 1963 1964 if (is_written && PM_SCAN_OP_IS_WP(p) && 1965 uffd_wp_range(vma, addr, PAGE_SIZE, true) < 0) 1966 return -EINVAL; 1967 } 1968 1969 cond_resched(); 1970 return ret; 1971 } 1972
On 4/6/23 4:40 PM, kernel test robot wrote:
Hi Muhammad,
kernel test robot noticed the following build errors:
[auto build test ERROR on akpm-mm/mm-everything] [also build test ERROR on next-20230406] [cannot apply to linus/master v6.3-rc5] [If your patch is applied to the wrong git tree, kindly drop us a note. And when submitting patch, we suggest to use '--base' as documented in https://git-scm.com/docs/git-format-patch#_base_tree_information]
url: https://github.com/intel-lab-lkp/linux/commits/Muhammad-Usama-Anjum/userfaul... base: https://git.kernel.org/pub/scm/linux/kernel/git/akpm/mm.git mm-everything patch link: https://lore.kernel.org/r/20230406074005.1784728-3-usama.anjum%40collabora.c... patch subject: [PATCH v12 2/5] fs/proc/task_mmu: Implement IOCTL to get and optionally clear info about PTEs config: arc-randconfig-r023-20230405 (https://download.01.org/0day-ci/archive/20230406/202304061914.N1Hmx12N-lkp@i...) compiler: arceb-elf-gcc (GCC) 12.1.0 reproduce (this is a W=1 build): wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross chmod +x ~/bin/make.cross # https://github.com/intel-lab-lkp/linux/commit/f13abb36f64c77913509da8ca15751... git remote add linux-review https://github.com/intel-lab-lkp/linux git fetch --no-tags linux-review Muhammad-Usama-Anjum/userfaultfd-UFFD_FEATURE_WP_ASYNC/20230406-154314 git checkout f13abb36f64c77913509da8ca157512d2fb9f031 # save the config file mkdir build_dir && cp config build_dir/.config COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0 make.cross W=1 O=build_dir ARCH=arc olddefconfig COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0 make.cross W=1 O=build_dir ARCH=arc SHELL=/bin/bash fs/proc/
If you fix the issue, kindly add following tag where applicable | Reported-by: kernel test robot lkp@intel.com | Link: https://lore.kernel.org/oe-kbuild-all/202304061914.N1Hmx12N-lkp@intel.com/
All error/warnings (new ones prefixed by >>):
fs/proc/task_mmu.c: In function 'pagemap_scan_pmd_entry':
fs/proc/task_mmu.c:1921:37: error: 'HPAGE_SIZE' undeclared (first use in this function); did you mean 'PAGE_SIZE'?
1921 | ((end - start < HPAGE_SIZE) || | ^~~~~~~~~~ | PAGE_SIZE
It seems arc architecture supports CONFIG_TRANSPARENT_HUGEPAGE, but it doesn't define HPAGE_SIZE. It only defines PAGE_SIZE in include/uapi/asm/page.h? AFAIK HPAGE_SIZE must be defined when CONFIG_TRANSPARENT_HUGEPAGE is enabled. What can be the solution here for arc?
The remaining build failures are because the wrong tree. I base my patches on latest next, while the bot has based patches on mm-everything. I guess today's next would have latest mm stuff, a rebase would make things correct or I'll shift to mm-everything.
On 4/6/23 5:56 PM, Muhammad Usama Anjum wrote:
On 4/6/23 4:40 PM, kernel test robot wrote:
Hi Muhammad,
kernel test robot noticed the following build errors:
[auto build test ERROR on akpm-mm/mm-everything] [also build test ERROR on next-20230406] [cannot apply to linus/master v6.3-rc5] [If your patch is applied to the wrong git tree, kindly drop us a note. And when submitting patch, we suggest to use '--base' as documented in https://git-scm.com/docs/git-format-patch#_base_tree_information]
url: https://github.com/intel-lab-lkp/linux/commits/Muhammad-Usama-Anjum/userfaul... base: https://git.kernel.org/pub/scm/linux/kernel/git/akpm/mm.git mm-everything patch link: https://lore.kernel.org/r/20230406074005.1784728-3-usama.anjum%40collabora.c... patch subject: [PATCH v12 2/5] fs/proc/task_mmu: Implement IOCTL to get and optionally clear info about PTEs config: arc-randconfig-r023-20230405 (https://download.01.org/0day-ci/archive/20230406/202304061914.N1Hmx12N-lkp@i...) compiler: arceb-elf-gcc (GCC) 12.1.0 reproduce (this is a W=1 build): wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross chmod +x ~/bin/make.cross # https://github.com/intel-lab-lkp/linux/commit/f13abb36f64c77913509da8ca15751... git remote add linux-review https://github.com/intel-lab-lkp/linux git fetch --no-tags linux-review Muhammad-Usama-Anjum/userfaultfd-UFFD_FEATURE_WP_ASYNC/20230406-154314 git checkout f13abb36f64c77913509da8ca157512d2fb9f031 # save the config file mkdir build_dir && cp config build_dir/.config COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0 make.cross W=1 O=build_dir ARCH=arc olddefconfig COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0 make.cross W=1 O=build_dir ARCH=arc SHELL=/bin/bash fs/proc/
If you fix the issue, kindly add following tag where applicable | Reported-by: kernel test robot lkp@intel.com | Link: https://lore.kernel.org/oe-kbuild-all/202304061914.N1Hmx12N-lkp@intel.com/
All error/warnings (new ones prefixed by >>):
fs/proc/task_mmu.c: In function 'pagemap_scan_pmd_entry':
fs/proc/task_mmu.c:1921:37: error: 'HPAGE_SIZE' undeclared (first use in this function); did you mean 'PAGE_SIZE'?
1921 | ((end - start < HPAGE_SIZE) || | ^~~~~~~~~~ | PAGE_SIZEIt seems arc architecture supports CONFIG_TRANSPARENT_HUGEPAGE, but it doesn't define HPAGE_SIZE. It only defines PAGE_SIZE in include/uapi/asm/page.h? AFAIK HPAGE_SIZE must be defined when CONFIG_TRANSPARENT_HUGEPAGE is enabled. What can be the solution here for arc?
I'm still looking for solution. Vineet do you have some thoughts?
The remaining build failures are because the wrong tree. I base my patches on latest next, while the bot has based patches on mm-everything. I guess today's next would have latest mm stuff, a rebase would make things correct or I'll shift to mm-everything.
Hi Vineet,
It seems arc architecture supports CONFIG_TRANSPARENT_HUGEPAGE, but it doesn't define HPAGE_SIZE. It only defines PAGE_SIZE in include/uapi/asm/page.h? AFAIK HPAGE_SIZE must be defined when CONFIG_TRANSPARENT_HUGEPAGE is enabled. What can be the solution here for arc?
Should I just compile out this code for arc architecture specifically?
Thanks, Usama
On 6/7/23 10:45 AM, Muhammad Usama Anjum wrote:
On 4/6/23 5:56 PM, Muhammad Usama Anjum wrote:
On 4/6/23 4:40 PM, kernel test robot wrote:
Hi Muhammad,
kernel test robot noticed the following build errors:
[auto build test ERROR on akpm-mm/mm-everything] [also build test ERROR on next-20230406] [cannot apply to linus/master v6.3-rc5] [If your patch is applied to the wrong git tree, kindly drop us a note. And when submitting patch, we suggest to use '--base' as documented in https://git-scm.com/docs/git-format-patch#_base_tree_information]
url: https://github.com/intel-lab-lkp/linux/commits/Muhammad-Usama-Anjum/userfaul... base: https://git.kernel.org/pub/scm/linux/kernel/git/akpm/mm.git mm-everything patch link: https://lore.kernel.org/r/20230406074005.1784728-3-usama.anjum%40collabora.c... patch subject: [PATCH v12 2/5] fs/proc/task_mmu: Implement IOCTL to get and optionally clear info about PTEs config: arc-randconfig-r023-20230405 (https://download.01.org/0day-ci/archive/20230406/202304061914.N1Hmx12N-lkp@i...) compiler: arceb-elf-gcc (GCC) 12.1.0 reproduce (this is a W=1 build): wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross chmod +x ~/bin/make.cross # https://github.com/intel-lab-lkp/linux/commit/f13abb36f64c77913509da8ca15751... git remote add linux-review https://github.com/intel-lab-lkp/linux git fetch --no-tags linux-review Muhammad-Usama-Anjum/userfaultfd-UFFD_FEATURE_WP_ASYNC/20230406-154314 git checkout f13abb36f64c77913509da8ca157512d2fb9f031 # save the config file mkdir build_dir && cp config build_dir/.config COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0 make.cross W=1 O=build_dir ARCH=arc olddefconfig COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0 make.cross W=1 O=build_dir ARCH=arc SHELL=/bin/bash fs/proc/
If you fix the issue, kindly add following tag where applicable | Reported-by: kernel test robot lkp@intel.com | Link: https://lore.kernel.org/oe-kbuild-all/202304061914.N1Hmx12N-lkp@intel.com/
All error/warnings (new ones prefixed by >>):
fs/proc/task_mmu.c: In function 'pagemap_scan_pmd_entry':
fs/proc/task_mmu.c:1921:37: error: 'HPAGE_SIZE' undeclared (first use in this function); did you mean 'PAGE_SIZE'?
1921 | ((end - start < HPAGE_SIZE) || | ^~~~~~~~~~ | PAGE_SIZEIt seems arc architecture supports CONFIG_TRANSPARENT_HUGEPAGE, but it doesn't define HPAGE_SIZE. It only defines PAGE_SIZE in include/uapi/asm/page.h? AFAIK HPAGE_SIZE must be defined when CONFIG_TRANSPARENT_HUGEPAGE is enabled. What can be the solution here for arc?
I'm still looking for solution. Vineet do you have some thoughts?
The remaining build failures are because the wrong tree. I base my patches on latest next, while the bot has based patches on mm-everything. I guess today's next would have latest mm stuff, a rebase would make things correct or I'll shift to mm-everything.
Hi Muhammad,
kernel test robot noticed the following build errors:
[auto build test ERROR on akpm-mm/mm-everything] [also build test ERROR on next-20230406] [cannot apply to linus/master v6.3-rc5] [If your patch is applied to the wrong git tree, kindly drop us a note. And when submitting patch, we suggest to use '--base' as documented in https://git-scm.com/docs/git-format-patch#_base_tree_information]
url: https://github.com/intel-lab-lkp/linux/commits/Muhammad-Usama-Anjum/userfaul... base: https://git.kernel.org/pub/scm/linux/kernel/git/akpm/mm.git mm-everything patch link: https://lore.kernel.org/r/20230406074005.1784728-3-usama.anjum%40collabora.c... patch subject: [PATCH v12 2/5] fs/proc/task_mmu: Implement IOCTL to get and optionally clear info about PTEs config: arm-randconfig-r021-20230405 (https://download.01.org/0day-ci/archive/20230406/202304061924.m0OL9fIj-lkp@i...) compiler: clang version 17.0.0 (https://github.com/llvm/llvm-project 67409911353323ca5edf2049ef0df54132fa1ca7) reproduce (this is a W=1 build): wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross chmod +x ~/bin/make.cross # install arm cross compiling tool for clang build # apt-get install binutils-arm-linux-gnueabi # https://github.com/intel-lab-lkp/linux/commit/f13abb36f64c77913509da8ca15751... git remote add linux-review https://github.com/intel-lab-lkp/linux git fetch --no-tags linux-review Muhammad-Usama-Anjum/userfaultfd-UFFD_FEATURE_WP_ASYNC/20230406-154314 git checkout f13abb36f64c77913509da8ca157512d2fb9f031 # save the config file mkdir build_dir && cp config build_dir/.config COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=arm olddefconfig COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=arm SHELL=/bin/bash fs/
If you fix the issue, kindly add following tag where applicable | Reported-by: kernel test robot lkp@intel.com | Link: https://lore.kernel.org/oe-kbuild-all/202304061924.m0OL9fIj-lkp@intel.com/
All errors (new ones prefixed by >>):
fs/proc/task_mmu.c:1965:47: error: too few arguments to function call, expected 5, have 4
uffd_wp_range(vma, addr, PAGE_SIZE, true) < 0) ~~~~~~~~~~~~~ ^ include/linux/userfaultfd_k.h:215:20: note: 'uffd_wp_range' declared here static inline long uffd_wp_range(struct mm_struct *dst_mm, ^ 1 error generated.
vim +1965 fs/proc/task_mmu.c
1948 1949 for (addr = start; addr < end && !ret; pte++, addr += PAGE_SIZE) { 1950 pte = pte_offset_map_lock(vma->vm_mm, pmd, addr, &ptl); 1951 1952 is_written = !is_pte_uffd_wp(*pte); 1953 is_file = vma->vm_file; 1954 is_present = pte_present(*pte); 1955 is_swap = is_swap_pte(*pte); 1956 1957 pte_unmap_unlock(pte, ptl); 1958 1959 ret = pagemap_scan_output(is_written, is_file, is_present, 1960 is_swap, p, addr, 1); 1961 if (ret < 0) 1962 return ret; 1963 1964 if (is_written && PM_SCAN_OP_IS_WP(p) &&
1965 uffd_wp_range(vma, addr, PAGE_SIZE, true) < 0)
1966 return -EINVAL; 1967 } 1968 1969 cond_resched(); 1970 return ret; 1971 } 1972
On Thu, 6 Apr 2023 at 09:40, Muhammad Usama Anjum usama.anjum@collabora.com wrote:>
This IOCTL, PAGEMAP_SCAN on pagemap file can be used to get and/or clear the info about page table entries. The following operations are supported in this ioctl:
- Get the information if the pages have been written-to (PAGE_IS_WRITTEN), file mapped (PAGE_IS_FILE), present (PAGE_IS_PRESENT) or swapped (PAGE_IS_SWAPPED).
- Find pages which have been written-to and write protect the pages (atomic PAGE_IS_WRITTEN + PAGEMAP_WP_ENGAGE)
This IOCTL can be extended to get information about more PTE bits.
[...]
--- a/fs/proc/task_mmu.c +++ b/fs/proc/task_mmu.c
[...]
+static inline bool is_pte_uffd_wp(pte_t pte) +{
return ((pte_present(pte) && pte_uffd_wp(pte)) ||pte_swp_uffd_wp_any(pte));
Nit: outer parentheses are not needed for `return`ed value -- please remove. (Same in other places.)
@@ -1768,11 +1789,416 @@ static int pagemap_release(struct inode *inode, struct file *file) return 0; }
+#define PM_SCAN_FOUND_MAX_PAGES (1) +#define PM_SCAN_BITS_ALL (PAGE_IS_WRITTEN | PAGE_IS_FILE | \
PAGE_IS_PRESENT | PAGE_IS_SWAPPED)+#define PM_SCAN_OPS (PM_SCAN_OP_GET | PM_SCAN_OP_WP) +#define PM_SCAN_OP_IS_WP(a) (a->flags & PM_SCAN_OP_WP)
Nit: PM_SCAN_DO_UFFD_WP()? It would shift the hint in the name from what op is executed to what behaviour is requested.
+#define PM_SCAN_BITMAP(wt, file, present, swap) \
(wt | file << 1 | present << 2 | swap << 3)
Please parenthesize macro arguments ("(wt)", "(file)", etc.) to not have to worry about operator precedence when passed a complex expression.
I
[...]
+static inline bool pagemap_scan_is_written_set(struct pagemap_scan_private *p)
pagemap_scan_checks_page_written? or similar 'scan is written' doesn't seem to convey the expected intention. The function is used only once in ..._test_walk(), so maybe just inline, possibly using a temporary `bool` to make the condition easier to read?
[...]
+static int pagemap_scan_output(bool wt, bool file, bool pres, bool swap,
Could you try out how the code would look when passing the bitmap instead of separate booleans?
struct pagemap_scan_private *p,unsigned long addr, unsigned int n_pages)+{
[...]
if ((cur->start + cur->len * PAGE_SIZE == addr) &&(cur->bitmap == bitmap)) {
Nit: bitmap check is cheaper, so I'd put it first. BTW, inner parentheses are not needed here.
cur->len += n_pages;p->found_pages += n_pages;if (p->max_pages && (p->found_pages == p->max_pages))return PM_SCAN_FOUND_MAX_PAGES;return 0;}if (!p->vec_index || ((p->vec_index + 1) < p->vec_len)) {
It looks that `if (p->vec_index < p->vec_len)` is enough here - if we have vec_len == 0 here, then we'd not fit the entry in the userspace buffer anyway. Am I missing something?
if (cur->len) {memcpy(&p->vec[p->vec_index], cur,sizeof(struct page_region));p->vec_index++;}cur->start = addr;cur->len = n_pages;cur->bitmap = bitmap;p->found_pages += n_pages;if (p->max_pages && (p->found_pages == p->max_pages))return PM_SCAN_FOUND_MAX_PAGES;return 0;}return -ENOSPC;+}
+static inline int pagemap_scan_deposit(struct pagemap_scan_private *p,
struct page_region __user *vec,unsigned long *vec_index)
..._deposit() is used only in single place - please inline.
[...]
+static int pagemap_scan_pmd_entry(pmd_t *pmd, unsigned long start,
unsigned long end, struct mm_walk *walk)+{
struct pagemap_scan_private *p = walk->private;bool is_written, is_file, is_present, is_swap;struct vm_area_struct *vma = walk->vma;unsigned long addr = end;spinlock_t *ptl;int ret = 0;pte_t *pte;+#ifdef CONFIG_TRANSPARENT_HUGEPAGE
ptl = pmd_trans_huge_lock(pmd, vma);if (ptl) {
Nit: `page_lock` or `pt_lock` to make it easier to guess the purpose?
unsigned long n_pages = (end - start)/PAGE_SIZE;is_written = !is_pmd_uffd_wp(*pmd);is_file = vma->vm_file;is_present = pmd_present(*pmd);is_swap = is_swap_pmd(*pmd);spin_unlock(ptl);/** Break huge page into small pages if the WP operation need to* be performed is on a portion of the huge page or if max_pages* pages limit would exceed.
BTW, could the `max_pages` limit be relaxed a bit (in that it would be possible to return more pages if they all merge into the last vector entry) so that it would not need to split otherwise-matching huge page? It would remove the need for this special handling in the kernel and splitting the page by this read-only-appearing ioctl?
*/if (is_written && PM_SCAN_OP_IS_WP(p) &&((end - start < HPAGE_SIZE) ||(p->max_pages &&(p->max_pages - p->found_pages) < n_pages))) {split_huge_pmd(vma, pmd, start);goto process_smaller_pages;}if (p->max_pages &&p->found_pages + n_pages > p->max_pages)n_pages = p->max_pages - p->found_pages;ret = pagemap_scan_output(is_written, is_file, is_present,is_swap, p, start, n_pages);if (ret < 0)return ret;if (is_written && PM_SCAN_OP_IS_WP(p) &&uffd_wp_range(vma, start, HPAGE_SIZE, true) < 0)ret = -EINVAL;
Why not propagate the error from uffd_wp_range()?
[...]
+static long do_pagemap_cmd(struct mm_struct *mm,
struct pm_scan_arg __user *uarg)
Please rename the function to `do_pagemap_scan` as it implements just this single ioctl now.
+{
[...]
start = (unsigned long)untagged_addr(arg.start);vec = (struct page_region *)(unsigned long)untagged_addr(arg.vec);
Is the inner cast needed?
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.flags = arg.flags;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.cur.len = 0;p.cur.start = 0;p.vec = NULL;p.vec_len = (PAGEMAP_WALK_SIZE >> PAGE_SHIFT);
Nit: parentheses are not needed here, please remove.
/** 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 between different walks and* store the p.cur at the end of the walk to the user buffer.*/p.vec = kmalloc_array(p.vec_len, sizeof(struct page_region),GFP_KERNEL);if (!p.vec)return -ENOMEM;walk_start = walk_end = start;while (walk_end < end && !ret) {
The loop will stop if a previous iteration returned ENOSPC (and the error will be lost) - is it intended?
p.vec_index = 0;empty_slots = arg.vec_len - vec_index;p.vec_len = min(p.vec_len, empty_slots);walk_end = (walk_start + PAGEMAP_WALK_SIZE) & PAGEMAP_WALK_MASK;if (walk_end > end)walk_end = end;mmap_read_lock(mm);ret = walk_page_range(mm, walk_start, walk_end,&pagemap_scan_ops, &p);mmap_read_unlock(mm);if (ret && ret != -ENOSPC && ret != PM_SCAN_FOUND_MAX_PAGES)goto free_data;walk_start = walk_end;if (p.vec_index) {if (copy_to_user(&vec[vec_index], p.vec,p.vec_index *sizeof(struct page_region))) {
sizeof(*p.vec) ?
ret = -EFAULT;goto free_data;}vec_index += p.vec_index;}}ret = pagemap_scan_deposit(&p, vec, &vec_index);if (!ret)ret = vec_index;+free_data:
kfree(p.vec);return ret;+}
+static long pagemap_scan_ioctl(struct file *file, unsigned int cmd,
unsigned long arg)+{
struct pm_scan_arg __user *uarg = (struct pm_scan_arg __user *)arg;
This is specific to PAGEMAP_SCAN ioctl, so should go into do_pagemap_cmd().
struct mm_struct *mm = file->private_data;switch (cmd) {case PAGEMAP_SCAN:return do_pagemap_cmd(mm, uarg);
[...]
--- a/include/linux/userfaultfd_k.h +++ b/include/linux/userfaultfd_k.h @@ -210,6 +210,14 @@ extern bool userfaultfd_wp_async(struct vm_area_struct *vma);
#else /* CONFIG_USERFAULTFD */
+static inline long uffd_wp_range(struct mm_struct *dst_mm,
struct vm_area_struct *vma,unsigned long start, unsigned long len,bool enable_wp)+{
return 0;+}
/* mm helpers */ static inline vm_fault_t handle_userfault(struct vm_fault *vmf, unsigned long reason)
Shouldn't this part be in the patch introducing uffd_wp_range()?
Best Regards Michał Mirosław
Hello,
Thank you so much for the review. Do you have any thoughts on the build error on arc architecture? https://lore.kernel.org/all/e3c82373-256a-6297-bcb4-5e1179a2cbe2@collabora.c...
On 4/6/23 8:52 PM, Michał Mirosław wrote:
On Thu, 6 Apr 2023 at 09:40, Muhammad Usama Anjum usama.anjum@collabora.com wrote:>
This IOCTL, PAGEMAP_SCAN on pagemap file can be used to get and/or clear the info about page table entries. The following operations are supported in this ioctl:
- Get the information if the pages have been written-to (PAGE_IS_WRITTEN), file mapped (PAGE_IS_FILE), present (PAGE_IS_PRESENT) or swapped (PAGE_IS_SWAPPED).
- Find pages which have been written-to and write protect the pages (atomic PAGE_IS_WRITTEN + PAGEMAP_WP_ENGAGE)
This IOCTL can be extended to get information about more PTE bits.
[...]
--- a/fs/proc/task_mmu.c +++ b/fs/proc/task_mmu.c
[...]
+static inline bool is_pte_uffd_wp(pte_t pte) +{
return ((pte_present(pte) && pte_uffd_wp(pte)) ||pte_swp_uffd_wp_any(pte));Nit: outer parentheses are not needed for `return`ed value -- please remove. (Same in other places.)
Will remove.
@@ -1768,11 +1789,416 @@ static int pagemap_release(struct inode *inode, struct file *file) return 0; }
+#define PM_SCAN_FOUND_MAX_PAGES (1) +#define PM_SCAN_BITS_ALL (PAGE_IS_WRITTEN | PAGE_IS_FILE | \
PAGE_IS_PRESENT | PAGE_IS_SWAPPED)+#define PM_SCAN_OPS (PM_SCAN_OP_GET | PM_SCAN_OP_WP) +#define PM_SCAN_OP_IS_WP(a) (a->flags & PM_SCAN_OP_WP)
Nit: PM_SCAN_DO_UFFD_WP()? It would shift the hint in the name from what op is executed to what behaviour is requested.
Will do.
+#define PM_SCAN_BITMAP(wt, file, present, swap) \
(wt | file << 1 | present << 2 | swap << 3)Please parenthesize macro arguments ("(wt)", "(file)", etc.) to not have to worry about operator precedence when passed a complex expression.
Like this? #define PM_SCAN_BITMAP(wt, file, present, swap) \ ((wt) | (file << 1) | (present << 2) | (swap << 3))
I
[...]
+static inline bool pagemap_scan_is_written_set(struct pagemap_scan_private *p)
pagemap_scan_checks_page_written? or similar 'scan is written' doesn't seem to convey the expected intention. The function is used only once in ..._test_walk(), so maybe just inline, possibly using a temporary `bool` to make the condition easier to read?
I'll update the name of function. Using bool doesn't help much. Lets keep the same implementation.
[...]
+static int pagemap_scan_output(bool wt, bool file, bool pres, bool swap,
Could you try out how the code would look when passing the bitmap instead of separate booleans?
It doesn't look better. Right now we have less duplicate code.
struct pagemap_scan_private *p,unsigned long addr, unsigned int n_pages)+{
[...]
if ((cur->start + cur->len * PAGE_SIZE == addr) &&(cur->bitmap == bitmap)) {Nit: bitmap check is cheaper, so I'd put it first. BTW, inner parentheses are not needed here.
Will do.
cur->len += n_pages;p->found_pages += n_pages;if (p->max_pages && (p->found_pages == p->max_pages))return PM_SCAN_FOUND_MAX_PAGES;return 0;}if (!p->vec_index || ((p->vec_index + 1) < p->vec_len)) {It looks that `if (p->vec_index < p->vec_len)` is enough here - if we have vec_len == 0 here, then we'd not fit the entry in the userspace buffer anyway. Am I missing something?
No. I'd explained it with diagram last time: https://lore.kernel.org/all/3c8d9ea0-1382-be0c-8dd2-d490eedd3b55@collabora.c...
I'll add a concise comment here.
if (cur->len) {memcpy(&p->vec[p->vec_index], cur,sizeof(struct page_region));p->vec_index++;}cur->start = addr;cur->len = n_pages;cur->bitmap = bitmap;p->found_pages += n_pages;if (p->max_pages && (p->found_pages == p->max_pages))return PM_SCAN_FOUND_MAX_PAGES;return 0;}return -ENOSPC;+}
+static inline int pagemap_scan_deposit(struct pagemap_scan_private *p,
struct page_region __user *vec,unsigned long *vec_index)..._deposit() is used only in single place - please inline.
It is already inline.
[...]
+static int pagemap_scan_pmd_entry(pmd_t *pmd, unsigned long start,
unsigned long end, struct mm_walk *walk)+{
struct pagemap_scan_private *p = walk->private;bool is_written, is_file, is_present, is_swap;struct vm_area_struct *vma = walk->vma;unsigned long addr = end;spinlock_t *ptl;int ret = 0;pte_t *pte;+#ifdef CONFIG_TRANSPARENT_HUGEPAGE
ptl = pmd_trans_huge_lock(pmd, vma);if (ptl) {Nit: `page_lock` or `pt_lock` to make it easier to guess the purpose?
No, in this file ptl is used religiously for this lock. So I'll keep it ptl to keep consistency.
unsigned long n_pages = (end - start)/PAGE_SIZE;is_written = !is_pmd_uffd_wp(*pmd);is_file = vma->vm_file;is_present = pmd_present(*pmd);is_swap = is_swap_pmd(*pmd);spin_unlock(ptl);/** Break huge page into small pages if the WP operation need to* be performed is on a portion of the huge page or if max_pages* pages limit would exceed.BTW, could the `max_pages` limit be relaxed a bit (in that it would be possible to return more pages if they all merge into the last vector entry) so that it would not need to split otherwise-matching huge page? It would remove the need for this special handling in the kernel and splitting the page by this read-only-appearing ioctl?
No, this cannot be done. Otherwise we'll not be able to emulate Windows syscall GetWriteWatch() which specifies the exact number of pages. Usually in most of cases, either user will not use THP or not perform the operation on partial huge page. So this part is only there to keep things correct for those users who do use THP and partial pagemap_scan operations.
*/if (is_written && PM_SCAN_OP_IS_WP(p) &&((end - start < HPAGE_SIZE) ||(p->max_pages &&(p->max_pages - p->found_pages) < n_pages))) {split_huge_pmd(vma, pmd, start);goto process_smaller_pages;}if (p->max_pages &&p->found_pages + n_pages > p->max_pages)n_pages = p->max_pages - p->found_pages;ret = pagemap_scan_output(is_written, is_file, is_present,is_swap, p, start, n_pages);if (ret < 0)return ret;if (is_written && PM_SCAN_OP_IS_WP(p) &&uffd_wp_range(vma, start, HPAGE_SIZE, true) < 0)ret = -EINVAL;Why not propagate the error from uffd_wp_range()?
uffd_wp_range() returns status in long variable. We cannot return long in this function. So intead of type casting long to int and then return I've used -EINVAL. Would following be more suitable?
long ret2 = uffd_wp_range(vma, start, HPAGE_SIZE, true); if (ret2 < 0) return (int)ret2;
[...]
+static long do_pagemap_cmd(struct mm_struct *mm,
struct pm_scan_arg __user *uarg)Please rename the function to `do_pagemap_scan` as it implements just this single ioctl now.
Will do.
+{
[...]
start = (unsigned long)untagged_addr(arg.start);vec = (struct page_region *)(unsigned long)untagged_addr(arg.vec);Is the inner cast needed?
arg.vec remains 64-bit on 32-bit systems. So casting 64bit value directly to struct page_region pointer errors out. So I've added specific casting to unsigned long first before casting to pointers.
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.flags = arg.flags;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.cur.len = 0;p.cur.start = 0;p.vec = NULL;p.vec_len = (PAGEMAP_WALK_SIZE >> PAGE_SHIFT);Nit: parentheses are not needed here, please remove.
Will do.
/** 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 between different walks and* store the p.cur at the end of the walk to the user buffer.*/p.vec = kmalloc_array(p.vec_len, sizeof(struct page_region),GFP_KERNEL);if (!p.vec)return -ENOMEM;walk_start = walk_end = start;while (walk_end < end && !ret) {The loop will stop if a previous iteration returned ENOSPC (and the error will be lost) - is it intended?
It is intentional. -ENOSPC means that the user buffer is full even though there was more memory to walk over. We don't treat this error. So when buffer gets full, we stop walking over further as user buffer has gotten full and return as success.
p.vec_index = 0;empty_slots = arg.vec_len - vec_index;p.vec_len = min(p.vec_len, empty_slots);walk_end = (walk_start + PAGEMAP_WALK_SIZE) & PAGEMAP_WALK_MASK;if (walk_end > end)walk_end = end;mmap_read_lock(mm);ret = walk_page_range(mm, walk_start, walk_end,&pagemap_scan_ops, &p);mmap_read_unlock(mm);if (ret && ret != -ENOSPC && ret != PM_SCAN_FOUND_MAX_PAGES)goto free_data;walk_start = walk_end;if (p.vec_index) {if (copy_to_user(&vec[vec_index], p.vec,p.vec_index *sizeof(struct page_region))) {sizeof(*p.vec) ?
Sure.
ret = -EFAULT;goto free_data;}vec_index += p.vec_index;}}ret = pagemap_scan_deposit(&p, vec, &vec_index);if (!ret)ret = vec_index;+free_data:
kfree(p.vec);return ret;+}
+static long pagemap_scan_ioctl(struct file *file, unsigned int cmd,
unsigned long arg)+{
struct pm_scan_arg __user *uarg = (struct pm_scan_arg __user *)arg;This is specific to PAGEMAP_SCAN ioctl, so should go into do_pagemap_cmd().
Sure.
struct mm_struct *mm = file->private_data;switch (cmd) {case PAGEMAP_SCAN:return do_pagemap_cmd(mm, uarg);[...]
--- a/include/linux/userfaultfd_k.h +++ b/include/linux/userfaultfd_k.h @@ -210,6 +210,14 @@ extern bool userfaultfd_wp_async(struct vm_area_struct *vma);
#else /* CONFIG_USERFAULTFD */
+static inline long uffd_wp_range(struct mm_struct *dst_mm,
struct vm_area_struct *vma,unsigned long start, unsigned long len,bool enable_wp)+{
return 0;+}
/* mm helpers */ static inline vm_fault_t handle_userfault(struct vm_fault *vmf, unsigned long reason)
Shouldn't this part be in the patch introducing uffd_wp_range()?
We have not added uffd_wp_range() in previous patches. We just need this stub for this patch for the case when CONFIG_USERFAULTFD isn't enabled.
I'd this as separate patch before this patch. Mike asked me to merge it with this patch in order not to break bisectability. https://lore.kernel.org/all/ZBK+86eMMazwfhdx@kernel.org
Best Regards Michał Mirosław
On Thu, 6 Apr 2023 at 19:58, Muhammad Usama Anjum usama.anjum@collabora.com wrote:
Hello,
Thank you so much for the review. Do you have any thoughts on the build error on arc architecture? https://lore.kernel.org/all/e3c82373-256a-6297-bcb4-5e1179a2cbe2@collabora.c...
Maybe copy HPAGE_* defines from x86 and key on CONFIG_PGTABLE_LEVELS > 2? I don't know much about arc arch, though.
On 4/6/23 8:52 PM, Michał Mirosław wrote:
On Thu, 6 Apr 2023 at 09:40, Muhammad Usama Anjum usama.anjum@collabora.com wrote:>
[...]
+#define PM_SCAN_BITMAP(wt, file, present, swap) \
(wt | file << 1 | present << 2 | swap << 3)Please parenthesize macro arguments ("(wt)", "(file)", etc.) to not have to worry about operator precedence when passed a complex expression.
Like this? #define PM_SCAN_BITMAP(wt, file, present, swap) \ ((wt) | (file << 1) | (present << 2) | (swap << 3))
The value would be: ( (wt) | ((file) << 1) | ... ) IOW, each parameter should have parentheses around its name.
[...]
cur->len += n_pages;p->found_pages += n_pages;if (p->max_pages && (p->found_pages == p->max_pages))return PM_SCAN_FOUND_MAX_PAGES;return 0;}if (!p->vec_index || ((p->vec_index + 1) < p->vec_len)) {It looks that `if (p->vec_index < p->vec_len)` is enough here - if we have vec_len == 0 here, then we'd not fit the entry in the userspace buffer anyway. Am I missing something?
No. I'd explained it with diagram last time: https://lore.kernel.org/all/3c8d9ea0-1382-be0c-8dd2-d490eedd3b55@collabora.c...
I'll add a concise comment here.
So it seems, but I think the code changed a bit and maybe could be simplified now? Since p->vec_len == 0 is currently not valid, the field could count only the entries available in p->vec[] -- IOW: not include p->cur in the count.
BTW, `if (no space) return -ENOSPC` will avoid additional indentation for the non-merging case.
[...]
+static inline int pagemap_scan_deposit(struct pagemap_scan_private *p,
struct page_region __user *vec,unsigned long *vec_index)..._deposit() is used only in single place - please inline.
It is already inline.
Sorry. I mean: please paste the code in place of the single call.
[...]
/** Break huge page into small pages if the WP operation need to* be performed is on a portion of the huge page or if max_pages* pages limit would exceed.BTW, could the `max_pages` limit be relaxed a bit (in that it would be possible to return more pages if they all merge into the last vector entry) so that it would not need to split otherwise-matching huge page? It would remove the need for this special handling in the kernel and splitting the page by this read-only-appearing ioctl?
No, this cannot be done. Otherwise we'll not be able to emulate Windows syscall GetWriteWatch() which specifies the exact number of pages. Usually in most of cases, either user will not use THP or not perform the operation on partial huge page. So this part is only there to keep things correct for those users who do use THP and partial pagemap_scan operations.
I see that `GetWriteWatch` returns a list of pages not ranges of pages. That makes sense (more or less). (BTW, It could be emulated in userspace by caching the last not-fully-consumed range.)
*/if (is_written && PM_SCAN_OP_IS_WP(p) &&((end - start < HPAGE_SIZE) ||(p->max_pages &&(p->max_pages - p->found_pages) < n_pages))) {split_huge_pmd(vma, pmd, start);goto process_smaller_pages;}if (p->max_pages &&p->found_pages + n_pages > p->max_pages)n_pages = p->max_pages - p->found_pages;ret = pagemap_scan_output(is_written, is_file, is_present,is_swap, p, start, n_pages);if (ret < 0)return ret;
So let's simplify this:
if (p->max_pages && n_pages > max_pages - found_pages) n_pages = max_pages - found_pages;
if (is_written && DO_WP && n_pages != HPAGE_SIZE / PAGE_SIZE) { split_thp(); goto process_smaller_pages; }
BTW, THP handling could be extracted to a function that would return -EAGAIN if it has split the page or it wasn't a THP -- and that would mean `goto process_smaller_pages`.
Why not propagate the error from uffd_wp_range()?
uffd_wp_range() returns status in long variable. We cannot return long in this function. So intead of type casting long to int and then return I've used -EINVAL. Would following be more suitable?
long ret2 = uffd_wp_range(vma, start, HPAGE_SIZE, true); if (ret2 < 0) return (int)ret2;
I think it's ok, since negative values are expected to be error codes. And since you can't overflow int with HPAGE_SIZE pages, then I wouldn't use `ret2` but cast the return and add a comment why it's safe.
[...]
start = (unsigned long)untagged_addr(arg.start);vec = (struct page_region *)(unsigned long)untagged_addr(arg.vec);Is the inner cast needed?
arg.vec remains 64-bit on 32-bit systems. So casting 64bit value directly to struct page_region pointer errors out. So I've added specific casting to unsigned long first before casting to pointers.
I see. So to convey the intention, the `arg.start` and `arg.vec` should be casted to unsigned long, not the `untagged_addr()` return values.
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.flags = arg.flags;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.cur.len = 0;p.cur.start = 0;p.vec = NULL;p.vec_len = (PAGEMAP_WALK_SIZE >> PAGE_SHIFT);Nit: parentheses are not needed here, please remove.
Will do.
/** 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 between different walks and* store the p.cur at the end of the walk to the user buffer.*/p.vec = kmalloc_array(p.vec_len, sizeof(struct page_region),GFP_KERNEL);if (!p.vec)return -ENOMEM;walk_start = walk_end = start;while (walk_end < end && !ret) {The loop will stop if a previous iteration returned ENOSPC (and the error will be lost) - is it intended?
It is intentional. -ENOSPC means that the user buffer is full even though there was more memory to walk over. We don't treat this error. So when buffer gets full, we stop walking over further as user buffer has gotten full and return as success.
Thanks. What's the difference between -ENOSPC and PM_SCAN_FOUND_MAX_PAGES? They seem to result in the same effect (code flow).
[...]
--- a/include/linux/userfaultfd_k.h +++ b/include/linux/userfaultfd_k.h @@ -210,6 +210,14 @@ extern bool userfaultfd_wp_async(struct vm_area_struct *vma);
#else /* CONFIG_USERFAULTFD */
+static inline long uffd_wp_range(struct mm_struct *dst_mm,
struct vm_area_struct *vma,unsigned long start, unsigned long len,bool enable_wp)+{
return 0;+}
[...]
Shouldn't this part be in the patch introducing uffd_wp_range()?
We have not added uffd_wp_range() in previous patches. We just need this stub for this patch for the case when CONFIG_USERFAULTFD isn't enabled.
I'd this as separate patch before this patch. Mike asked me to merge it with this patch in order not to break bisectability. [1] https://lore.kernel.org/all/ZBK+86eMMazwfhdx@kernel.org
I would understand the reply [1] to mean that the uffd_wp_range() stub should go in the same patch where uffd_wp_range() is implemented. But uffd_wp_range() is already in (since f369b07c86140) so I don't see how having the stub in a separate commit sequenced before this one could break bisect?
Best Regards Michał Mirosław
On 4/7/23 1:00 AM, Michał Mirosław wrote:
On Thu, 6 Apr 2023 at 19:58, Muhammad Usama Anjum usama.anjum@collabora.com wrote:
Hello,
Thank you so much for the review. Do you have any thoughts on the build error on arc architecture? https://lore.kernel.org/all/e3c82373-256a-6297-bcb4-5e1179a2cbe2@collabora.c...
Maybe copy HPAGE_* defines from x86 and key on CONFIG_PGTABLE_LEVELS > 2? I don't know much about arc arch, though.
On 4/6/23 8:52 PM, Michał Mirosław wrote:
On Thu, 6 Apr 2023 at 09:40, Muhammad Usama Anjum usama.anjum@collabora.com wrote:>
[...]
+#define PM_SCAN_BITMAP(wt, file, present, swap) \
(wt | file << 1 | present << 2 | swap << 3)Please parenthesize macro arguments ("(wt)", "(file)", etc.) to not have to worry about operator precedence when passed a complex expression.
Like this? #define PM_SCAN_BITMAP(wt, file, present, swap) \ ((wt) | (file << 1) | (present << 2) | (swap << 3))
The value would be: ( (wt) | ((file) << 1) | ... ) IOW, each parameter should have parentheses around its name.
Will do.
[...]
cur->len += n_pages;p->found_pages += n_pages;if (p->max_pages && (p->found_pages == p->max_pages))return PM_SCAN_FOUND_MAX_PAGES;return 0;}if (!p->vec_index || ((p->vec_index + 1) < p->vec_len)) {It looks that `if (p->vec_index < p->vec_len)` is enough here - if we have vec_len == 0 here, then we'd not fit the entry in the userspace buffer anyway. Am I missing something?
No. I'd explained it with diagram last time: https://lore.kernel.org/all/3c8d9ea0-1382-be0c-8dd2-d490eedd3b55@collabora.c...
I'll add a concise comment here.
So it seems, but I think the code changed a bit and maybe could be simplified now? Since p->vec_len == 0 is currently not valid, the field could count only the entries available in p->vec[] -- IOW: not include p->cur in the count.
I see. But this'll not work as we need to count p->cur to don't go above the maximum count, p->vec_size.
BTW, `if (no space) return -ENOSPC` will avoid additional indentation for the non-merging case.
I'll update.
[...]
+static inline int pagemap_scan_deposit(struct pagemap_scan_private *p,
struct page_region __user *vec,unsigned long *vec_index)..._deposit() is used only in single place - please inline.
It is already inline.
Sorry. I mean: please paste the code in place of the single call.
I've made it a separate function to make the code look better in the caller function and logically easier to understand. This function is ugly. do_pagemap_scan() is also already very long function with lots of things happening. If you still insist, I'll remove this function.
[...]
/** Break huge page into small pages if the WP operation need to* be performed is on a portion of the huge page or if max_pages* pages limit would exceed.BTW, could the `max_pages` limit be relaxed a bit (in that it would be possible to return more pages if they all merge into the last vector entry) so that it would not need to split otherwise-matching huge page? It would remove the need for this special handling in the kernel and splitting the page by this read-only-appearing ioctl?
No, this cannot be done. Otherwise we'll not be able to emulate Windows syscall GetWriteWatch() which specifies the exact number of pages. Usually in most of cases, either user will not use THP or not perform the operation on partial huge page. So this part is only there to keep things correct for those users who do use THP and partial pagemap_scan operations.
I see that `GetWriteWatch` returns a list of pages not ranges of pages. That makes sense (more or less). (BTW, It could be emulated in userspace by caching the last not-fully-consumed range.)
First of all, caching is avoided as then state maintained is needed. This is probably not accepted in Wine upstream later. Secondly, even if we have cache, Get + WP operation would not be accurate when we ask only N pages, but it gets N + X pages where X pages will not be consumed by the application at this time.
*/if (is_written && PM_SCAN_OP_IS_WP(p) &&((end - start < HPAGE_SIZE) ||(p->max_pages &&(p->max_pages - p->found_pages) < n_pages))) {split_huge_pmd(vma, pmd, start);goto process_smaller_pages;}if (p->max_pages &&p->found_pages + n_pages > p->max_pages)n_pages = p->max_pages - p->found_pages;ret = pagemap_scan_output(is_written, is_file, is_present,is_swap, p, start, n_pages);if (ret < 0)return ret;So let's simplify this:
if (p->max_pages && n_pages > max_pages - found_pages) n_pages = max_pages - found_pages;
if (is_written && DO_WP && n_pages != HPAGE_SIZE / PAGE_SIZE) { split_thp(); goto process_smaller_pages; }
Clever!! This looks very sleek.
BTW, THP handling could be extracted to a function that would return -EAGAIN if it has split the page or it wasn't a THP -- and that would mean `goto process_smaller_pages`.
Other functions in this file handle the THP in this same way. So it feels like more intuitive that we follow to same pattern in this file.
Why not propagate the error from uffd_wp_range()?
uffd_wp_range() returns status in long variable. We cannot return long in this function. So intead of type casting long to int and then return I've used -EINVAL. Would following be more suitable?
long ret2 = uffd_wp_range(vma, start, HPAGE_SIZE, true); if (ret2 < 0) return (int)ret2;
I think it's ok, since negative values are expected to be error codes. And since you can't overflow int with HPAGE_SIZE pages, then I wouldn't use `ret2` but cast the return and add a comment why it's safe.
I'll update.
[...]
start = (unsigned long)untagged_addr(arg.start);vec = (struct page_region *)(unsigned long)untagged_addr(arg.vec);Is the inner cast needed?
arg.vec remains 64-bit on 32-bit systems. So casting 64bit value directly to struct page_region pointer errors out. So I've added specific casting to unsigned long first before casting to pointers.
I see. So to convey the intention, the `arg.start` and `arg.vec` should be casted to unsigned long, not the `untagged_addr()` return values.
I'll update.
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.flags = arg.flags;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.cur.len = 0;p.cur.start = 0;p.vec = NULL;p.vec_len = (PAGEMAP_WALK_SIZE >> PAGE_SHIFT);Nit: parentheses are not needed here, please remove.
Will do.
/** 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 between different walks and* store the p.cur at the end of the walk to the user buffer.*/p.vec = kmalloc_array(p.vec_len, sizeof(struct page_region),GFP_KERNEL);if (!p.vec)return -ENOMEM;walk_start = walk_end = start;while (walk_end < end && !ret) {The loop will stop if a previous iteration returned ENOSPC (and the error will be lost) - is it intended?
It is intentional. -ENOSPC means that the user buffer is full even though there was more memory to walk over. We don't treat this error. So when buffer gets full, we stop walking over further as user buffer has gotten full and return as success.
Thanks. What's the difference between -ENOSPC and PM_SCAN_FOUND_MAX_PAGES? They seem to result in the same effect (code flow).
-ENOSPC --> user buffer has been filled completely PM_SCAN_FOUND_MAX_PAGES --> max_pages have been found, user buffer may still have more space
[...]
--- a/include/linux/userfaultfd_k.h +++ b/include/linux/userfaultfd_k.h @@ -210,6 +210,14 @@ extern bool userfaultfd_wp_async(struct vm_area_struct *vma);
#else /* CONFIG_USERFAULTFD */
+static inline long uffd_wp_range(struct mm_struct *dst_mm,
struct vm_area_struct *vma,unsigned long start, unsigned long len,bool enable_wp)+{
return 0;+}
[...]
Shouldn't this part be in the patch introducing uffd_wp_range()?
We have not added uffd_wp_range() in previous patches. We just need this stub for this patch for the case when CONFIG_USERFAULTFD isn't enabled.
I'd this as separate patch before this patch. Mike asked me to merge it with this patch in order not to break bisectability. [1] https://lore.kernel.org/all/ZBK+86eMMazwfhdx@kernel.org
I would understand the reply [1] to mean that the uffd_wp_range() stub should go in the same patch where uffd_wp_range() is implemented. But uffd_wp_range() is already in (since f369b07c86140) so I don't see how having the stub in a separate commit sequenced before this one could break bisect?
Sorry, I mis-interpreted it. I'll make it a separate patch.
Best Regards Michał Mirosław
On Thu, 6 Apr 2023 at 23:04, Muhammad Usama Anjum usama.anjum@collabora.com wrote:
On 4/7/23 1:00 AM, Michał Mirosław wrote:
On Thu, 6 Apr 2023 at 19:58, Muhammad Usama Anjum usama.anjum@collabora.com wrote:
[...]
cur->len += n_pages;p->found_pages += n_pages;if (p->max_pages && (p->found_pages == p->max_pages))return PM_SCAN_FOUND_MAX_PAGES;return 0;}if (!p->vec_index || ((p->vec_index + 1) < p->vec_len)) {It looks that `if (p->vec_index < p->vec_len)` is enough here - if we have vec_len == 0 here, then we'd not fit the entry in the userspace buffer anyway. Am I missing something?
No. I'd explained it with diagram last time: https://lore.kernel.org/all/3c8d9ea0-1382-be0c-8dd2-d490eedd3b55@collabora.c...
I'll add a concise comment here.
So it seems, but I think the code changed a bit and maybe could be simplified now? Since p->vec_len == 0 is currently not valid, the field could count only the entries available in p->vec[] -- IOW: not include p->cur in the count.
I see. But this'll not work as we need to count p->cur to don't go above the maximum count, p->vec_size.
You can subtract 1 from p->vec_size before the page walk to account for the buffer in `cur`.
[...]
+static inline int pagemap_scan_deposit(struct pagemap_scan_private *p,
struct page_region __user *vec,unsigned long *vec_index)..._deposit() is used only in single place - please inline.
It is already inline.
Sorry. I mean: please paste the code in place of the single call.
I've made it a separate function to make the code look better in the caller function and logically easier to understand. This function is ugly. do_pagemap_scan() is also already very long function with lots of things happening. If you still insist, I'll remove this function.
Please do remove - it will make the copy to userspace code all neatly together.
[...]
*/if (is_written && PM_SCAN_OP_IS_WP(p) &&((end - start < HPAGE_SIZE) ||(p->max_pages &&(p->max_pages - p->found_pages) < n_pages))) {split_huge_pmd(vma, pmd, start);goto process_smaller_pages;}if (p->max_pages &&p->found_pages + n_pages > p->max_pages)n_pages = p->max_pages - p->found_pages;ret = pagemap_scan_output(is_written, is_file, is_present,is_swap, p, start, n_pages);if (ret < 0)return ret;So let's simplify this:
if (p->max_pages && n_pages > max_pages - found_pages) n_pages = max_pages - found_pages;
if (is_written && DO_WP && n_pages != HPAGE_SIZE / PAGE_SIZE) { split_thp(); goto process_smaller_pages; }
Clever!! This looks very sleek.
BTW, THP handling could be extracted to a function that would return -EAGAIN if it has split the page or it wasn't a THP -- and that would mean `goto process_smaller_pages`.
Other functions in this file handle the THP in this same way. So it feels like more intuitive that we follow to same pattern in this file.
I'll leave it to you. Extracting THP support would avoid a goto and #ifdef inside a function, though (and make the function smaller).
/** 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 between different walks and* store the p.cur at the end of the walk to the user buffer.*/p.vec = kmalloc_array(p.vec_len, sizeof(struct page_region),GFP_KERNEL);if (!p.vec)return -ENOMEM;walk_start = walk_end = start;while (walk_end < end && !ret) {The loop will stop if a previous iteration returned ENOSPC (and the error will be lost) - is it intended?
It is intentional. -ENOSPC means that the user buffer is full even though there was more memory to walk over. We don't treat this error. So when buffer gets full, we stop walking over further as user buffer has gotten full and return as success.
Thanks. What's the difference between -ENOSPC and PM_SCAN_FOUND_MAX_PAGES? They seem to result in the same effect (code flow).
-ENOSPC --> user buffer has been filled completely PM_SCAN_FOUND_MAX_PAGES --> max_pages have been found, user buffer may still have more space
What is the difference in code behaviour when those two cases are compared? (I'd expect none.)
Best Regards Michał Mirosław
On 4/7/23 12:34 PM, Michał Mirosław wrote:
On Thu, 6 Apr 2023 at 23:04, Muhammad Usama Anjum usama.anjum@collabora.com wrote:
On 4/7/23 1:00 AM, Michał Mirosław wrote:
On Thu, 6 Apr 2023 at 19:58, Muhammad Usama Anjum usama.anjum@collabora.com wrote:
[...]
cur->len += n_pages;p->found_pages += n_pages;if (p->max_pages && (p->found_pages == p->max_pages))return PM_SCAN_FOUND_MAX_PAGES;return 0;}if (!p->vec_index || ((p->vec_index + 1) < p->vec_len)) {It looks that `if (p->vec_index < p->vec_len)` is enough here - if we have vec_len == 0 here, then we'd not fit the entry in the userspace buffer anyway. Am I missing something?
No. I'd explained it with diagram last time: https://lore.kernel.org/all/3c8d9ea0-1382-be0c-8dd2-d490eedd3b55@collabora.c...
I'll add a concise comment here.
So it seems, but I think the code changed a bit and maybe could be simplified now? Since p->vec_len == 0 is currently not valid, the field could count only the entries available in p->vec[] -- IOW: not include p->cur in the count.
I see. But this'll not work as we need to count p->cur to don't go above the maximum count, p->vec_size.
You can subtract 1 from p->vec_size before the page walk to account for the buffer in `cur`.
Yeah, it can be done. But I've thought about it. It would look more uglier. I'll have to subtract 1 from vec_len in the start and then add 1 in the end. The current algorithm seems simpler.
[...]
+static inline int pagemap_scan_deposit(struct pagemap_scan_private *p,
struct page_region __user *vec,unsigned long *vec_index)..._deposit() is used only in single place - please inline.
It is already inline.
Sorry. I mean: please paste the code in place of the single call.
I've made it a separate function to make the code look better in the caller function and logically easier to understand. This function is ugly. do_pagemap_scan() is also already very long function with lots of things happening. If you still insist, I'll remove this function.
Please do remove - it will make the copy to userspace code all neatly together.
Will remove it.
[...]
*/if (is_written && PM_SCAN_OP_IS_WP(p) &&((end - start < HPAGE_SIZE) ||(p->max_pages &&(p->max_pages - p->found_pages) < n_pages))) {split_huge_pmd(vma, pmd, start);goto process_smaller_pages;}if (p->max_pages &&p->found_pages + n_pages > p->max_pages)n_pages = p->max_pages - p->found_pages;ret = pagemap_scan_output(is_written, is_file, is_present,is_swap, p, start, n_pages);if (ret < 0)return ret;So let's simplify this:
if (p->max_pages && n_pages > max_pages - found_pages) n_pages = max_pages - found_pages;
if (is_written && DO_WP && n_pages != HPAGE_SIZE / PAGE_SIZE) { split_thp(); goto process_smaller_pages; }
Clever!! This looks very sleek.
BTW, THP handling could be extracted to a function that would return -EAGAIN if it has split the page or it wasn't a THP -- and that would mean `goto process_smaller_pages`.
Other functions in this file handle the THP in this same way. So it feels like more intuitive that we follow to same pattern in this file.
I'll leave it to you. Extracting THP support would avoid a goto and #ifdef inside a function, though (and make the function smaller).
/** 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 between different walks and* store the p.cur at the end of the walk to the user buffer.*/p.vec = kmalloc_array(p.vec_len, sizeof(struct page_region),GFP_KERNEL);if (!p.vec)return -ENOMEM;walk_start = walk_end = start;while (walk_end < end && !ret) {The loop will stop if a previous iteration returned ENOSPC (and the error will be lost) - is it intended?
It is intentional. -ENOSPC means that the user buffer is full even though there was more memory to walk over. We don't treat this error. So when buffer gets full, we stop walking over further as user buffer has gotten full and return as success.
Thanks. What's the difference between -ENOSPC and PM_SCAN_FOUND_MAX_PAGES? They seem to result in the same effect (code flow).
-ENOSPC --> user buffer has been filled completely PM_SCAN_FOUND_MAX_PAGES --> max_pages have been found, user buffer may still have more space
What is the difference in code behaviour when those two cases are compared? (I'd expect none.)
There is difference: We add data to user buffer. If it succeeds with return code 0, we engage the WP. If it succeeds with PM_SCAN_FOUND_MAX_PAGES, we still engage the WP. But if we get -ENOSPC, we don't perform engage as the data wasn't added to the user buffer.
Best Regards Michał Mirosław
On Fri, 7 Apr 2023 at 12:04, Muhammad Usama Anjum usama.anjum@collabora.com wrote:
On 4/7/23 12:34 PM, Michał Mirosław wrote:
On Thu, 6 Apr 2023 at 23:04, Muhammad Usama Anjum usama.anjum@collabora.com wrote:
On 4/7/23 1:00 AM, Michał Mirosław wrote:
On Thu, 6 Apr 2023 at 19:58, Muhammad Usama Anjum usama.anjum@collabora.com wrote:
[...]
> + /* > + * 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 between different walks and > + * store the p.cur at the end of the walk to the user buffer. > + */ > + p.vec = kmalloc_array(p.vec_len, sizeof(struct page_region), > + GFP_KERNEL); > + if (!p.vec) > + return -ENOMEM; > + > + walk_start = walk_end = start; > + while (walk_end < end && !ret) {
The loop will stop if a previous iteration returned ENOSPC (and the error will be lost) - is it intended?
It is intentional. -ENOSPC means that the user buffer is full even though there was more memory to walk over. We don't treat this error. So when buffer gets full, we stop walking over further as user buffer has gotten full and return as success.
Thanks. What's the difference between -ENOSPC and PM_SCAN_FOUND_MAX_PAGES? They seem to result in the same effect (code flow).
-ENOSPC --> user buffer has been filled completely PM_SCAN_FOUND_MAX_PAGES --> max_pages have been found, user buffer may still have more space
What is the difference in code behaviour when those two cases are compared? (I'd expect none.)
There is difference: We add data to user buffer. If it succeeds with return code 0, we engage the WP. If it succeeds with PM_SCAN_FOUND_MAX_PAGES, we still engage the WP. But if we get -ENOSPC, we don't perform engage as the data wasn't added to the user buffer.
Thanks! I see it now. I see a few more corner cases here: 1. If we did engage WP but fail to copy the vector we return -EFAULT but the WP is already engaged. I'm not sure this is something worth guarding against, but documenting that would be helpful I think. 2. If uffd_wp_range() fails, but we have already processed pages earlier, we should treat the error like ENOSPC and back out the failed range (the earier changes would be lost otherwise).
Best Regards Michał Mirosław
On 4/7/23 3:14 PM, Michał Mirosław wrote:
On Fri, 7 Apr 2023 at 12:04, Muhammad Usama Anjum usama.anjum@collabora.com wrote:
On 4/7/23 12:34 PM, Michał Mirosław wrote:
On Thu, 6 Apr 2023 at 23:04, Muhammad Usama Anjum usama.anjum@collabora.com wrote:
On 4/7/23 1:00 AM, Michał Mirosław wrote:
On Thu, 6 Apr 2023 at 19:58, Muhammad Usama Anjum usama.anjum@collabora.com wrote:
[...]
>> + /* >> + * 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 between different walks and >> + * store the p.cur at the end of the walk to the user buffer. >> + */ >> + p.vec = kmalloc_array(p.vec_len, sizeof(struct page_region), >> + GFP_KERNEL); >> + if (!p.vec) >> + return -ENOMEM; >> + >> + walk_start = walk_end = start; >> + while (walk_end < end && !ret) { > > The loop will stop if a previous iteration returned ENOSPC (and the > error will be lost) - is it intended? It is intentional. -ENOSPC means that the user buffer is full even though there was more memory to walk over. We don't treat this error. So when buffer gets full, we stop walking over further as user buffer has gotten full and return as success.
Thanks. What's the difference between -ENOSPC and PM_SCAN_FOUND_MAX_PAGES? They seem to result in the same effect (code flow).
-ENOSPC --> user buffer has been filled completely PM_SCAN_FOUND_MAX_PAGES --> max_pages have been found, user buffer may still have more space
What is the difference in code behaviour when those two cases are compared? (I'd expect none.)
There is difference: We add data to user buffer. If it succeeds with return code 0, we engage the WP. If it succeeds with PM_SCAN_FOUND_MAX_PAGES, we still engage the WP. But if we get -ENOSPC, we don't perform engage as the data wasn't added to the user buffer.
Thanks! I see it now. I see a few more corner cases here:
- If we did engage WP but fail to copy the vector we return -EFAULT
but the WP is already engaged. I'm not sure this is something worth guarding against, but documenting that would be helpful I think.
Sure.
- If uffd_wp_range() fails, but we have already processed pages
earlier, we should treat the error like ENOSPC and back out the failed range (the earier changes would be lost otherwise).
Backing out is easier to do for hugepages. But for normal pages, we'll have to write some code to find where the current data was added (in cur or in vec) and back out from that. I'll have to write some more code to avoid the side-effects as well.
But aren't we going over-engineering here? Error occurred and we are trying to keep the previously generated correct data and returning successfully still to the user? I don't think we should do this. An error is error. We should return the error simply even if the memory flags would get lost. We don't know what caused the error in uffd_wp_range(). Under normal situation, we there shouldn't have had error.
Best Regards Michał Mirosław
On Fri, 7 Apr 2023 at 13:11, Muhammad Usama Anjum usama.anjum@collabora.com wrote:
On 4/7/23 3:14 PM, Michał Mirosław wrote:
On Fri, 7 Apr 2023 at 12:04, Muhammad Usama Anjum usama.anjum@collabora.com wrote:
On 4/7/23 12:34 PM, Michał Mirosław wrote:
On Thu, 6 Apr 2023 at 23:04, Muhammad Usama Anjum usama.anjum@collabora.com wrote:
On 4/7/23 1:00 AM, Michał Mirosław wrote:
On Thu, 6 Apr 2023 at 19:58, Muhammad Usama Anjum usama.anjum@collabora.com wrote:
[...]
>>> + /* >>> + * 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 between different walks and >>> + * store the p.cur at the end of the walk to the user buffer. >>> + */ >>> + p.vec = kmalloc_array(p.vec_len, sizeof(struct page_region), >>> + GFP_KERNEL); >>> + if (!p.vec) >>> + return -ENOMEM; >>> + >>> + walk_start = walk_end = start; >>> + while (walk_end < end && !ret) { >> >> The loop will stop if a previous iteration returned ENOSPC (and the >> error will be lost) - is it intended? > It is intentional. -ENOSPC means that the user buffer is full even though > there was more memory to walk over. We don't treat this error. So when > buffer gets full, we stop walking over further as user buffer has gotten > full and return as success.
Thanks. What's the difference between -ENOSPC and PM_SCAN_FOUND_MAX_PAGES? They seem to result in the same effect (code flow).
-ENOSPC --> user buffer has been filled completely PM_SCAN_FOUND_MAX_PAGES --> max_pages have been found, user buffer may still have more space
What is the difference in code behaviour when those two cases are compared? (I'd expect none.)
There is difference: We add data to user buffer. If it succeeds with return code 0, we engage the WP. If it succeeds with PM_SCAN_FOUND_MAX_PAGES, we still engage the WP. But if we get -ENOSPC, we don't perform engage as the data wasn't added to the user buffer.
Thanks! I see it now. I see a few more corner cases here:
- If we did engage WP but fail to copy the vector we return -EFAULT
but the WP is already engaged. I'm not sure this is something worth guarding against, but documenting that would be helpful I think.
Sure.
- If uffd_wp_range() fails, but we have already processed pages
earlier, we should treat the error like ENOSPC and back out the failed range (the earier changes would be lost otherwise).
Backing out is easier to do for hugepages. But for normal pages, we'll have to write some code to find where the current data was added (in cur or in vec) and back out from that. I'll have to write some more code to avoid the side-effects as well.
If I read the code correctly, the last page should always be in `cur` and on failure only a single page is needed to be backed out. Did I miss something?
But aren't we going over-engineering here? Error occurred and we are trying to keep the previously generated correct data and returning successfully still to the user? I don't think we should do this. An error is error. We should return the error simply even if the memory flags would get lost. We don't know what caused the error in uffd_wp_range(). Under normal situation, we there shouldn't have had error.
In this case it means that on (intermittent) allocation error we get inconsistent or non-deterministic results. I wouldn't want to be the one debugging this later - I'd prefer either the syscall be "exception-safe" (give consistent and predictable output) or kill the process.
Best Regards Michał Mirosław
On 4/11/23 2:29 PM, Michał Mirosław wrote:
On Fri, 7 Apr 2023 at 13:11, Muhammad Usama Anjum usama.anjum@collabora.com wrote:
On 4/7/23 3:14 PM, Michał Mirosław wrote:
On Fri, 7 Apr 2023 at 12:04, Muhammad Usama Anjum usama.anjum@collabora.com wrote:
On 4/7/23 12:34 PM, Michał Mirosław wrote:
On Thu, 6 Apr 2023 at 23:04, Muhammad Usama Anjum usama.anjum@collabora.com wrote:
On 4/7/23 1:00 AM, Michał Mirosław wrote: > On Thu, 6 Apr 2023 at 19:58, Muhammad Usama Anjum > usama.anjum@collabora.com wrote:
[...]
>>>> + /* >>>> + * 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 between different walks and >>>> + * store the p.cur at the end of the walk to the user buffer. >>>> + */ >>>> + p.vec = kmalloc_array(p.vec_len, sizeof(struct page_region), >>>> + GFP_KERNEL); >>>> + if (!p.vec) >>>> + return -ENOMEM; >>>> + >>>> + walk_start = walk_end = start; >>>> + while (walk_end < end && !ret) { >>> >>> The loop will stop if a previous iteration returned ENOSPC (and the >>> error will be lost) - is it intended? >> It is intentional. -ENOSPC means that the user buffer is full even though >> there was more memory to walk over. We don't treat this error. So when >> buffer gets full, we stop walking over further as user buffer has gotten >> full and return as success. > > Thanks. What's the difference between -ENOSPC and > PM_SCAN_FOUND_MAX_PAGES? They seem to result in the same effect (code > flow). -ENOSPC --> user buffer has been filled completely PM_SCAN_FOUND_MAX_PAGES --> max_pages have been found, user buffer may still have more space
What is the difference in code behaviour when those two cases are compared? (I'd expect none.)
There is difference: We add data to user buffer. If it succeeds with return code 0, we engage the WP. If it succeeds with PM_SCAN_FOUND_MAX_PAGES, we still engage the WP. But if we get -ENOSPC, we don't perform engage as the data wasn't added to the user buffer.
Thanks! I see it now. I see a few more corner cases here:
- If we did engage WP but fail to copy the vector we return -EFAULT
but the WP is already engaged. I'm not sure this is something worth guarding against, but documenting that would be helpful I think.
Sure.
- If uffd_wp_range() fails, but we have already processed pages
earlier, we should treat the error like ENOSPC and back out the failed range (the earier changes would be lost otherwise).
Backing out is easier to do for hugepages. But for normal pages, we'll have to write some code to find where the current data was added (in cur or in vec) and back out from that. I'll have to write some more code to avoid the side-effects as well.
If I read the code correctly, the last page should always be in `cur` and on failure only a single page is needed to be backed out. Did I miss something?
I'm leaving using uffd_wp_range() in next revision as it is costing performance. This will not be needed in next revision.
But aren't we going over-engineering here? Error occurred and we are trying to keep the previously generated correct data and returning successfully still to the user? I don't think we should do this. An error is error. We should return the error simply even if the memory flags would get lost. We don't know what caused the error in uffd_wp_range(). Under normal situation, we there shouldn't have had error.
In this case it means that on (intermittent) allocation error we get inconsistent or non-deterministic results. I wouldn't want to be the one debugging this later - I'd prefer either the syscall be "exception-safe" (give consistent and predictable output) or kill the process.
Best Regards Michał Mirosław
On Thu, 6 Apr 2023 at 09:40, Muhammad Usama Anjum usama.anjum@collabora.com wrote: [...]
--- a/fs/proc/task_mmu.c +++ b/fs/proc/task_mmu.c
[...]
+static int pagemap_scan_pmd_entry(pmd_t *pmd, unsigned long start,
unsigned long end, struct mm_walk *walk)+{
struct pagemap_scan_private *p = walk->private;bool is_written, is_file, is_present, is_swap;struct vm_area_struct *vma = walk->vma;unsigned long addr = end;spinlock_t *ptl;int ret = 0;pte_t *pte;+#ifdef CONFIG_TRANSPARENT_HUGEPAGE
ptl = pmd_trans_huge_lock(pmd, vma);if (ptl) {
[...]
return ret;}+process_smaller_pages:
if (pmd_trans_unstable(pmd))return 0;
Why pmd_trans_unstable() is needed here and not only after split_huge_pmd()?
Best regards Michał Mirosław
On 4/7/23 1:12 AM, Michał Mirosław wrote:
On Thu, 6 Apr 2023 at 09:40, Muhammad Usama Anjum usama.anjum@collabora.com wrote: [...]
--- a/fs/proc/task_mmu.c +++ b/fs/proc/task_mmu.c
[...]
+static int pagemap_scan_pmd_entry(pmd_t *pmd, unsigned long start,
unsigned long end, struct mm_walk *walk)+{
struct pagemap_scan_private *p = walk->private;bool is_written, is_file, is_present, is_swap;struct vm_area_struct *vma = walk->vma;unsigned long addr = end;spinlock_t *ptl;int ret = 0;pte_t *pte;+#ifdef CONFIG_TRANSPARENT_HUGEPAGE
ptl = pmd_trans_huge_lock(pmd, vma);if (ptl) {[...]
return ret;}+process_smaller_pages:
if (pmd_trans_unstable(pmd))return 0;Why pmd_trans_unstable() is needed here and not only after split_huge_pmd()?
I'm not entirely sure. But the idea is if THP is unstable, we should return. As it doesn't seem like after splitting THP can be unstable, we should not check it. Do you agree with the following?
--- a/fs/proc/task_mmu.c +++ b/fs/proc/task_mmu.c @@ -1957,11 +1957,11 @@ static int pagemap_scan_pmd_entry(pmd_t *pmd, unsigned long start,
return ret; } -process_smaller_pages: + if (pmd_trans_unstable(pmd)) return 0; #endif - +process_smaller_pages: for (addr = start; addr < end && !ret; pte++, addr += PAGE_SIZE)
On Thu, 6 Apr 2023 at 23:12, Muhammad Usama Anjum usama.anjum@collabora.com wrote:
On 4/7/23 1:12 AM, Michał Mirosław wrote:
On Thu, 6 Apr 2023 at 09:40, Muhammad Usama Anjum usama.anjum@collabora.com wrote: [...]
--- a/fs/proc/task_mmu.c +++ b/fs/proc/task_mmu.c
[...]
+static int pagemap_scan_pmd_entry(pmd_t *pmd, unsigned long start,
unsigned long end, struct mm_walk *walk)+{
[...]
+#ifdef CONFIG_TRANSPARENT_HUGEPAGE
ptl = pmd_trans_huge_lock(pmd, vma);if (ptl) {[...]
return ret;}+process_smaller_pages:
if (pmd_trans_unstable(pmd))return 0;Why pmd_trans_unstable() is needed here and not only after split_huge_pmd()?
I'm not entirely sure. But the idea is if THP is unstable, we should return. As it doesn't seem like after splitting THP can be unstable, we should not check it. Do you agree with the following?
The description of pmd_trans_unstable() [1] seems to indicate that it is needed only after split_huge_pmd().
[1] https://elixir.bootlin.com/linux/v6.3-rc5/source/include/linux/pgtable.h#L13...
Best Regards Michał Mirosław
On 4/7/23 12:23 PM, Michał Mirosław wrote:
On Thu, 6 Apr 2023 at 23:12, Muhammad Usama Anjum usama.anjum@collabora.com wrote:
On 4/7/23 1:12 AM, Michał Mirosław wrote:
On Thu, 6 Apr 2023 at 09:40, Muhammad Usama Anjum usama.anjum@collabora.com wrote: [...]
--- a/fs/proc/task_mmu.c +++ b/fs/proc/task_mmu.c
[...]
+static int pagemap_scan_pmd_entry(pmd_t *pmd, unsigned long start,
unsigned long end, struct mm_walk *walk)+{
[...]
+#ifdef CONFIG_TRANSPARENT_HUGEPAGE
ptl = pmd_trans_huge_lock(pmd, vma);if (ptl) {[...]
return ret;}+process_smaller_pages:
if (pmd_trans_unstable(pmd))return 0;Why pmd_trans_unstable() is needed here and not only after split_huge_pmd()?
I'm not entirely sure. But the idea is if THP is unstable, we should return. As it doesn't seem like after splitting THP can be unstable, we should not check it. Do you agree with the following?
The description of pmd_trans_unstable() [1] seems to indicate that it is needed only after split_huge_pmd().
[1] https://elixir.bootlin.com/linux/v6.3-rc5/source/include/linux/pgtable.h#L13...
Sorry, yeah pmd_trans_unstable() is need after split. But it is also needed in normal case when ptl is NULL to rule out the case if pmd is unstable before performing operation on normal pages:
ptl = pmd_trans_huge_lock(pmd, vma); if (ptl) { ... } if (pmd_trans_unstable(pmd)) return 0;
This file has usage examples of pmd_trans_unstable():
https://elixir.bootlin.com/linux/v6.3-rc5/source/fs/proc/task_mmu.c#L634 https://elixir.bootlin.com/linux/v6.3-rc5/source/fs/proc/task_mmu.c#L1195 https://elixir.bootlin.com/linux/v6.3-rc5/source/fs/proc/task_mmu.c#L1543 https://elixir.bootlin.com/linux/v6.3-rc5/source/fs/proc/task_mmu.c#L1887
So we are good with what we have in this patch.
Best Regards Michał Mirosław
On Fri, 7 Apr 2023 at 11:35, Muhammad Usama Anjum usama.anjum@collabora.com wrote:
On 4/7/23 12:23 PM, Michał Mirosław wrote:
On Thu, 6 Apr 2023 at 23:12, Muhammad Usama Anjum usama.anjum@collabora.com wrote:
On 4/7/23 1:12 AM, Michał Mirosław wrote:
On Thu, 6 Apr 2023 at 09:40, Muhammad Usama Anjum usama.anjum@collabora.com wrote: [...]
--- a/fs/proc/task_mmu.c +++ b/fs/proc/task_mmu.c
[...]
+static int pagemap_scan_pmd_entry(pmd_t *pmd, unsigned long start,
unsigned long end, struct mm_walk *walk)+{
[...]
+#ifdef CONFIG_TRANSPARENT_HUGEPAGE
ptl = pmd_trans_huge_lock(pmd, vma);if (ptl) {[...]
return ret;}+process_smaller_pages:
if (pmd_trans_unstable(pmd))return 0;Why pmd_trans_unstable() is needed here and not only after split_huge_pmd()?
I'm not entirely sure. But the idea is if THP is unstable, we should return. As it doesn't seem like after splitting THP can be unstable, we should not check it. Do you agree with the following?
The description of pmd_trans_unstable() [1] seems to indicate that it is needed only after split_huge_pmd().
[1] https://elixir.bootlin.com/linux/v6.3-rc5/source/include/linux/pgtable.h#L13...
Sorry, yeah pmd_trans_unstable() is need after split. But it is also needed in normal case when ptl is NULL to rule out the case if pmd is unstable before performing operation on normal pages:
ptl = pmd_trans_huge_lock(pmd, vma); if (ptl) { ... } if (pmd_trans_unstable(pmd)) return 0;
This file has usage examples of pmd_trans_unstable():
https://elixir.bootlin.com/linux/v6.3-rc5/source/fs/proc/task_mmu.c#L634 https://elixir.bootlin.com/linux/v6.3-rc5/source/fs/proc/task_mmu.c#L1195 https://elixir.bootlin.com/linux/v6.3-rc5/source/fs/proc/task_mmu.c#L1543 https://elixir.bootlin.com/linux/v6.3-rc5/source/fs/proc/task_mmu.c#L1887
So we are good with what we have in this patch.
Shouldn't we signal ACTION_AGAIN then in order to call .pte_hole?
Best Regards Michał Mirosław
On 4/7/23 3:04 PM, Michał Mirosław wrote:
On Fri, 7 Apr 2023 at 11:35, Muhammad Usama Anjum usama.anjum@collabora.com wrote:
On 4/7/23 12:23 PM, Michał Mirosław wrote:
On Thu, 6 Apr 2023 at 23:12, Muhammad Usama Anjum usama.anjum@collabora.com wrote:
On 4/7/23 1:12 AM, Michał Mirosław wrote:
On Thu, 6 Apr 2023 at 09:40, Muhammad Usama Anjum usama.anjum@collabora.com wrote: [...]
--- a/fs/proc/task_mmu.c +++ b/fs/proc/task_mmu.c
[...]
+static int pagemap_scan_pmd_entry(pmd_t *pmd, unsigned long start,
unsigned long end, struct mm_walk *walk)+{
[...]
+#ifdef CONFIG_TRANSPARENT_HUGEPAGE
ptl = pmd_trans_huge_lock(pmd, vma);if (ptl) {[...]
return ret;}+process_smaller_pages:
if (pmd_trans_unstable(pmd))return 0;Why pmd_trans_unstable() is needed here and not only after split_huge_pmd()?
I'm not entirely sure. But the idea is if THP is unstable, we should return. As it doesn't seem like after splitting THP can be unstable, we should not check it. Do you agree with the following?
The description of pmd_trans_unstable() [1] seems to indicate that it is needed only after split_huge_pmd().
[1] https://elixir.bootlin.com/linux/v6.3-rc5/source/include/linux/pgtable.h#L13...
Sorry, yeah pmd_trans_unstable() is need after split. But it is also needed in normal case when ptl is NULL to rule out the case if pmd is unstable before performing operation on normal pages:
ptl = pmd_trans_huge_lock(pmd, vma); if (ptl) { ... } if (pmd_trans_unstable(pmd)) return 0;
This file has usage examples of pmd_trans_unstable():
https://elixir.bootlin.com/linux/v6.3-rc5/source/fs/proc/task_mmu.c#L634 https://elixir.bootlin.com/linux/v6.3-rc5/source/fs/proc/task_mmu.c#L1195 https://elixir.bootlin.com/linux/v6.3-rc5/source/fs/proc/task_mmu.c#L1543 https://elixir.bootlin.com/linux/v6.3-rc5/source/fs/proc/task_mmu.c#L1887
So we are good with what we have in this patch.
Shouldn't we signal ACTION_AGAIN then in order to call .pte_hole?
I'm not sure. I've not done research on it if we need to signal ACTION_AGAIN as this function pagemap_scan_pmd_entry() mimics how pagemap_pmd_range() handles reads to the pagemap file. pagemap_pmd_range() isn't doing anything if pmd is unstable. Hence we also not doing anything.
Best Regards Michał Mirosław
On Fri, 7 Apr 2023 at 12:15, Muhammad Usama Anjum usama.anjum@collabora.com wrote:
On 4/7/23 3:04 PM, Michał Mirosław wrote:
On Fri, 7 Apr 2023 at 11:35, Muhammad Usama Anjum usama.anjum@collabora.com wrote:
On 4/7/23 12:23 PM, Michał Mirosław wrote:
On Thu, 6 Apr 2023 at 23:12, Muhammad Usama Anjum usama.anjum@collabora.com wrote:
On 4/7/23 1:12 AM, Michał Mirosław wrote:
On Thu, 6 Apr 2023 at 09:40, Muhammad Usama Anjum usama.anjum@collabora.com wrote: [...] > --- a/fs/proc/task_mmu.c > +++ b/fs/proc/task_mmu.c [...] > +static int pagemap_scan_pmd_entry(pmd_t *pmd, unsigned long start, > + unsigned long end, struct mm_walk *walk) > +{
[...]
> +#ifdef CONFIG_TRANSPARENT_HUGEPAGE > + ptl = pmd_trans_huge_lock(pmd, vma); > + if (ptl) { [...] > + return ret; > + } > +process_smaller_pages: > + if (pmd_trans_unstable(pmd)) > + return 0;
Why pmd_trans_unstable() is needed here and not only after split_huge_pmd()?
I'm not entirely sure. But the idea is if THP is unstable, we should return. As it doesn't seem like after splitting THP can be unstable, we should not check it. Do you agree with the following?
The description of pmd_trans_unstable() [1] seems to indicate that it is needed only after split_huge_pmd().
[1] https://elixir.bootlin.com/linux/v6.3-rc5/source/include/linux/pgtable.h#L13...
Sorry, yeah pmd_trans_unstable() is need after split. But it is also needed in normal case when ptl is NULL to rule out the case if pmd is unstable before performing operation on normal pages:
ptl = pmd_trans_huge_lock(pmd, vma); if (ptl) { ... } if (pmd_trans_unstable(pmd)) return 0;
This file has usage examples of pmd_trans_unstable():
https://elixir.bootlin.com/linux/v6.3-rc5/source/fs/proc/task_mmu.c#L634 https://elixir.bootlin.com/linux/v6.3-rc5/source/fs/proc/task_mmu.c#L1195 https://elixir.bootlin.com/linux/v6.3-rc5/source/fs/proc/task_mmu.c#L1543 https://elixir.bootlin.com/linux/v6.3-rc5/source/fs/proc/task_mmu.c#L1887
So we are good with what we have in this patch.
Shouldn't we signal ACTION_AGAIN then in order to call .pte_hole?
I'm not sure. I've not done research on it if we need to signal ACTION_AGAIN as this function pagemap_scan_pmd_entry() mimics how pagemap_pmd_range() handles reads to the pagemap file. pagemap_pmd_range() isn't doing anything if pmd is unstable. Hence we also not doing anything.
Doesn't this mean that if we scan a file-backed vma we would miss non-present parts of the mapping in the output?
Best Regards Michał Mirosław
On 4/7/23 3:21 PM, Michał Mirosław wrote:
On Fri, 7 Apr 2023 at 12:15, Muhammad Usama Anjum usama.anjum@collabora.com wrote:
On 4/7/23 3:04 PM, Michał Mirosław wrote:
On Fri, 7 Apr 2023 at 11:35, Muhammad Usama Anjum usama.anjum@collabora.com wrote:
On 4/7/23 12:23 PM, Michał Mirosław wrote:
On Thu, 6 Apr 2023 at 23:12, Muhammad Usama Anjum usama.anjum@collabora.com wrote:
On 4/7/23 1:12 AM, Michał Mirosław wrote: > On Thu, 6 Apr 2023 at 09:40, Muhammad Usama Anjum > usama.anjum@collabora.com wrote: > [...] >> --- a/fs/proc/task_mmu.c >> +++ b/fs/proc/task_mmu.c > [...] >> +static int pagemap_scan_pmd_entry(pmd_t *pmd, unsigned long start, >> + unsigned long end, struct mm_walk *walk) >> +{
[...]
>> +#ifdef CONFIG_TRANSPARENT_HUGEPAGE >> + ptl = pmd_trans_huge_lock(pmd, vma); >> + if (ptl) { > [...] >> + return ret; >> + } >> +process_smaller_pages: >> + if (pmd_trans_unstable(pmd)) >> + return 0; > > Why pmd_trans_unstable() is needed here and not only after split_huge_pmd()? I'm not entirely sure. But the idea is if THP is unstable, we should return. As it doesn't seem like after splitting THP can be unstable, we should not check it. Do you agree with the following?
The description of pmd_trans_unstable() [1] seems to indicate that it is needed only after split_huge_pmd().
[1] https://elixir.bootlin.com/linux/v6.3-rc5/source/include/linux/pgtable.h#L13...
Sorry, yeah pmd_trans_unstable() is need after split. But it is also needed in normal case when ptl is NULL to rule out the case if pmd is unstable before performing operation on normal pages:
ptl = pmd_trans_huge_lock(pmd, vma); if (ptl) { ... } if (pmd_trans_unstable(pmd)) return 0;
This file has usage examples of pmd_trans_unstable():
https://elixir.bootlin.com/linux/v6.3-rc5/source/fs/proc/task_mmu.c#L634 https://elixir.bootlin.com/linux/v6.3-rc5/source/fs/proc/task_mmu.c#L1195 https://elixir.bootlin.com/linux/v6.3-rc5/source/fs/proc/task_mmu.c#L1543 https://elixir.bootlin.com/linux/v6.3-rc5/source/fs/proc/task_mmu.c#L1887
So we are good with what we have in this patch.
Shouldn't we signal ACTION_AGAIN then in order to call .pte_hole?
I'm not sure. I've not done research on it if we need to signal ACTION_AGAIN as this function pagemap_scan_pmd_entry() mimics how pagemap_pmd_range() handles reads to the pagemap file. pagemap_pmd_range() isn't doing anything if pmd is unstable. Hence we also not doing anything.
Doesn't this mean that if we scan a file-backed vma we would miss non-present parts of the mapping in the output?
I'm trying to mimic the same information through ioctl which is attained by reading the file. We'll only miss the unstable VMA here. I'm don't know about how often the PMD is unstable and its effects.
Best Regards Michał Mirosław
New IOCTL and macros has been added in the kernel sources. Update the tools header file as well.
Signed-off-by: Muhammad Usama Anjum usama.anjum@collabora.com --- tools/include/uapi/linux/fs.h | 53 +++++++++++++++++++++++++++++++++++ 1 file changed, 53 insertions(+)
diff --git a/tools/include/uapi/linux/fs.h b/tools/include/uapi/linux/fs.h index b7b56871029c..47879c38ce2f 100644 --- a/tools/include/uapi/linux/fs.h +++ b/tools/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 the bitmap 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 bitmap 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 bitmap; +}; + +/* + * 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 */
Add some explanation and method to use write-protection and written-to on memory range.
Signed-off-by: Muhammad Usama Anjum usama.anjum@collabora.com --- Changes in v11: - Add more documentation --- Documentation/admin-guide/mm/pagemap.rst | 56 ++++++++++++++++++++++++ 1 file changed, 56 insertions(+)
diff --git a/Documentation/admin-guide/mm/pagemap.rst b/Documentation/admin-guide/mm/pagemap.rst index c8f380271cad..a3e08f15b433 100644 --- a/Documentation/admin-guide/mm/pagemap.rst +++ b/Documentation/admin-guide/mm/pagemap.rst @@ -227,3 +227,59 @@ Before Linux 3.11 pagemap bits 55-60 were used for "page-shift" (which is always 12 at most architectures). Since Linux 3.11 their meaning changes after first clear of soft-dirty bits. Since Linux 4.2 they are used for flags unconditionally. + +Pagemap Scan IOCTL +================== + +The ``PAGEMAP_SCAN`` IOCTL on the pagemap file can be used to get or optionally +clear the info about page table entries. The following operations are supported +in this IOCTL: +- Get the information if the pages have been written-to (``PAGE_IS_WRITTEN``), + file mapped (``PAGE_IS_FILE``), present (``PAGE_IS_PRESENT``) or swapped + (``PAGE_IS_SWAPPED``). +- Find pages which have been written-to and write protect the pages atomically + (atomic ``PAGE_IS_WRITTEN + PAGEMAP_WP_ENGAGE``) + +The ``struct pm_scan_arg`` is used as the argument of the IOCTL. + 1. The size of the ``struct pm_scan_arg`` must be specified in the ``size`` + field. This field will be helpful in recognizing the structure if extensions + are done later. + 2. The flags can be specified in the ``flags`` field. The ``PM_SCAN_OP_GET`` + and ``PM_SCAN_OP_WP`` are the only added flag at this time. + 3. The range is specified through ``start`` and ``len``. + 4. The output buffer of ``struct page_region`` array and size is specified in + ``vec`` and ``vec_len``. + 5. The optional maximum requested pages are specified in the ``max_pages``. + 6. The masks are specified in ``required_mask``, ``anyof_mask``, + ``excluded_ mask`` and ``return_mask``. + 1. To find if ``PAGE_IS_WRITTEN`` flag is set for pages which have + ``PAGE_IS_FILE`` set and ``PAGE_IS_SWAPPED`` un-set, ``required_mask`` + is set to ``PAGE_IS_FILE``, ``exclude_mask`` is set to + ``PAGE_IS_SWAPPED`` and ``return_mask`` is set to ``PAGE_IS_WRITTEN``. + The output buffer in ``vec`` and length must be specified in ``vec_len``. + 2. To find pages which have either ``PAGE_IS_FILE`` or ``PAGE_IS_SWAPPED`` + set, ``anyof_masks`` is set to ``PAGE_IS_FILE | PAGE_IS_SWAPPED``. + 3. To find written pages and engage write protect, ``PAGE_IS_WRITTEN`` is + specified in ``required_mask`` and ``return_mask``. In addition to + specifying the output buffer in ``vec`` and length in ``vec_len``, the + ``PAGEMAP_WP_ENGAGE`` is specified in ``flags`` to perform write protect + on the range as well. + +The ``PAGE_IS_WRITTEN`` flag can be considered as the better and correct +alternative of soft-dirty flag. It doesn't get affected by household chores (VMA +merging) of the kernel and hence the user can find the true soft-dirty pages +only. This IOCTL adds the atomic way to find which pages have been written and +write protect those pages again. This kind of operation is needed to efficiently +find out which pages have changed in the memory. + +To get information about which pages have been written-to or optionally write +protect the pages, following must be performed first in order: + 1. The userfaultfd file descriptor is created with ``userfaultfd`` syscall. + 2. The ``UFFD_FEATURE_WP_UNPOPULATED`` and ``UFFD_FEATURE_WP_ASYNC`` features + are set by ``UFFDIO_API`` IOCTL. + 3. The memory range is registered with ``UFFDIO_REGISTER_MODE_WP`` mode + through ``UFFDIO_REGISTER`` IOCTL. + 4. Then the any part of the registered memory or the whole memory region must + be write protected using the ``UFFDIO_WRITEPROTECT`` IOCTL. + 5. Now the ``PAGEMAP_SCAN`` IOCTL can be used to either just find pages which + have been written-to or optionally write protect the pages as well.
Add pagemap ioctl tests. Add several different types of tests to judge the correction of the interface.
Signed-off-by: Muhammad Usama Anjum usama.anjum@collabora.com --- Changes in v12: - Updates and add more memory type tests
Changes in v11: - Rebase on top of next-20230216 and update tests
Chages in v7: - Add and update all test cases
Changes in v6: - Rename variables
Changes in v4: - Updated all the tests to conform to new IOCTL
Changes in v3: - Add another test to do sanity of flags
Changes in v2: - Update the tests to use the ioctl interface instead of syscall --- tools/testing/selftests/mm/.gitignore | 1 + tools/testing/selftests/mm/Makefile | 4 +- tools/testing/selftests/mm/config | 1 + tools/testing/selftests/mm/pagemap_ioctl.c | 1301 ++++++++++++++++++++ tools/testing/selftests/mm/run_vmtests.sh | 4 + 5 files changed, 1310 insertions(+), 1 deletion(-) create mode 100644 tools/testing/selftests/mm/pagemap_ioctl.c mode change 100644 => 100755 tools/testing/selftests/mm/run_vmtests.sh
diff --git a/tools/testing/selftests/mm/.gitignore b/tools/testing/selftests/mm/.gitignore index 1f8c36a9fa10..9e7e0ae26582 100644 --- a/tools/testing/selftests/mm/.gitignore +++ b/tools/testing/selftests/mm/.gitignore @@ -17,6 +17,7 @@ mremap_dontunmap mremap_test on-fault-limit transhuge-stress +pagemap_ioctl protection_keys protection_keys_32 protection_keys_64 diff --git a/tools/testing/selftests/mm/Makefile b/tools/testing/selftests/mm/Makefile index 6f7279202788..d4000e90a6ee 100644 --- a/tools/testing/selftests/mm/Makefile +++ b/tools/testing/selftests/mm/Makefile @@ -30,7 +30,7 @@ MACHINE ?= $(shell echo $(uname_M) | sed -e 's/aarch64.*/arm64/' -e 's/ppc64.*/p MAKEFLAGS += --no-builtin-rules
CFLAGS = -Wall -I $(top_srcdir) -I $(top_srcdir)/tools/include/uapi $(EXTRA_CFLAGS) $(KHDR_INCLUDES) -LDLIBS = -lrt -lpthread +LDLIBS = -lrt -lpthread -lm TEST_GEN_FILES = cow TEST_GEN_FILES += compaction_test TEST_GEN_FILES += gup_test @@ -56,6 +56,7 @@ TEST_GEN_FILES += on-fault-limit TEST_GEN_FILES += thuge-gen TEST_GEN_FILES += transhuge-stress TEST_GEN_FILES += userfaultfd +TEST_GEN_PROGS += pagemap_ioctl TEST_GEN_PROGS += soft-dirty TEST_GEN_PROGS += split_huge_page_test TEST_GEN_FILES += ksm_tests @@ -108,6 +109,7 @@ $(OUTPUT)/cow: vm_util.c $(OUTPUT)/khugepaged: vm_util.c $(OUTPUT)/ksm_functional_tests: vm_util.c $(OUTPUT)/madv_populate: vm_util.c +$(OUTPUT)/pagemap_ioctl: vm_util.c $(OUTPUT)/soft-dirty: vm_util.c $(OUTPUT)/split_huge_page_test: vm_util.c $(OUTPUT)/userfaultfd: vm_util.c diff --git a/tools/testing/selftests/mm/config b/tools/testing/selftests/mm/config index be087c4bc396..4309916f629e 100644 --- a/tools/testing/selftests/mm/config +++ b/tools/testing/selftests/mm/config @@ -1,5 +1,6 @@ CONFIG_SYSVIPC=y CONFIG_USERFAULTFD=y +CONFIG_PTE_MARKER_UFFD_WP=y CONFIG_TEST_VMALLOC=m CONFIG_DEVICE_PRIVATE=y CONFIG_TEST_HMM=m diff --git a/tools/testing/selftests/mm/pagemap_ioctl.c b/tools/testing/selftests/mm/pagemap_ioctl.c new file mode 100644 index 000000000000..abfd3b03457a --- /dev/null +++ b/tools/testing/selftests/mm/pagemap_ioctl.c @@ -0,0 +1,1301 @@ +// SPDX-License-Identifier: GPL-2.0 +#define _GNU_SOURCE +#include <stdio.h> +#include <fcntl.h> +#include <string.h> +#include <sys/mman.h> +#include <errno.h> +#include <malloc.h> +#include "vm_util.h" +#include "../kselftest.h" +#include <linux/types.h> +#include <linux/memfd.h> +#include <linux/userfaultfd.h> +#include <linux/fs.h> +#include <sys/ioctl.h> +#include <sys/stat.h> +#include <math.h> +#include <asm/unistd.h> +#include <pthread.h> +#include <sys/resource.h> +#include <assert.h> +#include <sys/ipc.h> +#include <sys/shm.h> + +#define PAGEMAP_BITS_ALL (PAGE_IS_WRITTEN | PAGE_IS_FILE | \ + PAGE_IS_PRESENT | PAGE_IS_SWAPPED) +#define PAGEMAP_NON_WRITTEN_BITS (PAGE_IS_FILE | PAGE_IS_PRESENT | \ + PAGE_IS_SWAPPED) + +#define TEST_ITERATIONS 10 +#define PAGEMAP "/proc/self/pagemap" +int pagemap_fd; +int uffd; +int page_size; +int hpage_size; + +static long pagemap_ioctl(void *start, int len, void *vec, int vec_len, int flag, + int max_pages, long required_mask, long anyof_mask, long excluded_mask, + long return_mask) +{ + struct pm_scan_arg arg; + + arg.start = (uintptr_t)start; + arg.len = len; + arg.vec = (uintptr_t)vec; + arg.vec_len = vec_len; + arg.flags = flag; + arg.size = sizeof(struct pm_scan_arg); + arg.max_pages = max_pages; + arg.required_mask = required_mask; + arg.anyof_mask = anyof_mask; + arg.excluded_mask = excluded_mask; + arg.return_mask = return_mask; + + return ioctl(pagemap_fd, PAGEMAP_SCAN, &arg); +} + +int init_uffd(void) +{ + struct uffdio_api uffdio_api; + + uffd = syscall(__NR_userfaultfd, O_CLOEXEC | O_NONBLOCK); + if (uffd == -1) + ksft_exit_fail_msg("uffd syscall failed\n"); + + uffdio_api.api = UFFD_API; + uffdio_api.features = UFFD_FEATURE_WP_UNPOPULATED | UFFD_FEATURE_WP_ASYNC | + UFFD_FEATURE_WP_HUGETLBFS_SHMEM; + if (ioctl(uffd, UFFDIO_API, &uffdio_api)) + ksft_exit_fail_msg("UFFDIO_API\n"); + + if (!(uffdio_api.api & UFFDIO_REGISTER_MODE_WP) || + !(uffdio_api.features & UFFD_FEATURE_WP_UNPOPULATED) || + !(uffdio_api.features & UFFD_FEATURE_WP_ASYNC) || + !(uffdio_api.features & UFFD_FEATURE_WP_HUGETLBFS_SHMEM)) + ksft_exit_fail_msg("UFFDIO_API error %llu\n", uffdio_api.api); + + return 0; +} + +int wp_init(void *lpBaseAddress, int dwRegionSize) +{ + struct uffdio_register uffdio_register; + struct uffdio_writeprotect wp; + + uffdio_register.range.start = (unsigned long)lpBaseAddress; + uffdio_register.range.len = dwRegionSize; + uffdio_register.mode = UFFDIO_REGISTER_MODE_WP; + if (ioctl(uffd, UFFDIO_REGISTER, &uffdio_register)) + ksft_exit_fail_msg("ioctl(UFFDIO_REGISTER) %d %s\n", errno, strerror(errno)); + + if (!(uffdio_register.ioctls & UFFDIO_WRITEPROTECT)) + ksft_exit_fail_msg("ioctl set is incorrect\n"); + + wp.range.start = (unsigned long)lpBaseAddress; + wp.range.len = dwRegionSize; + wp.mode = UFFDIO_WRITEPROTECT_MODE_WP; + + if (ioctl(uffd, UFFDIO_WRITEPROTECT, &wp)) + ksft_exit_fail_msg("ioctl(UFFDIO_WRITEPROTECT)\n"); + + return 0; +} + +int wp_free(void *lpBaseAddress, int dwRegionSize) +{ + struct uffdio_register uffdio_register; + + uffdio_register.range.start = (unsigned long)lpBaseAddress; + uffdio_register.range.len = dwRegionSize; + uffdio_register.mode = UFFDIO_REGISTER_MODE_WP; + if (ioctl(uffd, UFFDIO_UNREGISTER, &uffdio_register.range)) + ksft_exit_fail_msg("ioctl unregister failure\n"); + return 0; +} + +int wp_addr_range(void *lpBaseAddress, int dwRegionSize) +{ + struct uffdio_writeprotect wp; + + wp.range.start = (unsigned long)lpBaseAddress; + wp.range.len = dwRegionSize; + wp.mode = UFFDIO_WRITEPROTECT_MODE_WP; + + if (ioctl(uffd, UFFDIO_WRITEPROTECT, &wp)) + ksft_exit_fail_msg("ioctl(UFFDIO_WRITEPROTECT)\n"); + + return 0; +} + +void *gethugetlb_mem(int size, int *shmid) +{ + char *mem; + + if (shmid) { + *shmid = shmget(2, size, SHM_HUGETLB | IPC_CREAT | SHM_R | SHM_W); + if (*shmid < 0) + ksft_exit_fail_msg("shmget error\n"); + + mem = shmat(*shmid, 0, 0); + if (mem == (char *)-1) { + shmctl(*shmid, IPC_RMID, NULL); + ksft_exit_fail_msg("Shared memory attach failure\n"); + } + } else { + mem = mmap(NULL, size, PROT_READ | PROT_WRITE, + MAP_ANONYMOUS | MAP_HUGETLB | MAP_PRIVATE, -1, 0); + if (mem == MAP_FAILED) + ksft_exit_fail_msg("mmap of hugetlbfs file failed\n"); + } + + return mem; +} + +int userfaultfd_tests(void) +{ + int mem_size, vec_size, written, num_pages = 16; + char *mem, *vec; + + mem_size = num_pages * page_size; + mem = mmap(NULL, mem_size, PROT_NONE, MAP_PRIVATE | MAP_ANON, -1, 0); + if (mem == MAP_FAILED) + ksft_exit_fail_msg("error nomem\n"); + + wp_init(mem, mem_size); + + /* Change protection of pages differently */ + mprotect(mem, mem_size/8, PROT_READ|PROT_WRITE); + mprotect(mem + 1 * mem_size/8, mem_size/8, PROT_READ); + mprotect(mem + 2 * mem_size/8, mem_size/8, PROT_READ|PROT_WRITE); + mprotect(mem + 3 * mem_size/8, mem_size/8, PROT_READ); + mprotect(mem + 4 * mem_size/8, mem_size/8, PROT_READ|PROT_WRITE); + mprotect(mem + 5 * mem_size/8, mem_size/8, PROT_NONE); + mprotect(mem + 6 * mem_size/8, mem_size/8, PROT_READ|PROT_WRITE); + mprotect(mem + 7 * mem_size/8, mem_size/8, PROT_READ); + + wp_addr_range(mem + (mem_size/16), mem_size - 2 * (mem_size/8)); + wp_addr_range(mem, mem_size); + + vec_size = mem_size/page_size; + vec = malloc(sizeof(struct page_region) * vec_size); + + written = pagemap_ioctl(mem, mem_size, vec, 1, PM_SCAN_OP_GET | PM_SCAN_OP_WP, + vec_size - 2, PAGE_IS_WRITTEN, 0, 0, PAGE_IS_WRITTEN); + if (written < 0) + ksft_exit_fail_msg("error %d %d %s\n", written, errno, strerror(errno)); + + ksft_test_result(written == 0, "%s all new pages must not be written (dirty)\n", __func__); + + wp_free(mem, mem_size); + munmap(mem, mem_size); + free(vec); + return 0; +} + +int sanity_tests_sd(void) +{ + char *mem, *m[2]; + int mem_size, vec_size, ret, ret2, ret3, i, num_pages = 10; + struct page_region *vec, *vec2; + + vec_size = 100; + mem_size = num_pages * page_size; + + vec = malloc(sizeof(struct page_region) * vec_size); + if (!vec) + ksft_exit_fail_msg("error nomem\n"); + + vec2 = malloc(sizeof(struct page_region) * vec_size); + if (!vec2) + ksft_exit_fail_msg("error nomem\n"); + + mem = mmap(NULL, mem_size, PROT_READ | PROT_WRITE, MAP_PRIVATE | MAP_ANON, -1, 0); + if (mem == MAP_FAILED) + ksft_exit_fail_msg("error nomem\n"); + + wp_init(mem, mem_size); + wp_addr_range(mem, mem_size); + + /* 1. wrong operation */ + ksft_test_result(pagemap_ioctl(mem, 0, vec, vec_size, PM_SCAN_OP_GET, + 0, PAGEMAP_BITS_ALL, 0, 0, PAGEMAP_BITS_ALL) < 0, + "%s memory size must be valid\n", __func__); + + ksft_test_result(pagemap_ioctl(mem, mem_size, NULL, vec_size, PM_SCAN_OP_GET, + 0, PAGEMAP_BITS_ALL, 0, 0, PAGEMAP_BITS_ALL) < 0, + "%s output buffer must be specified\n", __func__); + + ksft_test_result(pagemap_ioctl(mem, mem_size, vec, 0, PM_SCAN_OP_GET, + 0, PAGEMAP_BITS_ALL, 0, 0, PAGEMAP_BITS_ALL) < 0, + "%s output buffer size must be valid\n", __func__); + + ksft_test_result(pagemap_ioctl(mem, mem_size, vec, vec_size, -1, + 0, PAGE_IS_WRITTEN, 0, 0, PAGE_IS_WRITTEN) < 0, + "%s wrong flag specified\n", __func__); + + ksft_test_result(pagemap_ioctl(mem, mem_size, vec, vec_size, PM_SCAN_OP_WP, + 0, PAGE_IS_WRITTEN, 0, 0, PAGE_IS_WRITTEN) < 0, + "%s PM_SCAN_OP_WP cannot be used without PM_SCAN_OP_GET\n", __func__); + + ksft_test_result(pagemap_ioctl(mem, mem_size, vec, vec_size, + PM_SCAN_OP_GET | PM_SCAN_OP_WP | 0xFF, + 0, PAGE_IS_WRITTEN, 0, 0, PAGE_IS_WRITTEN) < 0, + "%s flag has extra bits specified\n", __func__); + + ksft_test_result(pagemap_ioctl(mem, mem_size, vec, vec_size, PM_SCAN_OP_GET, + 0, 0, 0, 0, PAGE_IS_WRITTEN) < 0, + "%s no selection mask is specified\n", __func__); + + ksft_test_result(pagemap_ioctl(mem, mem_size, vec, vec_size, PM_SCAN_OP_GET, + 0, PAGE_IS_WRITTEN, PAGE_IS_WRITTEN, 0, 0) < 0, + "%s no return mask is specified\n", __func__); + + ksft_test_result(pagemap_ioctl(mem, mem_size, vec, vec_size, PM_SCAN_OP_GET, + 0, PAGE_IS_WRITTEN, 0, 0, 0x1000) < 0, + "%s wrong return mask specified\n", __func__); + + ksft_test_result(pagemap_ioctl(mem, mem_size, vec, vec_size, PM_SCAN_OP_GET | PM_SCAN_OP_WP, + 0, 0xFFF, PAGE_IS_WRITTEN, 0, PAGE_IS_WRITTEN) < 0, + "%s mixture of correct and wrong flag\n", __func__); + + ksft_test_result(pagemap_ioctl(mem, mem_size, vec, vec_size, PM_SCAN_OP_GET | PM_SCAN_OP_WP, + 0, 0, 0, PAGEMAP_BITS_ALL, PAGE_IS_WRITTEN) < 0, + "%s PAGEMAP_BITS_ALL cannot be specified with PM_SCAN_OP_WP\n", __func__); + + /* 2. Clear area with larger vec size */ + ret = pagemap_ioctl(mem, mem_size, vec, vec_size, PM_SCAN_OP_GET | PM_SCAN_OP_WP, 0, + PAGE_IS_WRITTEN, 0, 0, PAGE_IS_WRITTEN); + ksft_test_result(ret >= 0, "%s Clear area with larger vec size\n", __func__); + + /* 3. Repeated pattern of written and non-written pages */ + for (i = 0; i < mem_size; i += 2 * page_size) + mem[i]++; + + ret = pagemap_ioctl(mem, mem_size, vec, vec_size, PM_SCAN_OP_GET, 0, PAGE_IS_WRITTEN, 0, + 0, PAGE_IS_WRITTEN); + if (ret < 0) + ksft_exit_fail_msg("error %d %d %s\n", ret, errno, strerror(errno)); + + ksft_test_result(ret == mem_size/(page_size * 2), + "%s Repeated pattern of written and non-written pages\n", __func__); + + /* 4. Repeated pattern of written and non-written pages in parts */ + ret = pagemap_ioctl(mem, mem_size, vec, vec_size, PM_SCAN_OP_GET | PM_SCAN_OP_WP, + num_pages/2 - 2, PAGE_IS_WRITTEN, 0, 0, PAGE_IS_WRITTEN); + if (ret < 0) + ksft_exit_fail_msg("error %d %d %s\n", ret, errno, strerror(errno)); + + ret2 = pagemap_ioctl(mem, mem_size, vec, 2, PM_SCAN_OP_GET, 0, PAGE_IS_WRITTEN, 0, 0, + PAGE_IS_WRITTEN); + if (ret2 < 0) + ksft_exit_fail_msg("error %d %d %s\n", ret2, errno, strerror(errno)); + + ret3 = pagemap_ioctl(mem, mem_size, vec, vec_size, PM_SCAN_OP_GET | PM_SCAN_OP_WP, + 0, PAGE_IS_WRITTEN, 0, 0, PAGE_IS_WRITTEN); + if (ret3 < 0) + ksft_exit_fail_msg("error %d %d %s\n", ret3, errno, strerror(errno)); + + ksft_test_result((ret + ret3) == num_pages/2 && ret2 == 2, + "%s Repeated pattern of written and non-written pages in parts\n", + __func__); + + /* 5. only get 2 dirty pages and clear them as well */ + vec_size = mem_size/page_size; + memset(mem, -1, mem_size); + + /* get and clear second and third pages */ + ret = pagemap_ioctl(mem + page_size, 2 * page_size, vec, 1, PM_SCAN_OP_GET | PM_SCAN_OP_WP, + 2, PAGE_IS_WRITTEN, 0, 0, PAGE_IS_WRITTEN); + if (ret < 0) + ksft_exit_fail_msg("error %d %d %s\n", ret, errno, strerror(errno)); + + ret2 = pagemap_ioctl(mem, mem_size, vec2, vec_size, PM_SCAN_OP_GET, 0, + PAGE_IS_WRITTEN, 0, 0, PAGE_IS_WRITTEN); + if (ret2 < 0) + ksft_exit_fail_msg("error %d %d %s\n", ret2, errno, strerror(errno)); + + ksft_test_result(ret == 1 && vec[0].len == 2 && + vec[0].start == (uintptr_t)(mem + page_size) && + ret2 == 2 && vec2[0].len == 1 && vec2[0].start == (uintptr_t)mem && + vec2[1].len == vec_size - 3 && + vec2[1].start == (uintptr_t)(mem + 3 * page_size), + "%s only get 2 written pages and clear them as well\n", __func__); + + wp_free(mem, mem_size); + munmap(mem, mem_size); + + /* 6. Two regions */ + m[0] = mmap(NULL, mem_size, PROT_READ | PROT_WRITE, MAP_PRIVATE | MAP_ANON, -1, 0); + if (m[0] == MAP_FAILED) + ksft_exit_fail_msg("error nomem\n"); + m[1] = mmap(NULL, mem_size, PROT_READ | PROT_WRITE, MAP_PRIVATE | MAP_ANON, -1, 0); + if (m[1] == MAP_FAILED) + ksft_exit_fail_msg("error nomem\n"); + + wp_init(m[0], mem_size); + wp_init(m[1], mem_size); + wp_addr_range(m[0], mem_size); + wp_addr_range(m[1], mem_size); + + memset(m[0], 'a', mem_size); + memset(m[1], 'b', mem_size); + + wp_addr_range(m[0], mem_size); + + ret = pagemap_ioctl(m[1], mem_size, vec, 1, PM_SCAN_OP_GET, 0, PAGE_IS_WRITTEN, 0, 0, + PAGE_IS_WRITTEN); + if (ret < 0) + ksft_exit_fail_msg("error %d %d %s\n", ret, errno, strerror(errno)); + + ksft_test_result(ret == 1 && vec[0].len == mem_size/page_size, + "%s Two regions\n", __func__); + + wp_free(m[0], mem_size); + wp_free(m[1], mem_size); + munmap(m[0], mem_size); + munmap(m[1], mem_size); + + free(vec); + free(vec2); + return 0; +} + +int base_tests(char *prefix, char *mem, int mem_size, int skip) +{ + int vec_size, written; + struct page_region *vec, *vec2; + + if (skip) { + ksft_test_result_skip("%s all new pages must not be written (dirty)\n", prefix); + ksft_test_result_skip("%s all pages must be written (dirty)\n", prefix); + ksft_test_result_skip("%s all pages dirty other than first and the last one\n", + prefix); + ksft_test_result_skip("%s only middle page dirty\n", prefix); + ksft_test_result_skip("%s only two middle pages dirty\n", prefix); + return 0; + } + + vec_size = mem_size/page_size; + vec = malloc(sizeof(struct page_region) * vec_size); + vec2 = malloc(sizeof(struct page_region) * vec_size); + + /* 1. all new pages must be not be written (dirty) */ + written = pagemap_ioctl(mem, mem_size, vec, 1, PM_SCAN_OP_GET | PM_SCAN_OP_WP, vec_size - 2, + PAGE_IS_WRITTEN, 0, 0, PAGE_IS_WRITTEN); + if (written < 0) + ksft_exit_fail_msg("error %d %d %s\n", written, errno, strerror(errno)); + + ksft_test_result(written == 0, "%s all new pages must not be written (dirty)\n", prefix); + + /* 2. all pages must be written */ + memset(mem, -1, mem_size); + + written = pagemap_ioctl(mem, mem_size, vec, 1, PM_SCAN_OP_GET, 0, PAGE_IS_WRITTEN, 0, 0, + PAGE_IS_WRITTEN); + if (written < 0) + ksft_exit_fail_msg("error %d %d %s\n", written, errno, strerror(errno)); + + ksft_test_result(written == 1 && vec[0].len == mem_size/page_size, + "%s all pages must be written (dirty)\n", prefix); + + /* 3. all pages dirty other than first and the last one */ + wp_addr_range(mem, mem_size); + memset(mem + page_size, 0, mem_size - (2 * page_size)); + + written = pagemap_ioctl(mem, mem_size, vec, 1, PM_SCAN_OP_GET, 0, PAGE_IS_WRITTEN, 0, 0, + PAGE_IS_WRITTEN); + if (written < 0) + ksft_exit_fail_msg("error %d %d %s\n", written, errno, strerror(errno)); + + ksft_test_result(written == 1 && vec[0].len >= vec_size - 2 && vec[0].len <= vec_size, + "%s all pages dirty other than first and the last one\n", prefix); + + /* 4. only middle page dirty */ + wp_addr_range(mem, mem_size); + mem[vec_size/2 * page_size]++; + + written = pagemap_ioctl(mem, mem_size, vec, vec_size, PM_SCAN_OP_GET, 0, PAGE_IS_WRITTEN, + 0, 0, PAGE_IS_WRITTEN); + if (written < 0) + ksft_exit_fail_msg("error %d %d %s\n", written, errno, strerror(errno)); + + ksft_test_result(written == 1 && vec[0].len >= 1, + "%s only middle page dirty\n", prefix); + + /* 5. only two middle pages dirty and walk over only middle pages */ + wp_addr_range(mem, mem_size); + mem[vec_size/2 * page_size]++; + mem[(vec_size/2 + 1) * page_size]++; + + written = pagemap_ioctl(&mem[vec_size/2 * page_size], 2 * page_size, vec, 1, PM_SCAN_OP_GET, + 0, PAGE_IS_WRITTEN, 0, 0, PAGE_IS_WRITTEN); + if (written < 0) + ksft_exit_fail_msg("error %d %d %s\n", written, errno, strerror(errno)); + + ksft_test_result(written == 1 && vec[0].start == (uintptr_t)(&mem[vec_size/2 * page_size]) + && vec[0].len == 2, + "%s only two middle pages dirty\n", prefix); + + free(vec); + free(vec2); + return 0; +} + +void *gethugepage(int map_size) +{ + int ret; + char *map; + + map = memalign(hpage_size, map_size); + if (!map) + ksft_exit_fail_msg("memalign failed %d %s\n", errno, strerror(errno)); + + ret = madvise(map, map_size, MADV_HUGEPAGE); + if (ret) + ksft_exit_fail_msg("madvise failed %d %d %s\n", ret, errno, strerror(errno)); + + return map; +} + +int hpage_unit_tests(void) +{ + char *map; + int ret, ret2; + size_t num_pages = 10; + int map_size = hpage_size * num_pages; + int vec_size = map_size/page_size; + struct page_region *vec, *vec2; + + vec = malloc(sizeof(struct page_region) * vec_size); + vec2 = malloc(sizeof(struct page_region) * vec_size); + if (!vec || !vec2) + ksft_exit_fail_msg("malloc failed\n"); + + map = gethugepage(map_size); + if (map) { + wp_init(map, map_size); + wp_addr_range(map, map_size); + + /* 1. all new huge page must not be written (dirty) */ + ret = pagemap_ioctl(map, map_size, vec, vec_size, PM_SCAN_OP_GET | PM_SCAN_OP_WP, 0, + PAGE_IS_WRITTEN, 0, 0, PAGE_IS_WRITTEN); + if (ret < 0) + ksft_exit_fail_msg("error %d %d %s\n", ret, errno, strerror(errno)); + + ksft_test_result(ret == 0, "%s all new huge page must not be written (dirty)\n", + __func__); + + /* 2. all the huge page must not be written */ + ret = pagemap_ioctl(map, map_size, vec, vec_size, PM_SCAN_OP_GET, 0, + PAGE_IS_WRITTEN, 0, 0, PAGE_IS_WRITTEN); + if (ret < 0) + ksft_exit_fail_msg("error %d %d %s\n", ret, errno, strerror(errno)); + + ksft_test_result(ret == 0, "%s all the huge page must not be written\n", __func__); + + /* 3. all the huge page must be written and clear dirty as well */ + memset(map, -1, map_size); + ret = pagemap_ioctl(map, map_size, vec, vec_size, PM_SCAN_OP_GET | PM_SCAN_OP_WP, + 0, PAGE_IS_WRITTEN, 0, 0, PAGE_IS_WRITTEN); + if (ret < 0) + ksft_exit_fail_msg("error %d %d %s\n", ret, errno, strerror(errno)); + + ksft_test_result(ret == 1 && vec[0].start == (uintptr_t)map && + vec[0].len == vec_size && vec[0].bitmap == PAGE_IS_WRITTEN, + "%s all the huge page must be written and clear\n", __func__); + + /* 4. only middle page written */ + wp_free(map, map_size); + free(map); + map = gethugepage(map_size); + wp_init(map, map_size); + wp_addr_range(map, map_size); + map[vec_size/2 * page_size]++; + + ret = pagemap_ioctl(map, map_size, vec, vec_size, PM_SCAN_OP_GET, 0, + PAGE_IS_WRITTEN, 0, 0, PAGE_IS_WRITTEN); + if (ret < 0) + ksft_exit_fail_msg("error %d %d %s\n", ret, errno, strerror(errno)); + + ksft_test_result(ret == 1 && vec[0].len > 0, + "%s only middle page written\n", __func__); + + wp_free(map, map_size); + free(map); + } else { + ksft_test_result_skip("all new huge page must be written\n"); + ksft_test_result_skip("all the huge page must not be written\n"); + ksft_test_result_skip("all the huge page must be written and clear\n"); + ksft_test_result_skip("only middle page written\n"); + } + + /* 5. clear first half of huge page */ + map = gethugepage(map_size); + if (map) { + wp_init(map, map_size); + wp_addr_range(map, map_size); + + memset(map, 0, map_size); + + wp_addr_range(map, map_size/2); + + ret = pagemap_ioctl(map, map_size, vec, vec_size, PM_SCAN_OP_GET, 0, + PAGE_IS_WRITTEN, 0, 0, PAGE_IS_WRITTEN); + if (ret < 0) + ksft_exit_fail_msg("error %d %d %s\n", ret, errno, strerror(errno)); + + ksft_test_result(ret == 1 && vec[0].len == vec_size/2 && + vec[0].start == (uintptr_t)(map + map_size/2), + "%s clear first half of huge page\n", __func__); + wp_free(map, map_size); + free(map); + } else { + ksft_test_result_skip("clear first half of huge page\n"); + } + + /* 6. clear first half of huge page with limited buffer */ + map = gethugepage(map_size); + if (map) { + wp_init(map, map_size); + wp_addr_range(map, map_size); + + memset(map, 0, map_size); + + ret = pagemap_ioctl(map, map_size, vec, vec_size, PM_SCAN_OP_GET | PM_SCAN_OP_WP, + vec_size/2, PAGE_IS_WRITTEN, 0, 0, PAGE_IS_WRITTEN); + if (ret < 0) + ksft_exit_fail_msg("error %d %d %s\n", ret, errno, strerror(errno)); + + ret = pagemap_ioctl(map, map_size, vec, vec_size, PM_SCAN_OP_GET, 0, + PAGE_IS_WRITTEN, 0, 0, PAGE_IS_WRITTEN); + if (ret < 0) + ksft_exit_fail_msg("error %d %d %s\n", ret, errno, strerror(errno)); + + ksft_test_result(ret == 1 && vec[0].len == vec_size/2 && + vec[0].start == (uintptr_t)(map + map_size/2), + "%s clear first half of huge page with limited buffer\n", + __func__); + wp_free(map, map_size); + free(map); + } else { + ksft_test_result_skip("clear first half of huge page with limited buffer\n"); + } + + /* 7. clear second half of huge page */ + map = gethugepage(map_size); + if (map) { + wp_init(map, map_size); + wp_addr_range(map, map_size); + + memset(map, -1, map_size); + + ret = pagemap_ioctl(map + map_size/2, map_size/2, vec, vec_size, + PM_SCAN_OP_GET | PM_SCAN_OP_WP, vec_size/2, PAGE_IS_WRITTEN, 0, + 0, PAGE_IS_WRITTEN); + if (ret < 0) + ksft_exit_fail_msg("error %d %d %s\n", ret, errno, strerror(errno)); + + ret = pagemap_ioctl(map, map_size, vec, vec_size, PM_SCAN_OP_GET, 0, + PAGE_IS_WRITTEN, 0, 0, PAGE_IS_WRITTEN); + if (ret < 0) + ksft_exit_fail_msg("error %d %d %s\n", ret, errno, strerror(errno)); + + ksft_test_result(ret == 1 && vec[0].len == vec_size/2, + "%s clear second half huge page\n", __func__); + wp_free(map, map_size); + free(map); + } else { + ksft_test_result_skip("clear second half huge page\n"); + } + + /* 8. get half huge page */ + map = gethugepage(map_size); + if (map) { + wp_init(map, map_size); + wp_addr_range(map, map_size); + + memset(map, -1, map_size); + usleep(100); + + ret = pagemap_ioctl(map, map_size, vec, 1, PM_SCAN_OP_GET | PM_SCAN_OP_WP, + hpage_size/(2*page_size), PAGE_IS_WRITTEN, 0, 0, + PAGE_IS_WRITTEN); + if (ret < 0) + ksft_exit_fail_msg("error %d %d %s\n", ret, errno, strerror(errno)); + + ksft_test_result(ret == 1 && vec[0].len == hpage_size/(2*page_size), + "%s get half huge page\n", __func__); + + ret2 = pagemap_ioctl(map, map_size, vec, vec_size, PM_SCAN_OP_GET, 0, + PAGE_IS_WRITTEN, 0, 0, PAGE_IS_WRITTEN); + if (ret2 < 0) + ksft_exit_fail_msg("error %d %d %s\n", ret2, errno, strerror(errno)); + + ksft_test_result(ret2 == 1 && vec[0].len == (map_size - hpage_size/2)/page_size, + "%s get half huge page\n", __func__); + + wp_free(map, map_size); + free(map); + } else { + ksft_test_result_skip("get half huge page\n"); + } + + free(vec); + free(vec2); + return 0; +} + +int unmapped_region_tests(void) +{ + void *start = (void *)0x10000000; + int written, len = 0x00040000; + int vec_size = len / page_size; + struct page_region *vec = malloc(sizeof(struct page_region) * vec_size); + + /* 1. Get written pages */ + written = pagemap_ioctl(start, len, vec, vec_size, PM_SCAN_OP_GET, 0, + PAGEMAP_NON_WRITTEN_BITS, 0, 0, PAGEMAP_NON_WRITTEN_BITS); + if (written < 0) + ksft_exit_fail_msg("error %d %d %s\n", written, errno, strerror(errno)); + + ksft_test_result(written >= 0, "%s Get status of pages\n", __func__); + + free(vec); + return 0; +} + +static void test_simple(void) +{ + int i; + char *map; + struct page_region vec; + + map = aligned_alloc(page_size, page_size); + if (!map) + ksft_exit_fail_msg("aligned_alloc failed\n"); + + wp_init(map, page_size); + wp_addr_range(map, page_size); + + for (i = 0 ; i < TEST_ITERATIONS; i++) { + if (pagemap_ioctl(map, page_size, &vec, 1, PM_SCAN_OP_GET, 0, + PAGE_IS_WRITTEN, 0, 0, PAGE_IS_WRITTEN) == 1) { + ksft_print_msg("written bit was 1, but should be 0 (i=%d)\n", i); + break; + } + + wp_addr_range(map, page_size); + /* Write something to the page to get the written bit enabled on the page */ + map[0]++; + + if (pagemap_ioctl(map, page_size, &vec, 1, PM_SCAN_OP_GET, 0, + PAGE_IS_WRITTEN, 0, 0, PAGE_IS_WRITTEN) == 0) { + ksft_print_msg("written bit was 0, but should be 1 (i=%d)\n", i); + break; + } + + wp_addr_range(map, page_size); + } + wp_free(map, page_size); + free(map); + + ksft_test_result(i == TEST_ITERATIONS, "Test %s\n", __func__); +} + +int sanity_tests(void) +{ + int mem_size, vec_size, ret, fd, i, buf_size; + struct page_region *vec; + char *mem, *fmem; + struct stat sbuf; + + /* 1. wrong operation */ + mem_size = 10 * page_size; + vec_size = mem_size / page_size; + + vec = malloc(sizeof(struct page_region) * vec_size); + mem = mmap(NULL, mem_size, PROT_READ | PROT_WRITE, MAP_PRIVATE | MAP_ANON, -1, 0); + if (mem == MAP_FAILED || vec == MAP_FAILED) + ksft_exit_fail_msg("error nomem\n"); + + wp_init(mem, mem_size); + wp_addr_range(mem, mem_size); + + ksft_test_result(pagemap_ioctl(mem, mem_size, vec, vec_size, + PM_SCAN_OP_GET | PM_SCAN_OP_WP, 0, PAGEMAP_BITS_ALL, 0, 0, + PAGEMAP_BITS_ALL) < 0, + "%s clear op can only be specified with PAGE_IS_WRITTEN\n", __func__); + ksft_test_result(pagemap_ioctl(mem, mem_size, vec, vec_size, PM_SCAN_OP_GET, 0, + PAGEMAP_BITS_ALL, 0, 0, PAGEMAP_BITS_ALL) >= 0, + "%s required_mask specified\n", __func__); + ksft_test_result(pagemap_ioctl(mem, mem_size, vec, vec_size, PM_SCAN_OP_GET, 0, + 0, PAGEMAP_BITS_ALL, 0, PAGEMAP_BITS_ALL) >= 0, + "%s anyof_mask specified\n", __func__); + ksft_test_result(pagemap_ioctl(mem, mem_size, vec, vec_size, PM_SCAN_OP_GET, 0, + 0, 0, PAGEMAP_BITS_ALL, PAGEMAP_BITS_ALL) >= 0, + "%s excluded_mask specified\n", __func__); + ksft_test_result(pagemap_ioctl(mem, mem_size, vec, vec_size, PM_SCAN_OP_GET, 0, + PAGEMAP_BITS_ALL, PAGEMAP_BITS_ALL, 0, + PAGEMAP_BITS_ALL) >= 0, + "%s required_mask and anyof_mask specified\n", __func__); + wp_free(mem, mem_size); + munmap(mem, mem_size); + + /* 2. Get sd and present pages with anyof_mask */ + mem = mmap(NULL, mem_size, PROT_READ | PROT_WRITE, MAP_PRIVATE | MAP_ANON, -1, 0); + if (mem == MAP_FAILED) + ksft_exit_fail_msg("error nomem\n"); + wp_init(mem, mem_size); + wp_addr_range(mem, mem_size); + + memset(mem, 0, mem_size); + + ret = pagemap_ioctl(mem, mem_size, vec, vec_size, PM_SCAN_OP_GET, 0, + 0, PAGEMAP_BITS_ALL, 0, PAGEMAP_BITS_ALL); + ksft_test_result(ret >= 0 && vec[0].start == (uintptr_t)mem && vec[0].len == vec_size && + vec[0].bitmap == (PAGE_IS_WRITTEN | PAGE_IS_PRESENT), + "%s Get sd and present pages with anyof_mask\n", __func__); + + /* 3. Get sd and present pages with required_mask */ + ret = pagemap_ioctl(mem, mem_size, vec, vec_size, PM_SCAN_OP_GET, 0, + PAGEMAP_BITS_ALL, 0, 0, PAGEMAP_BITS_ALL); + ksft_test_result(ret >= 0 && vec[0].start == (uintptr_t)mem && vec[0].len == vec_size && + vec[0].bitmap == (PAGE_IS_WRITTEN | PAGE_IS_PRESENT), + "%s Get all the pages with required_mask\n", __func__); + + /* 4. Get sd and present pages with required_mask and anyof_mask */ + ret = pagemap_ioctl(mem, mem_size, vec, vec_size, PM_SCAN_OP_GET, 0, + PAGE_IS_WRITTEN, PAGE_IS_PRESENT, 0, PAGEMAP_BITS_ALL); + ksft_test_result(ret >= 0 && vec[0].start == (uintptr_t)mem && vec[0].len == vec_size && + vec[0].bitmap == (PAGE_IS_WRITTEN | PAGE_IS_PRESENT), + "%s Get sd and present pages with required_mask and anyof_mask\n", + __func__); + + /* 5. Don't get sd pages */ + ret = pagemap_ioctl(mem, mem_size, vec, vec_size, PM_SCAN_OP_GET, 0, + 0, 0, PAGE_IS_WRITTEN, PAGEMAP_BITS_ALL); + ksft_test_result(ret == 0, "%s Don't get sd pages\n", __func__); + + /* 6. Don't get present pages */ + ret = pagemap_ioctl(mem, mem_size, vec, vec_size, PM_SCAN_OP_GET, 0, + 0, 0, PAGE_IS_PRESENT, PAGEMAP_BITS_ALL); + ksft_test_result(ret == 0, "%s Don't get present pages\n", __func__); + + wp_free(mem, mem_size); + munmap(mem, mem_size); + + /* 8. Find written present pages with return mask */ + mem = mmap(NULL, mem_size, PROT_READ | PROT_WRITE, MAP_PRIVATE | MAP_ANON, -1, 0); + if (mem == MAP_FAILED) + ksft_exit_fail_msg("error nomem\n"); + wp_init(mem, mem_size); + wp_addr_range(mem, mem_size); + + memset(mem, 0, mem_size); + + ret = pagemap_ioctl(mem, mem_size, vec, vec_size, PM_SCAN_OP_GET, 0, + 0, PAGEMAP_BITS_ALL, 0, PAGE_IS_WRITTEN); + ksft_test_result(ret >= 0 && vec[0].start == (uintptr_t)mem && vec[0].len == vec_size && + vec[0].bitmap == PAGE_IS_WRITTEN, + "%s Find written present pages with return mask\n", __func__); + wp_free(mem, mem_size); + munmap(mem, mem_size); + + /* 9. Memory mapped file */ + fd = open(__FILE__, O_RDONLY); + if (fd < 0) + ksft_exit_fail_msg("%s Memory mapped file\n"); + + ret = stat(__FILE__, &sbuf); + if (ret < 0) + ksft_exit_fail_msg("error %d %d %s\n", ret, errno, strerror(errno)); + + fmem = mmap(NULL, sbuf.st_size, PROT_READ, MAP_PRIVATE, fd, 0); + if (fmem == MAP_FAILED) + ksft_exit_fail_msg("error nomem %ld %s\n", errno, strerror(errno)); + + ret = pagemap_ioctl(fmem, sbuf.st_size, vec, vec_size, PM_SCAN_OP_GET, 0, + 0, PAGEMAP_NON_WRITTEN_BITS, 0, PAGEMAP_NON_WRITTEN_BITS); + + ksft_test_result(ret >= 0 && vec[0].start == (uintptr_t)fmem && + vec[0].len == ceilf((float)sbuf.st_size/page_size) && + vec[0].bitmap == PAGE_IS_FILE, + "%s Memory mapped file\n", __func__); + + munmap(fmem, sbuf.st_size); + close(fd); + + /* 10. Create and read/write to a memory mapped file*/ + buf_size = page_size * 10; + + fd = open(__FILE__".tmp2", O_RDWR | O_CREAT, 0777); + if (fd < 0) + ksft_exit_fail_msg("Create and read/write to a memory mapped file: %s\n", + strerror(errno)); + + for (i = 0; i < buf_size; i++) + if (write(fd, "c", 1) < 0) + ksft_exit_fail_msg("Create and read/write to a memory mapped file\n"); + + fmem = mmap(NULL, buf_size, PROT_READ | PROT_WRITE, MAP_PRIVATE, fd, 0); + if (fmem == MAP_FAILED) + ksft_exit_fail_msg("error nomem %ld %s\n", errno, strerror(errno)); + + wp_init(fmem, buf_size); + wp_addr_range(fmem, buf_size); + + for (i = 0; i < buf_size; i++) + fmem[i] = i; + + ret = pagemap_ioctl(fmem, buf_size, vec, vec_size, PM_SCAN_OP_GET, 0, + PAGE_IS_WRITTEN | PAGE_IS_FILE, PAGE_IS_PRESENT | PAGE_IS_SWAPPED, 0, + PAGEMAP_BITS_ALL); + + ksft_test_result(ret >= 0 && vec[0].start == (uintptr_t)fmem && + vec[0].len == (buf_size/page_size) && + (vec[0].bitmap | PAGE_IS_WRITTEN) && (vec[0].bitmap | PAGE_IS_FILE), + "%s Read/write to private memory mapped file\n", __func__); + + wp_free(fmem, buf_size); + munmap(fmem, buf_size); + close(fd); + + free(vec); + return 0; +} + +int mprotect_tests(void) +{ + int ret; + char *mem, *mem2; + struct page_region vec; + int pagemap_fd = open("/proc/self/pagemap", O_RDONLY); + + if (pagemap_fd < 0) { + fprintf(stderr, "open() failed\n"); + exit(1); + } + + /* 1. Map two pages */ + mem = mmap(0, 2 * page_size, PROT_READ|PROT_WRITE, MAP_PRIVATE | MAP_ANON, -1, 0); + if (mem == MAP_FAILED) + ksft_exit_fail_msg("error nomem\n"); + wp_init(mem, 2 * page_size); + wp_addr_range(mem, 2 * page_size); + + /* Populate both pages. */ + memset(mem, 1, 2 * page_size); + + ret = pagemap_ioctl(mem, 2 * page_size, &vec, 1, PM_SCAN_OP_GET, 0, PAGE_IS_WRITTEN, + 0, 0, PAGE_IS_WRITTEN); + if (ret < 0) + ksft_exit_fail_msg("error %d %d %s\n", ret, errno, strerror(errno)); + + ksft_test_result(ret == 1 && vec.len == 2, "%s Both pages written\n", __func__); + + /* 2. Start tracking */ + wp_addr_range(mem, 2 * page_size); + + ksft_test_result(pagemap_ioctl(mem, 2 * page_size, &vec, 1, PM_SCAN_OP_GET, 0, + PAGE_IS_WRITTEN, 0, 0, PAGE_IS_WRITTEN) == 0, + "%s Both pages are not written (dirty)\n", __func__); + + /* 3. Remap the second page */ + mem2 = mmap(mem + page_size, page_size, PROT_READ|PROT_WRITE, + MAP_PRIVATE|MAP_ANON|MAP_FIXED, -1, 0); + if (mem2 == MAP_FAILED) + ksft_exit_fail_msg("error nomem\n"); + wp_init(mem2, page_size); + wp_addr_range(mem2, page_size); + + /* Protect + unprotect. */ + mprotect(mem, page_size, PROT_NONE); + mprotect(mem, 2 * page_size, PROT_READ); + mprotect(mem, 2 * page_size, PROT_READ|PROT_WRITE); + + /* Modify both pages. */ + memset(mem, 2, 2 * page_size); + + /* Protect + unprotect. */ + mprotect(mem, page_size, PROT_NONE); + mprotect(mem, page_size, PROT_READ); + mprotect(mem, page_size, PROT_READ|PROT_WRITE); + + ret = pagemap_ioctl(mem, 2 * page_size, &vec, 1, PM_SCAN_OP_GET, 0, PAGE_IS_WRITTEN, + 0, 0, PAGE_IS_WRITTEN); + if (ret < 0) + ksft_exit_fail_msg("error %d %d %s\n", ret, errno, strerror(errno)); + + ksft_test_result(ret == 1 && vec.len == 2, + "%s Both pages written after remap and mprotect\n", __func__); + + /* 4. Clear and make the pages written */ + wp_addr_range(mem, 2 * page_size); + + memset(mem, 'A', 2 * page_size); + + ret = pagemap_ioctl(mem, 2 * page_size, &vec, 1, PM_SCAN_OP_GET, 0, PAGE_IS_WRITTEN, + 0, 0, PAGE_IS_WRITTEN); + if (ret < 0) + ksft_exit_fail_msg("error %d %d %s\n", ret, errno, strerror(errno)); + + ksft_test_result(ret == 1 && vec.len == 2, + "%s Clear and make the pages written\n", __func__); + + wp_free(mem, 2 * page_size); + munmap(mem, 2 * page_size); + return 0; +} + +/* transact test */ +static const unsigned int nthreads = 6, pages_per_thread = 32, access_per_thread = 8; +static pthread_barrier_t start_barrier, end_barrier; +static unsigned int extra_thread_faults; +static unsigned int iter_count = 1000; +static volatile int finish; + +static ssize_t get_dirty_pages_reset(char *mem, unsigned int count, + int reset, int page_size) +{ + struct pm_scan_arg arg = {0}; + struct page_region rgns[256]; + int i, j, cnt, ret; + + arg.size = sizeof(struct pm_scan_arg); + arg.start = (uintptr_t)mem; + arg.max_pages = count; + arg.len = count * page_size; + arg.vec = (uintptr_t)rgns; + arg.vec_len = sizeof(rgns) / sizeof(*rgns); + arg.flags = PM_SCAN_OP_GET; + if (reset) + arg.flags |= PM_SCAN_OP_WP; + arg.required_mask = PAGE_IS_WRITTEN; + arg.return_mask = PAGE_IS_WRITTEN; + + ret = ioctl(pagemap_fd, PAGEMAP_SCAN, &arg); + if (ret < 0) + ksft_exit_fail_msg("ioctl failed\n"); + + cnt = 0; + for (i = 0; i < ret; ++i) { + if (rgns[i].bitmap != PAGE_IS_WRITTEN) + ksft_exit_fail_msg("wrong bitmap\n"); + + for (j = 0; j < rgns[i].len; ++j) + cnt++; + } + + return cnt; +} + +void *thread_proc(void *mem) +{ + int *m = mem; + long curr_faults, faults; + struct rusage r; + unsigned int i; + int ret; + + if (getrusage(RUSAGE_THREAD, &r)) + ksft_exit_fail_msg("getrusage\n"); + + curr_faults = r.ru_minflt; + + while (!finish) { + ret = pthread_barrier_wait(&start_barrier); + if (ret && ret != PTHREAD_BARRIER_SERIAL_THREAD) + ksft_exit_fail_msg("pthread_barrier_wait\n"); + + for (i = 0; i < access_per_thread; ++i) + __atomic_add_fetch(m + i * (0x1000 / sizeof(*m)), 1, __ATOMIC_SEQ_CST); + + ret = pthread_barrier_wait(&end_barrier); + if (ret && ret != PTHREAD_BARRIER_SERIAL_THREAD) + ksft_exit_fail_msg("pthread_barrier_wait\n"); + + if (getrusage(RUSAGE_THREAD, &r)) + ksft_exit_fail_msg("getrusage\n"); + + faults = r.ru_minflt - curr_faults; + if (faults < access_per_thread) + ksft_exit_fail_msg("faults < access_per_thread"); + + __atomic_add_fetch(&extra_thread_faults, faults - access_per_thread, + __ATOMIC_SEQ_CST); + curr_faults = r.ru_minflt; + } + + return NULL; +} + +static void transact_test(int page_size) +{ + unsigned int i, count, extra_pages; + pthread_t th; + char *mem; + int ret, c; + + if (pthread_barrier_init(&start_barrier, NULL, nthreads + 1)) + ksft_exit_fail_msg("pthread_barrier_init\n"); + + if (pthread_barrier_init(&end_barrier, NULL, nthreads + 1)) + ksft_exit_fail_msg("pthread_barrier_init\n"); + + mem = mmap(NULL, 0x1000 * nthreads * pages_per_thread, PROT_READ | PROT_WRITE, + MAP_ANONYMOUS | MAP_PRIVATE, -1, 0); + if (mem == MAP_FAILED) + ksft_exit_fail_msg("Error mmap %s.\n", strerror(errno)); + + wp_init(mem, 0x1000 * nthreads * pages_per_thread); + wp_addr_range(mem, 0x1000 * nthreads * pages_per_thread); + + memset(mem, 0, 0x1000 * nthreads * pages_per_thread); + + count = get_dirty_pages_reset(mem, nthreads * pages_per_thread, 1, page_size); + ksft_test_result(count > 0, "%s count %d\n", __func__, count); + count = get_dirty_pages_reset(mem, nthreads * pages_per_thread, 1, page_size); + ksft_test_result(count == 0, "%s count %d\n", __func__, count); + + finish = 0; + for (i = 0; i < nthreads; ++i) + pthread_create(&th, NULL, thread_proc, mem + 0x1000 * i * pages_per_thread); + + extra_pages = 0; + for (i = 0; i < iter_count; ++i) { + count = 0; + + ret = pthread_barrier_wait(&start_barrier); + if (ret && ret != PTHREAD_BARRIER_SERIAL_THREAD) + ksft_exit_fail_msg("pthread_barrier_wait\n"); + + count = get_dirty_pages_reset(mem, nthreads * pages_per_thread, 1, + page_size); + + ret = pthread_barrier_wait(&end_barrier); + if (ret && ret != PTHREAD_BARRIER_SERIAL_THREAD) + ksft_exit_fail_msg("pthread_barrier_wait\n"); + + if (count > nthreads * access_per_thread) + ksft_exit_fail_msg("Too big count %d expected %d, iter %d\n", + count, nthreads * access_per_thread, i); + + c = get_dirty_pages_reset(mem, nthreads * pages_per_thread, 1, page_size); + count += c; + + if (c > nthreads * access_per_thread) { + ksft_test_result_fail(" %s count > nthreads\n", __func__); + return; + } + + if (count != nthreads * access_per_thread) { + /* + * The purpose of the test is to make sure that no page updates are lost + * when the page updates and read-resetting soft dirty flags are performed + * in parallel. However, it is possible that the application will get the + * soft dirty flags twice on the two consecutive read-resets. This seems + * unavoidable as soft dirty flag is handled in software through page faults + * in kernel. While the updating the flags is supposed to be synchronized + * between page fault handling and read-reset, it is possible that + * read-reset happens after page fault PTE update but before the application + * re-executes write instruction. So read-reset gets the flag, clears write + * access and application gets page fault again for the same write. + */ + if (count < nthreads * access_per_thread) { + ksft_test_result_fail("Lost update, iter %d, %d vs %d.\n", i, count, + nthreads * access_per_thread); + return; + } + + extra_pages += count - nthreads * access_per_thread; + } + } + + pthread_barrier_wait(&start_barrier); + finish = 1; + pthread_barrier_wait(&end_barrier); + + ksft_test_result_pass("%s Extra pages %u (%.1lf%%), extra thread faults %d.\n", __func__, + extra_pages, + 100.0 * extra_pages / (iter_count * nthreads * access_per_thread), + extra_thread_faults); +} + +int main(void) +{ + int mem_size, shmid, buf_size, fd, i, ret; + char *mem, *map, *fmem; + struct stat sbuf; + + ksft_print_header(); + ksft_set_plan(83); + + page_size = getpagesize(); + hpage_size = read_pmd_pagesize(); + + pagemap_fd = open(PAGEMAP, O_RDWR); + if (pagemap_fd < 0) + return -EINVAL; + + if (init_uffd()) + ksft_exit_fail_msg("uffd init failed\n"); + + /* + * Written (dirty) PTE bit tests + */ + + /* 1. Sanity testing */ + sanity_tests_sd(); + + /* 2. Normal page testing */ + mem_size = 10 * page_size; + mem = mmap(NULL, mem_size, PROT_READ | PROT_WRITE, MAP_PRIVATE | MAP_ANON, -1, 0); + if (mem == MAP_FAILED) + ksft_exit_fail_msg("error nomem\n"); + wp_init(mem, mem_size); + wp_addr_range(mem, mem_size); + + base_tests("Page testing:", mem, mem_size, 0); + + wp_free(mem, mem_size); + munmap(mem, mem_size); + + /* 3. Large page testing */ + mem_size = 512 * 10 * page_size; + mem = mmap(NULL, mem_size, PROT_READ | PROT_WRITE, MAP_PRIVATE | MAP_ANON, -1, 0); + if (mem == MAP_FAILED) + ksft_exit_fail_msg("error nomem\n"); + wp_init(mem, mem_size); + wp_addr_range(mem, mem_size); + + base_tests("Large Page testing:", mem, mem_size, 0); + + wp_free(mem, mem_size); + munmap(mem, mem_size); + + /* 4. Huge page testing */ + map = gethugepage(hpage_size); + if (map) { + wp_init(map, hpage_size); + wp_addr_range(map, hpage_size); + base_tests("Huge page testing:", map, hpage_size, 0); + wp_free(map, hpage_size); + free(map); + } else { + base_tests("Huge page testing:", NULL, 0, 1); + } + + /* 5. Hugetlb page testing */ + mem_size = 2*1024*1024; + mem = gethugetlb_mem(mem_size, &shmid); + if (mem) { + wp_init(mem, mem_size); + wp_addr_range(mem, mem_size); + + base_tests("Hugetlb shmem testing:", mem, mem_size, 0); + + wp_free(mem, mem_size); + shmctl(shmid, IPC_RMID, NULL); + } else { + base_tests("Hugetlb shmem testing:", NULL, 0, 1); + } + + /* 6. Hugetlb page testing */ + mem = gethugetlb_mem(mem_size, NULL); + if (mem) { + wp_init(mem, mem_size); + wp_addr_range(mem, mem_size); + + base_tests("Hugetlb mem testing:", mem, mem_size, 0); + + wp_free(mem, mem_size); + } else { + base_tests("Hugetlb mem testing:", NULL, 0, 1); + } + + /* 7. file memory testing */ + buf_size = page_size * 10; + + fd = open(__FILE__".tmp0", O_RDWR | O_CREAT, 0777); + if (fd < 0) + ksft_exit_fail_msg("Create and read/write to a memory mapped file: %s\n", + strerror(errno)); + + for (i = 0; i < buf_size; i++) + if (write(fd, "c", 1) < 0) + ksft_exit_fail_msg("Create and read/write to a memory mapped file\n"); + + ret = stat(__FILE__".tmp0", &sbuf); + if (ret < 0) + ksft_exit_fail_msg("error %d %d %s\n", ret, errno, strerror(errno)); + + fmem = mmap(NULL, sbuf.st_size, PROT_READ | PROT_WRITE, MAP_PRIVATE, fd, 0); + if (fmem == MAP_FAILED) + ksft_exit_fail_msg("error nomem %ld %s\n", errno, strerror(errno)); + + wp_init(fmem, sbuf.st_size); + wp_addr_range(fmem, sbuf.st_size); + + base_tests("File memory testing:", fmem, sbuf.st_size, 0); + + wp_free(fmem, sbuf.st_size); + munmap(fmem, sbuf.st_size); + close(fd); + + /* 8888. file memory testing */ + buf_size = page_size * 10; + + fd = memfd_create(__FILE__".tmp00", MFD_NOEXEC_SEAL); + if (fd < 0) + ksft_exit_fail_msg("Create and read/write to a memory mapped file: %s\n", + strerror(errno)); + + if (ftruncate(fd, buf_size)) + ksft_exit_fail_msg("Error ftruncate\n"); + + for (i = 0; i < buf_size; i++) + if (write(fd, "c", 1) < 0) + ksft_exit_fail_msg("Create and read/write to a memory mapped file\n"); + + fmem = mmap(NULL, buf_size, PROT_READ | PROT_WRITE, MAP_PRIVATE, fd, 0); + if (fmem == MAP_FAILED) + ksft_exit_fail_msg("error nomem %ld %s\n", errno, strerror(errno)); + + wp_init(fmem, buf_size); + wp_addr_range(fmem, buf_size); + + base_tests("File anonymous memory testing:", fmem, buf_size, 0); + + wp_free(fmem, buf_size); + munmap(fmem, buf_size); + close(fd); + + /* 8. Huge page tests */ + hpage_unit_tests(); + + /* 9. Iterative test */ + test_simple(); + + /* 10. Mprotect test */ + mprotect_tests(); + + /* 11. Transact test */ + transact_test(page_size); + + /* + * Other PTE bit tests + */ + + /* 1. Sanity testing */ + sanity_tests(); + + /* 2. Unmapped address test */ + unmapped_region_tests(); + + /* 3. Userfaultfd tests */ + userfaultfd_tests(); + + close(pagemap_fd); + return ksft_exit_pass(); +} diff --git a/tools/testing/selftests/mm/run_vmtests.sh b/tools/testing/selftests/mm/run_vmtests.sh old mode 100644 new mode 100755 index aea09b014330..4c171c144fe6 --- a/tools/testing/selftests/mm/run_vmtests.sh +++ b/tools/testing/selftests/mm/run_vmtests.sh @@ -50,6 +50,8 @@ separated by spaces: memory protection key tests - soft_dirty test soft dirty page bit semantics +- pagemap + test pagemap_scan IOCTL - cow test copy-on-write semantics example: ./run_vmtests.sh -t "hmm mmap ksm" @@ -276,6 +278,8 @@ fi
CATEGORY="soft_dirty" run_test ./soft-dirty
+CATEGORY="pagemap" run_test ./pagemap_ioctl + # COW tests CATEGORY="cow" run_test ./cow
linux-kselftest-mirror@lists.linaro.org
-
kernel test robot -
Michał Mirosław -
Muhammad Usama Anjum