On 2021-03-12 6:38 p.m., Ira Weiny wrote: > On Thu, Mar 11, 2021 at 04:31:34PM -0700, Logan Gunthorpe wrote: >> Introduce pci_p2pdma_should_map_bus() which is meant to be called by >> DMA map functions to determine how to map a given p2pdma page. >> >> pci_p2pdma_bus_offset() is also added to allow callers to get the bus >> offset if they need to map the bus address. >> >> Signed-off-by: Logan Gunthorpe <logang@xxxxxxxxxxxx> >> --- >> drivers/pci/p2pdma.c | 50 ++++++++++++++++++++++++++++++++++++++ >> include/linux/pci-p2pdma.h | 11 +++++++++ >> 2 files changed, 61 insertions(+) >> >> diff --git a/drivers/pci/p2pdma.c b/drivers/pci/p2pdma.c >> index 7f6836732bce..66d16b7eb668 100644 >> --- a/drivers/pci/p2pdma.c >> +++ b/drivers/pci/p2pdma.c >> @@ -912,6 +912,56 @@ void pci_p2pdma_unmap_sg_attrs(struct device *dev, struct scatterlist *sg, >> } >> EXPORT_SYMBOL_GPL(pci_p2pdma_unmap_sg_attrs); >> >> +/** >> + * pci_p2pdma_bus_offset - returns the bus offset for a given page >> + * @page: page to get the offset for >> + * >> + * Must be passed a PCI p2pdma page. >> + */ >> +u64 pci_p2pdma_bus_offset(struct page *page) >> +{ >> + struct pci_p2pdma_pagemap *p2p_pgmap = to_p2p_pgmap(page->pgmap); >> + >> + WARN_ON(!is_pci_p2pdma_page(page)); > > Shouldn't this check be before the to_p2p_pgmap() call? The to_p2p_pgmap() call is just doing pointer arithmetic, so strictly speaking it doesn't need to be before. We just can't access p2p_pgmap until it has been checked. > And I've been told not > to introduce WARN_ON's. Should this be? > > if (!is_pci_p2pdma_page(page)) > return -1; In this case the WARN_ON is just to guard against misuse of the function. It should never happen unless a developer changes the code in a way that is incorrect. So I think that's the correct use of WARN_ON. Though I might change it to WARN and return, that seems safer. Logan