On Tue, Dec 19, 2017 at 07:32:01AM +0200, Leon Romanovsky wrote:
> On Wed, Dec 13, 2017 at 04:24:12PM -0800, Bryan Tan wrote:
> > @@ -229,8 +229,8 @@ struct ib_cq *pvrdma_create_cq(struct ib_device *ibdev,
> >
> > static void pvrdma_free_cq(struct pvrdma_dev *dev, struct pvrdma_cq *cq)
> > {
> > - atomic_dec(&cq->refcnt);
> > - wait_event(cq->wait, !atomic_read(&cq->refcnt));
> > + if (!refcount_dec_and_test(&cq->refcnt))
> > + wait_event(cq->wait, !refcount_read(&cq->refcnt));
>
> Don't you suppose to call to wait_event without condition on refcnt and
> sleep till refcnt == 0??
By the time we check this condition, there cannot be any new references
to this CQ. If refcnt is zero, there isn't any reason to call
wait_event.
However, we've changed this to use completions instead of wait queues
because there is a possibility of use after free here still. For SRQs we
use this pattern first to fix the problem of using refcount_dec, and
then a few commits later in the new patch series wait queues are
switched to completions. Please take a look at the v2 patch series.
Thanks!
Bryan
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
