Re: [PATCH 09/15] NTB: Use DMA Engine to Transmit and Receive

Dan Williams
2013-08-20T00:45:58Z

On 8/19/13 5:07 PM, "Jon Mason" <jon.mason@intel.com> wrote:

>> >> Is this for the case where we are bouncing back and forth between
>> >> sync/async? Otherwise I do not see how transactions could get out of
>> >> order given you allocate a channel once per queue. Is this comment
>> >> saying that the iowrite32 is somehow a fix, or is this comment a
>> >> FIXME?
>> >
>> >There is a case for a mix, the "copy_bytes" variable above switches to
>> >CPU for small transfers (which greatly increases throughput on small
>> >transfers). The caveat to it is the need to flush the DMA engine to
>> >prevent out-of-order. This comment is mainly an reminder of this
>>issue.
>>
>> So this is going forward with the stall as a known issue? The next
>>patch
>> should just do the sync to prevent the re-ordering, right?
>
>There is already a dma_sync_wait in the error path of ntb_async_rx to
>enforce the ordering. Do I need to change the comment (or move it) to
>make it more obvious what is happening?

Yeah, I think it just needs to move to the dma_sync_wait() otherwise it
seems like it¹s an open issue that needs fixing.

>>>> > + txd->callback = ntb_rx_copy_callback;
>> >> > + txd->callback_param = entry;
>> >> > +
>> >> > + cookie = dmaengine_submit(txd);
>> >> > + if (dma_submit_error(cookie))
>> >> > + goto err3;
>> >> > +
>> >> > + qp->last_cookie = cookie;
>> >> > +
>> >> > + dma_async_issue_pending(chan);
>> >>
>> >> hmm... can this go in ntb_process_rx() so that the submission is
>> >> batched? Cuts down on mmio.
>> >
>> >I moved it down to ntb_transport_rx (after the calls to
>> >ntb_process_rxc), and the performance seems to be roughly the same.
>>
>> Yeah, not expecting it to be noticeable, but conceptually
>>
>> submit
>> submit
>> submit
>> submit
>> issue
>>
>>
>> Is nicer than:
>>
>> submit
>> issue
>> submit
>> issue
>>
>>
>
>I agree, but I liked having all the dma engine awareness
>compartmentalized in the ntb_async_* and callbacks.

Ok, makes sense.

--
Dan