* [PATCH] document mmiowb and readX_relaxed a bit more in deviceiobook.tmpl
@ 2004-10-30 0:47 Jesse Barnes
2004-10-30 2:35 ` Benjamin Herrenschmidt
0 siblings, 1 reply; 3+ messages in thread
From: Jesse Barnes @ 2004-10-30 0:47 UTC (permalink / raw)
To: akpm, linux-kernel
[-- Attachment #1: Type: text/plain, Size: 343 bytes --]
This is a small patch to deviceiobook.tmpl to describe the new mmiowb routine
a bit more completely. I've also updated it to provide pointers to drivers
that do write flushing, use mmiowb, and use the readX_relaxed routines.
Acked-by: Grant Grundler <grundler@parisc-linux.org>
Signed-off-by: Jesse Barnes <jbarnes@sgi.com>
Thanks,
Jesse
[-- Attachment #2: deviceiobook-update.patch --]
[-- Type: text/plain, Size: 4139 bytes --]
===== Documentation/DocBook/deviceiobook.tmpl 1.5 vs edited =====
--- 1.5/Documentation/DocBook/deviceiobook.tmpl 2004-10-25 13:06:49 -07:00
+++ edited/Documentation/DocBook/deviceiobook.tmpl 2004-10-29 15:38:01 -07:00
@@ -195,7 +195,12 @@
be strongly ordered coming from different CPUs. Thus it's important
to properly protect parts of your driver that do memory-mapped writes
with locks and use the <function>mmiowb</function> to make sure they
- arrive in the order intended.
+ arrive in the order intended. Issuing a regular <function>readX
+ </function> will also ensure write ordering, but should only be used
+ when the driver has to be sure that the write has actually arrived
+ at the device (not that it's simply ordered with respect to other
+ writes), since a full <function>readX</function> is a relatively
+ expensive operation.
</para>
<para>
@@ -203,29 +208,50 @@
releasing a spinlock that protects regions using <function>writeb
</function> or similar functions that aren't surrounded by <function>
readb</function> calls, which will ensure ordering and flushing. The
- following example (again from qla1280.c) illustrates its use.
+ following pseudocode illustrates what might occur if write ordering
+ isn't guaranteed via <function>mmiowb</function> or one of the
+ <function>readX</function> functions.
</para>
<programlisting>
- sp->flags |= SRB_SENT;
- ha->actthreads++;
- WRT_REG_WORD(&reg->mailbox4, ha->req_ring_index);
-
- /*
- * A Memory Mapped I/O Write Barrier is needed to ensure that this write
- * of the request queue in register is ordered ahead of writes issued
- * after this one by other CPUs. Access to the register is protected
- * by the host_lock. Without the mmiowb, however, it is possible for
- * this CPU to release the host lock, another CPU acquire the host lock,
- * and write to the request queue in, and have the second write make it
- * to the chip first.
- */
- mmiowb(); /* posted write ordering */
+CPU A: spin_lock_irqsave(&dev_lock, flags)
+CPU A: ...
+CPU A: writel(newval, ring_ptr);
+CPU A: spin_unlock_irqrestore(&dev_lock, flags)
+ ...
+CPU B: spin_lock_irqsave(&dev_lock, flags)
+CPU B: writel(newval2, ring_ptr);
+CPU B: ...
+CPU B: spin_unlock_irqrestore(&dev_lock, flags)
</programlisting>
<para>
+ In the case above, newval2 could be written to ring_ptr before
+ newval. Fixing it is easy though:
+ </para>
+
+<programlisting>
+CPU A: spin_lock_irqsave(&dev_lock, flags)
+CPU A: ...
+CPU A: writel(newval, ring_ptr);
+CPU A: mmiowb(); /* ensure no other writes beat us to the device */
+CPU A: spin_unlock_irqrestore(&dev_lock, flags)
+ ...
+CPU B: spin_lock_irqsave(&dev_lock, flags)
+CPU B: writel(newval2, ring_ptr);
+CPU B: ...
+CPU B: mmiowb();
+CPU B: spin_unlock_irqrestore(&dev_lock, flags)
+</programlisting>
+
+ <para>
+ See tg3.c for a real world example of how to use <function>mmiowb
+ </function>
+ </para>
+
+ <para>
PCI ordering rules also guarantee that PIO read responses arrive
- after any outstanding DMA writes on that bus, since for some devices
+ after any outstanding DMA writes from that bus, since for some devices
the result of a <function>readb</function> call may signal to the
driver that a DMA transaction is complete. In many cases, however,
the driver may want to indicate that the next
@@ -234,7 +260,11 @@
<function>readb_relaxed</function> for these cases, although only
some platforms will honor the relaxed semantics. Using the relaxed
read functions will provide significant performance benefits on
- platforms that support it.
+ platforms that support it. The qla2xxx driver provides examples
+ of how to use <function>readX_relaxed</function>. In many cases,
+ a majority of the driver's <function>readX</function> calls can
+ safely be converted to <function>readX_relaxed</function> calls, since
+ only a few will indicate or depend on DMA completion.
</para>
</sect1>
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] document mmiowb and readX_relaxed a bit more in deviceiobook.tmpl
2004-10-30 0:47 [PATCH] document mmiowb and readX_relaxed a bit more in deviceiobook.tmpl Jesse Barnes
@ 2004-10-30 2:35 ` Benjamin Herrenschmidt
2004-11-01 17:36 ` Jesse Barnes
0 siblings, 1 reply; 3+ messages in thread
From: Benjamin Herrenschmidt @ 2004-10-30 2:35 UTC (permalink / raw)
To: Jesse Barnes; +Cc: Andrew Morton, Linux Kernel list
On Fri, 2004-10-29 at 17:47 -0700, Jesse Barnes wrote:
> This is a small patch to deviceiobook.tmpl to describe the new mmiowb routine
> a bit more completely. I've also updated it to provide pointers to drivers
> that do write flushing, use mmiowb, and use the readX_relaxed routines.
It's all good, but your semantics and description are very tailored to
your specific arch problem vs. unlock.
What about my suggestion of defining a broader semantic of mmiowb() as
beeing a barrier ordering MMIOs vs. the rest of the world ? The later
includes stores to memory _and_ spinlock/unlock.
I still intend to eventually make good use of that on ppc64 to get rid
of the expensive barriers we had to put in our writeX() implementations,
though I didn't have time to work on that yet.
Ben.
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] document mmiowb and readX_relaxed a bit more in deviceiobook.tmpl
2004-10-30 2:35 ` Benjamin Herrenschmidt
@ 2004-11-01 17:36 ` Jesse Barnes
0 siblings, 0 replies; 3+ messages in thread
From: Jesse Barnes @ 2004-11-01 17:36 UTC (permalink / raw)
To: Benjamin Herrenschmidt; +Cc: Andrew Morton, Linux Kernel list
On Friday, October 29, 2004 7:35 pm, Benjamin Herrenschmidt wrote:
> On Fri, 2004-10-29 at 17:47 -0700, Jesse Barnes wrote:
> > This is a small patch to deviceiobook.tmpl to describe the new mmiowb
> > routine a bit more completely. I've also updated it to provide pointers
> > to drivers that do write flushing, use mmiowb, and use the readX_relaxed
> > routines.
>
> It's all good, but your semantics and description are very tailored to
> your specific arch problem vs. unlock.
>
> What about my suggestion of defining a broader semantic of mmiowb() as
> beeing a barrier ordering MMIOs vs. the rest of the world ? The later
> includes stores to memory _and_ spinlock/unlock.
Yeah, that's ok with me, just be sure to update the documentation when you add
the PPC stuff. Seems like a worthwhile optimization.
thanks,
Jesse
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2004-11-01 17:37 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2004-10-30 0:47 [PATCH] document mmiowb and readX_relaxed a bit more in deviceiobook.tmpl Jesse Barnes
2004-10-30 2:35 ` Benjamin Herrenschmidt
2004-11-01 17:36 ` Jesse Barnes
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).