[openib-general] Re: [PATCH 2/6] [RFC] mthca kernel changes for resizeCQ

[Michael S. Tsirkin]

Quoting r. Roland Dreier <rolandd at cisco.com>:
> +static int mthca_resize_cq(struct ib_cq *ibcq, int entries, struct ib_udata *udata)
> +{
> +	struct mthca_dev *dev = to_mdev(ibcq->device);
> +	struct mthca_cq *cq = to_mcq(ibcq);
> +	struct mthca_resize_cq ucmd;
> +	u32 lkey;
> +	u8 status;
> +	int ret;
> +
> +	if (entries < 1 || entries > dev->limits.max_cqes)
> +		return -EINVAL;
> +
> +	entries = roundup_pow_of_two(entries + 1);
> +	if (entries == ibcq->cqe + 1)
> +		return 0;
> +
> +	if (cq->is_kernel) {
> +		ret = mthca_alloc_resize_buf(dev, cq, entries);
> +		if (ret)
> +			return ret;
> +		lkey = cq->resize_buf->buf.mr.ibmr.lkey;
> +	} else {
> +		if (ib_copy_from_udata(&ucmd, udata, sizeof ucmd))
> +			return -EFAULT;
> +		lkey = ucmd.lkey;
> +	}
> +
> +	ret = mthca_RESIZE_CQ(dev, cq->cqn, lkey, long_log2(entries), &status);
> +	if (status)
> +		ret = -EINVAL;
> +
> +	if (ret) {
> +		if (cq->resize_buf) {
> +			mthca_free_cq_buf(dev, &cq->resize_buf->buf,
> +					  cq->resize_buf->cqe);
> +			kfree(cq->resize_buf);
> +			spin_lock_irq(&cq->lock);
> +			cq->resize_buf = NULL;
> +			spin_unlock_irq(&cq->lock);
> +		}
> +		return ret;
> +	}
> +
> +	if (cq->is_kernel) {
> +		struct mthca_cq_buf tbuf;
> +		int tcqe;
> +
> +		spin_lock_irq(&cq->lock);
> +		if (cq->resize_buf->state == CQ_RESIZE_READY) {
> +			mthca_cq_resize_copy_cqes(cq);
> +			tbuf         = cq->buf;
> +			tcqe         = cq->ibcq.cqe;
> +			cq->buf      = cq->resize_buf->buf;
> +			cq->ibcq.cqe = cq->resize_buf->cqe;
> +		} else {
> +			tbuf = cq->resize_buf->buf;
> +			tcqe = cq->resize_buf->cqe;
> +		}
> +
> +		kfree(cq->resize_buf);
> +		cq->resize_buf = NULL;
> +		spin_unlock_irq(&cq->lock);
> +
> +		mthca_free_cq_buf(dev, &tbuf, tcqe);
> +	} else
> +		ibcq->cqe = entries - 1;
> +
> +	return 0;
> +}

I think I see a problem with this approach: if a ULP performs CQ poll
while mthca_RESIZE_CQ is in progress, it might get a false indication
that the CQ is empty since CQEs are being written to the new buffer
already.

As a result e.g. a ULP that does:

arm
while (poll cq) {
   handle cqe
}

will not empty the CQ and will deadlock.


-- 
Michael S. Tsirkin
Staff Engineer, Mellanox Technologies