[Following up a little late on this one]

Hi Grant and Tony,

On Wed, Feb 20, 2019 at 11:19:10AM +0000, Tony Chuang wrote:
> > -----Original Message-----
> > From: Grant Grundler [mailto:[email protected]]

> > This is another bug in the code actually: the code accessing tx
> > descriptor rings should be declared volatile since they are updated by
> > the device which is not visible to the compiler.  Compiler won't
> > optimize (or reorder) volatile code (by "volatile code" I mean code
> > that is accessing data marked "volatile").

If I'm understanding the code correctly, the TX ISR assumes that
everything between the last entry we processed (ring->r.rp) and a
certain point (cur_rp) is complete. The former index was saved during
the previous interrupt and the latter is determined via a register read.
So assuming properly synchronized interrupts (such as with PCIe MSI) and
no device-/firmware-related ordering bugs, then I think we're OK -- the
only "volatile" piece is the register read, which already embeds
volatile in the IO accessors.

> This is fixed in PATCH v5. We reset and use rx tag to sync dma.
> So the polling can be removed and the bus timeout has not happened again.

Yes, I tested the later versions of this series, and I didn't see this
problem so far. I'll follow up there with my Review/Test information
eventually.

Thanks,
Brian

Reply via email to