Subject: Re: [PATCH net 2/5] net/mlx5: Fix peer devlink set for SF representor devlink port

From: Simon Horman <[email-masked]>
Date: Fri, 10 May 2024 16:38:27 +0100

On Thu, May 09, 2024 at 02:29:48PM +0300, Tariq Toukan wrote:
> From: Shay Drory <[email-masked]>
> 
> The cited patch change register devlink flow, and neglect to reflect
> the changes for peer devlink set logic. Peer devlink set is
> triggering a call trace if done after devl_register.[1]
> 
> Hence, align peer devlink set logic with register devlink flow.
> 
> [1]
> WARNING: CPU: 4 PID: 3394 at net/devlink/core.c:155 devlink_rel_nested_in_add+0x177/0x180
> CPU: 4 PID: 3394 Comm: kworker/u40:1 Not tainted 6.9.0-rc4_for_linust_min_debug_2024_04_16_14_08 #1
> Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS rel-1.13.0-0-gf21b5a4aeb02-prebuilt.qemu.org 04/01/2014
> Workqueue: mlx5_vhca_event0 mlx5_vhca_state_work_handler [mlx5_core]
> RIP: 0010:devlink_rel_nested_in_add+0x177/0x180
> Call Trace:
>  <TASK>
>  ? __warn+0x78/0x120
>  ? devlink_rel_nested_in_add+0x177/0x180
>  ? report_bug+0x16d/0x180
>  ? handle_bug+0x3c/0x60
>  ? exc_invalid_op+0x14/0x70
>  ? asm_exc_invalid_op+0x16/0x20
>  ? devlink_port_init+0x30/0x30
>  ? devlink_port_type_clear+0x50/0x50
>  ? devlink_rel_nested_in_add+0x177/0x180
>  ? devlink_rel_nested_in_add+0xdd/0x180
>  mlx5_sf_mdev_event+0x74/0xb0 [mlx5_core]
>  notifier_call_chain+0x35/0xb0
>  blocking_notifier_call_chain+0x3d/0x60
>  mlx5_blocking_notifier_call_chain+0x22/0x30 [mlx5_core]
>  mlx5_sf_dev_probe+0x185/0x3e0 [mlx5_core]
>  auxiliary_bus_probe+0x38/0x80
>  ? driver_sysfs_add+0x51/0x80
>  really_probe+0xc5/0x3a0
>  ? driver_probe_device+0x90/0x90
>  __driver_probe_device+0x80/0x160
>  driver_probe_device+0x1e/0x90
>  __device_attach_driver+0x7d/0x100
>  bus_for_each_drv+0x80/0xd0
>  __device_attach+0xbc/0x1f0
>  bus_probe_device+0x86/0xa0
>  device_add+0x64f/0x860
>  __auxiliary_device_add+0x3b/0xa0
>  mlx5_sf_dev_add+0x139/0x330 [mlx5_core]
>  mlx5_sf_dev_state_change_handler+0x1e4/0x250 [mlx5_core]
>  notifier_call_chain+0x35/0xb0
>  blocking_notifier_call_chain+0x3d/0x60
>  mlx5_vhca_state_work_handler+0x151/0x200 [mlx5_core]
>  process_one_work+0x13f/0x2e0
>  worker_thread+0x2bd/0x3c0
>  ? rescuer_thread+0x410/0x410
>  kthread+0xc4/0xf0
>  ? kthread_complete_and_exit+0x20/0x20
>  ret_from_fork+0x2d/0x50
>  ? kthread_complete_and_exit+0x20/0x20
>  ret_from_fork_asm+0x11/0x20
>  </TASK>
> 
> Fixes: bf729988303a ("net/mlx5: Restore mistakenly dropped parts in register devlink flow")
> Fixes: c6e77aa9dd82 ("net/mlx5: Register devlink first under devlink lock")

Hi Tariq, Shay, all,

I agree that this patch addresses problems introduced by both of the
commits cited above. But I also note that they are both fixes for the
following commit. So I wonder if it should be cited in a Fixes tag too.

cf530217408e ("devlink: Notify users when objects are accessible")

> Signed-off-by: Shay Drory <[email-masked]>
> Reviewed-by: Moshe Shemesh <[email-masked]>
> Signed-off-by: Tariq Toukan <[email-masked]>

The above notwithstanding this looks good to me.

Reviewed-by: Simon Horman <[email-masked]>