Re: SCSI QLA not working on latest *-mm SN2

Jesse Barnes
2004-09-20T22:40:22Z

On Friday, September 17, 2004 11:10 pm, Grant Grundler wrote:
> Jesse Barnes wrote:
> ...
>
> > Btw Andrew (Vasquez), there's a small doc I put together that should
> > describe when you have to worry about PCI posting. It's in the tree:
> > Documentation/io_ordering.txt. If it's incomplete or confusing, just let
> > me know and I'll update it.
>
> Jesse,
> Both. incomplete and confusing.
> "concrete example of a hypothetical driver" wasn't my first warning
> this document needed work. :^)

Heh, yeah I noticed that on re-reading too :)

> I've hacked up the 2.6.9 version and even what I did still needs more work.
> Have time to correct my mistakes and answer the questions I ask?
>
> I'd be happy to review it again after you've done another round on it.
>
> []'s should all go away - used those to mark editorial notes.

Sure, thanks for reading it.

> Weakly Ordered Memory Mapped IO
> -------------------------------
>
> SGI Altix chipset implements weakly ordered Memory-Mapped I/O writes.
> On this platform, driver writers are responsible for ensuring I/O writes
> to memory-mapped addresses arrive in the order intended.

This is incorrect. I wrote this before I understood that 'pci write posting'
was the common term for describing the fact that writes from different CPUs
could arrive out of order. A s/weakly ordered/write posting would make the
existing document much more accurate.

> Like for PCI write posting problems, this is done by reading
> a 'safe' device or bridge register, causing the I/O chipset to
> flush pending writes to the device before any reads are issued.
> A driver would issue the "safe" read immediately prior to the exit
> of a critical section of code protected by spinlocks. This would
> ensure subsequent writes to I/O space arrived only after all prior
> writes (much like a memory barrier op, mb(), only with respect to I/O).
>
> Note: MMIO reads are expensive! Don't add MMIO reads after *every* MMIO
> write unless the device programming model absolutely requires it.

Yes!

>
>
> An example from a hypothetical device driver might help:
>
> ...
> CPU A: spin_lock_irqsave(&dev_lock, flags)
> CPU A: val = readl(my_status);
> 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: val = readl(my_status);
> CPU B: ...
> CPU B: writel(newval2, ring_ptr);
> CPU B: spin_unlock_irqrestore(&dev_lock, flags)
> ...
>
> In the case above, the device may receive newval2 before it receives
> newval, which could cause problems.
>
> [ Is this example broken or am I just staying up too late?
> The example is doing a readl() in the second critical section.
> Shouldn't that enforce the write ordering?
> ]

Yep, that's a bug. It should just be writes.

> The reads from safe_register will cause the I/O chipset to flush any
> pending writes before actually posting the read to the chipset, preventing
> possible data corruption.
>
> [ How about interactions with:
> o read_relaxed()?
> o DMA?
> o IO Port space reads?
> o IO Port space writes?
> ]

None that I know of.

Jesse