Re: [PATCH 1/2] fs/proc: extend the PAGEMAP_SCAN ioctl to report guard regions

[Lorenzo Stoakes <lorenzo.stoakes@oracle.com>]

On Thu, Mar 20, 2025 at 06:39:02AM +0000, Andrei Vagin wrote:
> From: Andrei Vagin <avagin@gmail.com>
>
> Introduce the PAGE_IS_GUARD flag in the PAGEMAP_SCAN ioctl to expose
> information about guard regions. This allows userspace tools, such as
> CRIU, to detect and handle guard regions.
>
> Signed-off-by: Andrei Vagin <avagin@gmail.com>

This looks fine thanks!

Reviewed-by: Lorenzo Stoakes <lorenzo.stoakes@oracle.com>

> ---
>  Documentation/admin-guide/mm/pagemap.rst | 1 +
>  fs/proc/task_mmu.c                       | 8 ++++++--
>  include/uapi/linux/fs.h                  | 1 +
>  3 files changed, 8 insertions(+), 2 deletions(-)
>
> diff --git a/Documentation/admin-guide/mm/pagemap.rst b/Documentation/admin-guide/mm/pagemap.rst
> index a297e824f990..7997b67ffc97 100644
> --- a/Documentation/admin-guide/mm/pagemap.rst
> +++ b/Documentation/admin-guide/mm/pagemap.rst
> @@ -234,6 +234,7 @@ Following flags about pages are currently supported:
>  - ``PAGE_IS_PFNZERO`` - Page has zero PFN
>  - ``PAGE_IS_HUGE`` - Page is PMD-mapped THP or Hugetlb backed
>  - ``PAGE_IS_SOFT_DIRTY`` - Page is soft-dirty
> +- ``PAGE_IS_GUARD`` - Page is a guard region

NIT: 'part of' a guard region.

>
>  The ``struct pm_scan_arg`` is used as the argument of the IOCTL.
>
> diff --git a/fs/proc/task_mmu.c b/fs/proc/task_mmu.c
> index c17615e21a5d..698d660bfee4 100644
> --- a/fs/proc/task_mmu.c
> +++ b/fs/proc/task_mmu.c
> @@ -2067,7 +2067,8 @@ static int pagemap_release(struct inode *inode, struct file *file)
>  #define PM_SCAN_CATEGORIES	(PAGE_IS_WPALLOWED | PAGE_IS_WRITTEN |	\
>  				 PAGE_IS_FILE |	PAGE_IS_PRESENT |	\
>  				 PAGE_IS_SWAPPED | PAGE_IS_PFNZERO |	\
> -				 PAGE_IS_HUGE | PAGE_IS_SOFT_DIRTY)
> +				 PAGE_IS_HUGE | PAGE_IS_SOFT_DIRTY |	\
> +				 PAGE_IS_GUARD)
>  #define PM_SCAN_FLAGS		(PM_SCAN_WP_MATCHING | PM_SCAN_CHECK_WPASYNC)
>
>  struct pagemap_scan_private {
> @@ -2108,8 +2109,11 @@ static unsigned long pagemap_page_category(struct pagemap_scan_private *p,
>  		if (!pte_swp_uffd_wp_any(pte))
>  			categories |= PAGE_IS_WRITTEN;
>

You don't show it, but kinda hate how we mark this as swapped even though,
you know, it's not... but we already do that for uffd pte markers, and
equally the guard marker for /proc/$pid/pagemap, so yeah it's consistent,
but still weird.

But fine, I guess that's something that already happened for uffd PTE
markers, and somebody searching for swapped is going to be shown guard
region pages too (and _right now_ would).

(This is a grumble/mumble rather than review comment, really :P)

> +		swp = pte_to_swp_entry(pte);

Total nit, but prefer either to add a newline after this or delete newline
after the block below just to indicate swp is used for all.

> +		if (is_guard_swp_entry(swp))
> +			categories |= PAGE_IS_GUARD;
> +

To be honest, nit-wise, I'd opt for just dropping this line... :) yeah I
know, the most petty thing ever! ;)

>  		if (p->masks_of_interest & PAGE_IS_FILE) {
> -			swp = pte_to_swp_entry(pte);
>  			if (is_pfn_swap_entry(swp) &&
>  			    !folio_test_anon(pfn_swap_entry_folio(swp)))
>  				categories |= PAGE_IS_FILE;
> diff --git a/include/uapi/linux/fs.h b/include/uapi/linux/fs.h
> index 2bbe00cf1248..8aa66c5f69b7 100644
> --- a/include/uapi/linux/fs.h
> +++ b/include/uapi/linux/fs.h
> @@ -363,6 +363,7 @@ typedef int __bitwise __kernel_rwf_t;
>  #define PAGE_IS_PFNZERO		(1 << 5)
>  #define PAGE_IS_HUGE		(1 << 6)
>  #define PAGE_IS_SOFT_DIRTY	(1 << 7)
> +#define PAGE_IS_GUARD		(1 << 8)
>
>  /*
>   * struct page_region - Page region with flags
> --
> 2.49.0.rc1.451.g8f38331e32-goog
>