On Fri, Jul 12, 2019 at 11:33:46AM +0000, Bernard Metzler wrote: > >diff --git a/drivers/infiniband/sw/siw/siw_verbs.c > >b/drivers/infiniband/sw/siw/siw_verbs.c > >index 32dc79d0e898..41c5ab293fe1 100644 > >+++ b/drivers/infiniband/sw/siw/siw_verbs.c > >@@ -1142,10 +1142,11 @@ int siw_req_notify_cq(struct ib_cq *base_cq, > >enum ib_cq_notify_flags flags) > > > > if ((flags & IB_CQ_SOLICITED_MASK) == IB_CQ_SOLICITED) > > /* CQ event for next solicited completion */ > >- smp_store_mb(*cq->notify, SIW_NOTIFY_SOLICITED); > >+ WRITE_ONCE(*cq->notify, SIW_NOTIFY_SOLICITED); > > else > > /* CQ event for any signalled completion */ > >- smp_store_mb(*cq->notify, SIW_NOTIFY_ALL); > >+ WRITE_ONCE(*cq->notify, SIW_NOTIFY_ALL); > >+ smp_wmb(); > > > > if (flags & IB_CQ_REPORT_MISSED_EVENTS) > > return cq->cq_put - cq->cq_get; > > > Hi Arnd, > Many thanks for pointing that out! Indeed, this CQ notification > mechanism does not take 32 bit architectures into account. > Since we have only three flags to hold here, it's probably better > to make it a 32bit value. That would remove the issue w/o > introducing extra smp_wmb(). I also prefer not to see smp_wmb() in drivers.. > I'd prefer smp_store_mb(), since on some architectures it shall be > more efficient. That would also make it sufficient to use > READ_ONCE. The READ_ONCE is confusing to me too, if you need store_release semantics then the reader also needs to pair with load_acquite - otherwise it doesn't work. Still, we need to do something rapidly to fix the i386 build, please revise right away.. Jason
