Commit c6e77aa9 authored by Shay Drory's avatar Shay Drory Committed by Jakub Kicinski
Browse files

net/mlx5: Register devlink first under devlink lock 



In case device is having a non fatal FW error during probe, the
driver will report the error to user via devlink. This will trigger
a WARN_ON, since mlx5 is calling devlink_register() last.
In order to avoid the WARN_ON[1], change mlx5 to invoke devl_register()
first under devlink lock.

[1]
WARNING: CPU: 5 PID: 227 at net/devlink/health.c:483 devlink_recover_notify.constprop.0+0xb8/0xc0
CPU: 5 PID: 227 Comm: kworker/u16:3 Not tainted 6.4.0-rc5_for_upstream_min_debug_2023_06_12_12_38 #1
Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS rel-1.13.0-0-gf21b5a4aeb02-prebuilt.qemu.org 04/01/2014
Workqueue: mlx5_health0000:08:00.0 mlx5_fw_reporter_err_work [mlx5_core]
RIP: 0010:devlink_recover_notify.constprop.0+0xb8/0xc0
Call Trace:
 <TASK>
 ? __warn+0x79/0x120
 ? devlink_recover_notify.constprop.0+0xb8/0xc0
 ? report_bug+0x17c/0x190
 ? handle_bug+0x3c/0x60
 ? exc_invalid_op+0x14/0x70
 ? asm_exc_invalid_op+0x16/0x20
 ? devlink_recover_notify.constprop.0+0xb8/0xc0
 devlink_health_report+0x4a/0x1c0
 mlx5_fw_reporter_err_work+0xa4/0xd0 [mlx5_core]
 process_one_work+0x1bb/0x3c0
 ? process_one_work+0x3c0/0x3c0
 worker_thread+0x4d/0x3c0
 ? process_one_work+0x3c0/0x3c0
 kthread+0xc6/0xf0
 ? kthread_complete_and_exit+0x20/0x20
 ret_from_fork+0x1f/0x30
 </TASK>

Fixes: cf530217 ("devlink: Notify users when objects are accessible")
Signed-off-by: default avatarShay Drory <shayd@nvidia.com>
Reviewed-by: default avatarMoshe Shemesh <moshe@nvidia.com>
Signed-off-by: default avatarSaeed Mahameed <saeedm@nvidia.com>
Signed-off-by: default avatarTariq Toukan <tariqt@nvidia.com>
Link: https://lore.kernel.org/r/20240409190820.227554-3-tariqt@nvidia.com


Signed-off-by: default avatarJakub Kicinski <kuba@kernel.org>
parent 0553e753

drivers/net/ethernet/mellanox/mlx5/core/main.c

+20 −17
Original line number Diff line number Diff line
@@ -1480,6 +1480,14 @@ int mlx5_init_one_devl_locked(struct mlx5_core_dev *dev) 
	if (err)

		goto err_register;



	err = mlx5_crdump_enable(dev);

	if (err)

		mlx5_core_err(dev, "mlx5_crdump_enable failed with error code %d\n", err);



	err = mlx5_hwmon_dev_register(dev);

	if (err)

		mlx5_core_err(dev, "mlx5_hwmon_dev_register failed with error code %d\n", err);



	mutex_unlock(&dev->intf_state_mutex);

	return 0;
@@ -1505,7 +1513,10 @@ int mlx5_init_one(struct mlx5_core_dev *dev) 
	int err;



	devl_lock(devlink);

	devl_register(devlink);

	err = mlx5_init_one_devl_locked(dev);

	if (err)

		devl_unregister(devlink);

	devl_unlock(devlink);

	return err;

}
@@ -1517,6 +1528,8 @@ void mlx5_uninit_one(struct mlx5_core_dev *dev) 
	devl_lock(devlink);

	mutex_lock(&dev->intf_state_mutex);



	mlx5_hwmon_dev_unregister(dev);

	mlx5_crdump_disable(dev);

	mlx5_unregister_device(dev);



	if (!test_bit(MLX5_INTERFACE_STATE_UP, &dev->intf_state)) {
@@ -1534,6 +1547,7 @@ void mlx5_uninit_one(struct mlx5_core_dev *dev) 
	mlx5_function_teardown(dev, true);

out:

	mutex_unlock(&dev->intf_state_mutex);

	devl_unregister(devlink);

	devl_unlock(devlink);

}
@@ -1680,16 +1694,20 @@ int mlx5_init_one_light(struct mlx5_core_dev *dev) 
	}



	devl_lock(devlink);

	devl_register(devlink);



	err = mlx5_devlink_params_register(priv_to_devlink(dev));

	devl_unlock(devlink);

	if (err) {

		mlx5_core_warn(dev, "mlx5_devlink_param_reg err = %d\n", err);

		goto query_hca_caps_err;

	}



	devl_unlock(devlink);

	return 0;



query_hca_caps_err:

	devl_unregister(devlink);

	devl_unlock(devlink);

	mlx5_function_disable(dev, true);

out:

	dev->state = MLX5_DEVICE_STATE_INTERNAL_ERROR;
@@ -1702,6 +1720,7 @@ void mlx5_uninit_one_light(struct mlx5_core_dev *dev) 


	devl_lock(devlink);

	mlx5_devlink_params_unregister(priv_to_devlink(dev));

	devl_unregister(devlink);

	devl_unlock(devlink);

	if (dev->state != MLX5_DEVICE_STATE_UP)

		return;
@@ -1943,16 +1962,7 @@ static int probe_one(struct pci_dev *pdev, const struct pci_device_id *id) 
		goto err_init_one;

	}



	err = mlx5_crdump_enable(dev);

	if (err)

		dev_err(&pdev->dev, "mlx5_crdump_enable failed with error code %d\n", err);



	err = mlx5_hwmon_dev_register(dev);

	if (err)

		mlx5_core_err(dev, "mlx5_hwmon_dev_register failed with error code %d\n", err);



	pci_save_state(pdev);

	devlink_register(devlink);

	return 0;



err_init_one:
@@ -1973,16 +1983,9 @@ static void remove_one(struct pci_dev *pdev) 
	struct devlink *devlink = priv_to_devlink(dev);



	set_bit(MLX5_BREAK_FW_WAIT, &dev->intf_state);

	/* mlx5_drain_fw_reset() and mlx5_drain_health_wq() are using

	 * devlink notify APIs.

	 * Hence, we must drain them before unregistering the devlink.

	 */

	mlx5_drain_fw_reset(dev);

	mlx5_drain_health_wq(dev);

	devlink_unregister(devlink);

	mlx5_sriov_disable(pdev, false);

	mlx5_hwmon_dev_unregister(dev);

	mlx5_crdump_disable(dev);

	mlx5_uninit_one(dev);

	mlx5_pci_close(dev);

	mlx5_mdev_uninit(dev);

drivers/net/ethernet/mellanox/mlx5/core/sf/dev/driver.c

+0 −1
Original line number Diff line number Diff line
@@ -101,7 +101,6 @@ static void mlx5_sf_dev_remove(struct auxiliary_device *adev) 
	devlink = priv_to_devlink(mdev);

	set_bit(MLX5_BREAK_FW_WAIT, &mdev->intf_state);

	mlx5_drain_health_wq(mdev);

	devlink_unregister(devlink);

	if (mlx5_dev_is_lightweight(mdev))

		mlx5_uninit_one_light(mdev);

	else