Re: [PATCH] secretmem: Remove uses of struct page

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

Hi Matthew,

This patch is causing a kernel splat when running the memfd_secret kernel mm
self-test.

This is because find_lock_page() handles errors from filemap_lock_folio(),
filtering them as NULL, but using filemap_lock_folio() directly does not.

I attach a fix-patch that fixes this for me locally...

Cheers, Lorenzo

On Fri, Jun 13, 2025 at 08:47:43PM +0100, Matthew Wilcox (Oracle) wrote:
> Use filemap_lock_folio() instead of find_lock_page() to retrieve
> a folio from the page cache.
>
> Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org>
> ---
>  mm/secretmem.c | 16 +++++++---------
>  1 file changed, 7 insertions(+), 9 deletions(-)
>
> diff --git a/mm/secretmem.c b/mm/secretmem.c
> index 589b26c2d553..dfa8d8d10bcb 100644
> --- a/mm/secretmem.c
> +++ b/mm/secretmem.c
> @@ -54,7 +54,6 @@ static vm_fault_t secretmem_fault(struct vm_fault *vmf)
>  	pgoff_t offset = vmf->pgoff;
>  	gfp_t gfp = vmf->gfp_mask;
>  	unsigned long addr;
> -	struct page *page;
>  	struct folio *folio;
>  	vm_fault_t ret;
>  	int err;
> @@ -65,16 +64,15 @@ static vm_fault_t secretmem_fault(struct vm_fault *vmf)
>  	filemap_invalidate_lock_shared(mapping);
>
>  retry:
> -	page = find_lock_page(mapping, offset);
> -	if (!page) {
> +	folio = filemap_lock_folio(mapping, offset);
> +	if (!folio) {
>  		folio = folio_alloc(gfp | __GFP_ZERO, 0);
>  		if (!folio) {
>  			ret = VM_FAULT_OOM;
>  			goto out;
>  		}
>
> -		page = &folio->page;
> -		err = set_direct_map_invalid_noflush(page);
> +		err = set_direct_map_invalid_noflush(folio_page(folio, 0));
>  		if (err) {
>  			folio_put(folio);
>  			ret = vmf_error(err);
> @@ -90,7 +88,7 @@ static vm_fault_t secretmem_fault(struct vm_fault *vmf)
>  			 * already happened when we marked the page invalid
>  			 * which guarantees that this call won't fail
>  			 */
> -			set_direct_map_default_noflush(page);
> +			set_direct_map_default_noflush(folio_page(folio, 0));
>  			if (err == -EEXIST)
>  				goto retry;
>
> @@ -98,11 +96,11 @@ static vm_fault_t secretmem_fault(struct vm_fault *vmf)
>  			goto out;
>  		}
>
> -		addr = (unsigned long)page_address(page);
> +		addr = (unsigned long)folio_address(folio);
>  		flush_tlb_kernel_range(addr, addr + PAGE_SIZE);
>  	}
>
> -	vmf->page = page;
> +	vmf->page = folio_file_page(folio, vmf->pgoff);

Small nit here: we've already put vmf->pgoff in offset so could just use offset
here.

Same goes for the opening check in this function actually...

>  	ret = VM_FAULT_LOCKED;
>
>  out:
> @@ -154,7 +152,7 @@ static int secretmem_migrate_folio(struct address_space *mapping,
>
>  static void secretmem_free_folio(struct folio *folio)
>  {
> -	set_direct_map_default_noflush(&folio->page);
> +	set_direct_map_default_noflush(folio_page(folio, 0));
>  	folio_zero_segment(folio, 0, folio_size(folio));
>  }
>
> --
> 2.47.2
>
>
>

----8<----
From 850c9ecb82859fe572452cad93b917964b648141 Mon Sep 17 00:00:00 2001
From: Lorenzo Stoakes <lorenzo.stoakes@oracle.com>
Date: Mon, 16 Jun 2025 14:56:27 +0100
Subject: [PATCH] fix

Signed-off-by: Lorenzo Stoakes <lorenzo.stoakes@oracle.com>
---
 mm/secretmem.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/mm/secretmem.c b/mm/secretmem.c
index dfa8d8d10bcb..873f23c6e92b 100644
--- a/mm/secretmem.c
+++ b/mm/secretmem.c
@@ -65,7 +65,7 @@ static vm_fault_t secretmem_fault(struct vm_fault *vmf)

 retry:
 	folio = filemap_lock_folio(mapping, offset);
-	if (!folio) {
+	if (IS_ERR(folio)) {
 		folio = folio_alloc(gfp | __GFP_ZERO, 0);
 		if (!folio) {
 			ret = VM_FAULT_OOM;
--
2.49.0