On Mon, Jun 12, 2023 at 12:34 PM Leon Romanovsky <leon@xxxxxxxxxx> wrote:
>
> On Fri, Jun 09, 2023 at 04:01:46AM -0700, Selvin Xavier wrote:
> > From: Kashyap Desai <kashyap.desai@xxxxxxxxxxxx>
> >
> > This interface will be used if the driver has not enabled interrupt
> > and/or interrupt is disabled for a short period of time.
> > Completion is not possible from interrupt so this interface does
> > self-polling.
> >
> > Signed-off-by: Kashyap Desai <kashyap.desai@xxxxxxxxxxxx>
> > Signed-off-by: Selvin Xavier <selvin.xavier@xxxxxxxxxxxx>
> > ---
> > drivers/infiniband/hw/bnxt_re/qplib_rcfw.c | 44 +++++++++++++++++++++++++++++-
> > drivers/infiniband/hw/bnxt_re/qplib_rcfw.h | 1 +
> > 2 files changed, 44 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/infiniband/hw/bnxt_re/qplib_rcfw.c b/drivers/infiniband/hw/bnxt_re/qplib_rcfw.c
> > index 15f6793..3215f8a 100644
> > --- a/drivers/infiniband/hw/bnxt_re/qplib_rcfw.c
> > +++ b/drivers/infiniband/hw/bnxt_re/qplib_rcfw.c
> > @@ -260,6 +260,44 @@ static int __send_message(struct bnxt_qplib_rcfw *rcfw,
> > return 0;
> > }
> >
> > +/**
> > + * __poll_for_resp - self poll completion for rcfw command
> > + * @rcfw - rcfw channel instance of rdev
> > + * @cookie - cookie to track the command
> > + * @opcode - rcfw submitted for given opcode
> > + *
> > + * It works same as __wait_for_resp except this function will
> > + * do self polling in sort interval since interrupt is disabled.
> > + * This function can not be called from non-sleepable context.
> > + *
> > + * Returns:
> > + * -ETIMEOUT if command is not completed in specific time interval.
> > + * 0 if command is completed by firmware.
> > + */
> > +static int __poll_for_resp(struct bnxt_qplib_rcfw *rcfw, u16 cookie,
> > + u8 opcode)
> > +{
> > + struct bnxt_qplib_cmdq_ctx *cmdq = &rcfw->cmdq;
> > + unsigned long issue_time;
> > + u16 cbit;
> > +
> > + cbit = cookie % rcfw->cmdq_depth;
> > + issue_time = jiffies;
> > +
> > + do {
> > + if (test_bit(ERR_DEVICE_DETACHED, &cmdq->flags))
> > + return bnxt_qplib_map_rc(opcode);
> > +
> > + usleep_range(1000, 1001);
> > +
> > + bnxt_qplib_service_creq(&rcfw->creq.creq_tasklet);
> > + if (!test_bit(cbit, cmdq->cmdq_bitmap))
> > + return 0;
> > + if (jiffies_to_msecs(jiffies - issue_time) > 10000)
> > + return -ETIMEDOUT;
> > + } while (true);
> > +};
> > +
> > static int __send_message_basic_sanity(struct bnxt_qplib_rcfw *rcfw,
> > struct bnxt_qplib_cmdqmsg *msg)
> > {
> > @@ -328,8 +366,10 @@ static int __bnxt_qplib_rcfw_send_message(struct bnxt_qplib_rcfw *rcfw,
> >
> > if (msg->block)
> > rc = __block_for_resp(rcfw, cookie, opcode);
> > - else
> > + else if (atomic_read(&rcfw->rcfw_intr_enabled))
>
> Why atomic_t? It doesn't eliminate the need of locking and if locking
> exists, you don't need atomic_t.
The intent of rcfw_intr_enabled was to avoid the locks. But I agree
that it will not help. We will refactor this
code and move it under a better locking scheme. Will remove atomic
variables in this path.
Thanks
>
> > rc = __wait_for_resp(rcfw, cookie, opcode);
> > + else
> > + rc = __poll_for_resp(rcfw, cookie, opcode);
> > if (rc) {
> > /* timed out */
> > dev_err(&rcfw->pdev->dev, "cmdq[%#x]=%#x timedout (%d)msec\n",
> > @@ -796,6 +836,7 @@ void bnxt_qplib_rcfw_stop_irq(struct bnxt_qplib_rcfw *rcfw, bool kill)
> > kfree(creq->irq_name);
> > creq->irq_name = NULL;
> > creq->requested = false;
> > + atomic_set(&rcfw->rcfw_intr_enabled, 0);
> > }
> >
> > void bnxt_qplib_disable_rcfw_channel(struct bnxt_qplib_rcfw *rcfw)
> > @@ -857,6 +898,7 @@ int bnxt_qplib_rcfw_start_irq(struct bnxt_qplib_rcfw *rcfw, int msix_vector,
> > creq->requested = true;
> >
> > bnxt_qplib_ring_nq_db(&creq->creq_db.dbinfo, res->cctx, true);
> > + atomic_inc(&rcfw->rcfw_intr_enabled);
> >
> > return 0;
> > }
> > diff --git a/drivers/infiniband/hw/bnxt_re/qplib_rcfw.h b/drivers/infiniband/hw/bnxt_re/qplib_rcfw.h
> > index b7bbbae..089e616 100644
> > --- a/drivers/infiniband/hw/bnxt_re/qplib_rcfw.h
> > +++ b/drivers/infiniband/hw/bnxt_re/qplib_rcfw.h
> > @@ -221,6 +221,7 @@ struct bnxt_qplib_rcfw {
> > u64 oos_prev;
> > u32 init_oos_stats;
> > u32 cmdq_depth;
> > + atomic_t rcfw_intr_enabled;
> > struct semaphore rcfw_inflight;
> > };
> >
> > --
> > 2.5.5
> >
>
>
Attachment:
smime.p7s
Description: S/MIME Cryptographic Signature
