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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <CE380608.2597D%djbw@fb.com>
Date:	Tue, 20 Aug 2013 00:45:54 +0000
From:	Dan Williams <djbw@...com>
To:	Jon Mason <jon.mason@...el.com>
CC:	Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
	Vinod Koul <vinod.koul@...el.com>,
	Dave Jiang <dave.jiang@...el.com>
Subject: Re: [PATCH 09/15] NTB: Use DMA Engine to Transmit and Receive



On 8/19/13 5:07 PM, "Jon Mason" <jon.mason@...el.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

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/