[v7,02/20] fs/dax: Return unmapped busy pages from dax_layout_busy_page_range()

Commit Message

Alistair Popple Feb. 4, 2025, 10:47 p.m. UTC

dax_layout_busy_page_range() is used by file systems to scan the DAX
page-cache to unmap mapping pages from user-space and to determine if
any pages in the given range are busy, either due to ongoing DMA or
other get_user_pages() usage.

Currently it checks to see the file mapping is mapped into user-space
with mapping_mapped() and returns early if not, skipping the check for
DMA busy pages. This is wrong as pages may still be undergoing DMA
access even if they have subsequently been unmapped from
user-space. Fix this by dropping the check for mapping_mapped().

Signed-off-by: Alistair Popple <[e-mail]>
Suggested-by: Dan Williams <[e-mail]>
Reviewed-by: Dan Williams <[e-mail]>
---
 fs/dax.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Balbir Singh Feb. 5, 2025, 3:15 a.m. UTC | #1

On 2/5/25 09:47, Alistair Popple wrote:
> dax_layout_busy_page_range() is used by file systems to scan the DAX
> page-cache to unmap mapping pages from user-space and to determine if
> any pages in the given range are busy, either due to ongoing DMA or
> other get_user_pages() usage.
> 
> Currently it checks to see the file mapping is mapped into user-space
> with mapping_mapped() and returns early if not, skipping the check for
> DMA busy pages. This is wrong as pages may still be undergoing DMA
> access even if they have subsequently been unmapped from
> user-space. Fix this by dropping the check for mapping_mapped().
> 
> Signed-off-by: Alistair Popple <[e-mail]>
> Suggested-by: Dan Williams <[e-mail]>
> Reviewed-by: Dan Williams <[e-mail]>
> ---
>  fs/dax.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/fs/dax.c b/fs/dax.c
> index 21b4740..5133568 100644
> --- a/fs/dax.c
> +++ b/fs/dax.c
> @@ -690,7 +690,7 @@ struct page *dax_layout_busy_page_range(struct address_space *mapping,
>  	if (IS_ENABLED(CONFIG_FS_DAX_LIMITED))
>  		return NULL;
>  
> -	if (!dax_mapping(mapping) || !mapping_mapped(mapping))
> +	if (!dax_mapping(mapping))
>  		return NULL;
>  
>  	/* If end == LLONG_MAX, all pages from start to till end of file */

I think the patch should probably also add

if (mapping_mapped(mapping))
	unmap_mapping_pages(mapping, start_idx, end_idx - start_idx + 1, 0);
But I don't think it's a blocker unmap_mapping_pages() should do the right thing internally

Acked-by: Balbir Singh <[e-mail]>

Patch

diff --git a/fs/dax.c b/fs/dax.c
index 21b4740..5133568 100644
--- a/fs/dax.c
+++ b/fs/dax.c
@@ -690,7 +690,7 @@  struct page *dax_layout_busy_page_range(struct address_space *mapping,
 	if (IS_ENABLED(CONFIG_FS_DAX_LIMITED))
 		return NULL;
 
-	if (!dax_mapping(mapping) || !mapping_mapped(mapping))
+	if (!dax_mapping(mapping))
 		return NULL;
 
 	/* If end == LLONG_MAX, all pages from start to till end of file */