On Mon, Sep 25, 2017 at 01:06:53PM -0400, Doug Ledford wrote:
> 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?
They don't allocate anything in this loop, but checking the pointers are
the same, see hns_roce_cmq_csq_done.
>
> > +#define HNS_ROCE_CMQ_TX_TIMEOUT 200
>
> or you could reduce the size of this define...
>
> --
> Doug Ledford <dledford@xxxxxxxxxx>
> 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@xxxxxxxxxxxxxxx
> More majordomo info at http://vger.kernel.org/majordomo-info.html
Attachment:
signature.asc
Description: PGP signature
