Re: [PATCH] lib/scatterlist: Provide a DMA page iterator

Jason Gunthorpe
2019-01-10T23:42:18Z

On Fri, Jan 04, 2019 at 03:35:31PM -0700, Jason Gunthorpe wrote:
> Commit 2db76d7c3c6d ("lib/scatterlist: sg_page_iter: support sg lists w/o
> backing pages") introduced the sg_page_iter_dma_address() function without
> providing a way to use it in the general case. If the sg_dma_len is not
> equal to the dma_length callers cannot safely use the
> for_each_sg_page/sg_page_iter_dma_address combination.
>
> Resolve this API mistake by providing a DMA specific iterator,
> for_each_sg_dma_page(), that uses the right length so
> sg_page_iter_dma_address() works as expected with all sglists. A new
> iterator type is introduced to provide compile-time safety against wrongly
> mixing accessors and iterators.
>
> Signed-off-by: Jason Gunthorpe <jgg@mellanox.com>
> drivers/gpu/drm/vmwgfx/vmwgfx_drv.h | 26 ++++++++++--
> drivers/gpu/drm/vmwgfx/vmwgfx_mob.c | 26 +++++++-----
> drivers/gpu/drm/vmwgfx/vmwgfx_ttm_buffer.c | 42 +++++++++++++------
> drivers/media/pci/intel/ipu3/ipu3-cio2.c | 4 +-
> include/linux/scatterlist.h | 49 ++++++++++++++++++----
> lib/scatterlist.c | 26 ++++++++++++
> 6 files changed, 134 insertions(+), 39 deletions(-)
>
> I'd like to run this patch through the RDMA tree as we have another
> series in the works that wants to use the for_each_sg_dma_page() API.
>
> The changes to vmwgfx make me nervous, it would be great if someone
> could test and ack them?
>
> Changes since the RFC:
> - Rework vmwgfx too [CH]
> - Use a distinct type for the DMA page iterator [CH]
> - Do not have a #ifdef [CH]

ChristophH: Will you ack?

Are you still OK with the vmwgfx reworking, or should we go back to
the original version that didn't have the type safety so this driver
can be left broken?

Thanks,
Jason