Re: [PATCH v5 2/2] media: usb: pwc: Don't use coherent DMA buffers for ISO transfer

[Tomasz Figa]

On Wed, Dec 19, 2018 at 4:51 PM Christoph Hellwig <hch@xxxxxxxxxxxxx> wrote:
>
> On Tue, Dec 18, 2018 at 06:48:03PM +0900, Tomasz Figa wrote:
> > > So as I said you can call dma_alloc_attrs with DMA_ATTR_NON_CONSISTENT
> > > in a loop with a suitably small chunk size, then stuff the results into
> > > a scatterlist and map that again for the device share with if you don't
> > > want a single contigous region.  You just have to either deal with
> > > non-contigous access from the kernel or use vmap and the right vmap
> > > cache flushing helpers.
> >
> > The point is that you didn't have to do this small chunk loop without
> > DMA_ATTR_NON_CONSISTENT, so it's at least inconsistent now and not
> > sure why it could be better than just a loop of alloc_page().
>
> You have to do it if you want to map the addresses for a second device.
>

The existing code that deals with dma_alloc_attrs() without
DMA_ATTR_NON_CONSISTENT would just call dma_get_sgtable_attrs() like
here:

https://elixir.bootlin.com/linux/v4.20-rc7/source/drivers/media/common/videobuf2/videobuf2-dma-contig.c#L366

and then dma_map_sg() for the other device like here;

https://elixir.bootlin.com/linux/v4.20-rc7/source/drivers/media/common/videobuf2/videobuf2-dma-contig.c#L283

> > > I would advice against new non-consistent users until this series
> > > goes through, mostly because dma_cache_sync is such an amazing bad
> > > API.  Otherwise things will just work at the allocation side, you'll
> > > just need to be careful to transfer ownership between the cpu and
> > > the device(s) carefully using the dma_sync_* APIs.
> >
> > Just to clarify, the actual code isn't very likely to surface any time
> > soon. so I assume it would be after this series lands.
> >
> > We will however need an API that can transparently handle both cases
> > of contiguous (without IOMMU) and page-by-page allocations (with
> > IOMMU) behind the scenes, like the current dma_alloc_attrs() without
> > DMA_ATTR_NON_CONSISTENT.
>
> Is the use case to then share the memory between multiples devices
> or just for a single device?  The latter case is generally easy, the
> former is rather more painful.

The former, but the convention has been to assume that the userspace
will choose the right (the most constrained typically) device to
allocate from or otherwise handle the import failure (e.g. by falling
back to copying into a buffer allocated from the importer).

Best regards,
Tomasz