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

[Dan Williams <djbw@fb.com>]

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