diff mbox series

[v2,for-next,05/17] RDMA/bnxt_re: Enhance the existing functions that wait for FW responses

Message ID 1686308514-11996-6-git-send-email-selvin.xavier@broadcom.com (mailing list archive)
State Accepted
Headers show
Series RDMA/bnxt_re: Control path updates | expand

Commit Message

Selvin Xavier June 9, 2023, 11:01 a.m. UTC 
From: Kashyap Desai <kashyap.desai@broadcom.com>

Use jiffies based timewait instead of counting iteration for
commands that block for FW response.

Also add a poll routine for control path commands. This is for
polling completion if the waiting commands timeout. This avoids cases
where the driver misses completion interrupts.

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 | 65 +++++++++++++++++++++++-------
 1 file changed, 51 insertions(+), 14 deletions(-)

Comments

Leon Romanovsky June 12, 2023, 7 a.m. UTC | #1 
On Fri, Jun 09, 2023 at 04:01:42AM -0700, Selvin Xavier wrote:
> From: Kashyap Desai <kashyap.desai@broadcom.com>
> 
> Use jiffies based timewait instead of counting iteration for
> commands that block for FW response.
> 
> Also add a poll routine for control path commands. This is for
> polling completion if the waiting commands timeout. This avoids cases
> where the driver misses completion interrupts.
> 
> 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 | 65 +++++++++++++++++++++++-------
>  1 file changed, 51 insertions(+), 14 deletions(-)
> 
> diff --git a/drivers/infiniband/hw/bnxt_re/qplib_rcfw.c b/drivers/infiniband/hw/bnxt_re/qplib_rcfw.c
> index 66121fb..3b242cc 100644
> --- a/drivers/infiniband/hw/bnxt_re/qplib_rcfw.c
> +++ b/drivers/infiniband/hw/bnxt_re/qplib_rcfw.c
> @@ -53,37 +53,74 @@
>  
>  static void bnxt_qplib_service_creq(struct tasklet_struct *t);
>  
> -/* Hardware communication channel */
> +/**
> + * __wait_for_resp   -	Don't hold the cpu context and wait for response
> + * @rcfw      -   rcfw channel instance of rdev
> + * @cookie    -   cookie to track the command
> + *
> + * Wait for command completion in sleepable context.
> + *
> + * Returns:
> + * 0 if command is completed by firmware.
> + * Non zero error code for rest of the case.

I don't see "return ret;" in this function.

> + */
>  static int __wait_for_resp(struct bnxt_qplib_rcfw *rcfw, u16 cookie)
>  {
>  	struct bnxt_qplib_cmdq_ctx *cmdq;
>  	u16 cbit;
> -	int rc;
> +	int ret;
>  
>  	cmdq = &rcfw->cmdq;
>  	cbit = cookie % rcfw->cmdq_depth;
> -	rc = wait_event_timeout(cmdq->waitq,
> -				!test_bit(cbit, cmdq->cmdq_bitmap),
> -				msecs_to_jiffies(RCFW_CMD_WAIT_TIME_MS));
> -	return rc ? 0 : -ETIMEDOUT;
> +
> +	do {
> +		/* Non zero means command completed */
> +		ret = wait_event_timeout(cmdq->waitq,
> +					 !test_bit(cbit, cmdq->cmdq_bitmap),
> +					 msecs_to_jiffies(10000));

Don't you need to check ret here?

> +
> +		if (!test_bit(cbit, cmdq->cmdq_bitmap))
> +			return 0;
> +
> +		bnxt_qplib_service_creq(&rcfw->creq.creq_tasklet);
> +
> +		if (!test_bit(cbit, cmdq->cmdq_bitmap))
> +			return 0;
> +
> +	} while (true);
>  };
>  
> +/**
> + * __block_for_resp   -	hold the cpu context and wait for response
> + * @rcfw      -   rcfw channel instance of rdev
> + * @cookie    -   cookie to track the command
> + *
> + * This function will hold the cpu (non-sleepable context) and
> + * wait for command completion. Maximum holding interval is 8 second.
> + *
> + * Returns:
> + * -ETIMEOUT if command is not completed in specific time interval.
> + * 0 if command is completed by firmware.
> + */
>  static int __block_for_resp(struct bnxt_qplib_rcfw *rcfw, u16 cookie)
>  {
> -	u32 count = RCFW_BLOCKED_CMD_WAIT_COUNT;
> -	struct bnxt_qplib_cmdq_ctx *cmdq;
> +	struct bnxt_qplib_cmdq_ctx *cmdq = &rcfw->cmdq;
> +	unsigned long issue_time = 0;
>  	u16 cbit;
>  
> -	cmdq = &rcfw->cmdq;
>  	cbit = cookie % rcfw->cmdq_depth;
> -	if (!test_bit(cbit, cmdq->cmdq_bitmap))
> -		goto done;
> +	issue_time = jiffies;
> +
>  	do {
>  		udelay(1);
> +
>  		bnxt_qplib_service_creq(&rcfw->creq.creq_tasklet);
> -	} while (test_bit(cbit, cmdq->cmdq_bitmap) && --count);
> -done:
> -	return count ? 0 : -ETIMEDOUT;
> +		if (!test_bit(cbit, cmdq->cmdq_bitmap))
> +			return 0;
> +
> +	} while (time_before(jiffies, issue_time + (8 * HZ)));
> +
> +	return -ETIMEDOUT;
>  };
>  
>  static int __send_message(struct bnxt_qplib_rcfw *rcfw,
> -- 
> 2.5.5
>
diff mbox series

Patch

diff --git a/drivers/infiniband/hw/bnxt_re/qplib_rcfw.c b/drivers/infiniband/hw/bnxt_re/qplib_rcfw.c
index 66121fb..3b242cc 100644
--- a/drivers/infiniband/hw/bnxt_re/qplib_rcfw.c
+++ b/drivers/infiniband/hw/bnxt_re/qplib_rcfw.c
@@ -53,37 +53,74 @@ 
 
 static void bnxt_qplib_service_creq(struct tasklet_struct *t);
 
-/* Hardware communication channel */
+/**
+ * __wait_for_resp   -	Don't hold the cpu context and wait for response
+ * @rcfw      -   rcfw channel instance of rdev
+ * @cookie    -   cookie to track the command
+ *
+ * Wait for command completion in sleepable context.
+ *
+ * Returns:
+ * 0 if command is completed by firmware.
+ * Non zero error code for rest of the case.
+ */
 static int __wait_for_resp(struct bnxt_qplib_rcfw *rcfw, u16 cookie)
 {
 	struct bnxt_qplib_cmdq_ctx *cmdq;
 	u16 cbit;
-	int rc;
+	int ret;
 
 	cmdq = &rcfw->cmdq;
 	cbit = cookie % rcfw->cmdq_depth;
-	rc = wait_event_timeout(cmdq->waitq,
-				!test_bit(cbit, cmdq->cmdq_bitmap),
-				msecs_to_jiffies(RCFW_CMD_WAIT_TIME_MS));
-	return rc ? 0 : -ETIMEDOUT;
+
+	do {
+		/* Non zero means command completed */
+		ret = wait_event_timeout(cmdq->waitq,
+					 !test_bit(cbit, cmdq->cmdq_bitmap),
+					 msecs_to_jiffies(10000));
+
+		if (!test_bit(cbit, cmdq->cmdq_bitmap))
+			return 0;
+
+		bnxt_qplib_service_creq(&rcfw->creq.creq_tasklet);
+
+		if (!test_bit(cbit, cmdq->cmdq_bitmap))
+			return 0;
+
+	} while (true);
 };
 
+/**
+ * __block_for_resp   -	hold the cpu context and wait for response
+ * @rcfw      -   rcfw channel instance of rdev
+ * @cookie    -   cookie to track the command
+ *
+ * This function will hold the cpu (non-sleepable context) and
+ * wait for command completion. Maximum holding interval is 8 second.
+ *
+ * Returns:
+ * -ETIMEOUT if command is not completed in specific time interval.
+ * 0 if command is completed by firmware.
+ */
 static int __block_for_resp(struct bnxt_qplib_rcfw *rcfw, u16 cookie)
 {
-	u32 count = RCFW_BLOCKED_CMD_WAIT_COUNT;
-	struct bnxt_qplib_cmdq_ctx *cmdq;
+	struct bnxt_qplib_cmdq_ctx *cmdq = &rcfw->cmdq;
+	unsigned long issue_time = 0;
 	u16 cbit;
 
-	cmdq = &rcfw->cmdq;
 	cbit = cookie % rcfw->cmdq_depth;
-	if (!test_bit(cbit, cmdq->cmdq_bitmap))
-		goto done;
+	issue_time = jiffies;
+
 	do {
 		udelay(1);
+
 		bnxt_qplib_service_creq(&rcfw->creq.creq_tasklet);
-	} while (test_bit(cbit, cmdq->cmdq_bitmap) && --count);
-done:
-	return count ? 0 : -ETIMEDOUT;
+		if (!test_bit(cbit, cmdq->cmdq_bitmap))
+			return 0;
+
+	} while (time_before(jiffies, issue_time + (8 * HZ)));
+
+	return -ETIMEDOUT;
 };
 
 static int __send_message(struct bnxt_qplib_rcfw *rcfw,