The subject does not make sense. All P2P is on ZONE_DEVICE pages.
It seems like this is about device private memory?
On Tue, Oct 15, 2024 at 06:23:45PM +0300, Yonatan Maman wrote:
> From: Yonatan Maman <Ymaman@xxxxxxxxxx>
>
> hmm_range_fault() natively triggers a page fault on device private
> pages, migrating them to RAM.
That "natively" above doesn't make sense to me.
> In some cases, such as with RDMA devices,
> the migration overhead between the device (e.g., GPU) and the CPU, and
> vice-versa, significantly damages performance. Thus, enabling Peer-to-
s/damages/degrades/
> Peer (P2P) DMA access for device private page might be crucial for
> minimizing data transfer overhead.
>
> This change introduces an API to support P2P connections for device
> private pages by implementing the following:
"This change.. " or "This patch.." is pointless, just explain what you
are doing.
>
> - Leveraging the struct pagemap_ops for P2P Page Callbacks. This
> callback involves mapping the page to MMIO and returning the
> corresponding PCI_P2P page.
While P2P uses the same underlying PCIe TLPs as MMIO, it is not
MMIO by definition, as memory mapped I/O is by definition about
the CPU memory mappping so that load and store instructions cause
the I/O. It also uses very different concepts in Linux.
> - Utilizing hmm_range_fault for Initializing P2P Connections. The API
There is no concept of a "connection" in PCIe dta transfers.
> also adds the HMM_PFN_REQ_TRY_P2P flag option for the
> hmm_range_fault caller to initialize P2P. If set, hmm_range_fault
> attempts initializing the P2P connection first, if the owner device
> supports P2P, using p2p_page. In case of failure or lack of support,
> hmm_range_fault will continue with the regular flow of migrating the
> page to RAM.
What is the need for the flag? As far as I can tell from reading
the series, the P2P mapping is entirely transparent to the callers
of hmm_range_fault.
> + /*
> + * Used for private (un-addressable) device memory only. Return a
> + * corresponding struct page, that can be mapped to device
> + * (e.g using dma_map_page)
> + */
> + struct page *(*get_dma_page_for_device)(struct page *private_page);
We are talking about P2P memory here. How do you manage to get a page
that dma_map_page can be used on? All P2P memory needs to use the P2P
aware dma_map_sg as the pages for P2P memory are just fake zone device
pages.
> + * P2P for supported pages, and according to caller request
> + * translate the private page to the match P2P page if it fails
> + * continue with the regular flow
> + */
> + if (is_device_private_entry(entry)) {
> + get_dma_page_handler =
> + pfn_swap_entry_to_page(entry)
> + ->pgmap->ops->get_dma_page_for_device;
> + if ((hmm_vma_walk->range->default_flags &
> + HMM_PFN_REQ_ALLOW_P2P) &&
> + get_dma_page_handler) {
> + dma_page = get_dma_page_handler(
> + pfn_swap_entry_to_page(entry));
This is really messy. You probably really want to share a branch
with the private page handling for the owner so that you only need
a single is_device_private_entry and can use a local variable for
to shortcut finding the page. Probably best done with a little helper:
Then this becomes:
static bool hmm_handle_device_private(struct hmm_range *range,
swp_entry_t entry, unsigned long *hmm_pfn)
{
struct page *page = pfn_swap_entry_to_page(entry);
struct dev_pagemap *pgmap = page->pgmap;
if (pgmap->owner == range->dev_private_owner) {
*hmm_pfn = swp_offset_pfn(entry);
goto found;
}
if (pgmap->ops->get_dma_page_for_device) {
*hmm_pfn =
page_to_pfn(pgmap->ops->get_dma_page_for_device(page));
goto found;
}
return false;
found:
*hmm_pfn |= HMM_PFN_VALID
if (is_writable_device_private_entry(entry))
*hmm_pfn |= HMM_PFN_WRITE;
return true;
}
which also makes it clear that returning a page from the method is
not that great, a PFN might work a lot better, e.g.
unsigned long (*device_private_dma_pfn)(struct page *page);
