On Fri, Apr 25, 2025 at 05:21:59PM -0700, Luis Chamberlain wrote:
> On Wed, Apr 23, 2025 at 11:12:52AM +0300, Leon Romanovsky wrote:
> > From: Christoph Hellwig <hch@xxxxxx>
> >
> > The current scheme with a single helper to determine the P2P status
> > and map a scatterlist segment force users to always use the map_sg
> > helper to DMA map, which we're trying to get away from because they
> > are very cache inefficient.
> >
> > Refactor the code so that there is a single helper that checks the P2P
> > state for a page, including the result that it is not a P2P page to
> > simplify the callers, and a second one to perform the address translation
> > for a bus mapped P2P transfer that does not depend on the scatterlist
> > structure.
> >
> > Signed-off-by: Christoph Hellwig <hch@xxxxxx>
> > Reviewed-by: Logan Gunthorpe <logang@xxxxxxxxxxxx>
> > Acked-by: Bjorn Helgaas <bhelgaas@xxxxxxxxxx>
> > Tested-by: Jens Axboe <axboe@xxxxxxxxx>
> > Signed-off-by: Leon Romanovsky <leonro@xxxxxxxxxx>
>
> Might make it easier for patch review to split off adding
> __pci_p2pdma_update_state() in a seprate patch first.
Original code __pci_p2pdma_update_state() had this code and was
dependent on SG, which we are removing in this patch.
if (state->map == PCI_P2PDMA_MAP_BUS_ADDR) {
sg->dma_address = sg_phys(sg) + state->bus_off;
sg_dma_len(sg) = sg->length;
sg_dma_mark_bus_address(sg);
}
So to split, we would need to introduce new version of __pci_p2pdma_update_state(),
rename existing one to something like __pci_p2pdma_update_state2() and
remove it in next patch. Such pattern of adding and immediately deleting
code is not welcomed.
> Other than that, looks good.
>
> Reviewed-by: Luis Chamberlain <mcgrof@xxxxxxxxxx>
Thanks
>
> Luis
