[PATCH v11 0/7] Implement IOCTL to get and optionally clear info about PTEs

[PATCH v11 0/7] Implement IOCTL to get and optionally clear info about PTEs

[Muhammad Usama Anjum]

These patches are based on next-20230307 and UFFD_FEATURE_WP_UNPOPULATED
patches from Peter.

*Changes in v11*
- Rebase on top of next-20230307
- Base patches on UFFD_FEATURE_WP_UNPOPULATED (https://lore.kernel.org/all/20230306213925.617814-1-peterx@redhat.com)
- 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

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.com/
[2] https://lore.kernel.org/all/20221220162606.1595355-1-usama.anjum@collabora.com
[3] https://lore.kernel.org/all/20221122115007.2787017-1-usama.anjum@collabora.com
[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 (7):
  userfaultfd: Add UFFD WP Async support
  userfaultfd: Define dummy uffd_wp_range()
  userfaultfd: update documentation to describe UFFD_FEATURE_WP_ASYNC
  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

 Documentation/admin-guide/mm/pagemap.rst     |  56 ++
 Documentation/admin-guide/mm/userfaultfd.rst |  21 +
 fs/proc/task_mmu.c                           | 366 ++++++++
 fs/userfaultfd.c                             |  25 +-
 include/linux/userfaultfd_k.h                |  14 +
 include/uapi/linux/fs.h                      |  53 ++
 include/uapi/linux/userfaultfd.h             |  11 +-
 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   | 920 +++++++++++++++++++
 tools/testing/selftests/mm/run_vmtests.sh    |   4 +
 14 files changed, 1549 insertions(+), 7 deletions(-)
 create mode 100644 tools/testing/selftests/mm/pagemap_ioctl.c
 mode change 100644 => 100755 tools/testing/selftests/mm/run_vmtests.sh

-- 
2.39.2

[PATCH v11 1/7] userfaultfd: Add UFFD WP Async support

[Muhammad Usama Anjum]

Add new WP Async mode (UFFD_FEATURE_WP_ASYNC) which resolves the page
faults on its own. It can be used to track that which pages have been
written-to from the time the pages were write-protected. It is very
efficient way to track the changes as uffd is by nature pte/pmd based.

UFFD synchronous WP sends the page faults to the userspace where the
pages which have been written-to can be tracked. But it is not efficient.
This is why this asynchronous version is being added. After setting the
WP Async, the pages which have been written to can be found in the pagemap
file or information can be obtained from the PAGEMAP_IOCTL.

Suggested-by: Peter Xu <peterx@redhat.com>
Signed-off-by: Muhammad Usama Anjum <usama.anjum@collabora.com>
---
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)
---
 fs/userfaultfd.c                 | 25 +++++++++++++++++++++++--
 include/linux/userfaultfd_k.h    |  6 ++++++
 include/uapi/linux/userfaultfd.h | 11 ++++++++++-
 mm/memory.c                      | 27 ++++++++++++++++++++++++---
 4 files changed, 63 insertions(+), 6 deletions(-)

diff --git a/fs/userfaultfd.c b/fs/userfaultfd.c
index dac0ebe39774..992b0b21cd59 100644
--- a/fs/userfaultfd.c
+++ b/fs/userfaultfd.c
@@ -1446,10 +1446,15 @@ static int userfaultfd_register(struct userfaultfd_ctx *ctx,
 			goto out_unlock;
 
 		/*
-		 * Note vmas containing huge pages
+		 * Note vmas containing huge pages. Hugetlb isn't supported
+		 * with UFFD_FEATURE_WP_ASYNC.
 		 */
-		if (is_vm_hugetlb_page(cur))
+		ret = -EINVAL;
+		if (is_vm_hugetlb_page(cur)) {
+			if (ctx->features & UFFD_FEATURE_WP_ASYNC)
+				goto out_unlock;
 			basic_ioctls = true;
+		}
 
 		found = true;
 	} for_each_vma_range(vmi, cur, end);
@@ -1874,6 +1879,10 @@ static int userfaultfd_writeprotect(struct userfaultfd_ctx *ctx,
 	mode_wp = uffdio_wp.mode & UFFDIO_WRITEPROTECT_MODE_WP;
 	mode_dontwake = uffdio_wp.mode & UFFDIO_WRITEPROTECT_MODE_DONTWAKE;
 
+	/* The unprotection is not supported if in async WP mode */
+	if (!mode_wp && (ctx->features & UFFD_FEATURE_WP_ASYNC))
+		return -EINVAL;
+
 	if (mode_wp && mode_dontwake)
 		return -EINVAL;
 
@@ -1957,6 +1966,13 @@ static int userfaultfd_continue(struct userfaultfd_ctx *ctx, unsigned long arg)
 	return ret;
 }
 
+int userfaultfd_wp_async(struct vm_area_struct *vma)
+{
+	struct userfaultfd_ctx *ctx = vma->vm_userfaultfd_ctx.ctx;
+
+	return (ctx && (ctx->features & UFFD_FEATURE_WP_ASYNC));
+}
+
 static inline unsigned int uffd_ctx_features(__u64 user_features)
 {
 	/*
@@ -1988,6 +2004,10 @@ static int userfaultfd_api(struct userfaultfd_ctx *ctx,
 	ret = -EPERM;
 	if ((features & UFFD_FEATURE_EVENT_FORK) && !capable(CAP_SYS_PTRACE))
 		goto err_out;
+	if ((features & UFFD_FEATURE_WP_ASYNC) &&
+	    !(features & UFFD_FEATURE_WP_UNPOPULATED))
+		goto err_out;
+
 	/* report all available features and ioctls to userland */
 	uffdio_api.features = UFFD_API_FEATURES;
 #ifndef CONFIG_HAVE_ARCH_USERFAULTFD_MINOR
@@ -2000,6 +2020,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 52cb3de88e20..b680c0ec8b85 100644
--- a/include/linux/userfaultfd_k.h
+++ b/include/linux/userfaultfd_k.h
@@ -178,6 +178,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 int userfaultfd_wp_async(struct vm_area_struct *vma);
 
 #else /* CONFIG_USERFAULTFD */
 
@@ -278,6 +279,11 @@ static inline bool userfaultfd_wp_unpopulated(struct vm_area_struct *vma)
 	return false;
 }
 
+static inline int userfaultfd_wp_async(struct vm_area_struct *vma)
+{
+	return false;
+}
+
 #endif /* CONFIG_USERFAULTFD */
 
 static inline bool pte_marker_entry_uffd_wp(swp_entry_t entry)
diff --git a/include/uapi/linux/userfaultfd.h b/include/uapi/linux/userfaultfd.h
index 90c958952bfc..00dbe7d6551b 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,13 @@ 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 only
+	 * supports anon and shmem (hugetlb isn't supported). It only takes
+	 * effect when a vma is registered with write-protection mode. Otherwise
+	 * the flag is ignored. It depends on UFFD_FEATURE_WP_UNPOPULATED.
 	 */
 #define UFFD_FEATURE_PAGEFAULT_FLAG_WP		(1<<0)
 #define UFFD_FEATURE_EVENT_FORK			(1<<1)
@@ -225,6 +233,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/memory.c b/mm/memory.c
index 8d135a814c60..341071c2c49a 100644
--- a/mm/memory.c
+++ b/mm/memory.c
@@ -3348,11 +3348,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;
 		}
 
 		/*
@@ -4824,8 +4841,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);
 	}
 
@@ -4837,6 +4857,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);
 
-- 
2.39.2

[PATCH v11 2/7] userfaultfd: Define dummy uffd_wp_range()

[Muhammad Usama Anjum]

Define uffd_wp_range() for the cases when CONFIG_USERFAULTFD isn't set.

Signed-off-by: Muhammad Usama Anjum <usama.anjum@collabora.com>
---
 include/linux/userfaultfd_k.h | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/include/linux/userfaultfd_k.h b/include/linux/userfaultfd_k.h
index b680c0ec8b85..fd1a1ecdb5f6 100644
--- a/include/linux/userfaultfd_k.h
+++ b/include/linux/userfaultfd_k.h
@@ -182,6 +182,14 @@ extern int userfaultfd_wp_async(struct vm_area_struct *vma);
 
 #else /* CONFIG_USERFAULTFD */
 
+extern 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)
-- 
2.39.2

[PATCH v11 3/7] userfaultfd: update documentation to describe UFFD_FEATURE_WP_ASYNC

[Muhammad Usama Anjum]

Explain the difference created by UFFD_FEATURE_WP_ASYNC to the write
protection (UFFDIO_WRITEPROTECT_MODE_WP) mode.

Suggested-by: Suggested-by: Peter Xu <peterx@redhat.com>
Signed-off-by: Muhammad Usama Anjum <usama.anjum@collabora.com>
---
Changes in v11:
- Update the documentation from reviews entirely from Peter
---
 Documentation/admin-guide/mm/userfaultfd.rst | 21 ++++++++++++++++++++
 1 file changed, 21 insertions(+)

diff --git a/Documentation/admin-guide/mm/userfaultfd.rst b/Documentation/admin-guide/mm/userfaultfd.rst
index 7dc823b56ca4..404d8aa8f09f 100644
--- a/Documentation/admin-guide/mm/userfaultfd.rst
+++ b/Documentation/admin-guide/mm/userfaultfd.rst
@@ -219,6 +219,27 @@ former will have ``UFFD_PAGEFAULT_FLAG_WP`` set, the latter
 you still need to supply a page when ``UFFDIO_REGISTER_MODE_MISSING`` was
 used.
 
+If the userfaultfd context (that has ``UFFDIO_REGISTER_MODE_WP`` registered
+against) has ``UFFD_FEATURE_WP_ASYNC`` feature enabled, it will work in
+async write protection mode.  It can be seen as a more accurate version of
+soft-dirty tracking, meanwhile the results will not be easily affected by
+other operations like vma merging.
+
+Comparing to the generic mode, the async mode will not generate any
+userfaultfd message when the protected memory range is written.  Instead, the
+kernel will automatically resolve the page fault immediately by dropping the
+uffd-wp bit in the pgtables.  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 be under track of uffd-wp async mode until the page is explicitly
+write-protected by ``UFFDIO_WRITEPROTECT`` ioctl 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.
+
+Currently ``UFFD_FEATURE_WP_ASYNC`` only support anonymous and shmem. Hugetlb is
+not yet supported.
+
 QEMU/KVM
 ========
 
-- 
2.39.2

[PATCH v11 4/7] fs/proc/task_mmu: Implement IOCTL to get and optionally clear info about PTEs

[Muhammad Usama Anjum]

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. This
IOCTL doesn't support hugetlbs at the moment. No information about
hugetlb can be obtained. This patch has evolved from a basic patch from
Gabriel Krisman Bertazi.

Signed-off-by: Muhammad Usama Anjum <usama.anjum@collabora.com>
---
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      | 366 ++++++++++++++++++++++++++++++++++++++++
 include/uapi/linux/fs.h |  53 ++++++
 2 files changed, 419 insertions(+)

diff --git a/fs/proc/task_mmu.c b/fs/proc/task_mmu.c
index 6a96e1713fd5..f8f796cf3439 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>
@@ -1132,6 +1133,18 @@ 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)));
+}
+
+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)
@@ -1760,11 +1773,364 @@ static int pagemap_release(struct inode *inode, struct file *file)
 	return 0;
 }
 
+#define PM_SCAN_BITS_ALL	(PAGE_IS_WRITTEN | PAGE_IS_FILE |	\
+				 PAGE_IS_PRESENT | PAGE_IS_SWAPPED)
+#define PM_SCAN_NON_WT_BITS	(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_wt_required(struct pagemap_scan_private *p)
+{
+	return	((p->required_mask & PAGE_IS_WRITTEN) ||
+		 (p->anyof_mask & PAGE_IS_WRITTEN) ||
+		 (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_wt_required(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;
+	bool cpy = true;
+
+	if (p->max_pages && (p->found_pages == p->max_pages))
+		return -ENOSPC;
+
+	if (!n_pages)
+		return -EINVAL;
+
+	if (p->required_mask)
+		cpy = ((p->required_mask & bitmap) == p->required_mask);
+	if (cpy && p->anyof_mask)
+		cpy = (p->anyof_mask & bitmap);
+	if (cpy && p->excluded_mask)
+		cpy = !(p->excluded_mask & bitmap);
+
+	bitmap = bitmap & p->return_mask;
+
+	if (cpy && bitmap) {
+		if ((cur->len) && (cur->bitmap == bitmap) &&
+		    (cur->start + cur->len * PAGE_SIZE == addr)) {
+
+			cur->len += n_pages;
+			p->found_pages += n_pages;
+		} else 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;
+		} else {
+			return -ENOSPC;
+		}
+	}
+
+	return 0;
+}
+
+static 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) {
+		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;
+	struct vm_area_struct *vma = walk->vma;
+	bool is_writ, is_file, is_pres, is_swap;
+	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;
+
+		is_writ = !is_pmd_uffd_wp(*pmd);
+		/*
+		 * Break huge page into small pages if operation needs to be
+		 * performed is on a portion of the huge page.
+		 */
+		if (is_writ && PM_SCAN_OP_IS_WP(p) &&
+		    (end - start < HPAGE_SIZE)) {
+			spin_unlock(ptl);
+
+			split_huge_pmd(vma, pmd, start);
+			goto process_smaller_pages;
+		}
+
+		n_pages = (end - start)/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(is_writ, vma->vm_file,
+					  pmd_present(*pmd), is_swap_pmd(*pmd),
+					  p, start, n_pages);
+		spin_unlock(ptl);
+
+		if (!ret && is_writ && PM_SCAN_OP_IS_WP(p) &&
+		    uffd_wp_range(walk->mm, vma, start, HPAGE_SIZE, true) < 0)
+			ret = -EINVAL;
+
+		return ret;
+	}
+process_smaller_pages:
+	if (pmd_trans_unstable(pmd))
+		return 0;
+#endif /* CONFIG_TRANSPARENT_HUGEPAGE */
+
+	for (addr = start; !ret && addr < end; pte++, addr += PAGE_SIZE) {
+		pte = pte_offset_map_lock(vma->vm_mm, pmd, addr, &ptl);
+
+		is_writ = !is_pte_uffd_wp(*pte);
+		is_file = vma->vm_file;
+		is_pres = pte_present(*pte);
+		is_swap = is_swap_pte(*pte);
+
+		pte_unmap_unlock(pte, ptl);
+
+		ret = pagemap_scan_output(is_writ, is_file, is_pres, is_swap,
+					  p, addr, 1);
+		if (ret)
+			break;
+
+		if (PM_SCAN_OP_IS_WP(p) && is_writ &&
+		    uffd_wp_range(walk->mm, vma, addr, PAGE_SIZE, true) < 0)
+			ret = -EINVAL;
+	}
+
+	cond_resched();
+	return ret;
+}
+
+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) {
+		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;
+}
+
+/* No hugetlb support is present. */
+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,
+};
+
+static bool pagemap_scan_args_valid(struct pm_scan_arg *arg,
+				    struct page_region __user *vec,
+				    unsigned long start)
+{
+	/* Detect illegal size, flags and masks */
+	if (arg->size != sizeof(struct pm_scan_arg))
+		return false;
+	if (arg->flags & ~PM_SCAN_OPS)
+		return false;
+	if ((arg->required_mask | arg->anyof_mask | arg->excluded_mask |
+	     arg->return_mask) & ~PM_SCAN_BITS_ALL)
+		return false;
+	if (!arg->required_mask && !arg->anyof_mask &&
+	    !arg->excluded_mask)
+		return false;
+	if (!arg->return_mask)
+		return false;
+
+	/* Validate memory ranges */
+	if (!(arg->flags & PM_SCAN_OP_GET))
+		return false;
+	if (!arg->vec)
+		return false;
+	if (arg->vec_len == 0)
+		return false;
+	if (!access_ok((void __user *)vec,
+		       arg->vec_len * sizeof(struct page_region)))
+		return false;
+
+	if (!IS_ALIGNED(start, PAGE_SIZE))
+		return false;
+	if (!access_ok((void __user *)start, arg->len))
+		return false;
+
+	if (PM_SCAN_OP_IS_WP(arg)) {
+		if (arg->required_mask & PM_SCAN_NON_WT_BITS)
+			return false;
+		if (arg->anyof_mask & PM_SCAN_NON_WT_BITS)
+			return false;
+		if (arg->excluded_mask & PM_SCAN_NON_WT_BITS)
+			return false;
+	}
+
+	return true;
+}
+
+static long do_pagemap_cmd(struct mm_struct *mm, struct pm_scan_arg *arg)
+{
+	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;
+	int ret = 0;
+
+	start = (unsigned long)untagged_addr(arg->start);
+	vec = (struct page_region *)(unsigned long)untagged_addr(arg->vec);
+
+	if (!pagemap_scan_args_valid(arg, vec, start))
+		return -EINVAL;
+
+	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.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) {
+		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))
+			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;
+	struct pm_scan_arg argument;
+
+	if (cmd == PAGEMAP_SCAN) {
+		if (copy_from_user(&argument, uarg,
+				   sizeof(struct pm_scan_arg)))
+			return -EFAULT;
+		return do_pagemap_cmd(mm, &argument);
+	}
+
+	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/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 */
-- 
2.39.2

[PATCH v11 5/7] tools headers UAPI: Update linux/fs.h with the kernel sources

[Muhammad Usama Anjum]

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 */
-- 
2.39.2

[PATCH v11 6/7] mm/pagemap: add documentation of PAGEMAP_SCAN IOCTL

[Muhammad Usama Anjum]

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 b5f970dc91e7..41815511011c 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.
-- 
2.39.2

[PATCH v11 7/7] selftests: mm: add pagemap ioctl tests

[Muhammad Usama Anjum]

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 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 | 920 +++++++++++++++++++++
 tools/testing/selftests/mm/run_vmtests.sh  |   4 +
 5 files changed, 929 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 fbf5646b1072..9fcba8e87f28 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..f0fc43694472
--- /dev/null
+++ b/tools/testing/selftests/mm/pagemap_ioctl.c
@@ -0,0 +1,920 @@
+// SPDX-License-Identifier: GPL-2.0
+#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/userfaultfd.h>
+#include <linux/fs.h>
+#include <sys/ioctl.h>
+#include <sys/stat.h>
+#include <math.h>
+#include <asm/unistd.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;
+	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))
+		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)\n");
+
+	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;
+}
+
+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;
+
+	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");
+	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);
+
+	/* 1. wrong operation */
+	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, 8,
+				       0, 0x1111, 0, 0, PAGE_IS_WRITTEN) < 0,
+			 "%s wrong mask specified\n", __func__);
+
+	ksft_test_result(pagemap_ioctl(mem, mem_size, vec, vec_size, 0,
+				       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_WP | PM_SCAN_OP_GET | 0x32,
+				       0, PAGE_IS_WRITTEN, 0, 0, PAGE_IS_WRITTEN) < 0,
+			 "%s mixture of correct and wrong flag\n", __func__);
+
+	ksft_test_result(pagemap_ioctl(mem, mem_size, NULL, 0, PM_SCAN_OP_WP,
+				       0, PAGE_IS_WRITTEN, 0, 0, PAGE_IS_WRITTEN) < 0,
+			 "%s PM_SCAN_OP_WP cannot be used without get\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, num_pages/5, 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, num_pages/2, PM_SCAN_OP_GET, 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__);
+
+	wp_free(mem, mem_size);
+	munmap(mem, mem_size);
+
+	/* 5. 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);
+
+	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);
+	return 0;
+}
+
+int base_tests(char *prefix, char *mem, int mem_size, int skip)
+{
+	int vec_size, ret, written, written2;
+	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);
+		ksft_test_result_skip("%s only get 2 dirty pages and clear them as well\n", prefix);
+		ksft_test_result_skip("%s Range clear only\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);
+
+	/* 6. only get 2 dirty pages and clear them as well */
+	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));
+
+	written = pagemap_ioctl(mem, mem_size, vec2, 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(ret == 1 && vec[0].len == 2 &&
+			 vec[0].start == (uintptr_t)(mem + page_size) &&
+			 written == 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", prefix);
+
+	/* 7. Range clear only */
+	memset(mem, -1, mem_size);
+
+	wp_addr_range(mem, mem_size);
+
+	written2 = pagemap_ioctl(mem, mem_size, vec, vec_size, PM_SCAN_OP_GET, 0,
+			       PAGE_IS_WRITTEN, 0, 0, PAGE_IS_WRITTEN);
+	if (written2 < 0)
+		ksft_exit_fail_msg("error %d %d %s\n", written2, errno, strerror(errno));
+
+	ksft_test_result(written2 == 0, "%s Range clear only\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));
+
+	wp_init(map, map_size);
+
+	return map;
+}
+
+int hpage_unit_tests(void)
+{
+	char *map;
+	int ret;
+	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) {
+		/* 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) {
+
+		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) {
+		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) {
+		memset(map, -1, map_size);
+
+		wp_addr_range(map + map_size/2, 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,
+				 "%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) {
+		memset(map, -1, map_size);
+
+		ret = pagemap_ioctl(map, map_size, vec, 1, PM_SCAN_OP_GET, 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__);
+		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)
+{
+	char *mem, *fmem;
+	int mem_size, vec_size, ret;
+	struct page_region *vec;
+
+	/* 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);
+
+	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);
+
+	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);
+
+	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 */
+	int fd;
+	struct stat sbuf;
+
+	fd = open(__FILE__, O_RDONLY);
+	if (fd < 0) {
+		ksft_test_result_skip("%s Memory mapped file\n");
+		goto free_vec_and_return;
+	}
+
+	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);
+
+free_vec_and_return:
+	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);
+
+	/* 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);
+
+	/* 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;
+}
+
+int main(void)
+{
+	char *mem, *map;
+	int mem_size;
+
+	ksft_print_header();
+	ksft_set_plan(57);
+
+	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);
+
+	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);
+
+	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) {
+		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);
+	}
+
+	/* 6. Huge page tests */
+	hpage_unit_tests();
+
+	/* 7. Iterative test */
+	test_simple();
+
+	/* 8. Mprotect test */
+	mprotect_tests();
+
+	/*
+	 * 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 8984e0bb58c7..f7e1370cc581
--- 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"
@@ -268,6 +270,8 @@ fi
 
 CATEGORY="soft_dirty" run_test ./soft-dirty
 
+CATEGORY="pagemap" run_test ./pagemap_ioctl
+
 # COW tests
 CATEGORY="cow" run_test ./cow
 
-- 
2.39.2

Re: [PATCH v11 0/7] Implement IOCTL to get and optionally clear info about PTEs

[Andrew Morton]

On Thu,  9 Mar 2023 18:57:11 +0500 Muhammad Usama Anjum <usama.anjum@collabora.com> wrote:

> 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].

It's a ton of new code and what I'm not seeing in here (might have
missed it?) is a clear statement of the value of this feature to our
users.

I see hints that CRIU would like it, but no description of how valuable
this is to CRIU's users.

So please spend some time preparing this info.

Also, are any other applications of this feature anticipated?  If so,
what are they?

IOW, please sell this stuff to us!

Re: [PATCH v11 0/7] Implement IOCTL to get and optionally clear info about PTEs

[Muhammad Usama Anjum]

Hi Andrew,

Thank you for your message.

On 3/10/23 12:58 AM, Andrew Morton wrote:
> On Thu,  9 Mar 2023 18:57:11 +0500 Muhammad Usama Anjum <usama.anjum@collabora.com> wrote:
> 
>> 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].
> 
> It's a ton of new code and what I'm not seeing in here (might have
> missed it?) is a clear statement of the value of this feature to our
> users.
Sorry, let me give some clear details about its value.

> 
> I see hints that CRIU would like it, but no description of how valuable
> this is to CRIU's users.
> 
> So please spend some time preparing this info.
> 
> Also, are any other applications of this feature anticipated?  If so,
> what are they?
> 
> IOW, please sell this stuff to us!
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.
At [10]:
> For Andrei's use case is to iteratively dump memory.

@Andrei and Danylo can elaborate more on their use cases.


*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-getwritewatch
[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.com
[8]
https://lore.kernel.org/all/20221220162606.1595355-1-usama.anjum@collabora.com
[9] https://lore.kernel.org/all/20230306213925.617814-1-peterx@redhat.com
[10] https://lore.kernel.org/all/20230125144529.1630917-1-mdanylo@google.com

-- 
BR,
Muhammad Usama Anjum

Re: [PATCH v11 4/7] fs/proc/task_mmu: Implement IOCTL to get and optionally clear info about PTEs

[Michał Mirosław]

On Thu, 9 Mar 2023 at 14:58, 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)
[...]
> --- 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>
> @@ -1132,6 +1133,18 @@ 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)));

Parentheses around pte_swp_uffd_wp_any() are redundant. Please remove
here and in all following if()s. (Nit: those extra parentheses are
used inconsistently in the patch anyway.)

[...]
> +static inline bool pagemap_scan_is_wt_required(struct pagemap_scan_private *p)

This seems to check if the PAGE_IS_WRITTEN flag is tested, so
"pagemap_scan_needs_wp_checks()"? Or maybe document/expand the "wt"
acronym as it seems used also on following code.

> +{
> +       return  ((p->required_mask & PAGE_IS_WRITTEN) ||
> +                (p->anyof_mask & PAGE_IS_WRITTEN) ||
> +                (p->excluded_mask & PAGE_IS_WRITTEN));

Nit: It looks like it should answer "do any of the masks contain
PAGE_IS_WRITTEN?" so maybe:

return (p->required_mask | p->anyof_mask | p->excluded_mask) & PAGE_IS_WRITTEN;

[...]

> +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;
> +       bool cpy = true;
> +
> +       if (p->max_pages && (p->found_pages == p->max_pages))
> +               return -ENOSPC;
> +
> +       if (!n_pages)
> +               return -EINVAL;
> +
> +       if (p->required_mask)
> +               cpy = ((p->required_mask & bitmap) == p->required_mask);
> +       if (cpy && p->anyof_mask)
> +               cpy = (p->anyof_mask & bitmap);
> +       if (cpy && p->excluded_mask)
> +               cpy = !(p->excluded_mask & bitmap);

Since the rest of the code is executed only when `cpy` is true, this
could just return early for easier understanding.

BTW, some of the tests are redundant. Eg: if required_mask == 0, then
`required_mask & x == required_mask` will always hold. Same for
`excluded_mask & x == 0`.

> +
> +       bitmap = bitmap & p->return_mask;

Nit: bitmap &= p->return_mask;

> +       if (cpy && bitmap) {

Assuming early returns on `!cpy` are done earlier:

if (!bitmap)
  return 0;

> +               if ((cur->len) && (cur->bitmap == bitmap) &&
> +                   (cur->start + cur->len * PAGE_SIZE == addr)) {

I'd recommend removing the extra parentheses as they make the code
less readable for me (too many parentheses to match visually).
The `cur->len` test seems redundant: is it possible to have
`cur->start == addr` in that case (I guess it would have to get
`n_pages == 0` in an earlier invocation)?

> +
> +                       cur->len += n_pages;
> +                       p->found_pages += n_pages;

Please add an early return so that 'else' chaining won't be necessary.

> +               } else if ((!p->vec_index) ||
> +                          ((p->vec_index + 1) < p->vec_len)) {

Can you explain this test? Why not just `p->vec_index < p->vec_len`? Or better:

if (vec_index >= p->vec_len)
    return -ENOSPC;

> +                       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;
> +               } else {
> +                       return -ENOSPC;
> +               }
> +       }
> +
> +       return 0;
> +}
[...]

> +static 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) {

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;
> +       struct vm_area_struct *vma = walk->vma;
> +       bool is_writ, is_file, is_pres, is_swap;
> +       unsigned long addr = end;
> +       spinlock_t *ptl;
> +       int ret = 0;
> +       pte_t *pte;
> +
> +#ifdef CONFIG_TRANSPARENT_HUGEPAGE

Is the `#ifdef` needed? `pmd_trans_huge_lock()` will always return
NULL if transparent hugepages are not compiled in. OTOH I see
BUILD_BUG() is possible in HPAGE_SIZE definition (irrelevant in this
case), so that would need to be worked around first.

> +       ptl = pmd_trans_huge_lock(pmd, vma);
> +       if (ptl) {
> +               unsigned long n_pages;
> +
> +               is_writ = !is_pmd_uffd_wp(*pmd);

`is_written`?

> +               /*
> +                * Break huge page into small pages if operation needs to be
> +                * performed is on a portion of the huge page.
> +                */
> +               if (is_writ && PM_SCAN_OP_IS_WP(p) &&
> +                   (end - start < HPAGE_SIZE)) {
> +                       spin_unlock(ptl);
> +
> +                       split_huge_pmd(vma, pmd, start);
> +                       goto process_smaller_pages;
> +               }
> +
> +               n_pages = (end - start)/PAGE_SIZE;
> +               if (p->max_pages &&
> +                   p->found_pages + n_pages >= p->max_pages)

Nit: greater-than is also correct and avoids no-op assignment.

> +                       n_pages = p->max_pages - p->found_pages;
> +
> +               ret = pagemap_scan_output(is_writ, vma->vm_file,
> +                                         pmd_present(*pmd), is_swap_pmd(*pmd),
> +                                         p, start, n_pages);
> +               spin_unlock(ptl);

if (ret || !is_written)
  return ret;

This will avoid those tests in the following if().

> +
> +               if (!ret && is_writ && PM_SCAN_OP_IS_WP(p) &&
> +                   uffd_wp_range(walk->mm, vma, start, HPAGE_SIZE, true) < 0)
> +                       ret = -EINVAL;
> +
> +               return ret;

After above early returns, this will be always `return 0;`.

> +       }
> +process_smaller_pages:
> +       if (pmd_trans_unstable(pmd))
> +               return 0;
> +#endif /* CONFIG_TRANSPARENT_HUGEPAGE */
> +
> +       for (addr = start; !ret && addr < end; pte++, addr += PAGE_SIZE) {

The `!ret` can be removed if the EINVAL case was to `break` by itself.

> +               pte = pte_offset_map_lock(vma->vm_mm, pmd, addr, &ptl);
> +
> +               is_writ = !is_pte_uffd_wp(*pte);
> +               is_file = vma->vm_file;
> +               is_pres = pte_present(*pte);
> +               is_swap = is_swap_pte(*pte);
> +
> +               pte_unmap_unlock(pte, ptl);
> +
> +               ret = pagemap_scan_output(is_writ, is_file, is_pres, is_swap,
> +                                         p, addr, 1);
> +               if (ret)
> +                       break;
> +
> +               if (PM_SCAN_OP_IS_WP(p) && is_writ &&
> +                   uffd_wp_range(walk->mm, vma, addr, PAGE_SIZE, true) < 0)
> +                       ret = -EINVAL;
> +       }
> +
> +       cond_resched();
> +       return ret;
> +}
> +
> +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) {

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;
> +}


> +/* No hugetlb support is present. */

"FIXME: hugetlb support is not implemented."? (There seems to be no
#ifdef CONFIG_HUGETLB or similar, so I guess the comment is about the
current implementation.)

> +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,
> +};
> +
> +static bool pagemap_scan_args_valid(struct pm_scan_arg *arg,
> +                                   struct page_region __user *vec,
> +                                   unsigned long start)
> +{
> +       /* Detect illegal size, flags and masks */
> +       if (arg->size != sizeof(struct pm_scan_arg))
> +               return false;
> +       if (arg->flags & ~PM_SCAN_OPS)
> +               return false;
> +       if ((arg->required_mask | arg->anyof_mask | arg->excluded_mask |
> +            arg->return_mask) & ~PM_SCAN_BITS_ALL)
> +               return false;

> +       if (!arg->required_mask && !arg->anyof_mask &&
> +           !arg->excluded_mask)
> +               return false;

Is there an assumption in the code that those checks are needed? I'd
expect that no selection criteria makes a valid page set?

> +       if (!arg->return_mask)
> +               return false;
> +
> +       /* Validate memory ranges */
> +       if (!(arg->flags & PM_SCAN_OP_GET))
> +               return false;
> +       if (!arg->vec)
> +               return false;
> +       if (arg->vec_len == 0)
> +               return false;

> +       if (!access_ok((void __user *)vec,
> +                      arg->vec_len * sizeof(struct page_region)))
> +               return false;

Is there a provision that userspace threads are all blocked from
manipulating mmaps during this ioctl()? If not, this is a TOCTOU bug
and the writes should be checked each time as another userspace thread
could remap the memory while the ioctl() is working. Anyway, the
return should be EFAULT for this case.

> +       if (!IS_ALIGNED(start, PAGE_SIZE))
> +               return false;
> +       if (!access_ok((void __user *)start, arg->len))
> +               return false;

This I guess want's to check if the range to be scanned is mapped -
but isn't this what the ioctl() should do during the scan? (But, also
see above.)

> +       if (PM_SCAN_OP_IS_WP(arg)) {

if (!...IS_WP) return true;

> +               if (arg->required_mask & PM_SCAN_NON_WT_BITS)
> +                       return false;
> +               if (arg->anyof_mask & PM_SCAN_NON_WT_BITS)
> +                       return false;
> +               if (arg->excluded_mask & PM_SCAN_NON_WT_BITS)
> +                       return false;

Please see: pagemap_scan_is_wt_required comment. Also, it seems this
constant is used only here, so ~PAGE_IS_WRITTEN might be enough?

[...]
> +static long do_pagemap_cmd(struct mm_struct *mm, struct pm_scan_arg *arg)
> +{
> +       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;
> +       int ret = 0;
> +
> +       start = (unsigned long)untagged_addr(arg->start);
> +       vec = (struct page_region *)(unsigned long)untagged_addr(arg->vec);
> +
> +       if (!pagemap_scan_args_valid(arg, vec, start))
> +               return -EINVAL;
> +
> +       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.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) {
> +               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))

if (ret && ret != -ENOSPC)

> +                       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;
> +       struct pm_scan_arg argument;
> +
> +       if (cmd == PAGEMAP_SCAN) {

switch() for easier expansion later?

> +               if (copy_from_user(&argument, uarg,
> +                                  sizeof(struct pm_scan_arg)))

sizeof(*argument);

Could you push this to do_pagemap_cmd()? In case this file gets more
ioctl() commands there won't be need to add more command-specific
structures in this function.

> +                       return -EFAULT;
> +               return do_pagemap_cmd(mm, &argument);
> +       }
> +
> +       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,

Is this correct? Would the code need a different userspace pointer
handling for 32-bit userspace on 64-bit kernel?

>  };
>  #endif /* CONFIG_PROC_PAGE_MONITOR */

Re: [PATCH v11 4/7] fs/proc/task_mmu: Implement IOCTL to get and optionally clear info about PTEs

[Peter Xu]

On Thu, Mar 09, 2023 at 06:57:15PM +0500, Muhammad Usama Anjum wrote:
> +	for (addr = start; !ret && addr < end; pte++, addr += PAGE_SIZE) {
> +		pte = pte_offset_map_lock(vma->vm_mm, pmd, addr, &ptl);
> +
> +		is_writ = !is_pte_uffd_wp(*pte);
> +		is_file = vma->vm_file;
> +		is_pres = pte_present(*pte);
> +		is_swap = is_swap_pte(*pte);
> +
> +		pte_unmap_unlock(pte, ptl);
> +
> +		ret = pagemap_scan_output(is_writ, is_file, is_pres, is_swap,
> +					  p, addr, 1);
> +		if (ret)
> +			break;
> +
> +		if (PM_SCAN_OP_IS_WP(p) && is_writ &&
> +		    uffd_wp_range(walk->mm, vma, addr, PAGE_SIZE, true) < 0)
> +			ret = -EINVAL;
> +	}

This is not real atomic..

Taking the spinlock for eacy pte is not only overkill but wrong in
atomicity because the pte can change right after spinlock unlocked.

Unfortunately you also cannot reuse uffd_wp_range() because that's not
atomic either, my fault here.  Probably I was thinking mostly from
soft-dirty pov on batching the collect+reset.

You need to take the spin lock, collect whatever bits, set/clear whatever
bits, only until then release the spin lock.

"Not atomic" means you can have some page got dirtied but you could miss
it.  Depending on how strict you want, I think it'll break apps like CRIU
if strict atomicity needed for migrating a process.  If we want to have a
new interface anyway, IMHO we'd better do that in the strict way.

Same comment applies to the THP handling (where I cut from the context).

-- 
Peter Xu

Re: [PATCH v11 4/7] fs/proc/task_mmu: Implement IOCTL to get and optionally clear info about PTEs

[Muhammad Usama Anjum]

On 3/15/23 8:55 PM, Peter Xu wrote:
> On Thu, Mar 09, 2023 at 06:57:15PM +0500, Muhammad Usama Anjum wrote:
>> +	for (addr = start; !ret && addr < end; pte++, addr += PAGE_SIZE) {
>> +		pte = pte_offset_map_lock(vma->vm_mm, pmd, addr, &ptl);
>> +
>> +		is_writ = !is_pte_uffd_wp(*pte);
>> +		is_file = vma->vm_file;
>> +		is_pres = pte_present(*pte);
>> +		is_swap = is_swap_pte(*pte);
>> +
>> +		pte_unmap_unlock(pte, ptl);
>> +
>> +		ret = pagemap_scan_output(is_writ, is_file, is_pres, is_swap,
>> +					  p, addr, 1);
>> +		if (ret)
>> +			break;
>> +
>> +		if (PM_SCAN_OP_IS_WP(p) && is_writ &&
>> +		    uffd_wp_range(walk->mm, vma, addr, PAGE_SIZE, true) < 0)
>> +			ret = -EINVAL;
>> +	}
> 
> This is not real atomic..
> 
> Taking the spinlock for eacy pte is not only overkill but wrong in
> atomicity because the pte can change right after spinlock unlocked.
Let me explain. It seems like wrong, but it isn't. In my rigorous testing,
it didn't show any side-effect.  Here we are finding out if a page is
written. If page is written, only then we clear it. Lets look at the
different possibilities here:
- If a page isn't written, we'll not clear it.
- If a page is written and there isn't any race, we'll clear written-to
flag by write protecting it.
- If a page is written but before clearing it, data is written again to the
page. The page would remain written and we'll clear it.
- If a page is written but before clearing it, it gets write protected,
we'll still write protected it. There is double right protection here, but
no side-effect.

Lets turn this into a truth table for easier understanding. Here first
coulmn and thrid column represents this above code. 2nd column represents
any other thread interacting with the page.

If page is written/dirty	some other task interacts	wp_page
no				does nothing			no
no				writes to page			no
no				wp the page			no
yes				does nothing			yes
yes				write to page			yes
yes				wp the page			yes

As you can see there isn't any side-effect happening. We aren't over doing
the wp or under-doing the write-protect.

Even if we were doing something wrong here and I bring the lock over all of
this, the pages get become written or wp just after unlocking. It is
expected. This current implementation doesn't seem to be breaking this.

Is my understanding wrong somewhere here? Can you point out?

Previous to this current locking design were either buggy or slower when
multiple threads were working on same pages. Current implementation removes
the limitations:
- The memcpy inside pagemap_scan_output is happening with pte unlocked.
- We are only wp a page if we have noted this page to be dirty
- No mm write lock is required. Only read lock works fine just like
userfaultfd_writeprotect() takes only read lock.

There is only one con here that we are locking and unlocking the pte lock
again and again.

Please have a look at my explanation and let me know what do you think.

> 
> Unfortunately you also cannot reuse uffd_wp_range() because that's not
> atomic either, my fault here.  Probably I was thinking mostly from
> soft-dirty pov on batching the collect+reset.
> 
> You need to take the spin lock, collect whatever bits, set/clear whatever
> bits, only until then release the spin lock.
> 
> "Not atomic" means you can have some page got dirtied but you could miss
> it.  Depending on how strict you want, I think it'll break apps like CRIU
> if strict atomicity needed for migrating a process.  If we want to have a
> new interface anyway, IMHO we'd better do that in the strict way.
In my rigorous multi-threaded testing where a lots of threads are working
on same set of pages, we aren't losing even a single update. I can share
the test if you want.

> 
> Same comment applies to the THP handling (where I cut from the context).
> 

-- 
BR,
Muhammad Usama Anjum

Re: [PATCH v11 4/7] fs/proc/task_mmu: Implement IOCTL to get and optionally clear info about PTEs

[Peter Xu]

On Wed, Mar 15, 2023 at 09:54:40PM +0500, Muhammad Usama Anjum wrote:
> On 3/15/23 8:55 PM, Peter Xu wrote:
> > On Thu, Mar 09, 2023 at 06:57:15PM +0500, Muhammad Usama Anjum wrote:
> >> +	for (addr = start; !ret && addr < end; pte++, addr += PAGE_SIZE) {
> >> +		pte = pte_offset_map_lock(vma->vm_mm, pmd, addr, &ptl);
> >> +
> >> +		is_writ = !is_pte_uffd_wp(*pte);
> >> +		is_file = vma->vm_file;
> >> +		is_pres = pte_present(*pte);
> >> +		is_swap = is_swap_pte(*pte);
> >> +
> >> +		pte_unmap_unlock(pte, ptl);
> >> +
> >> +		ret = pagemap_scan_output(is_writ, is_file, is_pres, is_swap,
> >> +					  p, addr, 1);
> >> +		if (ret)
> >> +			break;
> >> +
> >> +		if (PM_SCAN_OP_IS_WP(p) && is_writ &&
> >> +		    uffd_wp_range(walk->mm, vma, addr, PAGE_SIZE, true) < 0)
> >> +			ret = -EINVAL;
> >> +	}
> > 
> > This is not real atomic..
> > 
> > Taking the spinlock for eacy pte is not only overkill but wrong in
> > atomicity because the pte can change right after spinlock unlocked.
> Let me explain. It seems like wrong, but it isn't. In my rigorous testing,
> it didn't show any side-effect.  Here we are finding out if a page is
> written. If page is written, only then we clear it. Lets look at the
> different possibilities here:
> - If a page isn't written, we'll not clear it.
> - If a page is written and there isn't any race, we'll clear written-to
> flag by write protecting it.
> - If a page is written but before clearing it, data is written again to the
> page. The page would remain written and we'll clear it.
> - If a page is written but before clearing it, it gets write protected,
> we'll still write protected it. There is double right protection here, but
> no side-effect.
> 
> Lets turn this into a truth table for easier understanding. Here first
> coulmn and thrid column represents this above code. 2nd column represents
> any other thread interacting with the page.
> 
> If page is written/dirty	some other task interacts	wp_page
> no				does nothing			no
> no				writes to page			no
> no				wp the page			no
> yes				does nothing			yes
> yes				write to page			yes
> yes				wp the page			yes
> 
> As you can see there isn't any side-effect happening. We aren't over doing
> the wp or under-doing the write-protect.
> 
> Even if we were doing something wrong here and I bring the lock over all of
> this, the pages get become written or wp just after unlocking. It is
> expected. This current implementation doesn't seem to be breaking this.
> 
> Is my understanding wrong somewhere here? Can you point out?

Yes you're right.  With is_writ check it looks all fine.

> 
> Previous to this current locking design were either buggy or slower when
> multiple threads were working on same pages. Current implementation removes
> the limitations:
> - The memcpy inside pagemap_scan_output is happening with pte unlocked.

Why this has anything to worry?  Isn't that memcpy only applies to a
page_region struct?

> - We are only wp a page if we have noted this page to be dirty
> - No mm write lock is required. Only read lock works fine just like
> userfaultfd_writeprotect() takes only read lock.

I didn't even notice you used to use write lock.  Yes I think read lock is
suffice here.

> 
> There is only one con here that we are locking and unlocking the pte lock
> again and again.
> 
> Please have a look at my explanation and let me know what do you think.

I think this is fine as long as the semantics is correct, which I believe
is the case.  The spinlock can be optimized, but it can be done on top if
needs more involved changes.

> 
> > 
> > Unfortunately you also cannot reuse uffd_wp_range() because that's not
> > atomic either, my fault here.  Probably I was thinking mostly from
> > soft-dirty pov on batching the collect+reset.
> > 
> > You need to take the spin lock, collect whatever bits, set/clear whatever
> > bits, only until then release the spin lock.
> > 
> > "Not atomic" means you can have some page got dirtied but you could miss
> > it.  Depending on how strict you want, I think it'll break apps like CRIU
> > if strict atomicity needed for migrating a process.  If we want to have a
> > new interface anyway, IMHO we'd better do that in the strict way.
> In my rigorous multi-threaded testing where a lots of threads are working
> on same set of pages, we aren't losing even a single update. I can share
> the test if you want.

Good to have tests covering that.  I'd say you can add the test into
selftests along with the series when you repost if it's convenient.  It can
be part of an existing test or it can be a new one under mm/.

Thanks,

-- 
Peter Xu

Re: [PATCH v11 4/7] fs/proc/task_mmu: Implement IOCTL to get and optionally clear info about PTEs

[Muhammad Usama Anjum]

On 3/16/23 12:53 AM, Peter Xu wrote:
> On Wed, Mar 15, 2023 at 09:54:40PM +0500, Muhammad Usama Anjum wrote:
>> On 3/15/23 8:55 PM, Peter Xu wrote:
>>> On Thu, Mar 09, 2023 at 06:57:15PM +0500, Muhammad Usama Anjum wrote:
>>>> +	for (addr = start; !ret && addr < end; pte++, addr += PAGE_SIZE) {
>>>> +		pte = pte_offset_map_lock(vma->vm_mm, pmd, addr, &ptl);
>>>> +
>>>> +		is_writ = !is_pte_uffd_wp(*pte);
>>>> +		is_file = vma->vm_file;
>>>> +		is_pres = pte_present(*pte);
>>>> +		is_swap = is_swap_pte(*pte);
>>>> +
>>>> +		pte_unmap_unlock(pte, ptl);
>>>> +
>>>> +		ret = pagemap_scan_output(is_writ, is_file, is_pres, is_swap,
>>>> +					  p, addr, 1);
>>>> +		if (ret)
>>>> +			break;
>>>> +
>>>> +		if (PM_SCAN_OP_IS_WP(p) && is_writ &&
>>>> +		    uffd_wp_range(walk->mm, vma, addr, PAGE_SIZE, true) < 0)
>>>> +			ret = -EINVAL;
>>>> +	}
>>>
>>> This is not real atomic..
>>>
>>> Taking the spinlock for eacy pte is not only overkill but wrong in
>>> atomicity because the pte can change right after spinlock unlocked.
>> Let me explain. It seems like wrong, but it isn't. In my rigorous testing,
>> it didn't show any side-effect.  Here we are finding out if a page is
>> written. If page is written, only then we clear it. Lets look at the
>> different possibilities here:
>> - If a page isn't written, we'll not clear it.
>> - If a page is written and there isn't any race, we'll clear written-to
>> flag by write protecting it.
>> - If a page is written but before clearing it, data is written again to the
>> page. The page would remain written and we'll clear it.
>> - If a page is written but before clearing it, it gets write protected,
>> we'll still write protected it. There is double right protection here, but
>> no side-effect.
>>
>> Lets turn this into a truth table for easier understanding. Here first
>> coulmn and thrid column represents this above code. 2nd column represents
>> any other thread interacting with the page.
>>
>> If page is written/dirty	some other task interacts	wp_page
>> no				does nothing			no
>> no				writes to page			no
>> no				wp the page			no
>> yes				does nothing			yes
>> yes				write to page			yes
>> yes				wp the page			yes
>>
>> As you can see there isn't any side-effect happening. We aren't over doing
>> the wp or under-doing the write-protect.
>>
>> Even if we were doing something wrong here and I bring the lock over all of
>> this, the pages get become written or wp just after unlocking. It is
>> expected. This current implementation doesn't seem to be breaking this.
>>
>> Is my understanding wrong somewhere here? Can you point out?
> 
> Yes you're right.  With is_writ check it looks all fine.
> 
>>
>> Previous to this current locking design were either buggy or slower when
>> multiple threads were working on same pages. Current implementation removes
>> the limitations:
>> - The memcpy inside pagemap_scan_output is happening with pte unlocked.
> 
> Why this has anything to worry?  Isn't that memcpy only applies to a
> page_region struct?
Yeah, correct. I'm just saying that memcpy without pte lock is better than
memcpy with pte locked. :)

> 
>> - We are only wp a page if we have noted this page to be dirty
>> - No mm write lock is required. Only read lock works fine just like
>> userfaultfd_writeprotect() takes only read lock.
> 
> I didn't even notice you used to use write lock.  Yes I think read lock is
> suffice here.
> 
>>
>> There is only one con here that we are locking and unlocking the pte lock
>> again and again.
>>
>> Please have a look at my explanation and let me know what do you think.
> 
> I think this is fine as long as the semantics is correct, which I believe
> is the case.  The spinlock can be optimized, but it can be done on top if
> needs more involved changes.
> 
>>
>>>
>>> Unfortunately you also cannot reuse uffd_wp_range() because that's not
>>> atomic either, my fault here.  Probably I was thinking mostly from
>>> soft-dirty pov on batching the collect+reset.
>>>
>>> You need to take the spin lock, collect whatever bits, set/clear whatever
>>> bits, only until then release the spin lock.
>>>
>>> "Not atomic" means you can have some page got dirtied but you could miss
>>> it.  Depending on how strict you want, I think it'll break apps like CRIU
>>> if strict atomicity needed for migrating a process.  If we want to have a
>>> new interface anyway, IMHO we'd better do that in the strict way.
>> In my rigorous multi-threaded testing where a lots of threads are working
>> on same set of pages, we aren't losing even a single update. I can share
>> the test if you want.
> 
> Good to have tests covering that.  I'd say you can add the test into
> selftests along with the series when you repost if it's convenient.  It can
> be part of an existing test or it can be a new one under mm/.
Sure, I'll add it to the selftests.

Thank you for reviewing and asking the questions.

> 
> Thanks,
> 

-- 
BR,
Muhammad Usama Anjum

Re: [PATCH v11 2/7] userfaultfd: Define dummy uffd_wp_range()

[Mike Rapoport]

Hi,

On Thu, Mar 09, 2023 at 06:57:13PM +0500, Muhammad Usama Anjum wrote:
> Define uffd_wp_range() for the cases when CONFIG_USERFAULTFD isn't set.
> 
> Signed-off-by: Muhammad Usama Anjum <usama.anjum@collabora.com>
> ---
>  include/linux/userfaultfd_k.h | 8 ++++++++
>  1 file changed, 8 insertions(+)
> 
> diff --git a/include/linux/userfaultfd_k.h b/include/linux/userfaultfd_k.h
> index b680c0ec8b85..fd1a1ecdb5f6 100644
> --- a/include/linux/userfaultfd_k.h
> +++ b/include/linux/userfaultfd_k.h
> @@ -182,6 +182,14 @@ extern int userfaultfd_wp_async(struct vm_area_struct *vma);
>  
>  #else /* CONFIG_USERFAULTFD */
>  
> +extern inline long uffd_wp_range(struct mm_struct *dst_mm,

static inline

> +				 struct vm_area_struct *vma,
> +				 unsigned long start, unsigned long len,
> +				 bool enable_wp)
> +{
> +	return 0;
> +}
> +

I didn't see uffd_wp_range() defined in the previous patch.
Could be a rebase issue?

In any case, the stub should be defined in the same patch as the actual
function in order not to break bisectability.

>  /* mm helpers */
>  static inline vm_fault_t handle_userfault(struct vm_fault *vmf,
>  				unsigned long reason)
> -- 
> 2.39.2
> 

-- 
Sincerely yours,
Mike.

Re: [PATCH v11 4/7] fs/proc/task_mmu: Implement IOCTL to get and optionally clear info about PTEs

[Muhammad Usama Anjum]

Hi,

Thank you so much for reviewing.

On 3/13/23 9:02 PM, Michał Mirosław wrote:
> On Thu, 9 Mar 2023 at 14:58, 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)
> [...]
>> --- 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>
>> @@ -1132,6 +1133,18 @@ 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)));
> 
> Parentheses around pte_swp_uffd_wp_any() are redundant. Please remove
> here and in all following if()s. (Nit: those extra parentheses are
> used inconsistently in the patch anyway.)
I'll remove these in next version.

> 
> [...]
>> +static inline bool pagemap_scan_is_wt_required(struct pagemap_scan_private *p)
> 
> This seems to check if the PAGE_IS_WRITTEN flag is tested, so
> "pagemap_scan_needs_wp_checks()"? Or maybe document/expand the "wt"
> acronym as it seems used also on following code.
I'll expand wt.

> 
>> +{
>> +       return  ((p->required_mask & PAGE_IS_WRITTEN) ||
>> +                (p->anyof_mask & PAGE_IS_WRITTEN) ||
>> +                (p->excluded_mask & PAGE_IS_WRITTEN));
> 
> Nit: It looks like it should answer "do any of the masks contain
> PAGE_IS_WRITTEN?" so maybe:
> 
> return (p->required_mask | p->anyof_mask | p->excluded_mask) & PAGE_IS_WRITTEN;
I'll update.

> 
> [...]
> 
>> +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;
>> +       bool cpy = true;
>> +
>> +       if (p->max_pages && (p->found_pages == p->max_pages))
>> +               return -ENOSPC;
>> +
>> +       if (!n_pages)
>> +               return -EINVAL;
>> +
>> +       if (p->required_mask)
>> +               cpy = ((p->required_mask & bitmap) == p->required_mask);
>> +       if (cpy && p->anyof_mask)
>> +               cpy = (p->anyof_mask & bitmap);
>> +       if (cpy && p->excluded_mask)
>> +               cpy = !(p->excluded_mask & bitmap);
> 
> Since the rest of the code is executed only when `cpy` is true, this
> could just return early for easier understanding.
Hmm... I'll do the following:
	if (!cpy || !bitmap)
		return 0;

> 
> BTW, some of the tests are redundant. Eg: if required_mask == 0, then
> `required_mask & x == required_mask` will always hold. Same for
> `excluded_mask & x == 0`.
Correct. This is why I'm checking if required_mask is set and then
comparing bitmap with it. required_mask may be 0 if not set. This if will
ignore the subsequent check.

	if (p->required_mask)
		cpy = ((p->required_mask & bitmap) == p->required_mask);

I don't see any redundancy here. Please let me know otherwise?

> 
>> +
>> +       bitmap = bitmap & p->return_mask;
> 
> Nit: bitmap &= p->return_mask;
Sure. Will do.

Just for my knowledge, what does "Nit" signify if a comment is marked with it?

> 
>> +       if (cpy && bitmap) {
> 
> Assuming early returns on `!cpy` are done earlier:
> 
> if (!bitmap)
>   return 0;
I've posted condition above which would better suit here.

> 
>> +               if ((cur->len) && (cur->bitmap == bitmap) &&
>> +                   (cur->start + cur->len * PAGE_SIZE == addr)) {
> 
> I'd recommend removing the extra parentheses as they make the code
> less readable for me (too many parentheses to match visually).
I'll remove parenthesis.

> The `cur->len` test seems redundant: is it possible to have
> `cur->start == addr` in that case (I guess it would have to get
> `n_pages == 0` in an earlier invocation)?
No, both wouldn't work. cur->len == 0 means that it has only garbage. It is
essential to check the validity from cur->len before performing other
checks. Also cur->start can never be equal to addr as we are walking over
page addressing in serial manner. We want to see here if the current
address matches the previous data by finding the ending address of last
stored data (cur->start + cur->len * PAGE_SIZE).

> 
>> +
>> +                       cur->len += n_pages;
>> +                       p->found_pages += n_pages;
> 
> Please add an early return so that 'else' chaining won't be necessary.
I'll do it.

> 
>> +               } else if ((!p->vec_index) ||
>> +                          ((p->vec_index + 1) < p->vec_len)) {
> 
> Can you explain this test? Why not just `p->vec_index < p->vec_len`? Or better:
> 
> if (vec_index >= p->vec_len)
>     return -ENOSPC;

No, it'll not work. Lets leave it as it is. :)

It has gotten somewhat complex, but I don't have any other way to make it
simpler which works. First note the following points:
1) We walk over 512 page or 1 thp at a time to not over allocate memory in
kernel (p->vec).
2) We also want to merge the consective pages with same flags into one
struct page_region. p->vec of current walk may merge with next walk. So we
cannot write to user memory until we find the results of the next walk.

So most recent data is put into p->cur. When non-intersecting or mergeable
data is found, we move p->cur to p->vec[p->index] inside the page walk.
After the page walk, p->vec[0 to p->index] is moved to arg->vec. After all
the walks are over. We move the p->cur to arg->vec. It completes the data
transfer to user buffer.

	--------------
	|   p->cur   |
	--------------
	      |
	      |
	      V
	--------------
	|	     |
	|	     |
	|   p->vec   |
	|	     |
	|	     |
	--------------
	      |
	      |
	      V
	--------------
	|	     |
	|	     |
	|	     |
	|  arg->vec  |
	|	     |
	|	     |
	|	     |
	--------------


I'm so sorry that it has gotten this much complex. It was way simpler when
we were walking over all the memory in one go. But then we needed an
unbounded memory from the kernel which we don't want.

> 
>> +                       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;
>> +               } else {
>> +                       return -ENOSPC;
>> +               }
>> +       }
>> +
>> +       return 0;
>> +}
> [...]
> 
>> +static 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) {
> 
> if (!cur->len)
>   return 0;
Sure.

> 
>> +               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;
>> +       struct vm_area_struct *vma = walk->vma;
>> +       bool is_writ, is_file, is_pres, is_swap;
>> +       unsigned long addr = end;
>> +       spinlock_t *ptl;
>> +       int ret = 0;
>> +       pte_t *pte;
>> +
>> +#ifdef CONFIG_TRANSPARENT_HUGEPAGE
> 
> Is the `#ifdef` needed? `pmd_trans_huge_lock()` will always return
> NULL if transparent hugepages are not compiled in. OTOH I see
> BUILD_BUG() is possible in HPAGE_SIZE definition (irrelevant in this
> case), so that would need to be worked around first.
I'd got the build error [1]. So I'd added these. I've tested it again with
the same config. We don't need these #ifdef now. I'll remove these.

[1] https://lore.kernel.org/all/202211120107.cYLiq2cH-lkp@intel.com

> 
>> +       ptl = pmd_trans_huge_lock(pmd, vma);
>> +       if (ptl) {
>> +               unsigned long n_pages;
>> +
>> +               is_writ = !is_pmd_uffd_wp(*pmd);
> 
> `is_written`?
I'd kept it is_writ to match the pattern of is_file, is_pres and is_swap.
I'll update it to is_written and is_pres to is_present.

> 
>> +               /*
>> +                * Break huge page into small pages if operation needs to be
>> +                * performed is on a portion of the huge page.
>> +                */
>> +               if (is_writ && PM_SCAN_OP_IS_WP(p) &&
>> +                   (end - start < HPAGE_SIZE)) {
>> +                       spin_unlock(ptl);
>> +
>> +                       split_huge_pmd(vma, pmd, start);
>> +                       goto process_smaller_pages;
>> +               }
>> +
>> +               n_pages = (end - start)/PAGE_SIZE;
>> +               if (p->max_pages &&
>> +                   p->found_pages + n_pages >= p->max_pages)
> 
> Nit: greater-than is also correct and avoids no-op assignment.
Ohh... I'll update.

> 
>> +                       n_pages = p->max_pages - p->found_pages;
>> +
>> +               ret = pagemap_scan_output(is_writ, vma->vm_file,
>> +                                         pmd_present(*pmd), is_swap_pmd(*pmd),
>> +                                         p, start, n_pages);
>> +               spin_unlock(ptl);
> 
> if (ret || !is_written)
>   return ret;
> 
> This will avoid those tests in the following if().
Done.

> 
>> +
>> +               if (!ret && is_writ && PM_SCAN_OP_IS_WP(p) &&
>> +                   uffd_wp_range(walk->mm, vma, start, HPAGE_SIZE, true) < 0)
>> +                       ret = -EINVAL;
>> +
>> +               return ret;
> 
> After above early returns, this will be always `return 0;`.
Sure.

> 
>> +       }
>> +process_smaller_pages:
>> +       if (pmd_trans_unstable(pmd))
>> +               return 0;
>> +#endif /* CONFIG_TRANSPARENT_HUGEPAGE */
>> +
>> +       for (addr = start; !ret && addr < end; pte++, addr += PAGE_SIZE) {
> 
> The `!ret` can be removed if the EINVAL case was to `break` by itself.
Sure. Will do.

> 
>> +               pte = pte_offset_map_lock(vma->vm_mm, pmd, addr, &ptl);
>> +
>> +               is_writ = !is_pte_uffd_wp(*pte);
>> +               is_file = vma->vm_file;
>> +               is_pres = pte_present(*pte);
>> +               is_swap = is_swap_pte(*pte);
>> +
>> +               pte_unmap_unlock(pte, ptl);
>> +
>> +               ret = pagemap_scan_output(is_writ, is_file, is_pres, is_swap,
>> +                                         p, addr, 1);
>> +               if (ret)
>> +                       break;
>> +
>> +               if (PM_SCAN_OP_IS_WP(p) && is_writ &&
>> +                   uffd_wp_range(walk->mm, vma, addr, PAGE_SIZE, true) < 0)
>> +                       ret = -EINVAL;
>> +       }
>> +
>> +       cond_resched();
>> +       return ret;
>> +}
>> +
>> +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) {
> 
> if (!vma) return 0;
Will do.

> 
>> +               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;
>> +}
> 
> 
>> +/* No hugetlb support is present. */
> 
> "FIXME: hugetlb support is not implemented."? (There seems to be no
> #ifdef CONFIG_HUGETLB or similar, so I guess the comment is about the
> current implementation.)
I'm working on adding hugetlb support. I'll remove this comment.

> 
>> +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,
>> +};
>> +
>> +static bool pagemap_scan_args_valid(struct pm_scan_arg *arg,
>> +                                   struct page_region __user *vec,
>> +                                   unsigned long start)
>> +{
>> +       /* Detect illegal size, flags and masks */
>> +       if (arg->size != sizeof(struct pm_scan_arg))
>> +               return false;
>> +       if (arg->flags & ~PM_SCAN_OPS)
>> +               return false;
>> +       if ((arg->required_mask | arg->anyof_mask | arg->excluded_mask |
>> +            arg->return_mask) & ~PM_SCAN_BITS_ALL)
>> +               return false;
> 
>> +       if (!arg->required_mask && !arg->anyof_mask &&
>> +           !arg->excluded_mask)
>> +               return false;
> 
> Is there an assumption in the code that those checks are needed? I'd
> expect that no selection criteria makes a valid page set?
In my view, selection criterion must be specified for the ioctl to work. If
there is no criterio, user should go and read pagemap file directly. So the
assumption is that at least one selection criterion must be specified.

> 
>> +       if (!arg->return_mask)
>> +               return false;
>> +
>> +       /* Validate memory ranges */
>> +       if (!(arg->flags & PM_SCAN_OP_GET))
>> +               return false;
>> +       if (!arg->vec)
>> +               return false;
>> +       if (arg->vec_len == 0)
>> +               return false;
> 
>> +       if (!access_ok((void __user *)vec,
>> +                      arg->vec_len * sizeof(struct page_region)))
>> +               return false;
> 
> Is there a provision that userspace threads are all blocked from
> manipulating mmaps during this ioctl()? If not, this is a TOCTOU bug
> and the writes should be checked each time as another userspace thread
> could remap the memory while the ioctl() is working.
mincore() syscall is doing in the same way. It checks the validity in the
start only. What provision should I add? Isn't it obvious that the user
should not remap such memory?

> Anyway, the
> return should be EFAULT for this case.
I'll update.

> 
>> +       if (!IS_ALIGNED(start, PAGE_SIZE))
>> +               return false;
>> +       if (!access_ok((void __user *)start, arg->len))
>> +               return false;
> 
> This I guess want's to check if the range to be scanned is mapped -
> but isn't this what the ioctl() should do during the scan? (But, also
> see above.)
No, start represents the memory which the user wants to watch. User must
allocate this memory first and then pass the address to this ioctl to find
out the flags per page.

> 
>> +       if (PM_SCAN_OP_IS_WP(arg)) {
> 
> if (!...IS_WP) return true;
I was liking this way. Anyways I'll update.

> 
>> +               if (arg->required_mask & PM_SCAN_NON_WT_BITS)
>> +                       return false;
>> +               if (arg->anyof_mask & PM_SCAN_NON_WT_BITS)
>> +                       return false;
>> +               if (arg->excluded_mask & PM_SCAN_NON_WT_BITS)
>> +                       return false;
> 
> Please see: pagemap_scan_is_wt_required comment. Also, it seems this
> constant is used only here, so ~PAGE_IS_WRITTEN might be enough?
Yup, I'll update.

> 
> [...]
>> +static long do_pagemap_cmd(struct mm_struct *mm, struct pm_scan_arg *arg)
>> +{
>> +       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;
>> +       int ret = 0;
>> +
>> +       start = (unsigned long)untagged_addr(arg->start);
>> +       vec = (struct page_region *)(unsigned long)untagged_addr(arg->vec);
>> +
>> +       if (!pagemap_scan_args_valid(arg, vec, start))
>> +               return -EINVAL;
>> +
>> +       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.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) {
>> +               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))
> 
> if (ret && ret != -ENOSPC)
Sorry, I should have thought of this one. Thanks.

> 
>> +                       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;
>> +       struct pm_scan_arg argument;
>> +
>> +       if (cmd == PAGEMAP_SCAN) {
> 
> switch() for easier expansion later?
I'd switch here once. I'll add it again.

> 
>> +               if (copy_from_user(&argument, uarg,
>> +                                  sizeof(struct pm_scan_arg)))
> 
> sizeof(*argument);
> 
> Could you push this to do_pagemap_cmd()? In case this file gets more
> ioctl() commands there won't be need to add more command-specific
> structures in this function.
Sure, I'll update.

> 
>> +                       return -EFAULT;
>> +               return do_pagemap_cmd(mm, &argument);
>> +       }
>> +
>> +       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,
> 
> Is this correct? Would the code need a different userspace pointer
> handling for 32-bit userspace on 64-bit kernel?
Yeah, it is needed for 32-bit application to run on 64-bit kernel.

> 
>>  };
>>  #endif /* CONFIG_PROC_PAGE_MONITOR */

-- 
BR,
Muhammad Usama Anjum

Re: [PATCH v11 2/7] userfaultfd: Define dummy uffd_wp_range()

[Muhammad Usama Anjum]

On 3/16/23 12:02 PM, Mike Rapoport wrote:
> Hi,
> 
> On Thu, Mar 09, 2023 at 06:57:13PM +0500, Muhammad Usama Anjum wrote:
>> Define uffd_wp_range() for the cases when CONFIG_USERFAULTFD isn't set.
>>
>> Signed-off-by: Muhammad Usama Anjum <usama.anjum@collabora.com>
>> ---
>>  include/linux/userfaultfd_k.h | 8 ++++++++
>>  1 file changed, 8 insertions(+)
>>
>> diff --git a/include/linux/userfaultfd_k.h b/include/linux/userfaultfd_k.h
>> index b680c0ec8b85..fd1a1ecdb5f6 100644
>> --- a/include/linux/userfaultfd_k.h
>> +++ b/include/linux/userfaultfd_k.h
>> @@ -182,6 +182,14 @@ extern int userfaultfd_wp_async(struct vm_area_struct *vma);
>>  
>>  #else /* CONFIG_USERFAULTFD */
>>  
>> +extern inline long uffd_wp_range(struct mm_struct *dst_mm,
> 
> static inline
I'll update.

> 
>> +				 struct vm_area_struct *vma,
>> +				 unsigned long start, unsigned long len,
>> +				 bool enable_wp)
>> +{
>> +	return 0;
>> +}
>> +
> 
> I didn't see uffd_wp_range() defined in the previous patch.
> Could be a rebase issue?
> 
> In any case, the stub should be defined in the same patch as the actual
> function in order not to break bisectability.
This 2/7 patch is a preparatory patch for 3/7 patch. I'll merge both then.

> 
>>  /* mm helpers */
>>  static inline vm_fault_t handle_userfault(struct vm_fault *vmf,
>>  				unsigned long reason)
>> -- 
>> 2.39.2
>>
> 

-- 
BR,
Muhammad Usama Anjum

Re: [PATCH v11 1/7] userfaultfd: Add UFFD WP Async support

[Peter Xu]

[-- Attachment #1: Type: text/plain, Size: 2354 bytes --]

Hello, Muhammad,

On Thu, Mar 09, 2023 at 06:57:12PM +0500, Muhammad Usama Anjum wrote:
> Add new WP Async mode (UFFD_FEATURE_WP_ASYNC) which resolves the page
> faults on its own. It can be used to track that which pages have been
> written-to from the time the pages were write-protected. It is very
> efficient way to track the changes as uffd is by nature pte/pmd based.
> 
> UFFD synchronous WP sends the page faults to the userspace where the
> pages which have been written-to can be tracked. But it is not efficient.
> This is why this asynchronous version is being added. After setting the
> WP Async, the pages which have been written to can be found in the pagemap
> file or information can be obtained from the PAGEMAP_IOCTL.
> 
> Suggested-by: Peter Xu <peterx@redhat.com>
> Signed-off-by: Muhammad Usama Anjum <usama.anjum@collabora.com>

Here's the patch that can enable WP_ASYNC for all kinds of memories (as I
promised..).  Currently I only tested btrfs (besides the common three)
which is the major fs I use locally, but I guess it'll also enable the rest
no matter what's underneath, just like soft-dirty.

As I mentioned, I just feel it very unfortunate to have a lot of suffixes
for the UFFD_FEATURE_* on types of memory, and I hope we get rid of it for
this WP_ASYNC from the start because the workflow should really be similar
to anon/shmem handling for most of the rest, just a few tweaks here and
there.

I had a feeling that some type of special VMA will work weirdly, but let's
see.. so far I don't come up with any.

If the patch looks fine to you, please consider replace this patch with
patch 1 of mine where I attached.  Then patch 1 can be reviewed alongside
with your series.

Logically patch 1 can be reviewed separately too, because it works
perfectly afaiu without the atomic version of pagemap already.  But on my
side I don't think it justifies anything really matters, so unless someone
thinks it a good idea to post / review / merge it separately, you can keep
that with your new pagemap ioctl.

Patch 2 is only for your reference.  It's not for merging quality so please
don't put it into your series.  I do plan to cleanup the userfaultfd
selftests in the near future first (when I wrote this I am more eager to do
so..).  I also think your final pagemap test cases can cover quite a bit.

Thanks,

-- 
Peter Xu

[-- Attachment #2: 0001-userfaultfd-UFFD_FEATURE_WP_ASYNC.patch --]
[-- Type: text/plain, Size: 16602 bytes --]

From 69640a7df80c147f43b2926ec0b189343acc3362 Mon Sep 17 00:00:00 2001
From: Muhammad Usama Anjum <usama.anjum@collabora.com>
Date: Thu, 9 Mar 2023 18:57:12 +0500
Subject: [PATCH 1/2] userfaultfd: UFFD_FEATURE_WP_ASYNC

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 existance 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.

Signed-off-by: Muhammad Usama Anjum <usama.anjum@collabora.com>
Signed-off-by: Peter Xu <peterx@redhat.com>
---
 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 16843d5a4f65..2f924a33beff 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 8395605790f6..a3cb05cfbd9b 100644
--- a/fs/userfaultfd.c
+++ b/fs/userfaultfd.c
@@ -108,6 +108,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
@@ -1317,6 +1322,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;
 
@@ -1390,7 +1396,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;
 
 		/*
@@ -1449,7 +1455,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));
@@ -1548,6 +1554,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)))
@@ -1601,7 +1608,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;
@@ -1614,7 +1621,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
@@ -1951,6 +1958,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)
 {
 	/*
@@ -1982,6 +1994,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
@@ -1994,6 +2011,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 a2c53e98dfd6..19e81df921a5 100644
--- a/include/linux/userfaultfd_k.h
+++ b/include/linux/userfaultfd_k.h
@@ -159,11 +159,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
@@ -173,6 +184,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);
 }
@@ -195,6 +208,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 */
 
@@ -295,6 +309,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 8bfd07f4c143..88ab9fc56856 100644
--- a/mm/hugetlb.c
+++ b/mm/hugetlb.c
@@ -6084,21 +6084,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 c5f1bf906d0c..da639438af2b 100644
--- a/mm/memory.c
+++ b/mm/memory.c
@@ -3327,11 +3327,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;
 		}
 
 		/*
@@ -4803,8 +4820,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);
 	}
 
@@ -4816,6 +4836,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);
 
-- 
2.39.1


[-- Attachment #3: 0002-selftests-mm-Add-userfaultfd-unit-test.patch --]
[-- Type: text/plain, Size: 11052 bytes --]

From b948ecb4d42560e263a27e5be6cb361b444035fc Mon Sep 17 00:00:00 2001
From: Peter Xu <peterx@redhat.com>
Date: Wed, 15 Mar 2023 12:23:08 -0400
Subject: [PATCH 2/2] selftests/mm: Add userfaultfd unit test

Add a new test for userfaultfd unit test.

Signed-off-by: Peter Xu <peterx@redhat.com>
---
 tools/testing/selftests/mm/.gitignore         |   1 +
 tools/testing/selftests/mm/Makefile           |   2 +
 .../selftests/mm/userfaultfd-unit-test.c      | 407 ++++++++++++++++++
 3 files changed, 410 insertions(+)
 create mode 100644 tools/testing/selftests/mm/userfaultfd-unit-test.c

diff --git a/tools/testing/selftests/mm/.gitignore b/tools/testing/selftests/mm/.gitignore
index 1f8c36a9fa10..7404f27cba8d 100644
--- a/tools/testing/selftests/mm/.gitignore
+++ b/tools/testing/selftests/mm/.gitignore
@@ -22,6 +22,7 @@ protection_keys_32
 protection_keys_64
 madv_populate
 userfaultfd
+userfaultfd-unit-test
 mlock-intersect-test
 mlock-random-test
 virtual_address_range
diff --git a/tools/testing/selftests/mm/Makefile b/tools/testing/selftests/mm/Makefile
index fbf5646b1072..a540ab7c84ec 100644
--- a/tools/testing/selftests/mm/Makefile
+++ b/tools/testing/selftests/mm/Makefile
@@ -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_FILES += userfaultfd-unit-test
 TEST_GEN_PROGS += soft-dirty
 TEST_GEN_PROGS += split_huge_page_test
 TEST_GEN_FILES += ksm_tests
@@ -111,6 +112,7 @@ $(OUTPUT)/madv_populate: vm_util.c
 $(OUTPUT)/soft-dirty: vm_util.c
 $(OUTPUT)/split_huge_page_test: vm_util.c
 $(OUTPUT)/userfaultfd: vm_util.c
+$(OUTPUT)/userfaultfd-unit-test: vm_util.c
 
 ifeq ($(MACHINE),x86_64)
 BINARIES_32 := $(patsubst %,$(OUTPUT)/%,$(BINARIES_32))
diff --git a/tools/testing/selftests/mm/userfaultfd-unit-test.c b/tools/testing/selftests/mm/userfaultfd-unit-test.c
new file mode 100644
index 000000000000..cd7b4f564f3b
--- /dev/null
+++ b/tools/testing/selftests/mm/userfaultfd-unit-test.c
@@ -0,0 +1,407 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Userfaultfd unit tests.
+ *
+ *  Copyright (C) 2023  Red Hat, Inc.
+ */
+#define _GNU_SOURCE
+#include <stdio.h>
+#include <errno.h>
+#include <unistd.h>
+#include <stdlib.h>
+#include <sys/types.h>
+#include <sys/stat.h>
+#include <fcntl.h>
+#include <time.h>
+#include <signal.h>
+#include <poll.h>
+#include <string.h>
+#include <linux/mman.h>
+#include <sys/mman.h>
+#include <sys/syscall.h>
+#include <sys/ioctl.h>
+#include <sys/wait.h>
+#include <pthread.h>
+#include <linux/userfaultfd.h>
+#include <setjmp.h>
+#include <stdbool.h>
+#include <assert.h>
+#include <inttypes.h>
+#include <stdint.h>
+#include <sys/random.h>
+#include <semaphore.h>
+
+#include "../kselftest.h"
+#include "vm_util.h"
+
+/* 128MB */
+#define  ASYNC_MEM_SIZE  (128UL << 20)
+
+#define ALIGN_DOWN(x,a) ((x) & ~((a) - 1))
+#define PAGE_SIZE 4096
+
+int pagemap_fd;
+/* TODO: detect this */
+int thp_size = 2UL << 20;
+
+typedef unsigned char *(*mem_map_fn)(size_t size);
+
+unsigned char *anon_mem_map(size_t size);
+unsigned char *shmem_mem_map(size_t size);
+unsigned char *hugetlb_mem_map(size_t size);
+unsigned char *localfs_mem_map(size_t size);
+
+typedef enum {
+	TEST_ASYNC_ANON = 0,
+	TEST_ASYNC_SHMEM,
+	TEST_ASYNC_HUGETLB,
+	/* Direct test on `pwd`, with the hope that it's a local file system. */
+	TEST_ASYNC_LOCAL_FS,
+	TEST_ASYNC_NUM,
+} test_async_type;
+
+typedef struct {
+	const char *name;
+	/* madvise() used to zap a pte (only, but keep the data) */
+	int zap_madvise;
+	int page_size;
+	mem_map_fn mem_map;
+} async_ops_t;
+
+async_ops_t async_ops[TEST_ASYNC_NUM] = {
+	{
+		.name = "anonymous",
+		.zap_madvise = MADV_PAGEOUT,
+		.mem_map = anon_mem_map,
+		.page_size = PAGE_SIZE,
+	},
+	{
+		.name = "shmem",
+		.zap_madvise = MADV_DONTNEED,
+		.mem_map = shmem_mem_map,
+		.page_size = PAGE_SIZE,
+	},
+	{
+		.name = "hugetlb",
+		.zap_madvise = MADV_DONTNEED,
+		.mem_map = hugetlb_mem_map,
+		.page_size = (2UL << 20),
+	},
+	{
+		.name = "local-fs",
+		.zap_madvise = MADV_DONTNEED,
+		.mem_map = localfs_mem_map,
+		.page_size = PAGE_SIZE,
+	},
+};
+
+//#define debug(...) printf(__VA_ARGS__)
+#define debug(...)
+
+#define _err(fmt, ...)						\
+	do {							\
+		int ret = errno;				\
+		fprintf(stderr, "ERROR: " fmt, ##__VA_ARGS__);	\
+		fprintf(stderr, " (errno=%d, line=%d)\n",	\
+			ret, __LINE__);				\
+	} while (0)
+
+#define errexit(exitcode, fmt, ...)		\
+	do {					\
+		_err(fmt, ##__VA_ARGS__);	\
+		exit(exitcode);			\
+	} while (0)
+
+#define err(fmt, ...) errexit(1, fmt, ##__VA_ARGS__)
+
+#define PM_UFFD_WP                    (1UL << 57)
+
+#define  pagemap_check_wp(addr, wp) do {				\
+		if (!!(pagemap_get_entry(pagemap_fd, \
+					 (char *)addr) & PM_UFFD_WP) != wp) \
+			err("pagemap uffd-wp bit error: addr=0x%lx", \
+			    (unsigned long)addr);		     \
+	} while (0)
+
+static uint64_t random_uint64(void)
+{
+	uint64_t value;
+	int ret;
+
+	ret = getrandom(&value, sizeof(value), 0);
+	assert(ret == sizeof(value));
+
+	return value;
+}
+
+static int __userfaultfd_open_dev(void)
+{
+	int fd, _uffd;
+
+	fd = open("/dev/userfaultfd", O_RDWR | O_CLOEXEC);
+	if (fd < 0) {
+		perror("open(/dev/userfaultfd)");
+		return -1;
+	}
+
+	_uffd = ioctl(fd, USERFAULTFD_IOC_NEW, O_CLOEXEC);
+	if (_uffd < 0) {
+		perror("USERFAULTFD_IOC_NEW");
+		close(fd);
+		return -1;
+	}
+	close(fd);
+	return _uffd;
+}
+
+static void wp_range(int ufd, __u64 start, __u64 len, bool wp)
+{
+	struct uffdio_writeprotect prms;
+
+	/* Write protection page faults */
+	prms.range.start = start;
+	prms.range.len = len;
+	/* Undo write-protect, do wakeup after that */
+	prms.mode = wp ? UFFDIO_WRITEPROTECT_MODE_WP : 0;
+
+	if (ioctl(ufd, UFFDIO_WRITEPROTECT, &prms))
+		err("clear WP failed: address=0x%"PRIx64, (uint64_t)start);
+}
+
+unsigned char *anon_mem_map(size_t size)
+{
+	unsigned char *buffer;
+
+	buffer = mmap(NULL, size, PROT_READ|PROT_WRITE,
+		      MAP_PRIVATE|MAP_ANONYMOUS, -1, 0);
+
+	if (buffer == MAP_FAILED)
+		err("mmap(MAP_HUGETLB)");
+
+	return buffer;
+}
+
+unsigned char *shmem_mem_map(size_t size)
+{
+	unsigned char *buffer;
+
+	buffer = mmap(NULL, size, PROT_READ|PROT_WRITE,
+		      MAP_SHARED|MAP_ANONYMOUS, -1, 0);
+
+	if (buffer == MAP_FAILED)
+		err("mmap(MAP_HUGETLB)");
+
+	return buffer;
+}
+
+unsigned char *hugetlb_mem_map(size_t size)
+{
+	unsigned char *buffer;
+
+	buffer = mmap(NULL, size, PROT_READ|PROT_WRITE,
+		      MAP_SHARED|MAP_ANONYMOUS|MAP_HUGETLB|MAP_HUGE_2MB,
+		      -1, 0);
+
+	if (buffer == MAP_FAILED)
+		err("mmap(MAP_HUGETLB)");
+
+	return buffer;
+}
+
+unsigned char *localfs_mem_map(size_t size)
+{
+#define  TMP_LOCAL_PATH  "./test-async"
+	int ret, fd = open(TMP_LOCAL_PATH, O_RDWR | O_CREAT, 0644);
+	unsigned char *buffer = NULL;
+
+	if (fd < 0) {
+		perror("open()");
+		return NULL;
+	}
+
+	ret = ftruncate(fd, size);
+	if (ret) {
+		perror("ftruncate()");
+		close(fd);
+		return NULL;
+	}
+
+	buffer = mmap(NULL, size, PROT_READ|PROT_WRITE, MAP_SHARED, fd, 0);
+	if (buffer == MAP_FAILED) {
+		perror("mmap()");
+		close(fd);
+		return NULL;
+	}
+	close(fd);
+	unlink(TMP_LOCAL_PATH);
+
+	return buffer;
+}
+
+typedef struct {
+	async_ops_t *ops;
+	unsigned char *buffer;
+	size_t size;
+	volatile bool quit;
+} async_args_t;
+
+static void *uffd_zap_thread(void *data)
+{
+	async_args_t *args = data;
+	unsigned char *start = args->buffer;
+	size_t size = args->size;
+	async_ops_t *ops = args->ops;
+	unsigned int psize = ops->page_size;
+	int zap_madvise = ops->zap_madvise;
+	uint64_t offset;
+
+	debug("zap thread started\n");
+
+	while (!args->quit) {
+		offset = ALIGN_DOWN(random_uint64() % size, psize);
+		madvise(start + offset, psize, zap_madvise);
+		usleep(100);
+	}
+
+	debug("zap thread ended\n");
+
+	return NULL;
+}
+
+static bool bitmap_test(unsigned long *bitmap, unsigned long bit)
+{
+	unsigned long nbits = sizeof(unsigned long) * 8;
+
+	return bitmap[bit / nbits] & (1UL << (bit % nbits));
+}
+
+static bool test_async(async_ops_t *ops, int uffd,
+		       unsigned char *buffer, size_t size)
+{
+#define  LOOP_N  5
+	unsigned long *bitmap, npages, i;
+	pthread_t zap_tid;
+	async_args_t args = {
+		.buffer = buffer,
+		.size = size,
+		.ops = ops,
+		.quit = false,
+	};
+	int loops;
+
+	npages = size / ops->page_size;
+	bitmap = calloc(1, npages / 8);
+	assert(bitmap);
+
+	/* Random prefaults */
+	getrandom(bitmap, npages / 8, 0);
+	for (i = 0; i < npages; i++) {
+		if (bitmap_test(bitmap, i)) {
+			debug("prefault page %ld as write\n", i);
+			*(buffer + i * ops->page_size) = 1;
+		}
+	}
+
+	/* Create zapper to randomly zap pgtables */
+	pthread_create(&zap_tid, NULL, uffd_zap_thread, &args);
+
+	for (loops = 0; loops < LOOP_N; loops++) {
+		/* Start tracking, or reset, on all pages */
+		wp_range(uffd, (uintptr_t)buffer, size, true);
+
+		/* Random writes */
+		getrandom(bitmap, npages / 8, 0);
+		for (i = 0; i < npages; i++) {
+			if (bitmap_test(bitmap, i)) {
+				debug("update page %ld\n", i);
+				*(buffer + i * ops->page_size) = 2;
+			}
+		}
+
+		/* Verify pagemap tracked all the writes */
+		for (i = 0; i < npages; i++) {
+			debug("check page %ld\n", i);
+			pagemap_check_wp(buffer + i * ops->page_size,
+					 !bitmap_test(bitmap, i));
+		}
+	}
+
+	free(bitmap);
+	args.quit = true;
+	pthread_join(zap_tid, NULL);
+
+	return true;
+}
+
+static void test_uffd_wp_async_one(async_ops_t *ops)
+{
+	struct uffdio_register uffdio_register = {0};
+	struct uffdio_api uffdio_api;
+	unsigned char *buffer = NULL;
+	bool succeed = false;
+	int uffd = -1;
+
+	if (!ops->mem_map) {
+		ksft_test_result_skip("Userfaultfd-wp async (%s)\n",
+				      ops->name);
+		return;
+	}
+
+	buffer = ops->mem_map(ASYNC_MEM_SIZE);
+	if (!buffer)
+		goto out;
+
+	uffd = __userfaultfd_open_dev();
+	if (uffd < 0)
+		goto out;
+
+	uffdio_api.api = UFFD_API;
+	uffdio_api.features = UFFD_FEATURE_WP_ASYNC;
+	if (ioctl(uffd, UFFDIO_API, &uffdio_api)) {
+		perror("UFFDIO_API");
+		goto out;
+	}
+
+	uffdio_register.range.start = (unsigned long) buffer;
+	uffdio_register.range.len = ASYNC_MEM_SIZE;
+	uffdio_register.mode = UFFDIO_REGISTER_MODE_WP;
+	if (ioctl(uffd, UFFDIO_REGISTER, &uffdio_register)) {
+		perror("UFFDIO_REGISTER");
+		goto out;
+	}
+
+	succeed = test_async(ops, uffd, buffer, ASYNC_MEM_SIZE);
+
+	if (ioctl(uffd, UFFDIO_UNREGISTER, &uffdio_register.range)) {
+		perror("UFFDIO_UNREGISTER");
+		goto out;
+	}
+out:
+	if (uffd > 0)
+		close(uffd);
+	if (buffer)
+		munmap(buffer, ASYNC_MEM_SIZE);
+
+	if (succeed)
+		ksft_test_result_pass("Userfaultfd-wp async (%s)\n", ops->name);
+	else
+		ksft_test_result_fail("Userfaultfd-wp async (%s)\n", ops->name);
+}
+
+static void test_uffd_wp_async_all(void)
+{
+	test_async_type type;
+
+	for (type = 0; type < TEST_ASYNC_NUM; type++)
+		test_uffd_wp_async_one(&async_ops[type]);
+}
+
+int main(void)
+{
+	pagemap_fd = open("/proc/self/pagemap", O_RDONLY);
+
+	ksft_print_header();
+	ksft_set_plan(TEST_ASYNC_NUM);
+	test_uffd_wp_async_all();
+	return ksft_exit_pass();
+}
-- 
2.39.1

Re: [PATCH v11 4/7] fs/proc/task_mmu: Implement IOCTL to get and optionally clear info about PTEs

[Michał Mirosław]

On Thu, 16 Mar 2023 at 18:53, Muhammad Usama Anjum
<usama.anjum@collabora.com> wrote:
>
> Hi,
>
> Thank you so much for reviewing.
>
> On 3/13/23 9:02 PM, Michał Mirosław wrote:
> > On Thu, 9 Mar 2023 at 14:58, Muhammad Usama Anjum
> > <usama.anjum@collabora.com> wrote:
[...]
> >> --- a/fs/proc/task_mmu.c
> >> +++ b/fs/proc/task_mmu.c
[...]
> >> +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;
> >> +       bool cpy = true;
> >> +
> >> +       if (p->max_pages && (p->found_pages == p->max_pages))
> >> +               return -ENOSPC;
> >> +
> >> +       if (!n_pages)
> >> +               return -EINVAL;
> >> +
> >> +       if (p->required_mask)
> >> +               cpy = ((p->required_mask & bitmap) == p->required_mask);
> >> +       if (cpy && p->anyof_mask)
> >> +               cpy = (p->anyof_mask & bitmap);
> >> +       if (cpy && p->excluded_mask)
> >> +               cpy = !(p->excluded_mask & bitmap);
> >
> > Since the rest of the code is executed only when `cpy` is true, this
> > could just return early for easier understanding.
> Hmm... I'll do the following:
>         if (!cpy || !bitmap)
>                 return 0;
> > BTW, some of the tests are redundant. Eg: if required_mask == 0, then
> > `required_mask & x == required_mask` will always hold. Same for
> > `excluded_mask & x == 0`.
> Correct. This is why I'm checking if required_mask is set and then
> comparing bitmap with it. required_mask may be 0 if not set. This if will
> ignore the subsequent check.
>
>         if (p->required_mask)
>                 cpy = ((p->required_mask & bitmap) == p->required_mask);
>
> I don't see any redundancy here. Please let me know otherwise?
[...]
> >> +       if (cpy && bitmap) {
> >
> > Assuming early returns on `!cpy` are done earlier:
> >
> > if (!bitmap)
> >   return 0;
> I've posted condition above which would better suit here.
[...]

Since the `cpy` condition is updated and passed to each new branch
(IOW: after setting cpy = 0 for whatever reason all the further code
is skipped) you can drop the variable and do early returns everywhere.
E.g.:

if ((bitmap & p->required_mask) != p->required_mask)
  return 0;
if (p->anyof_mask && !(bitmap & p->anyof_mask))
  return 0;
if (bitmap & p->excluded_mask)
  return 0;
if (!bitmap)
  return 0;

Also you can take the "special" effect of masking with zero to be
always zero (and in C - false) to avoid testing for an empty mask
separately in most cases.

> Just for my knowledge, what does "Nit" signify if a comment is marked with it?

A low priority / cosmetic item that you might consider ignoring if a
fix is too expensive or controversial.

>> +               if ((cur->len) && (cur->bitmap == bitmap) &&
>> +                   (cur->start + cur->len * PAGE_SIZE == addr)) {
>
> I'd recommend removing the extra parentheses as they make the code
> less readable for me (too many parentheses to match visually).
I'll remove parenthesis.

[...]
>> The `cur->len` test seems redundant: is it possible to have
>> `cur->start == addr` in that case (I guess it would have to get
>> `n_pages == 0` in an earlier invocation)?
> No, both wouldn't work. cur->len == 0 means that it has only garbage. It is
> essential to check the validity from cur->len before performing other
> checks. Also cur->start can never be equal to addr as we are walking over
> page addressing in serial manner. We want to see here if the current
> address matches the previous data by finding the ending address of last
> stored data (cur->start + cur->len * PAGE_SIZE).

If cur->len == 0, then it doesn't matter if it gets merged or not - it
can be filtered out during the flush (see below).

[...]
> >> +               } else if ((!p->vec_index) ||
> >> +                          ((p->vec_index + 1) < p->vec_len)) {
> >
> > Can you explain this test? Why not just `p->vec_index < p->vec_len`? Or better:
> >
> > if (vec_index >= p->vec_len)
> >     return -ENOSPC;
>
> No, it'll not work. Lets leave it as it is. :)
>
> It has gotten somewhat complex, but I don't have any other way to make it
> simpler which works. First note the following points:
> 1) We walk over 512 page or 1 thp at a time to not over allocate memory in
> kernel (p->vec).
> 2) We also want to merge the consecutive pages with the same flags into one
> struct page_region. p->vec of current walk may merge with next walk. So we
> cannot write to user memory until we find the results of the next walk.
>
> So most recent data is put into p->cur. When non-intersecting or mergeable
> data is found, we move p->cur to p->vec[p->index] inside the page walk.
> After the page walk, p->vec[0 to p->index] is moved to arg->vec. After all
> the walks are over. We move the p->cur to arg->vec. It completes the data
> transfer to user buffer.
[...]
> I'm so sorry that it has gotten this much complex. It was way simpler when
> we were walking over all the memory in one go. But then we needed an
> unbounded memory from the kernel which we don't want.
[...]

I've gone through and hopefully understood the code. I'm not sure this
needs to be so complicated: when traversing a single PMD you can
always copy p->cur to p->vec[p->vec_index++] because you can have at
most pages_per_PMD non-merges (in the worst case the last page always
is left in p->cur and whole p->vec is used). After each PMD p->vec
needs a flush if p->vec_index > 0, skipping the dummy entry at front
(len == 0; if present). (This is mostly how it is implemented now, but
I propose to remove the "overflow" check and do the starting guard
removal only every PMD.)

BTW, the pagemap_scan_deposit() got me a bit confused: it seems that
it is just a copy of the p->vec flush to userspace. Please either use
it for both p->vec and p->cur flushing or inline.

BTW#2, I think the ENOSPC return in pagemap_scan_output() should
happen later - only if the pages would match and that caused the count
to exceed the limit. For THP n_pages should be truncated to the limit
(and ENOSPC returned right away) only after the pages were verified to
match.

[...]
> >> +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;
> >> +       struct vm_area_struct *vma = walk->vma;
> >> +       bool is_writ, is_file, is_pres, is_swap;
> >> +       unsigned long addr = end;
> >> +       spinlock_t *ptl;
> >> +       int ret = 0;
> >> +       pte_t *pte;
> >> +
> >> +#ifdef CONFIG_TRANSPARENT_HUGEPAGE
> >
> > Is the `#ifdef` needed? `pmd_trans_huge_lock()` will always return
> > NULL if transparent hugepages are not compiled in. OTOH I see
> > BUILD_BUG() is possible in HPAGE_SIZE definition (irrelevant in this
> > case), so that would need to be worked around first.
> I'd got the build error [1]. So I'd added these. I've tested it again with
> the same config. We don't need these #ifdef now. I'll remove these.

I mean that there are cases like [1] that actually need the #ifdef at
least to wrap HPAGE_SIZE usage. But maybe just this constant can be
wrapped so that we keep the code always compile-tested?

[1] https://elixir.bootlin.com/linux/v6.3-rc2/source/arch/mips/include/asm/page.h#L66

[...]
> >> +       if (!arg->required_mask && !arg->anyof_mask &&
> >> +           !arg->excluded_mask)
> >> +               return false;
> >
> > Is there an assumption in the code that those checks are needed? I'd
> > expect that no selection criteria makes a valid page set?
> In my view, selection criterion must be specified for the ioctl to work. If
> there is no criterio, user should go and read pagemap file directly. So the
> assumption is that at least one selection criterion must be specified.

Yes. I'm not sure we need to prevent multiple ways of doing the same
thing. But doesn't pagemap reading lack the range aggregation feature?

[...]
> >> +       if (!access_ok((void __user *)vec,
> >> +                      arg->vec_len * sizeof(struct page_region)))
> >> +               return false;
> >
> > Is there a provision that userspace threads are all blocked from
> > manipulating mmaps during this ioctl()? If not, this is a TOCTOU bug
> > and the writes should be checked each time as another userspace thread
> > could remap the memory while the ioctl() is working.
> mincore() syscall is doing in the same way. It checks the validity in the
> start only. What provision should I add? Isn't it obvious that the user
> should not remap such memory?

On the second look, I think the code already checks that while doing
copy_to_user(), so this check is redundant and can be removed.

> >
> >> +       if (!IS_ALIGNED(start, PAGE_SIZE))
> >> +               return false;
> >> +       if (!access_ok((void __user *)start, arg->len))
> >> +               return false;
> >
> > This I guess wants to check if the range to be scanned is mapped -
> > but isn't this what the ioctl() should do during the scan? (But, also
> > see above.)
> No, start represents the memory which the user wants to watch. User must
> allocate this memory first and then pass the address to this ioctl to find
> out the flags per page.

From:
+ * 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)
...

I'd expect the `start` field to just be a virtual address to start
scanning from. Does it need to be mapped? For CRIU usecase I'd start
with "start = 0" to find out all mappings, but 0 is (always) not
mapped. Is this supposed to only work on already discovered page
ranges? Anyway, I'd expect the code should be tolerant of another
thread changing the mappings while this ioctl() is walking the page
tables - is it so? If yes, then this check serves at most as an
optimization used only for an invalid call.

> >>  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,
> >
> > Is this correct? Would the code need a different userspace pointer
> > handling for 32-bit userspace on 64-bit kernel?
> Yeah, it is needed for 32-bit application to run on 64-bit kernel.

I mean is using the same function for both entry points correct? Don't
the pointers to userspace memory (e.g. arg->vec) need to be mapped for
32-bit process?

Best Regards

Michał Mirosław

Re: [PATCH v11 4/7] fs/proc/task_mmu: Implement IOCTL to get and optionally clear info about PTEs

[Muhammad Usama Anjum]

On 3/17/23 2:28 AM, Michał Mirosław wrote:
> On Thu, 16 Mar 2023 at 18:53, Muhammad Usama Anjum
> <usama.anjum@collabora.com> wrote:
>>
>> Hi,
>>
>> Thank you so much for reviewing.
>>
>> On 3/13/23 9:02 PM, Michał Mirosław wrote:
>>> On Thu, 9 Mar 2023 at 14:58, Muhammad Usama Anjum
>>> <usama.anjum@collabora.com> wrote:
> [...]
>>>> --- a/fs/proc/task_mmu.c
>>>> +++ b/fs/proc/task_mmu.c
> [...]
>>>> +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;
>>>> +       bool cpy = true;
>>>> +
>>>> +       if (p->max_pages && (p->found_pages == p->max_pages))
>>>> +               return -ENOSPC;
>>>> +
>>>> +       if (!n_pages)
>>>> +               return -EINVAL;
>>>> +
>>>> +       if (p->required_mask)
>>>> +               cpy = ((p->required_mask & bitmap) == p->required_mask);
>>>> +       if (cpy && p->anyof_mask)
>>>> +               cpy = (p->anyof_mask & bitmap);
>>>> +       if (cpy && p->excluded_mask)
>>>> +               cpy = !(p->excluded_mask & bitmap);
>>>
>>> Since the rest of the code is executed only when `cpy` is true, this
>>> could just return early for easier understanding.
>> Hmm... I'll do the following:
>>         if (!cpy || !bitmap)
>>                 return 0;
>>> BTW, some of the tests are redundant. Eg: if required_mask == 0, then
>>> `required_mask & x == required_mask` will always hold. Same for
>>> `excluded_mask & x == 0`.
>> Correct. This is why I'm checking if required_mask is set and then
>> comparing bitmap with it. required_mask may be 0 if not set. This if will
>> ignore the subsequent check.
>>
>>         if (p->required_mask)
>>                 cpy = ((p->required_mask & bitmap) == p->required_mask);
>>
>> I don't see any redundancy here. Please let me know otherwise?
> [...]
>>>> +       if (cpy && bitmap) {
>>>
>>> Assuming early returns on `!cpy` are done earlier:
>>>
>>> if (!bitmap)
>>>   return 0;
>> I've posted condition above which would better suit here.
> [...]
> 
> Since the `cpy` condition is updated and passed to each new branch
> (IOW: after setting cpy = 0 for whatever reason all the further code
> is skipped) you can drop the variable and do early returns everywhere.
> E.g.:
> 
> if ((bitmap & p->required_mask) != p->required_mask)
>   return 0;
> if (p->anyof_mask && !(bitmap & p->anyof_mask))
>   return 0;
> if (bitmap & p->excluded_mask)
>   return 0;
> if (!bitmap)
>   return 0;
Clever. Will do.

> 
> Also you can take the "special" effect of masking with zero to be
> always zero (and in C - false) to avoid testing for an empty mask
> separately in most cases.
Done.

> 
>> Just for my knowledge, what does "Nit" signify if a comment is marked with it?
> 
> A low priority / cosmetic item that you might consider ignoring if a
> fix is too expensive or controversial.
> 
>>> +               if ((cur->len) && (cur->bitmap == bitmap) &&
>>> +                   (cur->start + cur->len * PAGE_SIZE == addr)) {
>>
>> I'd recommend removing the extra parentheses as they make the code
>> less readable for me (too many parentheses to match visually).
> I'll remove parenthesis.
> 
> [...]
>>> The `cur->len` test seems redundant: is it possible to have
>>> `cur->start == addr` in that case (I guess it would have to get
>>> `n_pages == 0` in an earlier invocation)?
>> No, both wouldn't work. cur->len == 0 means that it has only garbage. It is
>> essential to check the validity from cur->len before performing other
>> checks. Also cur->start can never be equal to addr as we are walking over
>> page addressing in serial manner. We want to see here if the current
>> address matches the previous data by finding the ending address of last
>> stored data (cur->start + cur->len * PAGE_SIZE).
> 
> If cur->len == 0, then it doesn't matter if it gets merged or not - it
> can be filtered out during the flush (see below).

> 
> [...]
>>>> +               } else if ((!p->vec_index) ||
>>>> +                          ((p->vec_index + 1) < p->vec_len)) {
>>>
>>> Can you explain this test? Why not just `p->vec_index < p->vec_len`? Or better:
>>>
>>> if (vec_index >= p->vec_len)
>>>     return -ENOSPC;
>>
>> No, it'll not work. Lets leave it as it is. :)
>>
>> It has gotten somewhat complex, but I don't have any other way to make it
>> simpler which works. First note the following points:
>> 1) We walk over 512 page or 1 thp at a time to not over allocate memory in
>> kernel (p->vec).
>> 2) We also want to merge the consecutive pages with the same flags into one
>> struct page_region. p->vec of current walk may merge with next walk. So we
>> cannot write to user memory until we find the results of the next walk.
>>
>> So most recent data is put into p->cur. When non-intersecting or mergeable
>> data is found, we move p->cur to p->vec[p->index] inside the page walk.
>> After the page walk, p->vec[0 to p->index] is moved to arg->vec. After all
>> the walks are over. We move the p->cur to arg->vec. It completes the data
>> transfer to user buffer.
> [...]
>> I'm so sorry that it has gotten this much complex. It was way simpler when
>> we were walking over all the memory in one go. But then we needed an
>> unbounded memory from the kernel which we don't want.
> [...]
> 
> I've gone through and hopefully understood the code. I'm not sure this
> needs to be so complicated: when traversing a single PMD you can
> always copy p->cur to p->vec[p->vec_index++] because you can have at
> most pages_per_PMD non-merges (in the worst case the last page always
> is left in p->cur and whole p->vec is used). After each PMD p->vec
> needs a flush if p->vec_index > 0, skipping the dummy entry at front
> (len == 0; if present). (This is mostly how it is implemented now, but
> I propose to remove the "overflow" check and do the starting guard
> removal only every PMD.)
Sorry, unable to understand where to remove the guard?

> 
> BTW, the pagemap_scan_deposit() got me a bit confused: it seems that
> it is just a copy of the p->vec flush to userspace. Please either use
> it for both p->vec and p->cur flushing or inline.
I can inline this function if you say so, now that you understand all the
logic. I don't see what else can be done here.

> 
> BTW#2, I think the ENOSPC return in pagemap_scan_output() should
> happen later - only if the pages would match and that caused the count
> to exceed the limit. For THP n_pages should be truncated to the limit
> (and ENOSPC returned right away) only after the pages were verified to
> match.
We have 2 counters here:
* the p->max_pages optionally can be set to find out only N pages of
interest. So p->found_pages is counting this. We need to return early if
the page limit is complete.
* the p->vec_index keeps track of output buffer array size

> 
> [...]
>>>> +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;
>>>> +       struct vm_area_struct *vma = walk->vma;
>>>> +       bool is_writ, is_file, is_pres, is_swap;
>>>> +       unsigned long addr = end;
>>>> +       spinlock_t *ptl;
>>>> +       int ret = 0;
>>>> +       pte_t *pte;
>>>> +
>>>> +#ifdef CONFIG_TRANSPARENT_HUGEPAGE
>>>
>>> Is the `#ifdef` needed? `pmd_trans_huge_lock()` will always return
>>> NULL if transparent hugepages are not compiled in. OTOH I see
>>> BUILD_BUG() is possible in HPAGE_SIZE definition (irrelevant in this
>>> case), so that would need to be worked around first.
>> I'd got the build error [1]. So I'd added these. I've tested it again with
>> the same config. We don't need these #ifdef now. I'll remove these.
> 
> I mean that there are cases like [1] that actually need the #ifdef at
> least to wrap HPAGE_SIZE usage. But maybe just this constant can be
> wrapped so that we keep the code always compile-tested?
Some arch define HPAGE_SIZE even if huge page config isn't enabled and some
don't. Lets just add #ifdef CONFIG_TRANSPARENT_HUGEPAGE as it is just like
the similar code in this same file is using this same #ifdef.

> 
> [1] https://elixir.bootlin.com/linux/v6.3-rc2/source/arch/mips/include/asm/page.h#L66
> 
> [...]
>>>> +       if (!arg->required_mask && !arg->anyof_mask &&
>>>> +           !arg->excluded_mask)
>>>> +               return false;
>>>
>>> Is there an assumption in the code that those checks are needed? I'd
>>> expect that no selection criteria makes a valid page set?
>> In my view, selection criterion must be specified for the ioctl to work. If
>> there is no criterio, user should go and read pagemap file directly. So the
>> assumption is that at least one selection criterion must be specified.
> 
> Yes. I'm not sure we need to prevent multiple ways of doing the same
> thing. But doesn't pagemap reading lack the range aggregation feature?
Yeah, correct. But note that we are supporting only selective 4 flags in
this ioctl, not all pagemap flags. So it is useful for only those users who
depend only on these 4 flags. Out pagemap_ioctl interface is not so much
generic that we can cater anyone. Its interface is specific and we are
adding only those cases which are of our interest. So if someone wants
range aggregation from pagemap_ioctl, he'll need to add that flag in the
IOCTL first. When IOCTL support is added, he can specify the selection
criterion etc.

> 
> [...]
>>>> +       if (!access_ok((void __user *)vec,
>>>> +                      arg->vec_len * sizeof(struct page_region)))
>>>> +               return false;
>>>
>>> Is there a provision that userspace threads are all blocked from
>>> manipulating mmaps during this ioctl()? If not, this is a TOCTOU bug
>>> and the writes should be checked each time as another userspace thread
>>> could remap the memory while the ioctl() is working.
>> mincore() syscall is doing in the same way. It checks the validity in the
>> start only. What provision should I add? Isn't it obvious that the user
>> should not remap such memory?
> 
> On the second look, I think the code already checks that while doing
> copy_to_user(), so this check is redundant and can be removed.
I'll remove.

> 
>>>
>>>> +       if (!IS_ALIGNED(start, PAGE_SIZE))
>>>> +               return false;
>>>> +       if (!access_ok((void __user *)start, arg->len))
>>>> +               return false;
>>>
>>> This I guess wants to check if the range to be scanned is mapped -
>>> but isn't this what the ioctl() should do during the scan? (But, also
>>> see above.)
>> No, start represents the memory which the user wants to watch. User must
>> allocate this memory first and then pass the address to this ioctl to find
>> out the flags per page.
> 
> From:
> + * 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)
> ...
> 
> I'd expect the `start` field to just be a virtual address to start
> scanning from. Does it need to be mapped? For CRIU usecase I'd start
> with "start = 0" to find out all mappings, but 0 is (always) not
> mapped. Is this supposed to only work on already discovered page
> ranges? Anyway, I'd expect the code should be tolerant of another
> thread changing the mappings while this ioctl() is walking the page
> tables - is it so? If yes, then this check serves at most as an
> optimization used only for an invalid call.
Ohh... Ignore my previous comment. Yeah, any valid memory range can be
passed to view the page flags. This check just verifies if the memory range
is valid.

> 
>>>>  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,
>>>
>>> Is this correct? Would the code need a different userspace pointer
>>> handling for 32-bit userspace on 64-bit kernel?
>> Yeah, it is needed for 32-bit application to run on 64-bit kernel.
> 
> I mean is using the same function for both entry points correct? Don't
> the pointers to userspace memory (e.g. arg->vec) need to be mapped for
> 32-bit process?
No, every member is our argument structure is of 64 bit in our structure
which keeps memory layout same. So we don't need any specific conversion
here. (Even if we had any 32-bit variable, we just needed to make sure that
the layout remains the same in the memory.)

Thanks,
Usama

> 
> Best Regards
> 
> Michał Mirosław

-- 
BR,
Muhammad Usama Anjum

Re: [PATCH v11 1/7] userfaultfd: Add UFFD WP Async support

[Muhammad Usama Anjum]

Thanks you for sending. I'll perform testing and share results next.

On 3/17/23 12:20 AM, Peter Xu wrote:
> Hello, Muhammad,
> 
> On Thu, Mar 09, 2023 at 06:57:12PM +0500, Muhammad Usama Anjum wrote:
>> Add new WP Async mode (UFFD_FEATURE_WP_ASYNC) which resolves the page
>> faults on its own. It can be used to track that which pages have been
>> written-to from the time the pages were write-protected. It is very
>> efficient way to track the changes as uffd is by nature pte/pmd based.
>>
>> UFFD synchronous WP sends the page faults to the userspace where the
>> pages which have been written-to can be tracked. But it is not efficient.
>> This is why this asynchronous version is being added. After setting the
>> WP Async, the pages which have been written to can be found in the pagemap
>> file or information can be obtained from the PAGEMAP_IOCTL.
>>
>> Suggested-by: Peter Xu <peterx@redhat.com>
>> Signed-off-by: Muhammad Usama Anjum <usama.anjum@collabora.com>
> 
> Here's the patch that can enable WP_ASYNC for all kinds of memories (as I
> promised..).  Currently I only tested btrfs (besides the common three)
> which is the major fs I use locally, but I guess it'll also enable the rest
> no matter what's underneath, just like soft-dirty.
> 
> As I mentioned, I just feel it very unfortunate to have a lot of suffixes
> for the UFFD_FEATURE_* on types of memory, and I hope we get rid of it for
> this WP_ASYNC from the start because the workflow should really be similar
> to anon/shmem handling for most of the rest, just a few tweaks here and
> there.
> 
> I had a feeling that some type of special VMA will work weirdly, but let's
> see.. so far I don't come up with any.
> 
> If the patch looks fine to you, please consider replace this patch with
> patch 1 of mine where I attached.  Then patch 1 can be reviewed alongside
> with your series.
> 
> Logically patch 1 can be reviewed separately too, because it works
> perfectly afaiu without the atomic version of pagemap already.  But on my
> side I don't think it justifies anything really matters, so unless someone
> thinks it a good idea to post / review / merge it separately, you can keep
> that with your new pagemap ioctl.
> 
> Patch 2 is only for your reference.  It's not for merging quality so please
> don't put it into your series.  I do plan to cleanup the userfaultfd
> selftests in the near future first (when I wrote this I am more eager to do
> so..).  I also think your final pagemap test cases can cover quite a bit.
> 
> Thanks,
> 

-- 
BR,
Muhammad Usama Anjum

Re: [PATCH v11 4/7] fs/proc/task_mmu: Implement IOCTL to get and optionally clear info about PTEs

[Michał Mirosław]

On Fri, 17 Mar 2023 at 13:44, Muhammad Usama Anjum
<usama.anjum@collabora.com> wrote:
> On 3/17/23 2:28 AM, Michał Mirosław wrote:
> > On Thu, 16 Mar 2023 at 18:53, Muhammad Usama Anjum
> > <usama.anjum@collabora.com> wrote:
> >> On 3/13/23 9:02 PM, Michał Mirosław wrote:
> >>> On Thu, 9 Mar 2023 at 14:58, Muhammad Usama Anjum
> >>> <usama.anjum@collabora.com> wrote:
> > [...]
> >>>> --- a/fs/proc/task_mmu.c
> >>>> +++ b/fs/proc/task_mmu.c
> > [...]
> >>>> +static int pagemap_scan_output(bool wt, bool file, bool pres, bool swap,
> > [...]
> >>> The `cur->len` test seems redundant: is it possible to have
> >>> `cur->start == addr` in that case (I guess it would have to get
> >>> `n_pages == 0` in an earlier invocation)?
> >> No, both wouldn't work. cur->len == 0 means that it has only garbage. It is
> >> essential to check the validity from cur->len before performing other
> >> checks. Also cur->start can never be equal to addr as we are walking over
> >> page addressing in serial manner. We want to see here if the current
> >> address matches the previous data by finding the ending address of last
> >> stored data (cur->start + cur->len * PAGE_SIZE).
> >
> > If cur->len == 0, then it doesn't matter if it gets merged or not - it
> > can be filtered out during the flush (see below).
> > [...]
> >>>> +               } else if ((!p->vec_index) ||
> >>>> +                          ((p->vec_index + 1) < p->vec_len)) {
> >>>
> >>> Can you explain this test? Why not just `p->vec_index < p->vec_len`? Or better:
> >>>
> >>> if (vec_index >= p->vec_len)
> >>>     return -ENOSPC;
> >>
> >> No, it'll not work. Lets leave it as it is. :)
> >>
> >> It has gotten somewhat complex, but I don't have any other way to make it
> >> simpler which works. First note the following points:
> >> 1) We walk over 512 page or 1 thp at a time to not over allocate memory in
> >> kernel (p->vec).
> >> 2) We also want to merge the consecutive pages with the same flags into one
> >> struct page_region. p->vec of current walk may merge with next walk. So we
> >> cannot write to user memory until we find the results of the next walk.
> >>
> >> So most recent data is put into p->cur. When non-intersecting or mergeable
> >> data is found, we move p->cur to p->vec[p->index] inside the page walk.
> >> After the page walk, p->vec[0 to p->index] is moved to arg->vec. After all
> >> the walks are over. We move the p->cur to arg->vec. It completes the data
> >> transfer to user buffer.
> > [...]
> >> I'm so sorry that it has gotten this much complex. It was way simpler when
> >> we were walking over all the memory in one go. But then we needed an
> >> unbounded memory from the kernel which we don't want.
> > [...]
> >
> > I've gone through and hopefully understood the code. I'm not sure this
> > needs to be so complicated: when traversing a single PMD you can
> > always copy p->cur to p->vec[p->vec_index++] because you can have at
> > most pages_per_PMD non-merges (in the worst case the last page always
> > is left in p->cur and whole p->vec is used). After each PMD p->vec
> > needs a flush if p->vec_index > 0, skipping the dummy entry at front
> > (len == 0; if present). (This is mostly how it is implemented now, but
> > I propose to remove the "overflow" check and do the starting guard
> > removal only every PMD.)
> Sorry, unable to understand where to remove the guard?

Instead of checking for it in pagemap_scan_output() for each page you
can skip it in do_pagemap_cmd() when doing the flush.

> > BTW#2, I think the ENOSPC return in pagemap_scan_output() should
> > happen later - only if the pages would match and that caused the count
> > to exceed the limit. For THP n_pages should be truncated to the limit
> > (and ENOSPC returned right away) only after the pages were verified to
> > match.
> We have 2 counters here:
> * the p->max_pages optionally can be set to find out only N pages of
> interest. So p->found_pages is counting this. We need to return early if
> the page limit is complete.
> * the p->vec_index keeps track of output buffer array size

I think I get how the limits are supposed to work, but I also think
the implementation is not optimal. An example (assuming max_pages = 1
and vec_len = 1):
 - a matching page has been found
 - a second - non-matching - is tried but results in immediate -ENOSPC.
-> In this case I'd expect the early return to happen just after the
first page is found so that non
A similar problem occurs for hugepage: when the limit is hit (we found
>= max_pages, n_pages is possibly truncated), but the scan continues
until next page / PMD.

[...]
> >>>> +       if (!arg->required_mask && !arg->anyof_mask &&
> >>>> +           !arg->excluded_mask)
> >>>> +               return false;
> >>>
> >>> Is there an assumption in the code that those checks are needed? I'd
> >>> expect that no selection criteria makes a valid page set?
> >> In my view, selection criterion must be specified for the ioctl to work. If
> >> there is no criterio, user should go and read pagemap file directly. So the
> >> assumption is that at least one selection criterion must be specified.
> >
> > Yes. I'm not sure we need to prevent multiple ways of doing the same
> > thing. But doesn't pagemap reading lack the range aggregation feature?
> Yeah, correct. But note that we are supporting only selective 4 flags in
> this ioctl, not all pagemap flags. So it is useful for only those users who
> depend only on these 4 flags. Out pagemap_ioctl interface is not so much
> generic that we can cater anyone. Its interface is specific and we are
> adding only those cases which are of our interest. So if someone wants
> range aggregation from pagemap_ioctl, he'll need to add that flag in the
> IOCTL first. When IOCTL support is added, he can specify the selection
> criterion etc.

The available flag set is not a problem. An example usecase: dumping
the memory state for debugging: ioctl(return_mask=ALL) returns a
conveniently compact vector of ranges of pages that are actually used
by the process (not only having reserved the virtual space). This is
actually something that helps dumping processes with using tools like
AddressSanitizer that create huge sparse mappings.

Best Regards
Michał Mirosław

Re: [PATCH v11 4/7] fs/proc/task_mmu: Implement IOCTL to get and optionally clear info about PTEs

[Muhammad Usama Anjum]

On 3/17/23 7:15 PM, Michał Mirosław wrote:
> On Fri, 17 Mar 2023 at 13:44, Muhammad Usama Anjum
> <usama.anjum@collabora.com> wrote:
>> On 3/17/23 2:28 AM, Michał Mirosław wrote:
>>> On Thu, 16 Mar 2023 at 18:53, Muhammad Usama Anjum
>>> <usama.anjum@collabora.com> wrote:
>>>> On 3/13/23 9:02 PM, Michał Mirosław wrote:
>>>>> On Thu, 9 Mar 2023 at 14:58, Muhammad Usama Anjum
>>>>> <usama.anjum@collabora.com> wrote:
>>> [...]
>>>>>> --- a/fs/proc/task_mmu.c
>>>>>> +++ b/fs/proc/task_mmu.c
>>> [...]
>>>>>> +static int pagemap_scan_output(bool wt, bool file, bool pres, bool swap,
>>> [...]
>>>>> The `cur->len` test seems redundant: is it possible to have
>>>>> `cur->start == addr` in that case (I guess it would have to get
>>>>> `n_pages == 0` in an earlier invocation)?
>>>> No, both wouldn't work. cur->len == 0 means that it has only garbage. It is
>>>> essential to check the validity from cur->len before performing other
>>>> checks. Also cur->start can never be equal to addr as we are walking over
>>>> page addressing in serial manner. We want to see here if the current
>>>> address matches the previous data by finding the ending address of last
>>>> stored data (cur->start + cur->len * PAGE_SIZE).
>>>
>>> If cur->len == 0, then it doesn't matter if it gets merged or not - it
>>> can be filtered out during the flush (see below).
>>> [...]
>>>>>> +               } else if ((!p->vec_index) ||
>>>>>> +                          ((p->vec_index + 1) < p->vec_len)) {
>>>>>
>>>>> Can you explain this test? Why not just `p->vec_index < p->vec_len`? Or better:
>>>>>
>>>>> if (vec_index >= p->vec_len)
>>>>>     return -ENOSPC;
>>>>
>>>> No, it'll not work. Lets leave it as it is. :)
>>>>
>>>> It has gotten somewhat complex, but I don't have any other way to make it
>>>> simpler which works. First note the following points:
>>>> 1) We walk over 512 page or 1 thp at a time to not over allocate memory in
>>>> kernel (p->vec).
>>>> 2) We also want to merge the consecutive pages with the same flags into one
>>>> struct page_region. p->vec of current walk may merge with next walk. So we
>>>> cannot write to user memory until we find the results of the next walk.
>>>>
>>>> So most recent data is put into p->cur. When non-intersecting or mergeable
>>>> data is found, we move p->cur to p->vec[p->index] inside the page walk.
>>>> After the page walk, p->vec[0 to p->index] is moved to arg->vec. After all
>>>> the walks are over. We move the p->cur to arg->vec. It completes the data
>>>> transfer to user buffer.
>>> [...]
>>>> I'm so sorry that it has gotten this much complex. It was way simpler when
>>>> we were walking over all the memory in one go. But then we needed an
>>>> unbounded memory from the kernel which we don't want.
>>> [...]
>>>
>>> I've gone through and hopefully understood the code. I'm not sure this
>>> needs to be so complicated: when traversing a single PMD you can
>>> always copy p->cur to p->vec[p->vec_index++] because you can have at
>>> most pages_per_PMD non-merges (in the worst case the last page always
>>> is left in p->cur and whole p->vec is used). After each PMD p->vec
>>> needs a flush if p->vec_index > 0, skipping the dummy entry at front
>>> (len == 0; if present). (This is mostly how it is implemented now, but
>>> I propose to remove the "overflow" check and do the starting guard
>>> removal only every PMD.)
>> Sorry, unable to understand where to remove the guard?
> 
> Instead of checking for it in pagemap_scan_output() for each page you
> can skip it in do_pagemap_cmd() when doing the flush.
No, this cannot be done because in do_pagemap_cmd() we don't know that we
have space for new pages in the output buffer or not because the next page
may be aggregated to already present data.

> 
>>> BTW#2, I think the ENOSPC return in pagemap_scan_output() should
>>> happen later - only if the pages would match and that caused the count
>>> to exceed the limit. For THP n_pages should be truncated to the limit
>>> (and ENOSPC returned right away) only after the pages were verified to
>>> match.
>> We have 2 counters here:
>> * the p->max_pages optionally can be set to find out only N pages of
>> interest. So p->found_pages is counting this. We need to return early if
>> the page limit is complete.
>> * the p->vec_index keeps track of output buffer array size
> 
> I think I get how the limits are supposed to work, but I also think
> the implementation is not optimal. An example (assuming max_pages = 1
> and vec_len = 1):
>  - a matching page has been found
>  - a second - non-matching - is tried but results in immediate -ENOSPC.
> -> In this case I'd expect the early return to happen just after the
> first page is found so that non
> A similar problem occurs for hugepage: when the limit is hit (we found
>> = max_pages, n_pages is possibly truncated), but the scan continues
> until next page / PMD.
I'll try to check if I can optimize it. It seems like I should be able to
update this pretty easily by returning a negative status/error which
signifies that we have found the max_pages. Now just abort in sane way.

> 
> [...]
>>>>>> +       if (!arg->required_mask && !arg->anyof_mask &&
>>>>>> +           !arg->excluded_mask)
>>>>>> +               return false;
>>>>>
>>>>> Is there an assumption in the code that those checks are needed? I'd
>>>>> expect that no selection criteria makes a valid page set?
>>>> In my view, selection criterion must be specified for the ioctl to work. If
>>>> there is no criterio, user should go and read pagemap file directly. So the
>>>> assumption is that at least one selection criterion must be specified.
>>>
>>> Yes. I'm not sure we need to prevent multiple ways of doing the same
>>> thing. But doesn't pagemap reading lack the range aggregation feature?
>> Yeah, correct. But note that we are supporting only selective 4 flags in
>> this ioctl, not all pagemap flags. So it is useful for only those users who
>> depend only on these 4 flags. Out pagemap_ioctl interface is not so much
>> generic that we can cater anyone. Its interface is specific and we are
>> adding only those cases which are of our interest. So if someone wants
>> range aggregation from pagemap_ioctl, he'll need to add that flag in the
>> IOCTL first. When IOCTL support is added, he can specify the selection
>> criterion etc.
> 
> The available flag set is not a problem. An example usecase: dumping
> the memory state for debugging: ioctl(return_mask=ALL) returns a
> conveniently compact vector of ranges of pages that are actually used
> by the process (not only having reserved the virtual space). This is
> actually something that helps dumping processes with using tools like
> AddressSanitizer that create huge sparse mappings.
I don't know, we are adding more and more use cases as people are noticing
it. I've not thought about this use case. So I need more understanding
about it:
How should I identify "which pages are used"? Does use mean present and
swapped both? We we want to find present or swapped pages in other words
!pte_none pages and return in compact form, it can already be done by
ioctl(anyod_mask=PRESET | SWAPPED, return_mask=ALL).

> 
> Best Regards
> Michał Mirosław

-- 
BR,
Muhammad Usama Anjum

Re: [PATCH v11 0/7] Implement IOCTL to get and optionally clear info about PTEs

[Andrei Vagin]

On Thu, Mar 9, 2023 at 11:58 AM Andrew Morton <akpm@linux-foundation.org> wrote:
>
> On Thu,  9 Mar 2023 18:57:11 +0500 Muhammad Usama Anjum <usama.anjum@collabora.com> wrote:
>
> > 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].
>
> It's a ton of new code and what I'm not seeing in here (might have
> missed it?) is a clear statement of the value of this feature to our
> users.
>
> I see hints that CRIU would like it, but no description of how valuable
> this is to CRIU's users.


Hi Andrew,

The current interface works for CRIU, and I can't say we have anything
critical with it right now.

On the other hand, the new interface has a number of significant improvements:

* 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.


Thanks,
Andrei

>
> So please spend some time preparing this info.
>
> Also, are any other applications of this feature anticipated?  If so,
> what are they?
>
> IOW, please sell this stuff to us!

Re: [PATCH v11 1/7] userfaultfd: Add UFFD WP Async support

[Muhammad Usama Anjum]

[-- Attachment #1: Type: text/plain, Size: 3476 bytes --]

On 3/17/23 12:20 AM, Peter Xu wrote:
> Hello, Muhammad,
> 
> On Thu, Mar 09, 2023 at 06:57:12PM +0500, Muhammad Usama Anjum wrote:
>> Add new WP Async mode (UFFD_FEATURE_WP_ASYNC) which resolves the page
>> faults on its own. It can be used to track that which pages have been
>> written-to from the time the pages were write-protected. It is very
>> efficient way to track the changes as uffd is by nature pte/pmd based.
>>
>> UFFD synchronous WP sends the page faults to the userspace where the
>> pages which have been written-to can be tracked. But it is not efficient.
>> This is why this asynchronous version is being added. After setting the
>> WP Async, the pages which have been written to can be found in the pagemap
>> file or information can be obtained from the PAGEMAP_IOCTL.
>>
>> Suggested-by: Peter Xu <peterx@redhat.com>
>> Signed-off-by: Muhammad Usama Anjum <usama.anjum@collabora.com>
> 
> Here's the patch that can enable WP_ASYNC for all kinds of memories (as I
> promised..).  Currently I only tested btrfs (besides the common three)
> which is the major fs I use locally, but I guess it'll also enable the rest
> no matter what's underneath, just like soft-dirty.
> 
> As I mentioned, I just feel it very unfortunate to have a lot of suffixes
> for the UFFD_FEATURE_* on types of memory, and I hope we get rid of it for
> this WP_ASYNC from the start because the workflow should really be similar
> to anon/shmem handling for most of the rest, just a few tweaks here and
> there.
> 
> I had a feeling that some type of special VMA will work weirdly, but let's
> see.. so far I don't come up with any.
> 
> If the patch looks fine to you, please consider replace this patch with
> patch 1 of mine where I attached.  Then patch 1 can be reviewed alongside
> with your series.
> 
> Logically patch 1 can be reviewed separately too, because it works
> perfectly afaiu without the atomic version of pagemap already.  But on my
> side I don't think it justifies anything really matters, so unless someone
> thinks it a good idea to post / review / merge it separately, you can keep
> that with your new pagemap ioctl.
> 
> Patch 2 is only for your reference.  It's not for merging quality so please
> don't put it into your series.  I do plan to cleanup the userfaultfd
> selftests in the near future first (when I wrote this I am more eager to do
> so..).  I also think your final pagemap test cases can cover quite a bit.
> 
> Thanks,
Thank you so much for the patch. I've tested hugetlb mem. This patch is
working fine for hugetlb shmem:
*shmid = shmget(2, size, SHM_HUGETLB | IPC_CREAT | SHM_R | SHM_W);
mem = shmat(*shmid, 0, 0);

I've found slight issue with hugetlb mem which has been mmaped:
mem = mmap(NULL, size, PROT_READ | PROT_WRITE,
	   MAP_ANONYMOUS | MAP_HUGETLB | MAP_PRIVATE, -1, 0);
The issue is that even after witting to this memory, the wp flag is still
present there and memory doesn't appear to be dirty when it should have
been dirty. The temporary fix is to write to memory and write protect the
memory one extra time.

Here is how I'm checking if WP flag is set or not:
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));
}

I've isolated the reproducer inside kselftests by commenting the unrelated
code. Please have a look at the attached kselftest and follow from main or
search `//#define tmpfix` in the code.

-- 
BR,
Muhammad Usama Anjum

[-- Attachment #2: pagemap_ioctl.c --]
[-- Type: text/x-csrc, Size: 37570 bytes --]

// 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/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;// UFFDIO_REGISTER_MODE_MISSING |
	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; // UFFDIO_REGISTER_MODE_MISSING |
	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, 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, 8,
				       0, 0x1111, 0, 0, PAGE_IS_WRITTEN) < 0,
			 "%s wrong mask specified\n", __func__);

	ksft_test_result(pagemap_ioctl(mem, mem_size, vec, vec_size, 0,
				       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_WP | PM_SCAN_OP_GET | 0x32,
				       0, PAGE_IS_WRITTEN, 0, 0, PAGE_IS_WRITTEN) < 0,
			 "%s mixture of correct and wrong flag\n", __func__);

	ksft_test_result(pagemap_ioctl(mem, mem_size, NULL, 0, PM_SCAN_OP_WP,
				       0, PAGE_IS_WRITTEN, 0, 0, PAGE_IS_WRITTEN) < 0,
			 "%s PM_SCAN_OP_WP cannot be used without get\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 %d\n", __func__,
			 ret);

	/* 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 %d %d %d\n",
			 __func__, ret, ret2, ret3);

	/* 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 %d %d %d %d %d\n",
			 __func__, ret, vec[0].len, ret2, vec2[0].len, vec2[1].len);

	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) %d\n", prefix,
//			 written);

	/* 2. all pages must be written */
	memset(mem, -1, mem_size);

//#define tmpfix

#ifdef tmpfix
	/* fix for hugetlb mem */
	wp_addr_range(mem, mem_size);
	memset(mem, 1, mem_size);
#endif

	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) %d %d %d %d\n", prefix, written,
			 vec[0].len, vec[1].len, mem_size/page_size);

//	/* 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 %d %d\n", prefix,
//			 written, vec[0].len);
//
//	/* 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 %d %d\n",
				 __func__, ret, vec[0].len);

		/* 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 %d %d\n", __func__, ret,
				 vec[0].len);
		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);

		wp_addr_range(map + map_size/2, 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,
				 "%s clear second half huge page %d, %d\n", __func__, ret,
				 vec[0].len);
		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 %d %d %d\n", __func__, ret2, vec[0].len,
				 vec[1].len);

		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)
{
	char *mem, *fmem;
	int mem_size, vec_size, ret;
	struct page_region *vec;

	/* 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 */
	int fd;
	struct stat sbuf;

	fd = open(__FILE__, O_RDONLY);
	if (fd < 0) {
		ksft_test_result_skip("%s Memory mapped file\n");
		goto free_vec_and_return;
	}

	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);

free_vec_and_return:
	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)
{
	volatile 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, nthreads * access_per_thread %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;
	char *mem, *map;

	ksft_print_header();
	ksft_set_plan(66);

	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);
	}

//	/* 6. Huge page tests */
//	hpage_unit_tests();
//
//	/* 7. Iterative test */
//	test_simple();
//
//	/* 8. Mprotect test */
//	mprotect_tests();
//
//	/* 9. 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();
}

Re: [PATCH v11 0/7] Implement IOCTL to get and optionally clear info about PTEs

[Mike Rapoport]

On Mon, Mar 20, 2023 at 11:30:00AM -0700, Andrei Vagin wrote:
> On Thu, Mar 9, 2023 at 11:58 AM Andrew Morton <akpm@linux-foundation.org> wrote:
> >
> > On Thu,  9 Mar 2023 18:57:11 +0500 Muhammad Usama Anjum <usama.anjum@collabora.com> wrote:
> >
> > > 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].
> >
> > It's a ton of new code and what I'm not seeing in here (might have
> > missed it?) is a clear statement of the value of this feature to our
> > users.
> >
> > I see hints that CRIU would like it, but no description of how valuable
> > this is to CRIU's users.
> 
> Hi Andrew,
> 
> The current interface works for CRIU, and I can't say we have anything
> critical with it right now.
> 
> On the other hand, the new interface has a number of significant improvements:
> 
> * 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.

There is still a caveat in using userfaultfd for tracking dirty pages in
CRIU because we still don't support C/R of processes that use uffd. 
 
> Thanks,
> Andrei
> 
> >
> > So please spend some time preparing this info.
> >
> > Also, are any other applications of this feature anticipated?  If so,
> > what are they?
> >
> > IOW, please sell this stuff to us!

-- 
Sincerely yours,
Mike.

Re: [PATCH v11 0/7] Implement IOCTL to get and optionally clear info about PTEs

[Peter Xu]

On Tue, Mar 21, 2023 at 02:41:53PM +0200, Mike Rapoport wrote:
> On Mon, Mar 20, 2023 at 11:30:00AM -0700, Andrei Vagin wrote:
> > On Thu, Mar 9, 2023 at 11:58 AM Andrew Morton <akpm@linux-foundation.org> wrote:
> > >
> > > On Thu,  9 Mar 2023 18:57:11 +0500 Muhammad Usama Anjum <usama.anjum@collabora.com> wrote:
> > >
> > > > 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].
> > >
> > > It's a ton of new code and what I'm not seeing in here (might have
> > > missed it?) is a clear statement of the value of this feature to our
> > > users.
> > >
> > > I see hints that CRIU would like it, but no description of how valuable
> > > this is to CRIU's users.
> > 
> > Hi Andrew,
> > 
> > The current interface works for CRIU, and I can't say we have anything
> > critical with it right now.
> > 
> > On the other hand, the new interface has a number of significant improvements:
> > 
> > * 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.
> 
> There is still a caveat in using userfaultfd for tracking dirty pages in
> CRIU because we still don't support C/R of processes that use uffd. 

This reminded me whether the interface can also expose soft-dirty as a
ranged soft-dirty collector too to replace existing pagemap read()s?  Just
in case userfault cannot be used.  The code addition should be trivial IIUC.

Then maybe PAGE_IS_WRITTEN will be a name too generic, it can be two bits
PAGE_IS_UFFD_WP and PAGE_IS_SOFT_DIRTY, having PAGE_IS_UFFD_WP the inverted
meaning of current PAGE_IS_WRITTEN.

-- 
Peter Xu

Re: [PATCH v11 1/7] userfaultfd: Add UFFD WP Async support

[Peter Xu]

Hi, Muhammad,

On Tue, Mar 21, 2023 at 05:21:15PM +0500, Muhammad Usama Anjum wrote:
> Thank you so much for the patch. I've tested hugetlb mem. This patch is
> working fine for hugetlb shmem:
> *shmid = shmget(2, size, SHM_HUGETLB | IPC_CREAT | SHM_R | SHM_W);
> mem = shmat(*shmid, 0, 0);
> 
> I've found slight issue with hugetlb mem which has been mmaped:
> mem = mmap(NULL, size, PROT_READ | PROT_WRITE,
> 	   MAP_ANONYMOUS | MAP_HUGETLB | MAP_PRIVATE, -1, 0);
> The issue is that even after witting to this memory, the wp flag is still
> present there and memory doesn't appear to be dirty when it should have
> been dirty. The temporary fix is to write to memory and write protect the
> memory one extra time.

I looked into this today and found it's an existing bug that can trigger
with sync mode too.. as long as protection applied to unpopulated hugetlb
private mappings, then write to it.

I've sent a fix for it here and have you copied:

https://lore.kernel.org/linux-mm/20230321191840.1897940-1-peterx@redhat.com/T/#u

Please have a look and see whether it also fixes your issue.

PS: recently I added a warning in commit c2da319c2e2789 and that can indeed
capture this one when verifying using pagemap.  I'd guess your dmesg should
also contain something dumped.

Thanks,

-- 
Peter Xu

Re: [PATCH v11 1/7] userfaultfd: Add UFFD WP Async support

[Muhammad Usama Anjum]

On 3/22/23 12:25 AM, Peter Xu wrote:
> Hi, Muhammad,
> 
> On Tue, Mar 21, 2023 at 05:21:15PM +0500, Muhammad Usama Anjum wrote:
>> Thank you so much for the patch. I've tested hugetlb mem. This patch is
>> working fine for hugetlb shmem:
>> *shmid = shmget(2, size, SHM_HUGETLB | IPC_CREAT | SHM_R | SHM_W);
>> mem = shmat(*shmid, 0, 0);
>>
>> I've found slight issue with hugetlb mem which has been mmaped:
>> mem = mmap(NULL, size, PROT_READ | PROT_WRITE,
>> 	   MAP_ANONYMOUS | MAP_HUGETLB | MAP_PRIVATE, -1, 0);
>> The issue is that even after witting to this memory, the wp flag is still
>> present there and memory doesn't appear to be dirty when it should have
>> been dirty. The temporary fix is to write to memory and write protect the
>> memory one extra time.
> 
> I looked into this today and found it's an existing bug that can trigger
> with sync mode too.. as long as protection applied to unpopulated hugetlb
> private mappings, then write to it.
> 
> I've sent a fix for it here and have you copied:
> 
> https://lore.kernel.org/linux-mm/20230321191840.1897940-1-peterx@redhat.com/T/#u
> 
> Please have a look and see whether it also fixes your issue.
Thanks for sending the patch. I've replied on the sent patch.

> 
> PS: recently I added a warning in commit c2da319c2e2789 and that can indeed
> capture this one when verifying using pagemap.  I'd guess your dmesg should
> also contain something dumped.
I didn't had debug_vm config enabled. I've enabled it now. I'm getting only
the following stack trace in failure scenario:

ok 1 Hugetlb shmem testing: all new pages must not be written (dirty) 0
ok 2 Hugetlb shmem testing: all pages must be written (dirty) 1 512 0 512
ok 3 Hugetlb mem testing: all new pages must not be written (dirty) 0
[   10.086540] ------------[ cut here ]------------
[   10.087758] WARNING: CPU: 0 PID: 175 at
arch/x86/include/asm/pgtable.h:313 pagemap_scan_hugetlb_entry+0x19c/0x230
[   10.090208] Modules linked in:
[   10.091059] CPU: 0 PID: 175 Comm: pagemap_ioctl Not tainted
6.3.0-rc3-next-20230320-00010-gdc395ccf1882 #88
[   10.093224] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS
1.16.0-debian-1.16.0-5 04/01/2014
[   10.095879] RIP: 0010:pagemap_scan_hugetlb_entry+0x19c/0x230
[   10.097497] Code: 89 ca 41 89 c2 29 c8 4c 01 c2 49 39 d2 41 0f 43 c0 e9
53 ff ff ff 48 83 e2 9f 89 c7 31 ed 49 89 d1 83 e7 02 0f 84 30 ff ff ff
<0f> 0b 31 ff e9 27 ff ff ff 48 83 e2 9f 44 89 c0 bf 01 00 00 00 bd
[   10.102528] RSP: 0018:ffffb6cd80303d10 EFLAGS: 00010202
[   10.104002] RAX: 8000000000000ce7 RBX: 00007fcc84000000 RCX:
0000000000200000
[   10.105989] RDX: 80000002f7c00c87 RSI: 0000000000000001 RDI:
0000000000000002
[   10.108043] RBP: 0000000000000000 R08: 0000000000000200 R09:
80000002f7c00c87
[   10.110004] R10: ffffa08541e3220c R11: 0000000000000000 R12:
ffffa08541562420
[   10.112335] R13: ffffb6cd80303e70 R14: 00007fcc84000000 R15:
ffffffff8eae1520
[   10.114688] FS:  00007fcc8454b740(0000) GS:ffffa0886fc00000(0000)
knlGS:0000000000000000
[   10.116960] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[   10.118187] CR2: 00007fcc84000000 CR3: 0000000102838000 CR4:
0000000000750ef0
[   10.119628] PKRU: 55555554
[   10.120184] Call Trace:
[   10.120730]  <TASK>
[   10.121206]  __walk_page_range+0xbe/0x1b0
[   10.122048]  walk_page_range+0x15f/0x1a0
[   10.122869]  do_pagemap_cmd+0x239/0x390
[   10.123672]  __x64_sys_ioctl+0x8b/0xc0
[   10.124462]  do_syscall_64+0x3a/0x90
[   10.125227]  entry_SYSCALL_64_after_hwframe+0x72/0xdc
[   10.126326] RIP: 0033:0x7fcc8464bbab
[   10.127066] Code: 00 48 89 44 24 18 31 c0 48 8d 44 24 60 c7 04 24 10 00
00 00 48 89 44 24 08 48 8d 44 24 20 48 89 44 24 10 b8 10 00 00 00 0f 05
<89> c2 3d 00 f0 ff ff 77 1c 48 8b 44 24 18 64 48 2b 04 25 28 00 00
[   10.130868] RSP: 002b:00007fff9b864240 EFLAGS: 00000246 ORIG_RAX:
0000000000000010
[   10.132412] RAX: ffffffffffffffda RBX: 0000000000001000 RCX:
00007fcc8464bbab
[   10.133880] RDX: 00007fff9b8642c0 RSI: 00000000c0586610 RDI:
0000000000000003
[   10.135328] RBP: 00007fff9b864320 R08: 0000000000000001 R09:
0000000000000000
[   10.136790] R10: 00007fff9b864217 R11: 0000000000000246 R12:
0000000000000000
[   10.138285] R13: 00007fff9b8644f8 R14: 0000000000409df0 R15:
00007fcc84862020
[   10.139729]  </TASK>
[   10.140197] ---[ end trace 0000000000000000 ]---
not ok 4 Hugetlb mem testing: all pages must be written (dirty) 0
-2072900416 0 512

> 
> Thanks,
> 

-- 
BR,
Muhammad Usama Anjum