Re: [PATCH for-next 05/20] RDMA/hns: Add command queue support for hip08 RoCE driver

[Doug Ledford <dledford-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>]

On Wed, 2017-08-30 at 17:23 +0800, Wei Hu (Xavier) wrote:

So, we set the send timeout limit here:

> +	/* Setup Tx write back timeout */
> +	priv->cmq.tx_timeout = HNS_ROCE_CMQ_TX_TIMEOUT;
> +

[ snip ]

> +int hns_roce_cmq_send(struct hns_roce_dev *hr_dev,
> +		      struct hns_roce_cmq_desc *desc, int num)
> +{
> +	struct hns_roce_v2_priv *priv = (struct hns_roce_v2_priv
> *)hr_dev->priv;
> +	struct hns_roce_v2_cmq_ring *csq = &priv->cmq.csq;
> +	struct hns_roce_cmq_desc *desc_to_use;
> +	bool complete = false;
> +	u32 timeout = 0;
> +	int handle = 0;
> +	u16 desc_ret;
> +	int ret = 0;
> +	int ntc;
> +
> +	spin_lock_bh(&csq->lock);

and we take a bh lock here...

> +	if (num > hns_roce_cmq_space(csq)) {
> +		spin_unlock_bh(&csq->lock);
> +		return -EBUSY;
> +	}
> +
> +	/*
> +	 * Record the location of desc in the cmq for this time
> +	 * which will be use for hardware to write back
> +	 */
> +	ntc = csq->next_to_use;
> +
> +	while (handle < num) {
> +		desc_to_use = &csq->desc[csq->next_to_use];
> +		*desc_to_use = desc[handle];
> +		dev_dbg(hr_dev->dev, "set cmq desc:\n");
> +		csq->next_to_use++;
> +		if (csq->next_to_use == csq->desc_num)
> +			csq->next_to_use = 0;
> +		handle++;
> +	}
> +
> +	/* Write to hardware */
> +	roce_write(hr_dev, ROCEE_TX_CMQ_TAIL_REG, csq->next_to_use);
> +
> +	/*
> +	 * If the command is sync, wait for the firmware to write
> back,
> +	 * if multi descriptors to be sent, use the first one to
> check
> +	 */
> +	if ((desc->flag) & HNS_ROCE_CMD_FLAG_NO_INTR) {
> +		do {
> +			if (hns_roce_cmq_csq_done(hr_dev))
> +				break;
> +			usleep_range(1000, 2000);
> +			timeout++;
> +		} while (timeout < priv->cmq.tx_timeout);
> +	}

then we spin here for a maximum amount of time between 200 and 400ms,
so 1/4 to 1/2 a second.  All the time we are holding the bh lock on
this CPU.  That seems excessive to me.  If we are going to spin that
long, can you find a way to allocate/reserve your resources, send the
command, then drop the bh lock while you spin, and retake it before you
complete once the spinning is done?

> +#define HNS_ROCE_CMQ_TX_TIMEOUT			200

or you could reduce the size of this define...

-- 
Doug Ledford <dledford-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
    GPG KeyID: B826A3330E572FDD
    Key fingerprint = AE6B 1BDA 122B 23B4 265B  1274 B826 A333 0E57 2FDD

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html