devlink: protect health reporter operation with instance lock

Similar to other devlink objects, protect the reporters list
by devlink instance lock. Alongside add unlocked versions
of health reporter create/destroy functions and use them in drivers
on call paths where the instance lock is held.

Signed-off-by: Jiri Pirko <jiri@nvidia.com>
Reviewed-by: Jacob Keller <jacob.e.keller@intel.com>
Signed-off-by: Jakub Kicinski <kuba@kernel.org>

diff --git a/drivers/net/ethernet/mellanox/mlxsw/core.c b/drivers/net/ethernet/mellanox/mlxsw/core.c
index a0a06e2eff82e56690b97ba4c4c47ad86fc25604..33ef726e4d54fdf00d1054dd5363b74876c7aa54 100644 (file)
--- a/drivers/net/ethernet/mellanox/mlxsw/core.c
+++ b/drivers/net/ethernet/mellanox/mlxsw/core.c
@@ -2051,8 +2051,8 @@ static int mlxsw_core_health_init(struct mlxsw_core *mlxsw_core)
        if (!(mlxsw_core->bus->features & MLXSW_BUS_F_TXRX))
                return 0;
 
-       fw_fatal = devlink_health_reporter_create(devlink, &mlxsw_core_health_fw_fatal_ops,
-                                                 0, mlxsw_core);
+       fw_fatal = devl_health_reporter_create(devlink, &mlxsw_core_health_fw_fatal_ops,
+                                              0, mlxsw_core);
        if (IS_ERR(fw_fatal)) {
                dev_err(mlxsw_core->bus_info->dev, "Failed to create fw fatal reporter");
                return PTR_ERR(fw_fatal);
@@ -2072,7 +2072,7 @@ static int mlxsw_core_health_init(struct mlxsw_core *mlxsw_core)
 err_fw_fatal_config:
        mlxsw_core_trap_unregister(mlxsw_core, &mlxsw_core_health_listener, mlxsw_core);
 err_trap_register:
-       devlink_health_reporter_destroy(mlxsw_core->health.fw_fatal);
+       devl_health_reporter_destroy(mlxsw_core->health.fw_fatal);
        return err;
 }
 
@@ -2085,7 +2085,7 @@ static void mlxsw_core_health_fini(struct mlxsw_core *mlxsw_core)
        mlxsw_core_trap_unregister(mlxsw_core, &mlxsw_core_health_listener, mlxsw_core);
        /* Make sure there is no more event work scheduled */
        mlxsw_core_flush_owq();
-       devlink_health_reporter_destroy(mlxsw_core->health.fw_fatal);
+       devl_health_reporter_destroy(mlxsw_core->health.fw_fatal);
 }
 
 static void mlxsw_core_irq_event_handler_init(struct mlxsw_core *mlxsw_core)

diff --git a/drivers/net/netdevsim/health.c b/drivers/net/netdevsim/health.c
index aa77af4a68df6fc60a3ce9c081fc98c60df65158..eb04ed715d2d587651e04fdd4da248555bb9c170 100644 (file)
--- a/drivers/net/netdevsim/health.c
+++ b/drivers/net/netdevsim/health.c
@@ -233,16 +233,16 @@ int nsim_dev_health_init(struct nsim_dev *nsim_dev, struct devlink *devlink)
        int err;
 
        health->empty_reporter =
-               devlink_health_reporter_create(devlink,
-                                              &nsim_dev_empty_reporter_ops,
-                                              0, health);
+               devl_health_reporter_create(devlink,
+                                           &nsim_dev_empty_reporter_ops,
+                                           0, health);
        if (IS_ERR(health->empty_reporter))
                return PTR_ERR(health->empty_reporter);
 
        health->dummy_reporter =
-               devlink_health_reporter_create(devlink,
-                                              &nsim_dev_dummy_reporter_ops,
-                                              0, health);
+               devl_health_reporter_create(devlink,
+                                           &nsim_dev_dummy_reporter_ops,
+                                           0, health);
        if (IS_ERR(health->dummy_reporter)) {
                err = PTR_ERR(health->dummy_reporter);
                goto err_empty_reporter_destroy;
@@ -266,9 +266,9 @@ int nsim_dev_health_init(struct nsim_dev *nsim_dev, struct devlink *devlink)
        return 0;
 
 err_dummy_reporter_destroy:
-       devlink_health_reporter_destroy(health->dummy_reporter);
+       devl_health_reporter_destroy(health->dummy_reporter);
 err_empty_reporter_destroy:
-       devlink_health_reporter_destroy(health->empty_reporter);
+       devl_health_reporter_destroy(health->empty_reporter);
        return err;
 }
 
@@ -278,6 +278,6 @@ void nsim_dev_health_exit(struct nsim_dev *nsim_dev)
 
        debugfs_remove_recursive(health->ddir);
        kfree(health->recovered_break_msg);
-       devlink_health_reporter_destroy(health->dummy_reporter);
-       devlink_health_reporter_destroy(health->empty_reporter);
+       devl_health_reporter_destroy(health->dummy_reporter);
+       devl_health_reporter_destroy(health->empty_reporter);
 }

diff --git a/include/net/devlink.h b/include/net/devlink.h
index d7c9572e5bea86459158326899a7f93ef1c67bf3..0d64feaef7cbf763ae13399fcdc051bcce3a1c46 100644 (file)
--- a/include/net/devlink.h
+++ b/include/net/devlink.h
@@ -1865,18 +1865,34 @@ int devlink_fmsg_binary_pair_put(struct devlink_fmsg *fmsg, const char *name,
                                 const void *value, u32 value_len);
 
 struct devlink_health_reporter *
-devlink_health_reporter_create(struct devlink *devlink,
-                              const struct devlink_health_reporter_ops *ops,
-                              u64 graceful_period, void *priv);
+devl_port_health_reporter_create(struct devlink_port *port,
+                                const struct devlink_health_reporter_ops *ops,
+                                u64 graceful_period, void *priv);
 
 struct devlink_health_reporter *
 devlink_port_health_reporter_create(struct devlink_port *port,
                                    const struct devlink_health_reporter_ops *ops,
                                    u64 graceful_period, void *priv);
 
+struct devlink_health_reporter *
+devl_health_reporter_create(struct devlink *devlink,
+                           const struct devlink_health_reporter_ops *ops,
+                           u64 graceful_period, void *priv);
+
+struct devlink_health_reporter *
+devlink_health_reporter_create(struct devlink *devlink,
+                              const struct devlink_health_reporter_ops *ops,
+                              u64 graceful_period, void *priv);
+
+void
+devl_health_reporter_destroy(struct devlink_health_reporter *reporter);
+
 void
 devlink_health_reporter_destroy(struct devlink_health_reporter *reporter);
 
+void
+devl_port_health_reporter_destroy(struct devlink_health_reporter *reporter);
+
 void
 devlink_port_health_reporter_destroy(struct devlink_health_reporter *reporter);
 

diff --git a/net/devlink/leftover.c b/net/devlink/leftover.c
index f029e1bc68f60fe333ec29eeb9301a7d2c800c88..0263dc18e72d574e7e12eab6f2709a0e298a18f8 100644 (file)
--- a/net/devlink/leftover.c
+++ b/net/devlink/leftover.c
@@ -7337,8 +7337,8 @@ __devlink_health_reporter_create(struct devlink *devlink,
 }
 
 /**
- *     devlink_port_health_reporter_create - create devlink health reporter for
- *                                           specified port instance
+ *     devl_port_health_reporter_create - create devlink health reporter for
+ *                                        specified port instance
  *
  *     @port: devlink_port which should contain the new reporter
  *     @ops: ops
@@ -7346,12 +7346,13 @@ __devlink_health_reporter_create(struct devlink *devlink,
  *     @priv: priv
  */
 struct devlink_health_reporter *
-devlink_port_health_reporter_create(struct devlink_port *port,
-                                   const struct devlink_health_reporter_ops *ops,
-                                   u64 graceful_period, void *priv)
+devl_port_health_reporter_create(struct devlink_port *port,
+                                const struct devlink_health_reporter_ops *ops,
+                                u64 graceful_period, void *priv)
 {
        struct devlink_health_reporter *reporter;
 
+       devl_assert_locked(port->devlink);
        mutex_lock(&port->reporters_lock);
        if (__devlink_health_reporter_find_by_name(&port->reporter_list,
                                                   &port->reporters_lock, ops->name)) {
@@ -7370,10 +7371,26 @@ unlock:
        mutex_unlock(&port->reporters_lock);
        return reporter;
 }
+EXPORT_SYMBOL_GPL(devl_port_health_reporter_create);
+
+struct devlink_health_reporter *
+devlink_port_health_reporter_create(struct devlink_port *port,
+                                   const struct devlink_health_reporter_ops *ops,
+                                   u64 graceful_period, void *priv)
+{
+       struct devlink_health_reporter *reporter;
+       struct devlink *devlink = port->devlink;
+
+       devl_lock(devlink);
+       reporter = devl_port_health_reporter_create(port, ops,
+                                                   graceful_period, priv);
+       devl_unlock(devlink);
+       return reporter;
+}
 EXPORT_SYMBOL_GPL(devlink_port_health_reporter_create);
 
 /**
- *     devlink_health_reporter_create - create devlink health reporter
+ *     devl_health_reporter_create - create devlink health reporter
  *
  *     @devlink: devlink
  *     @ops: ops
@@ -7381,12 +7398,13 @@ EXPORT_SYMBOL_GPL(devlink_port_health_reporter_create);
  *     @priv: priv
  */
 struct devlink_health_reporter *
-devlink_health_reporter_create(struct devlink *devlink,
-                              const struct devlink_health_reporter_ops *ops,
-                              u64 graceful_period, void *priv)
+devl_health_reporter_create(struct devlink *devlink,
+                           const struct devlink_health_reporter_ops *ops,
+                           u64 graceful_period, void *priv)
 {
        struct devlink_health_reporter *reporter;
 
+       devl_assert_locked(devlink);
        mutex_lock(&devlink->reporters_lock);
        if (devlink_health_reporter_find_by_name(devlink, ops->name)) {
                reporter = ERR_PTR(-EEXIST);
@@ -7403,6 +7421,21 @@ unlock:
        mutex_unlock(&devlink->reporters_lock);
        return reporter;
 }
+EXPORT_SYMBOL_GPL(devl_health_reporter_create);
+
+struct devlink_health_reporter *
+devlink_health_reporter_create(struct devlink *devlink,
+                              const struct devlink_health_reporter_ops *ops,
+                              u64 graceful_period, void *priv)
+{
+       struct devlink_health_reporter *reporter;
+
+       devl_lock(devlink);
+       reporter = devl_health_reporter_create(devlink, ops,
+                                              graceful_period, priv);
+       devl_unlock(devlink);
+       return reporter;
+}
 EXPORT_SYMBOL_GPL(devlink_health_reporter_create);
 
 static void
@@ -7429,35 +7462,61 @@ __devlink_health_reporter_destroy(struct devlink_health_reporter *reporter)
 }
 
 /**
- *     devlink_health_reporter_destroy - destroy devlink health reporter
+ *     devl_health_reporter_destroy - destroy devlink health reporter
  *
  *     @reporter: devlink health reporter to destroy
  */
 void
-devlink_health_reporter_destroy(struct devlink_health_reporter *reporter)
+devl_health_reporter_destroy(struct devlink_health_reporter *reporter)
 {
        struct mutex *lock = &reporter->devlink->reporters_lock;
 
+       devl_assert_locked(reporter->devlink);
+
        mutex_lock(lock);
        __devlink_health_reporter_destroy(reporter);
        mutex_unlock(lock);
 }
+EXPORT_SYMBOL_GPL(devl_health_reporter_destroy);
+
+void
+devlink_health_reporter_destroy(struct devlink_health_reporter *reporter)
+{
+       struct devlink *devlink = reporter->devlink;
+
+       devl_lock(devlink);
+       devl_health_reporter_destroy(reporter);
+       devl_unlock(devlink);
+}
 EXPORT_SYMBOL_GPL(devlink_health_reporter_destroy);
 
 /**
- *     devlink_port_health_reporter_destroy - destroy devlink port health reporter
+ *     devl_port_health_reporter_destroy - destroy devlink port health reporter
  *
  *     @reporter: devlink health reporter to destroy
  */
 void
-devlink_port_health_reporter_destroy(struct devlink_health_reporter *reporter)
+devl_port_health_reporter_destroy(struct devlink_health_reporter *reporter)
 {
        struct mutex *lock = &reporter->devlink_port->reporters_lock;
 
+       devl_assert_locked(reporter->devlink);
+
        mutex_lock(lock);
        __devlink_health_reporter_destroy(reporter);
        mutex_unlock(lock);
 }
+EXPORT_SYMBOL_GPL(devl_port_health_reporter_destroy);
+
+void
+devlink_port_health_reporter_destroy(struct devlink_health_reporter *reporter)
+{
+       struct devlink *devlink = reporter->devlink;
+
+       devl_lock(devlink);
+       devl_port_health_reporter_destroy(reporter);
+       devl_unlock(devlink);
+}
 EXPORT_SYMBOL_GPL(devlink_port_health_reporter_destroy);
 
 static int
@@ -7805,12 +7864,11 @@ devlink_nl_cmd_health_reporter_get_dumpit(struct sk_buff *msg,
                unsigned long port_index;
                int idx = 0;
 
+               devl_lock(devlink);
+               if (!devl_is_registered(devlink))
+                       goto next_devlink;
+
                mutex_lock(&devlink->reporters_lock);
-               if (!devl_is_registered(devlink)) {
-                       mutex_unlock(&devlink->reporters_lock);
-                       devlink_put(devlink);
-                       continue;
-               }
 
                list_for_each_entry(reporter, &devlink->reporter_list,
                                    list) {
@@ -7824,6 +7882,7 @@ devlink_nl_cmd_health_reporter_get_dumpit(struct sk_buff *msg,
                                NLM_F_MULTI);
                        if (err) {
                                mutex_unlock(&devlink->reporters_lock);
+                               devl_unlock(devlink);
                                devlink_put(devlink);
                                state->idx = idx;
                                goto out;
@@ -7832,10 +7891,6 @@ devlink_nl_cmd_health_reporter_get_dumpit(struct sk_buff *msg,
                }
                mutex_unlock(&devlink->reporters_lock);
 
-               devl_lock(devlink);
-               if (!devl_is_registered(devlink))
-                       goto next_devlink;
-
                xa_for_each(&devlink->ports, port_index, port) {
                        mutex_lock(&port->reporters_lock);
                        list_for_each_entry(reporter, &port->reporter_list, list) {