Messages in this thread | | | Date | Fri, 12 Jul 2019 12:32:43 -0300 | From | Jason Gunthorpe <> | Subject | Re: Re: Re: Re: [PATCH] rdma/siw: avoid smp_store_mb() on a u64 |
| |
On Fri, Jul 12, 2019 at 03:24:09PM +0000, Bernard Metzler wrote: > > >To: "Bernard Metzler" <BMT@zurich.ibm.com> > >From: "Jason Gunthorpe" <jgg@ziepe.ca> > >Date: 07/12/2019 04:43PM > >Cc: "Arnd Bergmann" <arnd@arndb.de>, "Doug Ledford" > ><dledford@redhat.com>, "Peter Zijlstra" <peterz@infradead.org>, > >linux-rdma@vger.kernel.org, linux-kernel@vger.kernel.org > >Subject: [EXTERNAL] Re: Re: Re: [PATCH] rdma/siw: avoid > >smp_store_mb() on a u64 > > > >On Fri, Jul 12, 2019 at 02:35:50PM +0000, Bernard Metzler wrote: > > > >> >This looks wrong to me.. a userspace notification re-arm cannot be > >> >lost, so have a split READ/TEST/WRITE sequence can't possibly > >work? > >> > > >> >I'd expect an atomic test and clear here? > >> > >> We cannot avoid the case that the application re-arms the > >> CQ only after a CQE got placed. That is why folks are polling the > >> CQ once after re-arming it - to make sure they do not miss the > >> very last and single CQE which would have produced a CQ event. > > > >That is different, that is re-arm happing after a CQE placement and > >this can't be fixed. > > > >What I said is that a re-arm from userspace cannot be lost. So you > >can't blindly clear the arm flag with the WRITE_ONCE. It might be OK > >beacuse of the if, but... > > > >It is just goofy to write it without a 'test and clear' atomic. If > >the > >writer side consumes the notify it should always be done atomically. > > > Hmmm, I don't yet get why we should test and clear atomically, if we > clear anyway - is it because we want to avoid clearing a re-arm which > happens just after testing and before clearing?
It is just clearer as to the intent..
Are you trying to optimize away an atomic or something? That might work better as a dual counter scheme.
> Another complication -- test_and_set_bit() operates on a single > bit, but we have to test two bits, and reset both, if one is > set.
Why are two bits needed to represent armed and !armed?
Jason
| |