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

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

[Matthew Wilcox (Oracle)]

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

[PATCH 2/2] dax: Use folios more widely within DAX

[Matthew Wilcox (Oracle)]

Convert from pfn to folio instead of page and use those folios
throughout to avoid accesses to page->index and page->mapping.

Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org>
---
 fs/dax.c | 53 +++++++++++++++++++++++++++--------------------------
 1 file changed, 27 insertions(+), 26 deletions(-)

diff --git a/fs/dax.c b/fs/dax.c
index 21b47402b3dc..972febc6fb9d 100644
--- a/fs/dax.c
+++ b/fs/dax.c
@@ -320,38 +320,39 @@ static unsigned long dax_end_pfn(void *entry)
 	for (pfn = dax_to_pfn(entry); \
 			pfn < dax_end_pfn(entry); pfn++)
 
-static inline bool dax_page_is_shared(struct page *page)
+static inline bool dax_folio_is_shared(struct folio *folio)
 {
-	return page->mapping == PAGE_MAPPING_DAX_SHARED;
+	return folio->mapping == PAGE_MAPPING_DAX_SHARED;
 }
 
 /*
- * Set the page->mapping with PAGE_MAPPING_DAX_SHARED flag, increase the
+ * Set the folio->mapping with PAGE_MAPPING_DAX_SHARED flag, increase the
  * refcount.
  */
-static inline void dax_page_share_get(struct page *page)
+static inline void dax_folio_share_get(struct folio *folio)
 {
-	if (page->mapping != PAGE_MAPPING_DAX_SHARED) {
+	if (folio->mapping != PAGE_MAPPING_DAX_SHARED) {
 		/*
 		 * Reset the index if the page was already mapped
 		 * regularly before.
 		 */
-		if (page->mapping)
-			page->share = 1;
-		page->mapping = PAGE_MAPPING_DAX_SHARED;
+		if (folio->mapping)
+			folio->page.share = 1;
+		folio->mapping = PAGE_MAPPING_DAX_SHARED;
 	}
-	page->share++;
+	folio->page.share++;
 }
 
-static inline unsigned long dax_page_share_put(struct page *page)
+static inline unsigned long dax_folio_share_put(struct folio *folio)
 {
-	return --page->share;
+	return --folio->page.share;
 }
 
 /*
- * When it is called in dax_insert_entry(), the shared flag will indicate that
- * whether this entry is shared by multiple files.  If so, set the page->mapping
- * PAGE_MAPPING_DAX_SHARED, and use page->share as refcount.
+ * When it is called in dax_insert_entry(), the shared flag will indicate
+ * that whether this entry is shared by multiple files.  If so, set
+ * the folio->mapping PAGE_MAPPING_DAX_SHARED, and use page->share
+ * as refcount.
  */
 static void dax_associate_entry(void *entry, struct address_space *mapping,
 		struct vm_area_struct *vma, unsigned long address, bool shared)
@@ -364,14 +365,14 @@ static void dax_associate_entry(void *entry, struct address_space *mapping,
 
 	index = linear_page_index(vma, address & ~(size - 1));
 	for_each_mapped_pfn(entry, pfn) {
-		struct page *page = pfn_to_page(pfn);
+		struct folio *folio = pfn_folio(pfn);
 
 		if (shared) {
-			dax_page_share_get(page);
+			dax_folio_share_get(folio);
 		} else {
-			WARN_ON_ONCE(page->mapping);
-			page->mapping = mapping;
-			page->index = index + i++;
+			WARN_ON_ONCE(folio->mapping);
+			folio->mapping = mapping;
+			folio->index = index + i++;
 		}
 	}
 }
@@ -385,17 +386,17 @@ static void dax_disassociate_entry(void *entry, struct address_space *mapping,
 		return;
 
 	for_each_mapped_pfn(entry, pfn) {
-		struct page *page = pfn_to_page(pfn);
+		struct folio *folio = pfn_folio(pfn);
 
-		WARN_ON_ONCE(trunc && page_ref_count(page) > 1);
-		if (dax_page_is_shared(page)) {
+		WARN_ON_ONCE(trunc && folio_ref_count(folio) > 1);
+		if (dax_folio_is_shared(folio)) {
 			/* keep the shared flag if this page is still shared */
-			if (dax_page_share_put(page) > 0)
+			if (dax_folio_share_put(folio) > 0)
 				continue;
 		} else
-			WARN_ON_ONCE(page->mapping && page->mapping != mapping);
-		page->mapping = NULL;
-		page->index = 0;
+			WARN_ON_ONCE(folio->mapping && folio->mapping != mapping);
+		folio->mapping = NULL;
+		folio->index = 0;
 	}
 }
 
-- 
2.45.2

Re: [PATCH 2/2] dax: Use folios more widely within DAX

[jane.chu]

On 12/16/2024 7:53 AM, Matthew Wilcox (Oracle) wrote:

> Convert from pfn to folio instead of page and use those folios
> throughout to avoid accesses to page->index and page->mapping.
>
> Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org>
> ---
>   fs/dax.c | 53 +++++++++++++++++++++++++++--------------------------
>   1 file changed, 27 insertions(+), 26 deletions(-)
>
> diff --git a/fs/dax.c b/fs/dax.c
> index 21b47402b3dc..972febc6fb9d 100644
> --- a/fs/dax.c
> +++ b/fs/dax.c
> @@ -320,38 +320,39 @@ static unsigned long dax_end_pfn(void *entry)
>   	for (pfn = dax_to_pfn(entry); \
>   			pfn < dax_end_pfn(entry); pfn++)
>   
> -static inline bool dax_page_is_shared(struct page *page)
> +static inline bool dax_folio_is_shared(struct folio *folio)
>   {
> -	return page->mapping == PAGE_MAPPING_DAX_SHARED;
> +	return folio->mapping == PAGE_MAPPING_DAX_SHARED;
>   }
>   
>   /*
> - * Set the page->mapping with PAGE_MAPPING_DAX_SHARED flag, increase the
> + * Set the folio->mapping with PAGE_MAPPING_DAX_SHARED flag, increase the
>    * refcount.
>    */
> -static inline void dax_page_share_get(struct page *page)
> +static inline void dax_folio_share_get(struct folio *folio)
>   {
> -	if (page->mapping != PAGE_MAPPING_DAX_SHARED) {
> +	if (folio->mapping != PAGE_MAPPING_DAX_SHARED) {
>   		/*
>   		 * Reset the index if the page was already mapped
>   		 * regularly before.
>   		 */
> -		if (page->mapping)
> -			page->share = 1;
> -		page->mapping = PAGE_MAPPING_DAX_SHARED;
> +		if (folio->mapping)
> +			folio->page.share = 1;
> +		folio->mapping = PAGE_MAPPING_DAX_SHARED;
>   	}
> -	page->share++;
> +	folio->page.share++;
>   }
>   
> -static inline unsigned long dax_page_share_put(struct page *page)
> +static inline unsigned long dax_folio_share_put(struct folio *folio)
>   {
> -	return --page->share;
> +	return --folio->page.share;
>   }
>   
>   /*
> - * When it is called in dax_insert_entry(), the shared flag will indicate that
> - * whether this entry is shared by multiple files.  If so, set the page->mapping
> - * PAGE_MAPPING_DAX_SHARED, and use page->share as refcount.
> + * When it is called in dax_insert_entry(), the shared flag will indicate
> + * that whether this entry is shared by multiple files.  If so, set
> + * the folio->mapping PAGE_MAPPING_DAX_SHARED, and use page->share
> + * as refcount.
>    */
>   static void dax_associate_entry(void *entry, struct address_space *mapping,
>   		struct vm_area_struct *vma, unsigned long address, bool shared)
> @@ -364,14 +365,14 @@ static void dax_associate_entry(void *entry, struct address_space *mapping,
>   
>   	index = linear_page_index(vma, address & ~(size - 1));
>   	for_each_mapped_pfn(entry, pfn) {
> -		struct page *page = pfn_to_page(pfn);
> +		struct folio *folio = pfn_folio(pfn);
>   
>   		if (shared) {
> -			dax_page_share_get(page);
> +			dax_folio_share_get(folio);
>   		} else {
> -			WARN_ON_ONCE(page->mapping);
> -			page->mapping = mapping;
> -			page->index = index + i++;
> +			WARN_ON_ONCE(folio->mapping);
> +			folio->mapping = mapping;
> +			folio->index = index + i++;
>   		}
>   	}
>   }
> @@ -385,17 +386,17 @@ static void dax_disassociate_entry(void *entry, struct address_space *mapping,
>   		return;
>   
>   	for_each_mapped_pfn(entry, pfn) {
> -		struct page *page = pfn_to_page(pfn);
> +		struct folio *folio = pfn_folio(pfn);
>   
> -		WARN_ON_ONCE(trunc && page_ref_count(page) > 1);
> -		if (dax_page_is_shared(page)) {
> +		WARN_ON_ONCE(trunc && folio_ref_count(folio) > 1);
> +		if (dax_folio_is_shared(folio)) {
>   			/* keep the shared flag if this page is still shared */
> -			if (dax_page_share_put(page) > 0)
> +			if (dax_folio_share_put(folio) > 0)
>   				continue;
>   		} else
> -			WARN_ON_ONCE(page->mapping && page->mapping != mapping);
> -		page->mapping = NULL;
> -		page->index = 0;
> +			WARN_ON_ONCE(folio->mapping && folio->mapping != mapping);
> +		folio->mapping = NULL;
> +		folio->index = 0;
>   	}
>   }
>   

Looks good.

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

-jane

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

[jane.chu]

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

Re: [PATCH 2/2] dax: Use folios more widely within DAX

[Alistair Popple]

On Mon, Dec 16, 2024 at 03:53:56PM +0000, Matthew Wilcox (Oracle) wrote:
> Convert from pfn to folio instead of page and use those folios
> throughout to avoid accesses to page->index and page->mapping.
> 
> Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org>
> ---
>  fs/dax.c | 53 +++++++++++++++++++++++++++--------------------------
>  1 file changed, 27 insertions(+), 26 deletions(-)
> 
> diff --git a/fs/dax.c b/fs/dax.c
> index 21b47402b3dc..972febc6fb9d 100644
> --- a/fs/dax.c
> +++ b/fs/dax.c
> @@ -320,38 +320,39 @@ static unsigned long dax_end_pfn(void *entry)
>  	for (pfn = dax_to_pfn(entry); \
>  			pfn < dax_end_pfn(entry); pfn++)
>  
> -static inline bool dax_page_is_shared(struct page *page)
> +static inline bool dax_folio_is_shared(struct folio *folio)
>  {
> -	return page->mapping == PAGE_MAPPING_DAX_SHARED;
> +	return folio->mapping == PAGE_MAPPING_DAX_SHARED;

This will conflict with my series which introduces compound ZONE_DEVICE
pages to free up a PTE bit and allow FS DAX pages to be refcounted
normally. The main change is here -
https://lore.kernel.org/ linux-mm/39a896451e59b735f205e34da5510ead5e4cd47d.1732239628.git-series.apopple@nvidia.com/

I'm hoping we can get that in linux-next "soon".

>  }
>  
>  /*
> - * Set the page->mapping with PAGE_MAPPING_DAX_SHARED flag, increase the
> + * Set the folio->mapping with PAGE_MAPPING_DAX_SHARED flag, increase the
>   * refcount.
>   */
> -static inline void dax_page_share_get(struct page *page)
> +static inline void dax_folio_share_get(struct folio *folio)
>  {
> -	if (page->mapping != PAGE_MAPPING_DAX_SHARED) {
> +	if (folio->mapping != PAGE_MAPPING_DAX_SHARED) {
>  		/*
>  		 * Reset the index if the page was already mapped
>  		 * regularly before.
>  		 */
> -		if (page->mapping)
> -			page->share = 1;
> -		page->mapping = PAGE_MAPPING_DAX_SHARED;
> +		if (folio->mapping)
> +			folio->page.share = 1;

It also moves the share accounting to the folio as well so we could remove the
whole page->index/share union once that's merged.

> +		folio->mapping = PAGE_MAPPING_DAX_SHARED;
>  	}
> -	page->share++;
> +	folio->page.share++;
>  }
>  
> -static inline unsigned long dax_page_share_put(struct page *page)
> +static inline unsigned long dax_folio_share_put(struct folio *folio)
>  {
> -	return --page->share;
> +	return --folio->page.share;
>  }
>  
>  /*
> - * When it is called in dax_insert_entry(), the shared flag will indicate that
> - * whether this entry is shared by multiple files.  If so, set the page->mapping
> - * PAGE_MAPPING_DAX_SHARED, and use page->share as refcount.
> + * When it is called in dax_insert_entry(), the shared flag will indicate
> + * that whether this entry is shared by multiple files.  If so, set
> + * the folio->mapping PAGE_MAPPING_DAX_SHARED, and use page->share
> + * as refcount.
>   */
>  static void dax_associate_entry(void *entry, struct address_space *mapping,
>  		struct vm_area_struct *vma, unsigned long address, bool shared)
> @@ -364,14 +365,14 @@ static void dax_associate_entry(void *entry, struct address_space *mapping,
>  
>  	index = linear_page_index(vma, address & ~(size - 1));
>  	for_each_mapped_pfn(entry, pfn) {
> -		struct page *page = pfn_to_page(pfn);
> +		struct folio *folio = pfn_folio(pfn);
>  
>  		if (shared) {
> -			dax_page_share_get(page);
> +			dax_folio_share_get(folio);
>  		} else {
> -			WARN_ON_ONCE(page->mapping);
> -			page->mapping = mapping;
> -			page->index = index + i++;
> +			WARN_ON_ONCE(folio->mapping);
> +			folio->mapping = mapping;
> +			folio->index = index + i++;
>  		}
>  	}
>  }
> @@ -385,17 +386,17 @@ static void dax_disassociate_entry(void *entry, struct address_space *mapping,
>  		return;
>  
>  	for_each_mapped_pfn(entry, pfn) {
> -		struct page *page = pfn_to_page(pfn);
> +		struct folio *folio = pfn_folio(pfn);
>  
> -		WARN_ON_ONCE(trunc && page_ref_count(page) > 1);
> -		if (dax_page_is_shared(page)) {
> +		WARN_ON_ONCE(trunc && folio_ref_count(folio) > 1);
> +		if (dax_folio_is_shared(folio)) {
>  			/* keep the shared flag if this page is still shared */
> -			if (dax_page_share_put(page) > 0)
> +			if (dax_folio_share_put(folio) > 0)
>  				continue;
>  		} else
> -			WARN_ON_ONCE(page->mapping && page->mapping != mapping);
> -		page->mapping = NULL;
> -		page->index = 0;
> +			WARN_ON_ONCE(folio->mapping && folio->mapping != mapping);
> +		folio->mapping = NULL;
> +		folio->index = 0;
>  	}
>  }
>  
> -- 
> 2.45.2
> 
> 

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

[Dan Williams]

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.

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

[Alistair Popple]

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

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

[Matthew Wilcox]

On Wed, Jan 08, 2025 at 10:24:00AM +1100, Alistair Popple wrote:
> 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/

Looking at what's in linux-next today, there's no changes to
dax_set_mapping(), so this patch still applies cleanly (patch 2/2 is
obviated).  Can you look at this patch (1/2) again and apply it if it
seems good?

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

[Dan Williams]

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.

This is a safe conversion for the same reason that Alistair's conversion
to vmf_insert_folio_* is safe, folio metadata is always initialized for
device-dax.

Reviewed-by: Dan Williams <dan.j.williams@intel.com>

Andrew, can you pick this one up at the end of Alistair's series?

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

[Andrew Morton]

On Wed, 8 Jan 2025 10:24:00 +1100 Alistair Popple <apopple@nvidia.com> wrote:

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

I'll drop your v7 series from mm-unstable and shall add these two
patches from Matthew.