Re: [PATCH v2 5/5] libnvdimm: add DMA support for pmem blk-mq

From: Dave Jiang <dave.jiang@intel.com>
To: Dan Williams <dan.j.williams@intel.com>,
	"Koul, Vinod" <vinod.koul@intel.com>
Cc: Sinan Kaya <okaya@codeaurora.org>,
	"dmaengine@vger.kernel.org" <dmaengine@vger.kernel.org>,
	"linux-nvdimm@lists.01.org" <linux-nvdimm@lists.01.org>
Subject: Re: [PATCH v2 5/5] libnvdimm: add DMA support for pmem blk-mq
Date: Wed, 16 Aug 2017 10:16:31 -0700	[thread overview]
Message-ID: <57ee1a99-e9c7-0dc6-ade2-f4d40009e3d3@intel.com> (raw)
In-Reply-To: <CAPcyv4iZ+TaqLVmgreRCvbAFBZOmNaUDC4jm4WKzOGj0UrZS7A@mail.gmail.com>

On 08/16/2017 10:06 AM, Dan Williams wrote:
> On Wed, Aug 16, 2017 at 9:50 AM, Vinod Koul <vinod.koul@intel.com> wrote:
>> On Thu, Aug 03, 2017 at 09:14:13AM -0700, Dan Williams wrote:
>>>>>>>>>>>>>> Do we need a new API / new function, or new capability?
>>>>>>>>>>>>> Hmmm...you are right. I wonder if we need something like DMA_SG cap....
>>>>>>>>>>>>>
>>>>>>>>>>>>>
>>>>>>>>>>>>
>>>>>>>>>>>> Unfortunately, DMA_SG means something else. Maybe, we need DMA_MEMCPY_SG
>>>>>>>>>>>> to be similar with DMA_MEMSET_SG.
>>>>>>>>>>>
>>>>>>>>>>> I'm ok with that if Vinod is.
>>>>>>>>>>
>>>>>>>>>> So what exactly is the ask here, are you trying to do MEMCPY or SG or MEMSET
>>>>>>>>>> or all :). We should have done bitfields for this though...
>>>>>>>>>
>>>>>>>>> Add DMA_MEMCPY_SG to transaction type.
>>>>>>>>
>>>>>>>> Not MEMSET right, then why not use DMA_SG, DMA_SG is supposed for
>>>>>>>> scatterlist to scatterlist copy which is used to check for
>>>>>>>> device_prep_dma_sg() calls
>>>>>>>>
>>>>>>> Right. But we are doing flat buffer to/from scatterlist, not sg to sg. So
>>>>>>> we need something separate than what DMA_SG is used for.
>>>>>>
>>>>>> Hmm, its SG-buffer  and its memcpy, so should we call it DMA_SG_BUFFER,
>>>>>> since it is not memset (or is it) I would not call it memset, or maybe we
>>>>>> should also change DMA_SG to DMA_SG_SG to make it terribly clear :D
>>>>>
>>>>> I can create patches for both.
>>>>
>>>> Great, anyone who disagrees or can give better names :)
>>>
>>> All my suggestions would involve a lot more work. If we had infinite
>>> time we'd stop with the per-operation-type entry points and make this
>>> look like a typical driver sub-system that takes commands like
>>> block-devices or usb, but perhaps that ship has sailed.
>>
>> Can you elaborate on this :)
>>
>> I have been thinking about the need to redo the API. So lets discuss :)
> 
> The high level is straightforward, the devil is in the details. Define
> a generic dma command object, perhaps 'struct dma_io' certainly not
> 'struct dma_async_tx_descriptor', and have just one entry point
> per-driver. That 'struct dma_io' would carry a generic command number
> a target address and a scatterlist. The driver entry point would then
> convert and build the command to the hardware command format plus
> submission queue. The basic driving design principle is convert all
> the current function pointer complexity with the prep_* routines into
> data structure complexity in the common command format.
> 
> This trades off some efficiency because now you need to write the
> generic command and write the descriptor, but I think if the operation
> is worth offloading those conversion costs must already be in the
> noise.

Vinod, I think if you want to look at existing examples take a look at
the block layer request queue. Or even better blk-mq. I think this is
pretty close to what Dan is envisioning? Also, it's probably time we
looking into supporting hotplugging for DMA engines? Maybe this will
make it easier to do so. I'm willing to help and hoping that it will
make things easier for me for the next gen hardware.
_______________________________________________
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm