[PATCH] vfio/type1: Absorb num_pages_contiguous()

[PATCH] vfio/type1: Absorb num_pages_contiguous()

[Alex Williamson]

Objections were raised to adding this helper to common code with only a
single user and dubious generalism.  Pull it back into subsystem code.

Link: https://lore.kernel.org/all/CAHk-=whhYRMS7Xc9k_JBdrGvp++JLmU0T2xXEgn046hWrj7q8Q@mail.gmail.com/
Cc: David Hildenbrand <david@redhat.com>
Cc: Jason Gunthorpe <jgg@nvidia.com>
Cc: Li Zhe <lizhe.67@bytedance.com>
Signed-off-by: Alex Williamson <alex.williamson@redhat.com>
---
 drivers/vfio/vfio_iommu_type1.c | 22 ++++++++++++++++++++++
 include/linux/mm.h              | 23 -----------------------
 2 files changed, 22 insertions(+), 23 deletions(-)

diff --git a/drivers/vfio/vfio_iommu_type1.c b/drivers/vfio/vfio_iommu_type1.c
index 33384a8c152d..3f06a8d937fa 100644
--- a/drivers/vfio/vfio_iommu_type1.c
+++ b/drivers/vfio/vfio_iommu_type1.c
@@ -659,6 +659,28 @@ static long vpfn_pages(struct vfio_dma *dma,
 	return ret;
 }
 
+/*
+ * num_pages_contiguous() - determine the number of contiguous pages
+ * starting from the first page.
+ *
+ * Pages are contiguous if they represent contiguous PFNs. Depending on
+ * the memory model, this can mean that the addresses of the "struct page"s
+ * are not contiguous.
+ *
+ * @pages: an array of page pointers
+ * @nr_pages: length of the array
+ */
+static unsigned long num_pages_contiguous(struct page **pages, size_t nr_pages)
+{
+	size_t i;
+
+	for (i = 1; i < nr_pages; i++)
+		if (pages[i] != nth_page(pages[0], i))
+			break;
+
+	return i;
+}
+
 /*
  * Attempt to pin pages.  We really don't want to track all the pfns and
  * the iommu can only map chunks of consecutive pfns anyway, so get the
diff --git a/include/linux/mm.h b/include/linux/mm.h
index fae82df6d7d7..0ef2ba0c667a 100644
--- a/include/linux/mm.h
+++ b/include/linux/mm.h
@@ -1761,29 +1761,6 @@ static inline unsigned long page_to_section(const struct page *page)
 }
 #endif
 
-/*
- * num_pages_contiguous() - determine the number of contiguous pages
- * starting from the first page.
- *
- * Pages are contiguous if they represent contiguous PFNs. Depending on
- * the memory model, this can mean that the addresses of the "struct page"s
- * are not contiguous.
- *
- * @pages: an array of page pointers
- * @nr_pages: length of the array
- */
-static inline unsigned long num_pages_contiguous(struct page **pages,
-						 size_t nr_pages)
-{
-	size_t i;
-
-	for (i = 1; i < nr_pages; i++)
-		if (pages[i] != nth_page(pages[0], i))
-			break;
-
-	return i;
-}
-
 /**
  * folio_pfn - Return the Page Frame Number of a folio.
  * @folio: The folio.
-- 
2.50.1

Re: [PATCH] vfio/type1: Absorb num_pages_contiguous()

[David Hildenbrand]

On 05.08.25 03:24, Alex Williamson wrote:
> Objections were raised to adding this helper to common code with only a
> single user and dubious generalism.  Pull it back into subsystem code.
> 
> Link: https://lore.kernel.org/all/CAHk-=whhYRMS7Xc9k_JBdrGvp++JLmU0T2xXEgn046hWrj7q8Q@mail.gmail.com/
> Cc: David Hildenbrand <david@redhat.com>
> Cc: Jason Gunthorpe <jgg@nvidia.com>
> Cc: Li Zhe <lizhe.67@bytedance.com>
> Signed-off-by: Alex Williamson <alex.williamson@redhat.com>
> ---

So, we might have a version that Linus should be happy with, that we 
could likely place in mm/util.c.

Alex, how would you want to proceed with that?

-- 
Cheers,

David / dhildenb

Re: [PATCH] vfio/type1: Absorb num_pages_contiguous()

[Alex Williamson]

On Tue, 5 Aug 2025 15:27:35 +0200
David Hildenbrand <david@redhat.com> wrote:

> On 05.08.25 03:24, Alex Williamson wrote:
> > Objections were raised to adding this helper to common code with only a
> > single user and dubious generalism.  Pull it back into subsystem code.
> > 
> > Link: https://lore.kernel.org/all/CAHk-=whhYRMS7Xc9k_JBdrGvp++JLmU0T2xXEgn046hWrj7q8Q@mail.gmail.com/
> > Cc: David Hildenbrand <david@redhat.com>
> > Cc: Jason Gunthorpe <jgg@nvidia.com>
> > Cc: Li Zhe <lizhe.67@bytedance.com>
> > Signed-off-by: Alex Williamson <alex.williamson@redhat.com>
> > ---  
> 
> So, we might have a version that Linus should be happy with, that we 
> could likely place in mm/util.c.
> 
> Alex, how would you want to proceed with that?

Still reading the thread overnight, but if there's a better solution
that Linus won't balk at then let's do it.  Thanks,

Alex

Re: [PATCH] vfio/type1: Absorb num_pages_contiguous()

[Alex Williamson]

On Tue, 5 Aug 2025 07:56:43 -0600
Alex Williamson <alex.williamson@redhat.com> wrote:

> On Tue, 5 Aug 2025 15:27:35 +0200
> David Hildenbrand <david@redhat.com> wrote:
> 
> > On 05.08.25 03:24, Alex Williamson wrote:  
> > > Objections were raised to adding this helper to common code with only a
> > > single user and dubious generalism.  Pull it back into subsystem code.
> > > 
> > > Link: https://lore.kernel.org/all/CAHk-=whhYRMS7Xc9k_JBdrGvp++JLmU0T2xXEgn046hWrj7q8Q@mail.gmail.com/
> > > Cc: David Hildenbrand <david@redhat.com>
> > > Cc: Jason Gunthorpe <jgg@nvidia.com>
> > > Cc: Li Zhe <lizhe.67@bytedance.com>
> > > Signed-off-by: Alex Williamson <alex.williamson@redhat.com>
> > > ---    
> > 
> > So, we might have a version that Linus should be happy with, that we 
> > could likely place in mm/util.c.
> > 
> > Alex, how would you want to proceed with that?  
> 
> Still reading the thread overnight, but if there's a better solution
> that Linus won't balk at then let's do it.  Thanks,

How soon were you thinking of sending an alternate proposal?  I don't
want to make this any messier, but this patch gives us some breathing
room for the merge window.  Thanks,

Alex

Re: [PATCH] vfio/type1: Absorb num_pages_contiguous()

[Alex Williamson]

On Tue, 5 Aug 2025 12:15:58 -0600
Alex Williamson <alex.williamson@redhat.com> wrote:

> On Tue, 5 Aug 2025 07:56:43 -0600
> Alex Williamson <alex.williamson@redhat.com> wrote:
> 
> > On Tue, 5 Aug 2025 15:27:35 +0200
> > David Hildenbrand <david@redhat.com> wrote:
> >   
> > > On 05.08.25 03:24, Alex Williamson wrote:    
> > > > Objections were raised to adding this helper to common code with only a
> > > > single user and dubious generalism.  Pull it back into subsystem code.
> > > > 
> > > > Link: https://lore.kernel.org/all/CAHk-=whhYRMS7Xc9k_JBdrGvp++JLmU0T2xXEgn046hWrj7q8Q@mail.gmail.com/
> > > > Cc: David Hildenbrand <david@redhat.com>
> > > > Cc: Jason Gunthorpe <jgg@nvidia.com>
> > > > Cc: Li Zhe <lizhe.67@bytedance.com>
> > > > Signed-off-by: Alex Williamson <alex.williamson@redhat.com>
> > > > ---      
> > > 
> > > So, we might have a version that Linus should be happy with, that we 
> > > could likely place in mm/util.c.
> > > 
> > > Alex, how would you want to proceed with that?    
> > 
> > Still reading the thread overnight, but if there's a better solution
> > that Linus won't balk at then let's do it.  Thanks,  
> 
> How soon were you thinking of sending an alternate proposal?  I don't
> want to make this any messier, but this patch gives us some breathing
> room for the merge window.  Thanks,

After chatting with David off-list, I think we agree that just moving
this nth_page code into vfio is likely not the best solution since
there does seem to potentially be some agreement for an mm/util.c
helper.  However, it's too late for v6.17, so I've dropped this
optimization from my next branch with the expectation that we'll fix it
properly and re-queue it fro v6.18.  Thanks,

Alex

Re: [PATCH] vfio/type1: Absorb num_pages_contiguous()

[David Hildenbrand]

On 05.08.25 03:24, Alex Williamson wrote:
> Objections were raised to adding this helper to common code with only a
> single user and dubious generalism.  Pull it back into subsystem code.
> 
> Link: https://lore.kernel.org/all/CAHk-=whhYRMS7Xc9k_JBdrGvp++JLmU0T2xXEgn046hWrj7q8Q@mail.gmail.com/
> Cc: David Hildenbrand <david@redhat.com>
> Cc: Jason Gunthorpe <jgg@nvidia.com>
> Cc: Li Zhe <lizhe.67@bytedance.com>
> Signed-off-by: Alex Williamson <alex.williamson@redhat.com>
> ---

So, I took the original patch and
* moved the code to mm_inline.h (sounds like a better fit)
* Tweaked the patch description
* Tweaked the documentation and turned it into proper kerneldoc
* Made the function return "size_t" as well
* Use the page_to_section() trick to avoid nth_page().

Only compile-tested so far. Still running it through some cross compiles.


 From 36d67849bfdbc184990f21464c53585d35648616 Mon Sep 17 00:00:00 2001
From: Li Zhe <lizhe.67@bytedance.com>
Date: Thu, 10 Jul 2025 16:53:51 +0800
Subject: [PATCH] mm: introduce num_pages_contiguous()

Let's add a simple helper for determining the number of contiguous pages
that represent contiguous PFNs.

In an ideal world, this helper would be simpler or not even required.
Unfortunately, on some configs we still have to maintain (SPARSEMEM
without VMEMMAP), the memmap is allocated per memory section, and we might
run into weird corner cases of false positives when blindly testing for
contiguous pages only.

One example of such false positives would be a memory section-sized hole
that does not have a memmap. The surrounding memory sections might get
"struct pages" that are contiguous, but the PFNs are actually not.

This helper will, for example, be useful for determining contiguous PFNs
in a GUP result, to batch further operations across returned "struct
page"s. VFIO will utilize this interface to accelerate the VFIO DMA map
process.

Implementation based on Linus' suggestions to avoid new usage of
nth_page() where avoidable.

Suggested-by: Linus Torvalds <torvalds@linux-foundation.org>
Suggested-by: Jason Gunthorpe <jgg@ziepe.ca>
Signed-off-by: Li Zhe <lizhe.67@bytedance.com>
Co-developed-by: David Hildenbrand <david@redhat.com>
Signed-off-by: David Hildenbrand <david@redhat.com>
---
  include/linux/mm.h        |  7 ++++++-
  include/linux/mm_inline.h | 35 +++++++++++++++++++++++++++++++++++
  2 files changed, 41 insertions(+), 1 deletion(-)

diff --git a/include/linux/mm.h b/include/linux/mm.h
index fa538feaa8d95..2852bcd792745 100644
--- a/include/linux/mm.h
+++ b/include/linux/mm.h
@@ -1759,7 +1759,12 @@ static inline unsigned long page_to_section(const struct page *page)
  {
  	return (page->flags >> SECTIONS_PGSHIFT) & SECTIONS_MASK;
  }
-#endif
+#else /* !SECTION_IN_PAGE_FLAGS */
+static inline unsigned long page_to_section(const struct page *page)
+{
+	return 0;
+}
+#endif /* SECTION_IN_PAGE_FLAGS */
  
  /**
   * folio_pfn - Return the Page Frame Number of a folio.
diff --git a/include/linux/mm_inline.h b/include/linux/mm_inline.h
index 89b518ff097e6..58cb99b69f432 100644
--- a/include/linux/mm_inline.h
+++ b/include/linux/mm_inline.h
@@ -616,4 +616,39 @@ static inline bool vma_has_recency(struct vm_area_struct *vma)
  	return true;
  }
  
+/**
+ * num_pages_contiguous() - determine the number of contiguous pages
+ *                          that represent contiguous PFNs
+ * @pages: an array of page pointers
+ * @nr_pages: length of the array, at least 1
+ *
+ * Determine the number of contiguous pages that represent contiguous PFNs
+ * in @pages, starting from the first page.
+ *
+ * In kernel configs where contiguous pages might not imply contiguous PFNs
+ * over memory section boundaries, this function will stop at the memory
+ * section boundary.
+ *
+ * Returns the number of contiguous pages.
+ */
+static inline size_t num_pages_contiguous(struct page **pages, size_t nr_pages)
+{
+	struct page *cur_page = pages[0];
+	unsigned long section = page_to_section(cur_page);
+	size_t i;
+
+	for (i = 1; i < nr_pages; i++) {
+		if (++cur_page != pages[i])
+			break;
+		/*
+		 * In unproblematic kernel configs, page_to_section() == 0 and
+		 * the whole check will get optimized out.
+		 */
+		if (page_to_section(cur_page) != section)
+			break;
+	}
+
+	return i;
+}
+
  #endif
-- 
2.50.1


-- 
Cheers,

David / dhildenb

Re: [PATCH] vfio/type1: Absorb num_pages_contiguous()

[Jason Gunthorpe]

On Wed, Aug 06, 2025 at 02:35:15PM +0200, David Hildenbrand wrote:
> On 05.08.25 03:24, Alex Williamson wrote:
> +/**
> + * num_pages_contiguous() - determine the number of contiguous pages
> + *                          that represent contiguous PFNs
> + * @pages: an array of page pointers
> + * @nr_pages: length of the array, at least 1
> + *
> + * Determine the number of contiguous pages that represent contiguous PFNs
> + * in @pages, starting from the first page.
> + *
> + * In kernel configs where contiguous pages might not imply contiguous PFNs
> + * over memory section boundaries, this function will stop at the memory
> + * section boundary.

Maybe:

In some kernel configs contiguous PFNs will not have contiguous struct
pages. In these configurations num_pages_contiguous() will return a
smaller than ideal number. The caller should continue to check for pfn
contiguity after each call to num_pages_contiguous().

Jason

Re: [PATCH] vfio/type1: Absorb num_pages_contiguous()

[David Hildenbrand]

On 06.08.25 15:16, Jason Gunthorpe wrote:
> On Wed, Aug 06, 2025 at 02:35:15PM +0200, David Hildenbrand wrote:
>> On 05.08.25 03:24, Alex Williamson wrote:
>> +/**
>> + * num_pages_contiguous() - determine the number of contiguous pages
>> + *                          that represent contiguous PFNs
>> + * @pages: an array of page pointers
>> + * @nr_pages: length of the array, at least 1
>> + *
>> + * Determine the number of contiguous pages that represent contiguous PFNs
>> + * in @pages, starting from the first page.
>> + *
>> + * In kernel configs where contiguous pages might not imply contiguous PFNs
>> + * over memory section boundaries, this function will stop at the memory
>> + * section boundary.
> 
> Maybe:
> 
> In some kernel configs contiguous PFNs will not have contiguous struct
> pages. In these configurations num_pages_contiguous() will return a
> smaller than ideal number. The caller should continue to check for pfn
> contiguity after each call to num_pages_contiguous().

Yeah, sounds great, thanks!

-- 
Cheers,

David / dhildenb

Re: [PATCH] vfio/type1: Absorb num_pages_contiguous()

[lizhe.67]

On Wed, 6 Aug 2025 14:35:15 +0200, david@redhat.com wrote:
 
> On 05.08.25 03:24, Alex Williamson wrote:
> > Objections were raised to adding this helper to common code with only a
> > single user and dubious generalism.  Pull it back into subsystem code.
> > 
> > Link: https://lore.kernel.org/all/CAHk-=whhYRMS7Xc9k_JBdrGvp++JLmU0T2xXEgn046hWrj7q8Q@mail.gmail.com/
> > Cc: David Hildenbrand <david@redhat.com>
> > Cc: Jason Gunthorpe <jgg@nvidia.com>
> > Cc: Li Zhe <lizhe.67@bytedance.com>
> > Signed-off-by: Alex Williamson <alex.williamson@redhat.com>
> > ---
> 
> So, I took the original patch and
> * moved the code to mm_inline.h (sounds like a better fit)
> * Tweaked the patch description
> * Tweaked the documentation and turned it into proper kerneldoc
> * Made the function return "size_t" as well
> * Use the page_to_section() trick to avoid nth_page().
> 
> Only compile-tested so far. Still running it through some cross compiles.
> 
> 
>  From 36d67849bfdbc184990f21464c53585d35648616 Mon Sep 17 00:00:00 2001
> From: Li Zhe <lizhe.67@bytedance.com>
> Date: Thu, 10 Jul 2025 16:53:51 +0800
> Subject: [PATCH] mm: introduce num_pages_contiguous()
> 
> Let's add a simple helper for determining the number of contiguous pages
> that represent contiguous PFNs.
> 
> In an ideal world, this helper would be simpler or not even required.
> Unfortunately, on some configs we still have to maintain (SPARSEMEM
> without VMEMMAP), the memmap is allocated per memory section, and we might
> run into weird corner cases of false positives when blindly testing for
> contiguous pages only.
> 
> One example of such false positives would be a memory section-sized hole
> that does not have a memmap. The surrounding memory sections might get
> "struct pages" that are contiguous, but the PFNs are actually not.
> 
> This helper will, for example, be useful for determining contiguous PFNs
> in a GUP result, to batch further operations across returned "struct
> page"s. VFIO will utilize this interface to accelerate the VFIO DMA map
> process.
> 
> Implementation based on Linus' suggestions to avoid new usage of
> nth_page() where avoidable.
> 
> Suggested-by: Linus Torvalds <torvalds@linux-foundation.org>
> Suggested-by: Jason Gunthorpe <jgg@ziepe.ca>
> Signed-off-by: Li Zhe <lizhe.67@bytedance.com>
> Co-developed-by: David Hildenbrand <david@redhat.com>
> Signed-off-by: David Hildenbrand <david@redhat.com>
> ---
>   include/linux/mm.h        |  7 ++++++-
>   include/linux/mm_inline.h | 35 +++++++++++++++++++++++++++++++++++
>   2 files changed, 41 insertions(+), 1 deletion(-)
> 
> diff --git a/include/linux/mm.h b/include/linux/mm.h
> index fa538feaa8d95..2852bcd792745 100644
> --- a/include/linux/mm.h
> +++ b/include/linux/mm.h
> @@ -1759,7 +1759,12 @@ static inline unsigned long page_to_section(const struct page *page)
>   {
>   	return (page->flags >> SECTIONS_PGSHIFT) & SECTIONS_MASK;
>   }
> -#endif
> +#else /* !SECTION_IN_PAGE_FLAGS */
> +static inline unsigned long page_to_section(const struct page *page)
> +{
> +	return 0;
> +}
> +#endif /* SECTION_IN_PAGE_FLAGS */
>   
>   /**
>    * folio_pfn - Return the Page Frame Number of a folio.
> diff --git a/include/linux/mm_inline.h b/include/linux/mm_inline.h
> index 89b518ff097e6..58cb99b69f432 100644
> --- a/include/linux/mm_inline.h
> +++ b/include/linux/mm_inline.h
> @@ -616,4 +616,39 @@ static inline bool vma_has_recency(struct vm_area_struct *vma)
>   	return true;
>   }
>   
> +/**
> + * num_pages_contiguous() - determine the number of contiguous pages
> + *                          that represent contiguous PFNs
> + * @pages: an array of page pointers
> + * @nr_pages: length of the array, at least 1
> + *
> + * Determine the number of contiguous pages that represent contiguous PFNs
> + * in @pages, starting from the first page.
> + *
> + * In kernel configs where contiguous pages might not imply contiguous PFNs
> + * over memory section boundaries, this function will stop at the memory
> + * section boundary.
> + *
> + * Returns the number of contiguous pages.
> + */
> +static inline size_t num_pages_contiguous(struct page **pages, size_t nr_pages)
> +{
> +	struct page *cur_page = pages[0];
> +	unsigned long section = page_to_section(cur_page);
> +	size_t i;
> +
> +	for (i = 1; i < nr_pages; i++) {
> +		if (++cur_page != pages[i])
> +			break;
> +		/*
> +		 * In unproblematic kernel configs, page_to_section() == 0 and
> +		 * the whole check will get optimized out.
> +		 */
> +		if (page_to_section(cur_page) != section)
> +			break;
> +	}
> +
> +	return i;
> +}
> +
>   #endif

I sincerely appreciate your thoughtful revisions to this patch. The
code looks great.

Based on this patch, I reran the performance tests for the VFIO
optimizations. The results show no significant change from the
previous data.

Since num_pages_contiguous() is now defined in mm_inline.h, the
second patch "vfio/type1: optimize vfio_pin_pages_remote()" in this
series needs to include that header to resolve the build error.

diff --git a/drivers/vfio/vfio_iommu_type1.c b/drivers/vfio/vfio_iommu_type1.c
index 1136d7ac6b59..af98cb94153c 100644
--- a/drivers/vfio/vfio_iommu_type1.c
+++ b/drivers/vfio/vfio_iommu_type1.c
@@ -37,6 +37,7 @@
 #include <linux/vfio.h>
 #include <linux/workqueue.h>
 #include <linux/notifier.h>
+#include <linux/mm_inline.h>
 #include "vfio.h"
 
 #define DRIVER_VERSION  "0.2"

Thanks,
Zhe

Re: [PATCH] vfio/type1: Absorb num_pages_contiguous()

[Alex Williamson]

On Thu,  7 Aug 2025 12:14:19 +0800
lizhe.67@bytedance.com wrote:

> On Wed, 6 Aug 2025 14:35:15 +0200, david@redhat.com wrote:
>  
> > On 05.08.25 03:24, Alex Williamson wrote:  
> > > Objections were raised to adding this helper to common code with only a
> > > single user and dubious generalism.  Pull it back into subsystem code.
> > > 
> > > Link: https://lore.kernel.org/all/CAHk-=whhYRMS7Xc9k_JBdrGvp++JLmU0T2xXEgn046hWrj7q8Q@mail.gmail.com/
> > > Cc: David Hildenbrand <david@redhat.com>
> > > Cc: Jason Gunthorpe <jgg@nvidia.com>
> > > Cc: Li Zhe <lizhe.67@bytedance.com>
> > > Signed-off-by: Alex Williamson <alex.williamson@redhat.com>
> > > ---  
> > 
> > So, I took the original patch and
> > * moved the code to mm_inline.h (sounds like a better fit)
> > * Tweaked the patch description
> > * Tweaked the documentation and turned it into proper kerneldoc
> > * Made the function return "size_t" as well
> > * Use the page_to_section() trick to avoid nth_page().
> > 
> > Only compile-tested so far. Still running it through some cross compiles.
> > 
> > 
> >  From 36d67849bfdbc184990f21464c53585d35648616 Mon Sep 17 00:00:00 2001
> > From: Li Zhe <lizhe.67@bytedance.com>
> > Date: Thu, 10 Jul 2025 16:53:51 +0800
> > Subject: [PATCH] mm: introduce num_pages_contiguous()
> > 
> > Let's add a simple helper for determining the number of contiguous pages
> > that represent contiguous PFNs.
> > 
> > In an ideal world, this helper would be simpler or not even required.
> > Unfortunately, on some configs we still have to maintain (SPARSEMEM
> > without VMEMMAP), the memmap is allocated per memory section, and we might
> > run into weird corner cases of false positives when blindly testing for
> > contiguous pages only.
> > 
> > One example of such false positives would be a memory section-sized hole
> > that does not have a memmap. The surrounding memory sections might get
> > "struct pages" that are contiguous, but the PFNs are actually not.
> > 
> > This helper will, for example, be useful for determining contiguous PFNs
> > in a GUP result, to batch further operations across returned "struct
> > page"s. VFIO will utilize this interface to accelerate the VFIO DMA map
> > process.
> > 
> > Implementation based on Linus' suggestions to avoid new usage of
> > nth_page() where avoidable.
> > 
> > Suggested-by: Linus Torvalds <torvalds@linux-foundation.org>
> > Suggested-by: Jason Gunthorpe <jgg@ziepe.ca>
> > Signed-off-by: Li Zhe <lizhe.67@bytedance.com>
> > Co-developed-by: David Hildenbrand <david@redhat.com>
> > Signed-off-by: David Hildenbrand <david@redhat.com>
> > ---
> >   include/linux/mm.h        |  7 ++++++-
> >   include/linux/mm_inline.h | 35 +++++++++++++++++++++++++++++++++++
> >   2 files changed, 41 insertions(+), 1 deletion(-)
> > 
> > diff --git a/include/linux/mm.h b/include/linux/mm.h
> > index fa538feaa8d95..2852bcd792745 100644
> > --- a/include/linux/mm.h
> > +++ b/include/linux/mm.h
> > @@ -1759,7 +1759,12 @@ static inline unsigned long page_to_section(const struct page *page)
> >   {
> >   	return (page->flags >> SECTIONS_PGSHIFT) & SECTIONS_MASK;
> >   }
> > -#endif
> > +#else /* !SECTION_IN_PAGE_FLAGS */
> > +static inline unsigned long page_to_section(const struct page *page)
> > +{
> > +	return 0;
> > +}
> > +#endif /* SECTION_IN_PAGE_FLAGS */
> >   
> >   /**
> >    * folio_pfn - Return the Page Frame Number of a folio.
> > diff --git a/include/linux/mm_inline.h b/include/linux/mm_inline.h
> > index 89b518ff097e6..58cb99b69f432 100644
> > --- a/include/linux/mm_inline.h
> > +++ b/include/linux/mm_inline.h
> > @@ -616,4 +616,39 @@ static inline bool vma_has_recency(struct vm_area_struct *vma)
> >   	return true;
> >   }
> >   
> > +/**
> > + * num_pages_contiguous() - determine the number of contiguous pages
> > + *                          that represent contiguous PFNs
> > + * @pages: an array of page pointers
> > + * @nr_pages: length of the array, at least 1
> > + *
> > + * Determine the number of contiguous pages that represent contiguous PFNs
> > + * in @pages, starting from the first page.
> > + *
> > + * In kernel configs where contiguous pages might not imply contiguous PFNs
> > + * over memory section boundaries, this function will stop at the memory
> > + * section boundary.
> > + *
> > + * Returns the number of contiguous pages.
> > + */
> > +static inline size_t num_pages_contiguous(struct page **pages, size_t nr_pages)
> > +{
> > +	struct page *cur_page = pages[0];
> > +	unsigned long section = page_to_section(cur_page);
> > +	size_t i;
> > +
> > +	for (i = 1; i < nr_pages; i++) {
> > +		if (++cur_page != pages[i])
> > +			break;
> > +		/*
> > +		 * In unproblematic kernel configs, page_to_section() == 0 and
> > +		 * the whole check will get optimized out.
> > +		 */
> > +		if (page_to_section(cur_page) != section)
> > +			break;
> > +	}
> > +
> > +	return i;
> > +}
> > +
> >   #endif  
> 
> I sincerely appreciate your thoughtful revisions to this patch. The
> code looks great.
> 
> Based on this patch, I reran the performance tests for the VFIO
> optimizations. The results show no significant change from the
> previous data.
> 
> Since num_pages_contiguous() is now defined in mm_inline.h, the
> second patch "vfio/type1: optimize vfio_pin_pages_remote()" in this
> series needs to include that header to resolve the build error.
> 
> diff --git a/drivers/vfio/vfio_iommu_type1.c b/drivers/vfio/vfio_iommu_type1.c
> index 1136d7ac6b59..af98cb94153c 100644
> --- a/drivers/vfio/vfio_iommu_type1.c
> +++ b/drivers/vfio/vfio_iommu_type1.c
> @@ -37,6 +37,7 @@
>  #include <linux/vfio.h>
>  #include <linux/workqueue.h>
>  #include <linux/notifier.h>
> +#include <linux/mm_inline.h>
>  #include "vfio.h"
>  
>  #define DRIVER_VERSION  "0.2"

Hi Zhe,

Once we're all satisfied with the update, please post a full new
series.  Since we're restarting with fresh commits, please include the
fixes directly in the original patches.  Thanks,

Alex

Re: [PATCH] vfio/type1: Absorb num_pages_contiguous()

[lizhe.67]

On Thu, 7 Aug 2025 13:54:20 -0600, alex.williamson@redhat.com wrote:

> On Thu,  7 Aug 2025 12:14:19 +0800
> lizhe.67@bytedance.com wrote:
> 
> > On Wed, 6 Aug 2025 14:35:15 +0200, david@redhat.com wrote:
> >  
> > > On 05.08.25 03:24, Alex Williamson wrote:  
> > > > Objections were raised to adding this helper to common code with only a
> > > > single user and dubious generalism.  Pull it back into subsystem code.
> > > > 
> > > > Link: https://lore.kernel.org/all/CAHk-=whhYRMS7Xc9k_JBdrGvp++JLmU0T2xXEgn046hWrj7q8Q@mail.gmail.com/
> > > > Cc: David Hildenbrand <david@redhat.com>
> > > > Cc: Jason Gunthorpe <jgg@nvidia.com>
> > > > Cc: Li Zhe <lizhe.67@bytedance.com>
> > > > Signed-off-by: Alex Williamson <alex.williamson@redhat.com>
> > > > ---  
> > > 
> > > So, I took the original patch and
> > > * moved the code to mm_inline.h (sounds like a better fit)
> > > * Tweaked the patch description
> > > * Tweaked the documentation and turned it into proper kerneldoc
> > > * Made the function return "size_t" as well
> > > * Use the page_to_section() trick to avoid nth_page().
> > > 
> > > Only compile-tested so far. Still running it through some cross compiles.
> > > 
> > > 
> > >  From 36d67849bfdbc184990f21464c53585d35648616 Mon Sep 17 00:00:00 2001
> > > From: Li Zhe <lizhe.67@bytedance.com>
> > > Date: Thu, 10 Jul 2025 16:53:51 +0800
> > > Subject: [PATCH] mm: introduce num_pages_contiguous()
> > > 
> > > Let's add a simple helper for determining the number of contiguous pages
> > > that represent contiguous PFNs.
> > > 
> > > In an ideal world, this helper would be simpler or not even required.
> > > Unfortunately, on some configs we still have to maintain (SPARSEMEM
> > > without VMEMMAP), the memmap is allocated per memory section, and we might
> > > run into weird corner cases of false positives when blindly testing for
> > > contiguous pages only.
> > > 
> > > One example of such false positives would be a memory section-sized hole
> > > that does not have a memmap. The surrounding memory sections might get
> > > "struct pages" that are contiguous, but the PFNs are actually not.
> > > 
> > > This helper will, for example, be useful for determining contiguous PFNs
> > > in a GUP result, to batch further operations across returned "struct
> > > page"s. VFIO will utilize this interface to accelerate the VFIO DMA map
> > > process.
> > > 
> > > Implementation based on Linus' suggestions to avoid new usage of
> > > nth_page() where avoidable.
> > > 
> > > Suggested-by: Linus Torvalds <torvalds@linux-foundation.org>
> > > Suggested-by: Jason Gunthorpe <jgg@ziepe.ca>
> > > Signed-off-by: Li Zhe <lizhe.67@bytedance.com>
> > > Co-developed-by: David Hildenbrand <david@redhat.com>
> > > Signed-off-by: David Hildenbrand <david@redhat.com>
> > > ---
> > >   include/linux/mm.h        |  7 ++++++-
> > >   include/linux/mm_inline.h | 35 +++++++++++++++++++++++++++++++++++
> > >   2 files changed, 41 insertions(+), 1 deletion(-)
> > > 
> > > diff --git a/include/linux/mm.h b/include/linux/mm.h
> > > index fa538feaa8d95..2852bcd792745 100644
> > > --- a/include/linux/mm.h
> > > +++ b/include/linux/mm.h
> > > @@ -1759,7 +1759,12 @@ static inline unsigned long page_to_section(const struct page *page)
> > >   {
> > >   	return (page->flags >> SECTIONS_PGSHIFT) & SECTIONS_MASK;
> > >   }
> > > -#endif
> > > +#else /* !SECTION_IN_PAGE_FLAGS */
> > > +static inline unsigned long page_to_section(const struct page *page)
> > > +{
> > > +	return 0;
> > > +}
> > > +#endif /* SECTION_IN_PAGE_FLAGS */
> > >   
> > >   /**
> > >    * folio_pfn - Return the Page Frame Number of a folio.
> > > diff --git a/include/linux/mm_inline.h b/include/linux/mm_inline.h
> > > index 89b518ff097e6..58cb99b69f432 100644
> > > --- a/include/linux/mm_inline.h
> > > +++ b/include/linux/mm_inline.h
> > > @@ -616,4 +616,39 @@ static inline bool vma_has_recency(struct vm_area_struct *vma)
> > >   	return true;
> > >   }
> > >   
> > > +/**
> > > + * num_pages_contiguous() - determine the number of contiguous pages
> > > + *                          that represent contiguous PFNs
> > > + * @pages: an array of page pointers
> > > + * @nr_pages: length of the array, at least 1
> > > + *
> > > + * Determine the number of contiguous pages that represent contiguous PFNs
> > > + * in @pages, starting from the first page.
> > > + *
> > > + * In kernel configs where contiguous pages might not imply contiguous PFNs
> > > + * over memory section boundaries, this function will stop at the memory
> > > + * section boundary.
> > > + *
> > > + * Returns the number of contiguous pages.
> > > + */
> > > +static inline size_t num_pages_contiguous(struct page **pages, size_t nr_pages)
> > > +{
> > > +	struct page *cur_page = pages[0];
> > > +	unsigned long section = page_to_section(cur_page);
> > > +	size_t i;
> > > +
> > > +	for (i = 1; i < nr_pages; i++) {
> > > +		if (++cur_page != pages[i])
> > > +			break;
> > > +		/*
> > > +		 * In unproblematic kernel configs, page_to_section() == 0 and
> > > +		 * the whole check will get optimized out.
> > > +		 */
> > > +		if (page_to_section(cur_page) != section)
> > > +			break;
> > > +	}
> > > +
> > > +	return i;
> > > +}
> > > +
> > >   #endif  
> > 
> > I sincerely appreciate your thoughtful revisions to this patch. The
> > code looks great.
> > 
> > Based on this patch, I reran the performance tests for the VFIO
> > optimizations. The results show no significant change from the
> > previous data.
> > 
> > Since num_pages_contiguous() is now defined in mm_inline.h, the
> > second patch "vfio/type1: optimize vfio_pin_pages_remote()" in this
> > series needs to include that header to resolve the build error.
> > 
> > diff --git a/drivers/vfio/vfio_iommu_type1.c b/drivers/vfio/vfio_iommu_type1.c
> > index 1136d7ac6b59..af98cb94153c 100644
> > --- a/drivers/vfio/vfio_iommu_type1.c
> > +++ b/drivers/vfio/vfio_iommu_type1.c
> > @@ -37,6 +37,7 @@
> >  #include <linux/vfio.h>
> >  #include <linux/workqueue.h>
> >  #include <linux/notifier.h>
> > +#include <linux/mm_inline.h>
> >  #include "vfio.h"
> >  
> >  #define DRIVER_VERSION  "0.2"
> 
> Hi Zhe,
> 
> Once we're all satisfied with the update, please post a full new
> series.  Since we're restarting with fresh commits, please include the
> fixes directly in the original patches.  Thanks,

Got it. If no more comments come in over the next few days, I'll
send out a fresh patchset.

Thanks,
Zhe