| Message ID | 1686308514-11996-10-git-send-email-selvin.xavier@broadcom.com (mailing list archive) |
|---|---|
| State | Accepted |
| Headers | show |
| Series | RDMA/bnxt_re: Control path updates | expand |
On Fri, Jun 09, 2023 at 04:01:46AM -0700, Selvin Xavier wrote: > From: Kashyap Desai <kashyap.desai@broadcom.com> > > 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@broadcom.com> > Signed-off-by: Selvin Xavier <selvin.xavier@broadcom.com> > --- > 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. > 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 >
On Mon, Jun 12, 2023 at 12:34 PM Leon Romanovsky <leon@kernel.org> wrote: > > On Fri, Jun 09, 2023 at 04:01:46AM -0700, Selvin Xavier wrote: > > From: Kashyap Desai <kashyap.desai@broadcom.com> > > > > 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@broadcom.com> > > Signed-off-by: Selvin Xavier <selvin.xavier@broadcom.com> > > --- > > 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 > > > >
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)) 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; };