> has uncovered an issue in qede NIC driver [/drivers/net/ethernet/qlogic/qede] > and > this driver is seriously broken for basic L2. That's a bit extreme. There is a race condition bug that needs fixing.
> Before the series from John, driver was running fine, probably with hiding a > serious > bug underneath it which got exposed with his series. > > Driver uses mmiowb() after writing TX doorbell in function > qede_update_tx_producer() to synchronize the write between CPUs and > that TX doorbell area, driver maps with write combined memory using > ioremap_wc(). Yup, that's the problem. Write combined bar is relaxed-ordering bar, so you must use wmb() after the store to flush the wc_buffer. If driver gets scheduled on another cpu before the buffer is flushed and writes to the same address, we can be in trouble. > This is causing an issue where HW seems to be reading an unexpected producer > value > [not as was written on doorbell, probably an older producer] > and causing various SKB null condition hit while handling TX completion. This > seems to > be indicating that mmiowb() doesn't seems to be synchronizing/flushing > writes here, at least on x86_64 SMP, it's just like a compiler barrier [Not > really a > hardware fence]. > > If we replace mmiowb() with wmb() we never hit the problem. wmb() seems to be > adding some synchronization/flush mechanism here. > Moreover, if we map doorbell with ioremap_nocache() we never hit problem with > mmiowb() being there even. Makes sense. > Any suggestions or help about this barrier differences will be appreciated > here in > order to prepare a proper fix. Please prepare the fix with wmb() after the store to flush the write combined buffer > > Thanks in Advance !! > Manish Chopra
