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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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

> On Wed, 6 Aug 2025 14:35:15 +0200, david@xxxxxxxxxx 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@xxxxxxxxxxxxxx/
> > > Cc: David Hildenbrand <david@xxxxxxxxxx>
> > > Cc: Jason Gunthorpe <jgg@xxxxxxxxxx>
> > > Cc: Li Zhe <lizhe.67@xxxxxxxxxxxxx>
> > > Signed-off-by: Alex Williamson <alex.williamson@xxxxxxxxxx>
> > > ---  
> > 
> > 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@xxxxxxxxxxxxx>
> > 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@xxxxxxxxxxxxxxxxxxxx>
> > Suggested-by: Jason Gunthorpe <jgg@xxxxxxxx>
> > Signed-off-by: Li Zhe <lizhe.67@xxxxxxxxxxxxx>
> > Co-developed-by: David Hildenbrand <david@xxxxxxxxxx>
> > Signed-off-by: David Hildenbrand <david@xxxxxxxxxx>
> > ---
> >   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





[Index of Archives]     [KVM ARM]     [KVM ia64]     [KVM ppc]     [Virtualization Tools]     [Spice Development]     [Libvirt]     [Libvirt Users]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite Questions]     [Linux Kernel]     [Linux SCSI]     [XFree86]

  Powered by Linux