All of lore.kernel.org
 help / color / mirror / Atom feed
From: Ming Lei <ming.lei@canonical.com>
To: Alan Stern <stern@rowland.harvard.edu>
Cc: linux-kernel@vger.kernel.org,
	linux-arm-kernel@lists.infradead.org,
	Mark Salter <msalter@redhat.com>
Subject: Re: [PATCH 0/3] RFC: addition to DMA API
Date: Fri, 2 Sep 2011 08:59:58 +0800	[thread overview]
Message-ID: <CACVXFVO6PhfgH_90B7AvWxSReCeJJ8D+jpgrk065MPHok7jzhA@mail.gmail.com> (raw)
In-Reply-To: <Pine.LNX.4.44L0.1109011244340.1896-100000@iolanthe.rowland.org>

Hi,

On Fri, Sep 2, 2011 at 12:48 AM, Alan Stern <stern@rowland.harvard.edu> wrote:
> On Thu, 1 Sep 2011, Ming Lei wrote:
>
>> > Suppose A and B are _both_ part of the dma descriptor.  The device
>> > might see A==1 and B==0, if the memory accesses occur like this:
>> >
>> >        CPU             device
>> >        ---             ------
>> >        A = 1;
>> >        wmb();
>> >                        read B
>> >                        read A
>> >        B = 2;
>> >
>> > When this happens, the device will observe a non-atomic update of the
>> > descriptor.  There's no way to prevent this.
>>
>> If device doesn't find that B is 2, it will not fetch descriptor of A,
>> and will observe
>> a atomic update, which is just EHCI does for many cases(such as 4.10.2).
>
> You didn't read what I wrote above.  Suppose A and B are _both_ part of
> the same descriptor, like hw_token and hw_qtd_next.

I think the case(keep writing order between parts in a same dma descriptor)
is only in constant dma-poll master case, just like ehci/uhci.

General case is that memory barrier is required before linking one dma
descriptor into hardware queue but after the dma descriptor is prepared.

>
>> > The memory barrier in your qh_link_async() example can make sure that
>> > the device always sees consistent data.  It doesn't guarantee that the
>> > write to head->hw->hw_next will be flushed to memory in a reasonably
>> > short time, which is the problem you are trying to solve.
>>
>> Yes, up to now, it is the only case in which the flush can address to,
>> and in which kind of cases device will poll DMA coherent memory contiguously,
>> I am not sure if there are other devices except for EHCI(maybe have uhci/ohci).
>
> Yes: UHCI, OHCI, EHCI, and XHCI all poll memory constantly.

In fact, the flush may be not required for ohci and xhci case, since there is
already one mmio register writing at the end of .enqueue path in ohci/xhci
driver.(just a glance at the code of ohci/xhci, please correct if I am wrong)

For UHCI, looks like it has not been used on ARM, so maybe can ignore it.
UHCI is to support a slow usb 1.1 transfer, so I am wondering if the flush
can produce a obvious performance boost.

So looks like the flush only makes sense on EHCI.

If the above is not wrong, is it really needed to introduce a general DMA API
only for EHCI?


thanks,
--
Ming Lei

WARNING: multiple messages have this Message-ID (diff)
From: ming.lei@canonical.com (Ming Lei)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH 0/3] RFC: addition to DMA API
Date: Fri, 2 Sep 2011 08:59:58 +0800	[thread overview]
Message-ID: <CACVXFVO6PhfgH_90B7AvWxSReCeJJ8D+jpgrk065MPHok7jzhA@mail.gmail.com> (raw)
In-Reply-To: <Pine.LNX.4.44L0.1109011244340.1896-100000@iolanthe.rowland.org>

Hi,

On Fri, Sep 2, 2011 at 12:48 AM, Alan Stern <stern@rowland.harvard.edu> wrote:
> On Thu, 1 Sep 2011, Ming Lei wrote:
>
>> > Suppose A and B are _both_ part of the dma descriptor. ?The device
>> > might see A==1 and B==0, if the memory accesses occur like this:
>> >
>> > ? ? ? ?CPU ? ? ? ? ? ? device
>> > ? ? ? ?--- ? ? ? ? ? ? ------
>> > ? ? ? ?A = 1;
>> > ? ? ? ?wmb();
>> > ? ? ? ? ? ? ? ? ? ? ? ?read B
>> > ? ? ? ? ? ? ? ? ? ? ? ?read A
>> > ? ? ? ?B = 2;
>> >
>> > When this happens, the device will observe a non-atomic update of the
>> > descriptor. ?There's no way to prevent this.
>>
>> If device doesn't find that B is 2, it will not fetch descriptor of A,
>> and will observe
>> a atomic update, which is just EHCI does for many cases(such as 4.10.2).
>
> You didn't read what I wrote above. ?Suppose A and B are _both_ part of
> the same descriptor, like hw_token and hw_qtd_next.

I think the case(keep writing order between parts in a same dma descriptor)
is only in constant dma-poll master case, just like ehci/uhci.

General case is that memory barrier is required before linking one dma
descriptor into hardware queue but after the dma descriptor is prepared.

>
>> > The memory barrier in your qh_link_async() example can make sure that
>> > the device always sees consistent data. ?It doesn't guarantee that the
>> > write to head->hw->hw_next will be flushed to memory in a reasonably
>> > short time, which is the problem you are trying to solve.
>>
>> Yes, up to now, it is the only case in which the flush can address to,
>> and in which kind of cases device will poll DMA coherent memory contiguously,
>> I am not sure if there are other devices except for EHCI(maybe have uhci/ohci).
>
> Yes: UHCI, OHCI, EHCI, and XHCI all poll memory constantly.

In fact, the flush may be not required for ohci and xhci case, since there is
already one mmio register writing at the end of .enqueue path in ohci/xhci
driver.(just a glance at the code of ohci/xhci, please correct if I am wrong)

For UHCI, looks like it has not been used on ARM, so maybe can ignore it.
UHCI is to support a slow usb 1.1 transfer, so I am wondering if the flush
can produce a obvious performance boost.

So looks like the flush only makes sense on EHCI.

If the above is not wrong, is it really needed to introduce a general DMA API
only for EHCI?


thanks,
--
Ming Lei

  reply	other threads:[~2011-09-02  1:00 UTC|newest]

Thread overview: 64+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2011-08-31 21:30 [PATCH 0/3] RFC: addition to DMA API Mark Salter
2011-08-31 21:30 ` Mark Salter
2011-08-31 21:30 ` [PATCH 1/3] add dma_coherent_write_sync " Mark Salter
2011-08-31 21:30   ` Mark Salter
2011-09-01  2:59   ` Josh Cartwright
2011-09-01  2:59     ` Josh Cartwright
2011-09-01  9:57   ` Michał Mirosław
2011-09-01  9:57     ` Michał Mirosław
2011-09-01 12:36     ` Mark Salter
2011-09-01 12:36       ` Mark Salter
2011-09-06 14:30       ` Catalin Marinas
2011-09-06 14:30         ` Catalin Marinas
2011-08-31 21:30 ` [PATCH 2/3] define ARM-specific dma_coherent_write_sync Mark Salter
2011-08-31 21:30   ` Mark Salter
2011-09-06 14:32   ` Catalin Marinas
2011-09-06 14:32     ` Catalin Marinas
2011-09-06 14:37     ` Mark Salter
2011-09-06 14:37       ` Mark Salter
2011-09-06 14:48       ` Catalin Marinas
2011-09-06 14:48         ` Catalin Marinas
2011-09-06 15:02         ` Mark Salter
2011-09-06 15:02           ` Mark Salter
2011-10-03  1:40           ` Jon Masters
2011-10-03  1:40             ` Jon Masters
2011-10-03  8:44             ` Catalin Marinas
2011-10-03  8:44               ` Catalin Marinas
2011-10-03  9:24               ` Jon Masters
2011-10-03  9:24                 ` Jon Masters
2011-08-31 21:30 ` [PATCH 3/3] add dma_coherent_write_sync calls to USB EHCI driver Mark Salter
2011-08-31 21:30   ` Mark Salter
2011-09-01  2:33   ` Ming Lei
2011-09-01  2:33     ` Ming Lei
2011-09-01  2:09 ` [PATCH 0/3] RFC: addition to DMA API Ming Lei
2011-09-01  2:09   ` Ming Lei
2011-09-01  3:09   ` Alan Stern
2011-09-01  3:09     ` Alan Stern
2011-09-01  3:41     ` Ming Lei
2011-09-01  3:41       ` Ming Lei
2011-09-01  8:45       ` Will Deacon
2011-09-01  8:45         ` Will Deacon
2011-09-01  9:14         ` Ming Lei
2011-09-01  9:14           ` Ming Lei
2011-09-01 15:42           ` Alan Stern
2011-09-01 15:42             ` Alan Stern
2011-09-01 16:04             ` Russell King - ARM Linux
2011-09-01 16:04               ` Russell King - ARM Linux
2011-09-01 17:31               ` Will Deacon
2011-09-01 17:31                 ` Will Deacon
2011-09-01 18:07                 ` Russell King - ARM Linux
2011-09-01 18:07                   ` Russell King - ARM Linux
2011-09-01 19:14                 ` Mark Salter
2011-09-01 19:14                   ` Mark Salter
2011-09-01 15:22       ` Alan Stern
2011-09-01 15:22         ` Alan Stern
2011-09-01 15:56         ` Ming Lei
2011-09-01 15:56           ` Ming Lei
2011-09-01 16:48           ` Alan Stern
2011-09-01 16:48             ` Alan Stern
2011-09-02  0:59             ` Ming Lei [this message]
2011-09-02  0:59               ` Ming Lei
2011-09-02 13:53               ` Alan Stern
2011-09-02 13:53                 ` Alan Stern
2011-09-01  9:11 ` Will Deacon
2011-09-01  9:11   ` Will Deacon

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=CACVXFVO6PhfgH_90B7AvWxSReCeJJ8D+jpgrk065MPHok7jzhA@mail.gmail.com \
    --to=ming.lei@canonical.com \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=msalter@redhat.com \
    --cc=stern@rowland.harvard.edu \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.