Re: [PATCHv2 1/1] net/mlx4_core: avoid resetting HCA when accessing an offline device

[Tariq Toukan]

On 18/04/2018 4:31 PM, Zhu Yanjun wrote:

While a faulty cable is used or HCA firmware error, HCA device will
be offline. When the driver is accessing this offline device, the
following call trace will pop out.

"
...
   [] dump_stack+0x63/0x81
   [] panic+0xcc/0x21b
   [] mlx4_enter_error_state+0xba/0xf0 [mlx4_core]
   [] mlx4_cmd_reset_flow+0x38/0x60 [mlx4_core]
   [] mlx4_cmd_poll+0xc1/0x2e0 [mlx4_core]
   [] __mlx4_cmd+0xb0/0x160 [mlx4_core]
   [] mlx4_SENSE_PORT+0x54/0xd0 [mlx4_core]
   [] mlx4_dev_cap+0x4a4/0xb50 [mlx4_core]
...
"
In the above call trace, the function mlx4_cmd_poll calls the function
mlx4_cmd_post to access the HCA while HCA is offline. Then mlx4_cmd_post
returns an error -EIO. Per -EIO, the function mlx4_cmd_poll calls
mlx4_cmd_reset_flow to reset HCA. And the above call trace pops out.

This is not reasonable. Since HCA device is offline when it is being
accessed, it should not be reset again.

In this patch, since HCA is offline, the function mlx4_cmd_post returns
an error -EINVAL. Per -EINVAL, the function mlx4_cmd_poll directly returns
instead of resetting HCA.

CC: Srinivas Eeda 
CC: Junxiao Bi 
Suggested-by: Håkon Bugge 
Suggested-by: Tariq Toukan 
Signed-off-by: Zhu Yanjun 
---
V1->V2: Follow Tariq's advice, avoid the disturbance from other returned errors.
Since the returned values from the function mlx4_cmd_post are -EIO and -EINVAL,
to -EIO, the HCA device should be reset. To -EINVAL, that means that the 
function
mlx4_cmd_post is accessing an offline device. It is not necessary to reset HCA.
Go to label out directly.
---
  drivers/net/ethernet/mellanox/mlx4/cmd.c | 12 ++--
  1 file changed, 10 insertions(+), 2 deletions(-)



Reviewed-by: Tariq Toukan 

Thanks Zhu.

[PATCHv2 1/1] net/mlx4_core: avoid resetting HCA when accessing an offline device

[Zhu Yanjun]

While a faulty cable is used or HCA firmware error, HCA device will
be offline. When the driver is accessing this offline device, the
following call trace will pop out.

"
...
  [] dump_stack+0x63/0x81
  [] panic+0xcc/0x21b
  [] mlx4_enter_error_state+0xba/0xf0 [mlx4_core]
  [] mlx4_cmd_reset_flow+0x38/0x60 [mlx4_core]
  [] mlx4_cmd_poll+0xc1/0x2e0 [mlx4_core]
  [] __mlx4_cmd+0xb0/0x160 [mlx4_core]
  [] mlx4_SENSE_PORT+0x54/0xd0 [mlx4_core]
  [] mlx4_dev_cap+0x4a4/0xb50 [mlx4_core]
...
"
In the above call trace, the function mlx4_cmd_poll calls the function
mlx4_cmd_post to access the HCA while HCA is offline. Then mlx4_cmd_post
returns an error -EIO. Per -EIO, the function mlx4_cmd_poll calls
mlx4_cmd_reset_flow to reset HCA. And the above call trace pops out.

This is not reasonable. Since HCA device is offline when it is being
accessed, it should not be reset again.

In this patch, since HCA is offline, the function mlx4_cmd_post returns
an error -EINVAL. Per -EINVAL, the function mlx4_cmd_poll directly returns
instead of resetting HCA.

CC: Srinivas Eeda 
CC: Junxiao Bi 
Suggested-by: Håkon Bugge 
Suggested-by: Tariq Toukan 
Signed-off-by: Zhu Yanjun 
---
V1->V2: Follow Tariq's advice, avoid the disturbance from other returned errors.
Since the returned values from the function mlx4_cmd_post are -EIO and -EINVAL,
to -EIO, the HCA device should be reset. To -EINVAL, that means that the 
function
mlx4_cmd_post is accessing an offline device. It is not necessary to reset HCA.
Go to label out directly.
---
 drivers/net/ethernet/mellanox/mlx4/cmd.c | 12 ++--
 1 file changed, 10 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/mellanox/mlx4/cmd.c 
b/drivers/net/ethernet/mellanox/mlx4/cmd.c
index 6a9086d..df735b8 100644
--- a/drivers/net/ethernet/mellanox/mlx4/cmd.c
+++ b/drivers/net/ethernet/mellanox/mlx4/cmd.c
@@ -451,6 +451,8 @@ static int mlx4_cmd_post(struct mlx4_dev *dev, u64 
in_param, u64 out_param,
 * Device is going through error recovery
 * and cannot accept commands.
 */
+   mlx4_err(dev, "%s : Device is in error recovery.\n", __func__);
+   ret = -EINVAL;
goto out;
}
 
@@ -610,8 +612,11 @@ static int mlx4_cmd_poll(struct mlx4_dev *dev, u64 
in_param, u64 *out_param,
 
err = mlx4_cmd_post(dev, in_param, out_param ? *out_param : 0,
in_modifier, op_modifier, op, CMD_POLL_TOKEN, 0);
-   if (err)
+   if (err) {
+   if (err == -EINVAL)
+   goto out;
goto out_reset;
+   }
 
end = msecs_to_jiffies(timeout) + jiffies;
while (cmd_pending(dev) && time_before(jiffies, end)) {
@@ -710,8 +715,11 @@ static int mlx4_cmd_wait(struct mlx4_dev *dev, u64 
in_param, u64 *out_param,
 
err = mlx4_cmd_post(dev, in_param, out_param ? *out_param : 0,
in_modifier, op_modifier, op, context->token, 1);
-   if (err)
+   if (err) {
+   if (err == -EINVAL)
+   goto out;
goto out_reset;
+   }
 
if (op == MLX4_CMD_SENSE_PORT) {
ret_wait =
-- 
2.7.4