Re: [PATCH v4 2/8] dmaengine: change transaction type DMA_SG to DMA_SG_SG

[Dave Jiang <dave.jiang@intel.com>]

On 08/10/2017 12:05 PM, Dan Williams wrote:
> On Thu, Aug 10, 2017 at 9:22 AM, Dave Jiang <dave.jiang@intel.com> wrote:
>>
>>
>> On 08/09/2017 07:20 PM, Dan Williams wrote:
>>> On Wed, Aug 9, 2017 at 7:15 PM, Dan Williams <dan.j.williams@intel.com> wrote:
>>>> On Mon, Aug 7, 2017 at 9:39 AM, Dave Jiang <dave.jiang@intel.com> wrote:
>>>>> In preparation of adding an API to perform SG to/from buffer for dmaengine,
>>>>> we will change DMA_SG to DMA_SG_SG in order to explicitly making clear what
>>>>> this op type is for.
>>>>>
>>>>> Signed-off-by: Dave Jiang <dave.jiang@intel.com>
>>>>> ---
>>>>>  Documentation/dmaengine/provider.txt |    2 +-
>>>>>  drivers/crypto/ccp/ccp-dmaengine.c   |    2 +-
>>>>>  drivers/dma/at_hdmac.c               |    8 ++++----
>>>>>  drivers/dma/dmaengine.c              |    2 +-
>>>>>  drivers/dma/dmatest.c                |   12 ++++++------
>>>>>  drivers/dma/fsldma.c                 |    2 +-
>>>>>  drivers/dma/mv_xor.c                 |    6 +++---
>>>>>  drivers/dma/nbpfaxi.c                |    2 +-
>>>>>  drivers/dma/ste_dma40.c              |    6 +++---
>>>>>  drivers/dma/xgene-dma.c              |    4 ++--
>>>>>  drivers/dma/xilinx/zynqmp_dma.c      |    2 +-
>>>>>  include/linux/dmaengine.h            |    2 +-
>>>>>  12 files changed, 25 insertions(+), 25 deletions(-)
>>>>
>>>> Given the prevalence and age of DMA_SG I think we should call the new
>>>> op DMA_SG_SINGLE, or something like that, so that we don't surprise
>>>> someone who was expecting the old command type.
>>>
>>> Oh wait, you already call the new op DMA_MEMCPY_SG, so why does the
>>> old one need to change? ...and thinking about it further why do we
>>> need a new op at all? Couldn't we just pass in a single entry
>>> scatterlist that was setup on the stack with memcpy target address?
>>
>> That would probably work if we can do dma_map_sg() before submit and
>> dma_unmap_page() on completion since we'll lose the sg entry. I also
>> have concerns with the DMA_SG function provided in the ioatdma driver
>> since it really doesn't do scatter gather and it's all software
>> abstracted. It's not a big deal to support a single entry SG, but it
>> wouldn't be supporting real SG to SG setup without more complex code. I
>> worry about the overhead and messiness of it. Of course if you are ok
>> with providing a DMA_SG function that really doesn't do exactly what
>> it's advertised to do and only cater to pmem usage.
> 
> I'm fishing for a way to not to make the dmaengine operation-type
> proliferation problem worse. The sg-to-sg operation is odd in that
> typical usages of a scatterlist have a contiguous buffer as the source
> or destination. A change that gets us closer to that typical model is
> what I'm looking for and not adding baggage that we need to unwind
> later since everyone seems to agree that this "->prep_X() +
> ->submit()" model is broken. I'd like to revisit whether we actually
> need sg-to-sg vs that typical sg-to-buf model, and once we have the
> typical model can we refactor all commands to use a single entry point
> to the driver that takes a single scattler list parameter (similar to
> scsi, usb, etc).

Looking through the kernel it does not look like there's an actual in
kernel consumer for DMA_SG. That makes it rather difficult to determine
what it's used for. I have a feeling its usage is out of tree and thus
explain the deviation from typical kernel usages. I'm wondering if we
should to introduce DMA_SG_MEMCPY to set that as the standard and then
deprecate DMA_SG eventually.
_______________________________________________
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm