On Wed, Oct 23, 2019 at 09:49:14AM +0000, Michal Kalderon wrote: > > > > > + /* Wait for the connection setup to complete */ > > > > > + if > > (test_and_set_bit(QEDR_IWARP_CM_WAIT_FOR_CONNECT, > > > > > + &qp->iwarp_cm_flags)) > > > > > + wait_for_completion(&qp->iwarp_cm_comp); > > > > > + > > > > > + if > > (test_and_set_bit(QEDR_IWARP_CM_WAIT_FOR_DISCONNECT, > > > > > + &qp->iwarp_cm_flags)) > > > > > + wait_for_completion(&qp->iwarp_cm_comp); > > > > > } > > > > > > > > These atomics seem mis-named, and I'm unclear how they can both be > > > > waiting on the same completion? > > In IWARP the connection establishment and disconnect are offloaded to hw and asynchronous. > The first waits for CONNECT to complete. (completes asynchronously) > If we are in the middle of a connect that was offloaded (or after connect) the bit will be on and completion will be completed > once the connection is fully established. > We want to wait before destroying the qp due to hw constraints (can't destroy during connect). The atomics seem to be used in a 'something in progress' kind of way as the first thing to get the 'set' does whatever is happening here or waits for the otherone to finish doing it. > I didn't see a reason to use another completion structure, since > from what I read complete does comp->done++ and wait_for_completion > checks done and eventually does done-- so it can be used several > times if there is no need to distinguish which event occurred first > (in this case there is only one option first connect then disconnect > and disconnect can't occur without connect) I suppose it is OK here because both completion waits are sequential, but generally it is kind of sketchy as when QEDR_IWARP_CM_WAIT_FOR_CONNECT completes it will wake up something waiting for QEDR_IWARP_CM_WAIT_FOR_DISCONNECT as well. Jason
