On Wed, Aug 22, 2018 at 03:39:13PM -0500, Steve Wise wrote:
> > I see there are calls to comp_handler in the drivers. My best guess is
> > some driver has messed this up and is calling comp_handler after the
> > cq is destroyed?
>
> Maybe. The path that tickles this [1] is in iw_cxgb4. But the QP is
> being moved out of RTS, so the CQs should still be allocated.
Well, flush_qp obtains the CQ without any locking:
static void flush_qp(struct c4iw_qp *qhp)
[..]
rchp = to_c4iw_cq(qhp->ibqp.recv_cq);
(*rchp->ibcq.comp_handler)(&rchp->ibcq, rchp->ibcq.cq_context);
We shouldn't be able to destroy a CQ while a QP is using it though.
Due to this code:
static int create_qp(struct ib_uverbs_file *file,
[..]
if (attr.send_cq)
atomic_inc(&attr.send_cq->usecnt);
if (attr.recv_cq)
atomic_inc(&attr.recv_cq->usecnt);
so for this to be a problem the qp and the cq destroy would have to
race with flush_qp.
However peer_close() magically gets the 'ep' and 'qp':
static int peer_close(struct c4iw_dev *dev, struct sk_buff *skb)
{
ep = get_ep_from_tid(dev, tid);
if (!ep)
return 0;
mutex_lock(&ep->com.mutex);
ret = c4iw_modify_qp(ep->com.qp->rhp, ep->com.qp,
C4IW_QP_ATTR_NEXT_STATE, &attrs, 1);
get_ep_from_tid just gets a kref, doesn't hold a lock, and nothing
checks to see if ep->com or com.qp are valid pointers..
Sure looks racy with destroy_qp() to me..
Jason
