Re: [PATCH v4 0/6] iio: Add buffer write() support

[Jonathan Cameron <Jonathan.Cameron@Huawei.com>]

On Mon,  7 Aug 2023 13:21:07 +0200
Paul Cercueil <paul@crapouillou.net> wrote:

> [V3 was: "iio: new DMABUF based API, v3"][1]
> 
> Hi Jonathan,
> 
> This is a subset of my patchset that introduced a new interface based on
> DMABUF objects [1]. It adds write() support to the IIO buffer
> infrastructure.
> 
> The reason it is not the full IIO-DMABUF patchset, is because you
> requested performance benchmarks - and our current numbers are barely
> better (~ +10%) than the fileio interface. There is a good reason for
> that: V3 of the patchset switched from having the IIO core creating the
> DMABUFs backed by physically contiguous memory, to having the IIO core
> being a simple DMABUF importer, and having the DMABUFs created
> externally. We now use the udmabuf driver to create those, and they are
> allocated from paged memory. While this works perfectly fine, our
> buffers are now cut in 4 KiB chunks (pages), non-contiguous in memory,
> which causes the DMA hardware to create an IRQ storm, as it raises an
> interrupt after each 4 KiB in the worst case scenario.

Interesting. I'm guessing you don't necessarily need contiguous memory
and huge pages would get rid of most of that overhead?

Given embedded target those huge pages are hard to get so you need
hugetlb support to improve the chances of it working.  Some quick searching
suggests there is possible support on the way.
https://lore.kernel.org/linux-mm/20230817064623.3424348-1-vivek.kasireddy@intel.com/


> 
> Anyway, this is not directly a problem of the IIO-DMABUF code - but I
> can't really upstream a shiny new interface that I claim is much faster,
> without giving numbers.
> 
> So while we fix this (either by updating the DMA IP and driver to
> support scatter-gather)

Long run you almost always end up needing that unless contig requirements
are small and you want a robust solution.  I'm guessing no IOMMU to pretend
it's all contiguous... 

> or by hacking something quick to give us
> physically contiguous DMABUFs just for the benchmark), I thought it
> would make sense to upstream the few patches of the V3 patchset that are
> needed for the IIO-DMABUF interface but aren't directly related.

Good idea.

> 
> As for write() support, Nuno (Cc'd) said he will work on upstreaming the
> DAC counterpart of adc/adi-axi-adc.c in the next few weeks, so there
> will be a user for the buffer write() support. I hope you are okay with
> this - otherwise, we can just wait until this work is done and submit it
> all at once.

Absolutely fine, though I won't pick this up without the user also being
ready to go.

> 
> Changelog since v3:
> - [PATCH 2/6] is new;
> - [PATCH 3/6]: Drop iio_dma_buffer_space_available() function, and
>   update patch description accordingly;
> - [PATCH 6/6]: .space_available is now set to iio_dma_buffer_usage
>   (which is functionally the exact same).
> 
> Cheers,
> -Paul
> 
> [1] https://lore.kernel.org/all/20230403154800.215924-1-paul@crapouillou.net/
> 
> Alexandru Ardelean (1):
>   iio: buffer-dma: split iio_dma_buffer_fileio_free() function
> 
> Paul Cercueil (5):
>   iio: buffer-dma: Get rid of outgoing queue
>   iio: buffer-dma: Rename iio_dma_buffer_data_available()
>   iio: buffer-dma: Enable buffer write support
>   iio: buffer-dmaengine: Support specifying buffer direction
>   iio: buffer-dmaengine: Enable write support
> 
>  drivers/iio/adc/adi-axi-adc.c                 |   3 +-
>  drivers/iio/buffer/industrialio-buffer-dma.c  | 187 ++++++++++++------
>  .../buffer/industrialio-buffer-dmaengine.c    |  28 ++-
>  include/linux/iio/buffer-dma.h                |  11 +-
>  include/linux/iio/buffer-dmaengine.h          |   5 +-
>  5 files changed, 160 insertions(+), 74 deletions(-)
>