diff mbox series

[1/2] dax: Remove access to page->index

Message ID 20241216155408.8102-1-willy@infradead.org (mailing list archive)
State New
Headers show
Series [1/2] dax: Remove access to page->index | expand

Commit Message

Matthew Wilcox Dec. 16, 2024, 3:53 p.m. UTC
This looks like a complete mess (why are we setting page->index at page
fault time?), but I no longer care about DAX, and there's no reason to
let DAX hold us back from removing page->index.

Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org>
---
 drivers/dax/device.c | 9 ++++-----
 1 file changed, 4 insertions(+), 5 deletions(-)

Comments

Jane Chu Dec. 16, 2024, 5:49 p.m. UTC | #1
On 12/16/2024 7:53 AM, Matthew Wilcox (Oracle) wrote:

> This looks like a complete mess (why are we setting page->index at page
> fault time?), but I no longer care about DAX, and there's no reason to
> let DAX hold us back from removing page->index.
>
> Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org>
> ---
>   drivers/dax/device.c | 9 ++++-----
>   1 file changed, 4 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/dax/device.c b/drivers/dax/device.c
> index 6d74e62bbee0..bc871a34b9cd 100644
> --- a/drivers/dax/device.c
> +++ b/drivers/dax/device.c
> @@ -89,14 +89,13 @@ static void dax_set_mapping(struct vm_fault *vmf, pfn_t pfn,
>   			ALIGN_DOWN(vmf->address, fault_size));
>   
>   	for (i = 0; i < nr_pages; i++) {
> -		struct page *page = pfn_to_page(pfn_t_to_pfn(pfn) + i);
> +		struct folio *folio = pfn_folio(pfn_t_to_pfn(pfn) + i);
>   
> -		page = compound_head(page);
> -		if (page->mapping)
> +		if (folio->mapping)
>   			continue;
>   
> -		page->mapping = filp->f_mapping;
> -		page->index = pgoff + i;
> +		folio->mapping = filp->f_mapping;
> +		folio->index = pgoff + i;
>   	}
>   }
>   

Looks good.

Reviewed-by: Jane Chu <jane.chu@oracle.com>

-jane
Dan Williams Jan. 7, 2025, 12:43 a.m. UTC | #2
Matthew Wilcox (Oracle) wrote:
> This looks like a complete mess (why are we setting page->index at page
> fault time?)

Full story in Alistair's patches, but this a side effect of bypassing
the page allocator for instantiating file-backed mappings.

> but I no longer care about DAX, and there's no reason to
> let DAX hold us back from removing page->index.

Question is whether to move ahead with this now and have Alistair
rebase, or push ahead with getting Alistair's series into -next? I am
hoping that Alistair's series can move ahead this cycle, but still
catching up on the latest after the holiday break.
Alistair Popple Jan. 7, 2025, 11:24 p.m. UTC | #3
On Mon, Jan 06, 2025 at 04:43:13PM -0800, Dan Williams wrote:
> Matthew Wilcox (Oracle) wrote:
> > This looks like a complete mess (why are we setting page->index at page
> > fault time?)
> 
> Full story in Alistair's patches, but this a side effect of bypassing
> the page allocator for instantiating file-backed mappings.
> 
> > but I no longer care about DAX, and there's no reason to
> > let DAX hold us back from removing page->index.
> 
> Question is whether to move ahead with this now and have Alistair
> rebase, or push ahead with getting Alistair's series into -next? I am
> hoping that Alistair's series can move ahead this cycle, but still
> catching up on the latest after the holiday break.

The rebase probably isn't that hard, but if we push ahead with my series it's
largely unnecessary as it moves this over to the folio anyway. I've just posted
a respin on top of next-20241216 -
https://lore.kernel.org/linux-mm/cover.425da7c4e76c2749d0ad1734f972b06114e02d52.1736221254.git-series.apopple@nvidia.com/

 - Alistair
diff mbox series

Patch

diff --git a/drivers/dax/device.c b/drivers/dax/device.c
index 6d74e62bbee0..bc871a34b9cd 100644
--- a/drivers/dax/device.c
+++ b/drivers/dax/device.c
@@ -89,14 +89,13 @@  static void dax_set_mapping(struct vm_fault *vmf, pfn_t pfn,
 			ALIGN_DOWN(vmf->address, fault_size));
 
 	for (i = 0; i < nr_pages; i++) {
-		struct page *page = pfn_to_page(pfn_t_to_pfn(pfn) + i);
+		struct folio *folio = pfn_folio(pfn_t_to_pfn(pfn) + i);
 
-		page = compound_head(page);
-		if (page->mapping)
+		if (folio->mapping)
 			continue;
 
-		page->mapping = filp->f_mapping;
-		page->index = pgoff + i;
+		folio->mapping = filp->f_mapping;
+		folio->index = pgoff + i;
 	}
 }