All of lore.kernel.org
 help / color / mirror / Atom feed
From: Alan Stern <stern@rowland.harvard.edu>
To: Ming Lei <ming.lei@canonical.com>
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: Thu, 1 Sep 2011 11:22:51 -0400 (EDT)	[thread overview]
Message-ID: <Pine.LNX.4.44L0.1109011113210.1896-100000@iolanthe.rowland.org> (raw)
In-Reply-To: <CACVXFVOYaxGDZrwkHYc+h=tFhpYp+g95KZrDjg+bPH_N-9BT_g@mail.gmail.com>

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: TEXT/PLAIN; charset=UTF-8, Size: 2570 bytes --]

On Thu, 1 Sep 2011, Ming Lei wrote:

> I agree all about above, but what I described is from another view.
> I post out the example before explaining my idea further:
> 
> 
> 	CPU			device	
> 	A=1;
> 	wmb
> 	B=2;
> 					read B
> 					read A
> 
> one wmb is used to order 'A=1' and 'B=2', which will make the two write
> operations reach to physical memory as the order: 'A=1' first, 'B=2' second.
> Then the device can observe the two write events as the order above,
> so if device has seen 'B==2', then device will surely see 'A==1'.
> 
> Suppose writing to A is operation to update dma descriptor, the above example
> can make device always see a atomic update of descriptor, can't it?

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.

> My idea is that the memory access patterns are to be considered for
> writer of device driver. For example, many memory access patterns on
> EHCI hardware are described in detail.  Of course, device driver should
> make full use of the background info, below is a example from ehci driver:
> 
> qh_link_async():
> 
> 	/*prepare qh descriptor*/
> 	qh->qh_next = head->qh_next;
> 	qh->hw->hw_next = head->hw->hw_next;
> 	wmb ();
> 
> 	/*link the qh descriptor into hardware queue*/
> 	head->qh_next.qh = qh;
> 	head->hw->hw_next = dma;
> 
> so once EHCI fetches a qh with the address of 'dma', it will always see
> consistent content of qh descriptor, which could not be updated partially.

Yes, of course.  That's what memory barriers are intended for, to make 
sure that writes occur in the correct order.  Without the wmb(), the 
CPU might decide to write out the value of head->hw->hw_next before 
writing out the value of qh->hw->hw_next.  Then the device might see an 
inconsistent set of values.

None of this has anything to do with the write flushes you want to add.

> >> 2, most of such cases can be handled correctly by mb/wmb/rmb barriers.
> >
> > No, they can't.  See the third point above.
> 
> The example above has demoed that barriers can do it, hasn't it?

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.

Alan Stern


WARNING: multiple messages have this Message-ID (diff)
From: stern@rowland.harvard.edu (Alan Stern)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH 0/3] RFC: addition to DMA API
Date: Thu, 1 Sep 2011 11:22:51 -0400 (EDT)	[thread overview]
Message-ID: <Pine.LNX.4.44L0.1109011113210.1896-100000@iolanthe.rowland.org> (raw)
In-Reply-To: <CACVXFVOYaxGDZrwkHYc+h=tFhpYp+g95KZrDjg+bPH_N-9BT_g@mail.gmail.com>

On Thu, 1 Sep 2011, Ming Lei wrote:

> I agree all about above, but what I described is from another view.
> I post out the example before explaining my idea further:
> 
> 
> 	CPU			device	
> 	A=1;
> 	wmb
> 	B=2;
> 					read B
> 					read A
> 
> one wmb is used to order 'A=1' and 'B=2', which will make the two write
> operations reach to physical memory as the order: 'A=1' first, 'B=2' second.
> Then the device can observe the two write events as the order above,
> so if device has seen 'B==2', then device will surely see 'A==1'.
> 
> Suppose writing to A is operation to update dma descriptor, the above example
> can make device always see a atomic update of descriptor, can't it?

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.

> My idea is that the memory access patterns are to be considered for
> writer of device driver. For example, many memory access patterns on
> EHCI hardware are described in detail.  Of course, device driver should
> make full use of the background info, below is a example from ehci driver:
> 
> qh_link_async():
> 
> 	/*prepare qh descriptor*/
> 	qh->qh_next = head->qh_next;
> 	qh->hw->hw_next = head->hw->hw_next;
> 	wmb ();
> 
> 	/*link the qh descriptor into hardware queue*/
> 	head->qh_next.qh = qh;
> 	head->hw->hw_next = dma;
> 
> so once EHCI fetches a qh with the address of 'dma', it will always see
> consistent content of qh descriptor, which could not be updated partially.

Yes, of course.  That's what memory barriers are intended for, to make 
sure that writes occur in the correct order.  Without the wmb(), the 
CPU might decide to write out the value of head->hw->hw_next before 
writing out the value of qh->hw->hw_next.  Then the device might see an 
inconsistent set of values.

None of this has anything to do with the write flushes you want to add.

> >> 2, most of such cases can be handled correctly by mb/wmb/rmb barriers.
> >
> > No, they can't. ?See the third point above.
> 
> The example above has demoed that barriers can do it, hasn't it?

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.

Alan Stern

  parent reply	other threads:[~2011-09-01 15:22 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 [this message]
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
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=Pine.LNX.4.44L0.1109011113210.1896-100000@iolanthe.rowland.org \
    --to=stern@rowland.harvard.edu \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=ming.lei@canonical.com \
    --cc=msalter@redhat.com \
    /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.