Linux RDMA and InfiniBand development
 help / color / mirror / Atom feed
* [PATCH for-next] RDMA/efa: Extend admin timeout error print
@ 2025-07-02 15:20 Michael Margolin
  2025-07-03  6:46 ` Gal Pressman
  0 siblings, 1 reply; 3+ messages in thread
From: Michael Margolin @ 2025-07-02 15:20 UTC (permalink / raw)
  To: jgg, leon, linux-rdma; +Cc: sleybo, matua, gal.pressman, Yonatan Nachum

Add command context index to the printed message for additional debug
information.

Reviewed-by: Yonatan Nachum <ynachum@amazon.com>
Signed-off-by: Michael Margolin <mrgolin@amazon.com>
---
 drivers/infiniband/hw/efa/efa_com.c | 12 +++++++-----
 1 file changed, 7 insertions(+), 5 deletions(-)

diff --git a/drivers/infiniband/hw/efa/efa_com.c b/drivers/infiniband/hw/efa/efa_com.c
index bafd210dd43e..f1e88ee89bb8 100644
--- a/drivers/infiniband/hw/efa/efa_com.c
+++ b/drivers/infiniband/hw/efa/efa_com.c
@@ -1,6 +1,6 @@
 // SPDX-License-Identifier: GPL-2.0 OR BSD-2-Clause
 /*
- * Copyright 2018-2024 Amazon.com, Inc. or its affiliates. All rights reserved.
+ * Copyright 2018-2025 Amazon.com, Inc. or its affiliates. All rights reserved.
  */
 
 #include "efa_com.h"
@@ -557,17 +557,19 @@ static int efa_com_wait_and_process_admin_cq_interrupts(struct efa_comp_ctx *com
 		if (comp_ctx->status == EFA_CMD_COMPLETED)
 			ibdev_err_ratelimited(
 				aq->efa_dev,
-				"The device sent a completion but the driver didn't receive any MSI-X interrupt for admin cmd %s(%d) status %d (ctx: 0x%p, sq producer: %d, sq consumer: %d, cq consumer: %d)\n",
+				"The device sent a completion but the driver didn't receive any MSI-X interrupt for admin cmd %s(%d) status %d (ctx[%d]: 0x%p, sq producer: %d, sq consumer: %d, cq consumer: %d)\n",
 				efa_com_cmd_str(comp_ctx->cmd_opcode),
 				comp_ctx->cmd_opcode, comp_ctx->status,
-				comp_ctx, aq->sq.pc, aq->sq.cc, aq->cq.cc);
+				comp_ctx - aq->comp_ctx, comp_ctx, aq->sq.pc,
+				aq->sq.cc, aq->cq.cc);
 		else
 			ibdev_err_ratelimited(
 				aq->efa_dev,
-				"The device didn't send any completion for admin cmd %s(%d) status %d (ctx 0x%p, sq producer: %d, sq consumer: %d, cq consumer: %d)\n",
+				"The device didn't send any completion for admin cmd %s(%d) status %d (ctx[%d]: 0x%p, sq producer: %d, sq consumer: %d, cq consumer: %d)\n",
 				efa_com_cmd_str(comp_ctx->cmd_opcode),
 				comp_ctx->cmd_opcode, comp_ctx->status,
-				comp_ctx, aq->sq.pc, aq->sq.cc, aq->cq.cc);
+				comp_ctx - aq->comp_ctx, comp_ctx, aq->sq.pc,
+				aq->sq.cc, aq->cq.cc);
 
 		clear_bit(EFA_AQ_STATE_RUNNING_BIT, &aq->state);
 		err = -ETIME;
-- 
2.47.1


^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [PATCH for-next] RDMA/efa: Extend admin timeout error print
  2025-07-02 15:20 [PATCH for-next] RDMA/efa: Extend admin timeout error print Michael Margolin
@ 2025-07-03  6:46 ` Gal Pressman
  2025-07-03 12:20   ` Margolin, Michael
  0 siblings, 1 reply; 3+ messages in thread
From: Gal Pressman @ 2025-07-03  6:46 UTC (permalink / raw)
  To: Michael Margolin, jgg, leon, linux-rdma; +Cc: sleybo, matua, Yonatan Nachum

On 02/07/2025 18:20, Michael Margolin wrote:
> Add command context index to the printed message for additional debug
> information.
> 
> Reviewed-by: Yonatan Nachum <ynachum@amazon.com>
> Signed-off-by: Michael Margolin <mrgolin@amazon.com>
> ---
>  drivers/infiniband/hw/efa/efa_com.c | 12 +++++++-----
>  1 file changed, 7 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/infiniband/hw/efa/efa_com.c b/drivers/infiniband/hw/efa/efa_com.c
> index bafd210dd43e..f1e88ee89bb8 100644
> --- a/drivers/infiniband/hw/efa/efa_com.c
> +++ b/drivers/infiniband/hw/efa/efa_com.c
> @@ -1,6 +1,6 @@
>  // SPDX-License-Identifier: GPL-2.0 OR BSD-2-Clause
>  /*
> - * Copyright 2018-2024 Amazon.com, Inc. or its affiliates. All rights reserved.
> + * Copyright 2018-2025 Amazon.com, Inc. or its affiliates. All rights reserved.
>   */
>  
>  #include "efa_com.h"
> @@ -557,17 +557,19 @@ static int efa_com_wait_and_process_admin_cq_interrupts(struct efa_comp_ctx *com
>  		if (comp_ctx->status == EFA_CMD_COMPLETED)
>  			ibdev_err_ratelimited(
>  				aq->efa_dev,
> -				"The device sent a completion but the driver didn't receive any MSI-X interrupt for admin cmd %s(%d) status %d (ctx: 0x%p, sq producer: %d, sq consumer: %d, cq consumer: %d)\n",
> +				"The device sent a completion but the driver didn't receive any MSI-X interrupt for admin cmd %s(%d) status %d (ctx[%d]: 0x%p, sq producer: %d, sq consumer: %d, cq consumer: %d)\n",
>  				efa_com_cmd_str(comp_ctx->cmd_opcode),
>  				comp_ctx->cmd_opcode, comp_ctx->status,
> -				comp_ctx, aq->sq.pc, aq->sq.cc, aq->cq.cc);
> +				comp_ctx - aq->comp_ctx, comp_ctx, aq->sq.pc,
> +				aq->sq.cc, aq->cq.cc);
>  		else
>  			ibdev_err_ratelimited(
>  				aq->efa_dev,
> -				"The device didn't send any completion for admin cmd %s(%d) status %d (ctx 0x%p, sq producer: %d, sq consumer: %d, cq consumer: %d)\n",
> +				"The device didn't send any completion for admin cmd %s(%d) status %d (ctx[%d]: 0x%p, sq producer: %d, sq consumer: %d, cq consumer: %d)\n",
>  				efa_com_cmd_str(comp_ctx->cmd_opcode),
>  				comp_ctx->cmd_opcode, comp_ctx->status,
> -				comp_ctx, aq->sq.pc, aq->sq.cc, aq->cq.cc);
> +				comp_ctx - aq->comp_ctx, comp_ctx, aq->sq.pc,
> +				aq->sq.cc, aq->cq.cc);
>  
>  		clear_bit(EFA_AQ_STATE_RUNNING_BIT, &aq->state);
>  		err = -ETIME;

Arguably, there is no point in keeping the comp_ctx pointer print, as
you have nothing to compare the hashed pointer to. It could've been
useful if the pointer was also printed when the command is submitted,
but it isn't, so it's probably better to just remove it and keep the
index you added.

A better alternative might be storing the cmd_id inside the comp_ctx and
printing it instead, it contains more information.

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH for-next] RDMA/efa: Extend admin timeout error print
  2025-07-03  6:46 ` Gal Pressman
@ 2025-07-03 12:20   ` Margolin, Michael
  0 siblings, 0 replies; 3+ messages in thread
From: Margolin, Michael @ 2025-07-03 12:20 UTC (permalink / raw)
  To: Gal Pressman, jgg, leon, linux-rdma; +Cc: sleybo, matua, Yonatan Nachum


On 7/3/2025 9:46 AM, Gal Pressman wrote:
> Arguably, there is no point in keeping the comp_ctx pointer print, as
> you have nothing to compare the hashed pointer to. It could've been
> useful if the pointer was also printed when the command is submitted,
> but it isn't, so it's probably better to just remove it and keep the
> index you added.
>
> A better alternative might be storing the cmd_id inside the comp_ctx and
> printing it instead, it contains more information.

Nice idea, doing that. Thanks.

Michael


^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2025-07-03 12:21 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-07-02 15:20 [PATCH for-next] RDMA/efa: Extend admin timeout error print Michael Margolin
2025-07-03  6:46 ` Gal Pressman
2025-07-03 12:20   ` Margolin, Michael

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox