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

Doug Ledford
2017-09-25T17:06:53Z

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