From: Stanislav Kinsburskii <[email protected]> Sent: Monday,
April 20, 2026 9:22 AM
>
> On Mon, Apr 13, 2026 at 09:08:16PM +0000, Michael Kelley wrote:
> > From: Stanislav Kinsburskii <[email protected]> Sent:
> > Monday, March 30, 2026 1:04 PM
> > >
[snip]
> > > @@ -57,60 +58,61 @@ static int mshv_chunk_stride(struct page *page,
> > > /**
> > > * mshv_region_process_chunk - Processes a contiguous chunk of memory
> > > pages
> > > * in a region.
> > > - * @region : Pointer to the memory region structure.
> > > - * @flags : Flags to pass to the handler.
> > > - * @page_offset: Offset into the region's pages array to start
> > > processing.
> > > - * @page_count : Number of pages to process.
> > > - * @handler : Callback function to handle the chunk.
> > > + * @region : Pointer to the memory region structure.
> > > + * @flags : Flags to pass to the handler.
> > > + * @pfn_offset: Offset into the region's PFNs array to start processing.
> > > + * @pfn_count : Number of PFNs to process.
> > > + * @handler : Callback function to handle the chunk.
> > > *
> > > - * This function scans the region's pages starting from @page_offset,
> > > - * checking for contiguous present pages of the same size (normal or
> > > huge).
> > > - * It invokes @handler for the chunk of contiguous pages found. Returns
> > > the
> > > - * number of pages handled, or a negative error code if the first page is
> > > - * not present or the handler fails.
> > > + * This function scans the region's PFNs starting from @pfn_offset,
> > > + * checking for contiguous valid PFNs backed by pages of the same size
> > > + * (normal or huge). It invokes @handler for the chunk of contiguous
> > > valid
> > > + * PFNs found. Returns the number of PFNs handled, or a negative error
> > > code
> > > + * if the first PFN is invalid or the handler fails.
> > > *
> > > - * Note: The @handler callback must be able to handle both normal and
> > > huge
> > > - * pages.
> > > + * Note: The @handler callback must be able to handle valid PFNs backed
> > > by
> > > + * both normal and huge pages.
> > > *
> > > * Return: Number of pages handled, or negative error code.
> > > */
> > > -static long mshv_region_process_chunk(struct mshv_mem_region *region,
> > > - u32 flags,
> > > - u64 page_offset, u64 page_count,
> > > - int (*handler)(struct mshv_mem_region
> > > *region,
> > > - u32 flags,
> > > - u64 page_offset,
> > > - u64 page_count,
> > > - bool huge_page))
> > > +static long mshv_region_process_pfns(struct mshv_mem_region *region,
> > > + u32 flags,
> > > + u64 pfn_offset, u64 pfn_count,
> > > + int (*handler)(struct mshv_mem_region
> > > *region,
> > > + u32 flags,
> > > + u64 pfn_offset,
> > > + u64 pfn_count,
> > > + bool huge_page))
> > > {
> > > - u64 gfn = region->start_gfn + page_offset;
> > > + u64 gfn = region->start_gfn + pfn_offset;
> > > u64 count;
> > > - struct page *page;
> > > + unsigned long pfn;
> > > int stride, ret;
> > >
> > > - page = region->mreg_pages[page_offset];
> > > - if (!page)
> > > + pfn = region->mreg_pfns[pfn_offset];
> > > + if (!pfn_valid(pfn))
> > > return -EINVAL;
> > >
> > > - stride = mshv_chunk_stride(page, gfn, page_count);
> > > + stride = mshv_chunk_stride(pfn_to_page(pfn), gfn, pfn_count);
> > > if (stride < 0)
> > > return stride;
> > >
> > > /* Start at stride since the first stride is validated */
> > > - for (count = stride; count < page_count; count += stride) {
> > > - page = region->mreg_pages[page_offset + count];
> > > + for (count = stride; count < pfn_count ; count += stride) {
> > > + pfn = region->mreg_pfns[pfn_offset + count];
> > >
> > > - /* Break if current page is not present */
> > > - if (!page)
> > > + /* Break if current pfn is invalid */
> > > + if (!pfn_valid(pfn))
> >
> > pfn_valid() is a relatively expensive test to be doing in a loop
> > on what may be every single page. It does an RCU lock/unlock
> > and make other checks that aren't necessary here. Since
> > mreg_pfns[] is populated from mm calls, the only invalid PFNs
> > would be MSHV_INVALID_PFN that code in this module has
> > explicitly put there. Just testing against MSHV_INVALID_PFN
> > would be a lot faster here and elsewhere in this module. It's
> > really a "pfn set/not set" test. Defining a pfn_set() macro
> > here in this module that tests against MSHV_INVALID_PFN
> > would accomplish the same thing more efficiently.
> >
>
> Yes, we could do it the way you suggest. For completeness, I should add
> that pfn_valid() is expensive only on 32-bit ARM and ARC, which we
> don’t care about.
>
Could you elaborate? On x86, I'm seeing that pfn_valid() is about
220 bytes of code. It's the version in include/linux/mmzone.h, not
the simple version in include/asm-generic/memory_model.h. The
latter is used only for CONFIG_FLATMEM=y. Or is the root partition
kernel build setting CONFIG_FLATMEM_MANUAL and hence getting
the simple version?
Michael
