Merge branch 'devlink-parallel-commands'

Jiri Pirko says:

====================
net: devlink: allow parallel commands on multiple devlinks

Aim of this patchset is to remove devlink_mutex and eventually to enable
parallel ops on devlink netlink interface.
====================

Tested-by: Ido Schimmel <idosch@nvidia.com>
Reviewed-by: Jakub Kicinski <kuba@kernel.org>
Signed-off-by: David S. Miller <davem@davemloft.net>
This commit is contained in:
David S. Miller 2022-08-01 12:14:01 +01:00
commit d4d11f8ff1
5 changed files with 21 additions and 107 deletions

View File

@ -3958,11 +3958,9 @@ static int mlx4_devlink_reload_down(struct devlink *devlink, bool netns_change,
NL_SET_ERR_MSG_MOD(extack, "Namespace change is not supported");
return -EOPNOTSUPP;
}
devl_lock(devlink);
if (persist->num_vfs)
mlx4_warn(persist->dev, "Reload performed on PF, will cause reset on operating Virtual Functions\n");
mlx4_restart_one_down(persist->pdev);
devl_unlock(devlink);
return 0;
}
@ -3975,10 +3973,8 @@ static int mlx4_devlink_reload_up(struct devlink *devlink, enum devlink_reload_a
struct mlx4_dev_persistent *persist = dev->persist;
int err;
devl_lock(devlink);
*actions_performed = BIT(DEVLINK_RELOAD_ACTION_DRIVER_REINIT);
err = mlx4_restart_one_up(persist->pdev, true, devlink);
devl_unlock(devlink);
if (err)
mlx4_err(persist->dev, "mlx4_restart_one_up failed, ret=%d\n",
err);

View File

@ -164,7 +164,6 @@ static int mlx5_devlink_reload_down(struct devlink *devlink, bool netns_change,
NL_SET_ERR_MSG_MOD(extack, "reload while VFs are present is unfavorable");
}
devl_lock(devlink);
switch (action) {
case DEVLINK_RELOAD_ACTION_DRIVER_REINIT:
mlx5_unload_one_devl_locked(dev);
@ -181,7 +180,6 @@ static int mlx5_devlink_reload_down(struct devlink *devlink, bool netns_change,
ret = -EOPNOTSUPP;
}
devl_unlock(devlink);
return ret;
}
@ -192,7 +190,6 @@ static int mlx5_devlink_reload_up(struct devlink *devlink, enum devlink_reload_a
struct mlx5_core_dev *dev = devlink_priv(devlink);
int ret = 0;
devl_lock(devlink);
*actions_performed = BIT(action);
switch (action) {
case DEVLINK_RELOAD_ACTION_DRIVER_REINIT:
@ -211,7 +208,6 @@ static int mlx5_devlink_reload_up(struct devlink *devlink, enum devlink_reload_a
ret = -EOPNOTSUPP;
}
devl_unlock(devlink);
return ret;
}

View File

@ -1499,9 +1499,7 @@ mlxsw_devlink_core_bus_device_reload_down(struct devlink *devlink,
if (!(mlxsw_core->bus->features & MLXSW_BUS_F_RESET))
return -EOPNOTSUPP;
devl_lock(devlink);
mlxsw_core_bus_device_unregister(mlxsw_core, true);
devl_unlock(devlink);
return 0;
}
@ -1515,12 +1513,10 @@ mlxsw_devlink_core_bus_device_reload_up(struct devlink *devlink, enum devlink_re
*actions_performed = BIT(DEVLINK_RELOAD_ACTION_DRIVER_REINIT) |
BIT(DEVLINK_RELOAD_ACTION_FW_ACTIVATE);
devl_lock(devlink);
err = mlxsw_core_bus_device_register(mlxsw_core->bus_info,
mlxsw_core->bus,
mlxsw_core->bus_priv, true,
devlink, extack);
devl_unlock(devlink);
return err;
}

View File

@ -948,18 +948,15 @@ static int nsim_dev_reload_down(struct devlink *devlink, bool netns_change,
{
struct nsim_dev *nsim_dev = devlink_priv(devlink);
devl_lock(devlink);
if (nsim_dev->dont_allow_reload) {
/* For testing purposes, user set debugfs dont_allow_reload
* value to true. So forbid it.
*/
NL_SET_ERR_MSG_MOD(extack, "User forbid the reload for testing purposes");
devl_unlock(devlink);
return -EOPNOTSUPP;
}
nsim_dev_reload_destroy(nsim_dev);
devl_unlock(devlink);
return 0;
}
@ -970,19 +967,16 @@ static int nsim_dev_reload_up(struct devlink *devlink, enum devlink_reload_actio
struct nsim_dev *nsim_dev = devlink_priv(devlink);
int ret;
devl_lock(devlink);
if (nsim_dev->fail_reload) {
/* For testing purposes, user set debugfs fail_reload
* value to true. Fail right away.
*/
NL_SET_ERR_MSG_MOD(extack, "User setup the reload to fail for testing purposes");
devl_unlock(devlink);
return -EINVAL;
}
*actions_performed = BIT(DEVLINK_RELOAD_ACTION_DRIVER_REINIT);
ret = nsim_dev_reload_create(nsim_dev, extack);
devl_unlock(devlink);
return ret;
}

View File

@ -207,6 +207,7 @@ static const struct nla_policy devlink_selftest_nl_policy[DEVLINK_ATTR_SELFTEST_
static DEFINE_XARRAY_FLAGS(devlinks, XA_FLAGS_ALLOC);
#define DEVLINK_REGISTERED XA_MARK_1
#define DEVLINK_UNREGISTERING XA_MARK_2
/* devlink instances are open to the access from the user space after
* devlink_register() call. Such logical barrier allows us to have certain
@ -224,12 +225,6 @@ static DEFINE_XARRAY_FLAGS(devlinks, XA_FLAGS_ALLOC);
#define ASSERT_DEVLINK_NOT_REGISTERED(d) \
WARN_ON_ONCE(xa_get_mark(&devlinks, (d)->index, DEVLINK_REGISTERED))
/* devlink_mutex
*
* An overall lock guarding every operation coming from userspace.
*/
static DEFINE_MUTEX(devlink_mutex);
struct net *devlink_net(const struct devlink *devlink)
{
return read_pnet(&devlink->_net);
@ -305,6 +300,14 @@ retry:
devlink = xa_find_fn(&devlinks, indexp, ULONG_MAX, DEVLINK_REGISTERED);
if (!devlink)
goto unlock;
/* In case devlink_unregister() was already called and "unregistering"
* mark was set, do not allow to get a devlink reference here.
* This prevents live-lock of devlink_unregister() wait for completion.
*/
if (xa_get_mark(&devlinks, *indexp, DEVLINK_UNREGISTERING))
goto retry;
/* For a possible retry, the xa_find_after() should be always used */
xa_find_fn = xa_find_after;
if (!devlink_try_get(devlink))
@ -759,12 +762,6 @@ devlink_region_snapshot_get_by_id(struct devlink_region *region, u32 id)
#define DEVLINK_NL_FLAG_NEED_RATE_NODE BIT(3)
#define DEVLINK_NL_FLAG_NEED_LINECARD BIT(4)
/* The per devlink instance lock is taken by default in the pre-doit
* operation, yet several commands do not require this. The global
* devlink lock is taken and protects from disruption by user-calls.
*/
#define DEVLINK_NL_FLAG_NO_LOCK BIT(5)
static int devlink_nl_pre_doit(const struct genl_ops *ops,
struct sk_buff *skb, struct genl_info *info)
{
@ -773,14 +770,10 @@ static int devlink_nl_pre_doit(const struct genl_ops *ops,
struct devlink *devlink;
int err;
mutex_lock(&devlink_mutex);
devlink = devlink_get_from_attrs(genl_info_net(info), info->attrs);
if (IS_ERR(devlink)) {
mutex_unlock(&devlink_mutex);
if (IS_ERR(devlink))
return PTR_ERR(devlink);
}
if (~ops->internal_flags & DEVLINK_NL_FLAG_NO_LOCK)
devl_lock(devlink);
devl_lock(devlink);
info->user_ptr[0] = devlink;
if (ops->internal_flags & DEVLINK_NL_FLAG_NEED_PORT) {
devlink_port = devlink_port_get_from_info(devlink, info);
@ -822,10 +815,8 @@ static int devlink_nl_pre_doit(const struct genl_ops *ops,
return 0;
unlock:
if (~ops->internal_flags & DEVLINK_NL_FLAG_NO_LOCK)
devl_unlock(devlink);
devl_unlock(devlink);
devlink_put(devlink);
mutex_unlock(&devlink_mutex);
return err;
}
@ -840,10 +831,8 @@ static void devlink_nl_post_doit(const struct genl_ops *ops,
linecard = info->user_ptr[1];
devlink_linecard_put(linecard);
}
if (~ops->internal_flags & DEVLINK_NL_FLAG_NO_LOCK)
devl_unlock(devlink);
devl_unlock(devlink);
devlink_put(devlink);
mutex_unlock(&devlink_mutex);
}
static struct genl_family devlink_nl_family;
@ -1408,7 +1397,6 @@ static int devlink_nl_cmd_rate_get_dumpit(struct sk_buff *msg,
int idx = 0;
int err = 0;
mutex_lock(&devlink_mutex);
devlinks_xa_for_each_registered_get(sock_net(msg->sk), index, devlink) {
devl_lock(devlink);
list_for_each_entry(devlink_rate, &devlink->rate_list, list) {
@ -1433,7 +1421,6 @@ static int devlink_nl_cmd_rate_get_dumpit(struct sk_buff *msg,
devlink_put(devlink);
}
out:
mutex_unlock(&devlink_mutex);
if (err != -EMSGSIZE)
return err;
@ -1504,7 +1491,6 @@ static int devlink_nl_cmd_get_dumpit(struct sk_buff *msg,
int idx = 0;
int err;
mutex_lock(&devlink_mutex);
devlinks_xa_for_each_registered_get(sock_net(msg->sk), index, devlink) {
if (idx < start) {
idx++;
@ -1521,8 +1507,6 @@ static int devlink_nl_cmd_get_dumpit(struct sk_buff *msg,
idx++;
}
out:
mutex_unlock(&devlink_mutex);
cb->args[0] = idx;
return msg->len;
}
@ -1559,7 +1543,6 @@ static int devlink_nl_cmd_port_get_dumpit(struct sk_buff *msg,
int idx = 0;
int err;
mutex_lock(&devlink_mutex);
devlinks_xa_for_each_registered_get(sock_net(msg->sk), index, devlink) {
devl_lock(devlink);
list_for_each_entry(devlink_port, &devlink->port_list, list) {
@ -1583,8 +1566,6 @@ static int devlink_nl_cmd_port_get_dumpit(struct sk_buff *msg,
devlink_put(devlink);
}
out:
mutex_unlock(&devlink_mutex);
cb->args[0] = idx;
return msg->len;
}
@ -2238,7 +2219,6 @@ static int devlink_nl_cmd_linecard_get_dumpit(struct sk_buff *msg,
int idx = 0;
int err;
mutex_lock(&devlink_mutex);
devlinks_xa_for_each_registered_get(sock_net(msg->sk), index, devlink) {
mutex_lock(&devlink->linecards_lock);
list_for_each_entry(linecard, &devlink->linecard_list, list) {
@ -2265,8 +2245,6 @@ static int devlink_nl_cmd_linecard_get_dumpit(struct sk_buff *msg,
devlink_put(devlink);
}
out:
mutex_unlock(&devlink_mutex);
cb->args[0] = idx;
return msg->len;
}
@ -2503,7 +2481,6 @@ static int devlink_nl_cmd_sb_get_dumpit(struct sk_buff *msg,
int idx = 0;
int err;
mutex_lock(&devlink_mutex);
devlinks_xa_for_each_registered_get(sock_net(msg->sk), index, devlink) {
devl_lock(devlink);
list_for_each_entry(devlink_sb, &devlink->sb_list, list) {
@ -2527,8 +2504,6 @@ static int devlink_nl_cmd_sb_get_dumpit(struct sk_buff *msg,
devlink_put(devlink);
}
out:
mutex_unlock(&devlink_mutex);
cb->args[0] = idx;
return msg->len;
}
@ -2648,7 +2623,6 @@ static int devlink_nl_cmd_sb_pool_get_dumpit(struct sk_buff *msg,
int idx = 0;
int err = 0;
mutex_lock(&devlink_mutex);
devlinks_xa_for_each_registered_get(sock_net(msg->sk), index, devlink) {
if (!devlink->ops->sb_pool_get)
goto retry;
@ -2672,8 +2646,6 @@ retry:
devlink_put(devlink);
}
out:
mutex_unlock(&devlink_mutex);
if (err != -EMSGSIZE)
return err;
@ -2865,7 +2837,6 @@ static int devlink_nl_cmd_sb_port_pool_get_dumpit(struct sk_buff *msg,
int idx = 0;
int err = 0;
mutex_lock(&devlink_mutex);
devlinks_xa_for_each_registered_get(sock_net(msg->sk), index, devlink) {
if (!devlink->ops->sb_port_pool_get)
goto retry;
@ -2889,8 +2860,6 @@ retry:
devlink_put(devlink);
}
out:
mutex_unlock(&devlink_mutex);
if (err != -EMSGSIZE)
return err;
@ -3110,7 +3079,6 @@ devlink_nl_cmd_sb_tc_pool_bind_get_dumpit(struct sk_buff *msg,
int idx = 0;
int err = 0;
mutex_lock(&devlink_mutex);
devlinks_xa_for_each_registered_get(sock_net(msg->sk), index, devlink) {
if (!devlink->ops->sb_tc_pool_bind_get)
goto retry;
@ -3135,8 +3103,6 @@ retry:
devlink_put(devlink);
}
out:
mutex_unlock(&devlink_mutex);
if (err != -EMSGSIZE)
return err;
@ -4908,7 +4874,6 @@ static int devlink_nl_cmd_selftests_get_dumpit(struct sk_buff *msg,
int idx = 0;
int err = 0;
mutex_lock(&devlink_mutex);
devlinks_xa_for_each_registered_get(sock_net(msg->sk), index, devlink) {
if (idx < start || !devlink->ops->selftest_check)
goto inc;
@ -4927,7 +4892,6 @@ inc:
idx++;
devlink_put(devlink);
}
mutex_unlock(&devlink_mutex);
if (err != -EMSGSIZE)
return err;
@ -5393,7 +5357,6 @@ static int devlink_nl_cmd_param_get_dumpit(struct sk_buff *msg,
int idx = 0;
int err = 0;
mutex_lock(&devlink_mutex);
devlinks_xa_for_each_registered_get(sock_net(msg->sk), index, devlink) {
devl_lock(devlink);
list_for_each_entry(param_item, &devlink->param_list, list) {
@ -5419,8 +5382,6 @@ static int devlink_nl_cmd_param_get_dumpit(struct sk_buff *msg,
devlink_put(devlink);
}
out:
mutex_unlock(&devlink_mutex);
if (err != -EMSGSIZE)
return err;
@ -5621,7 +5582,6 @@ static int devlink_nl_cmd_port_param_get_dumpit(struct sk_buff *msg,
int idx = 0;
int err = 0;
mutex_lock(&devlink_mutex);
devlinks_xa_for_each_registered_get(sock_net(msg->sk), index, devlink) {
devl_lock(devlink);
list_for_each_entry(devlink_port, &devlink->port_list, list) {
@ -5652,8 +5612,6 @@ static int devlink_nl_cmd_port_param_get_dumpit(struct sk_buff *msg,
devlink_put(devlink);
}
out:
mutex_unlock(&devlink_mutex);
if (err != -EMSGSIZE)
return err;
@ -6208,7 +6166,6 @@ static int devlink_nl_cmd_region_get_dumpit(struct sk_buff *msg,
int idx = 0;
int err = 0;
mutex_lock(&devlink_mutex);
devlinks_xa_for_each_registered_get(sock_net(msg->sk), index, devlink) {
err = devlink_nl_cmd_region_get_devlink_dumpit(msg, cb, devlink,
&idx, start);
@ -6217,7 +6174,6 @@ static int devlink_nl_cmd_region_get_dumpit(struct sk_buff *msg,
goto out;
}
out:
mutex_unlock(&devlink_mutex);
cb->args[0] = idx;
return msg->len;
}
@ -6483,12 +6439,9 @@ static int devlink_nl_cmd_region_read_dumpit(struct sk_buff *skb,
start_offset = *((u64 *)&cb->args[0]);
mutex_lock(&devlink_mutex);
devlink = devlink_get_from_attrs(sock_net(cb->skb->sk), attrs);
if (IS_ERR(devlink)) {
err = PTR_ERR(devlink);
goto out_dev;
}
if (IS_ERR(devlink))
return PTR_ERR(devlink);
devl_lock(devlink);
@ -6588,8 +6541,6 @@ static int devlink_nl_cmd_region_read_dumpit(struct sk_buff *skb,
genlmsg_end(skb, hdr);
devl_unlock(devlink);
devlink_put(devlink);
mutex_unlock(&devlink_mutex);
return skb->len;
nla_put_failure:
@ -6597,8 +6548,6 @@ nla_put_failure:
out_unlock:
devl_unlock(devlink);
devlink_put(devlink);
out_dev:
mutex_unlock(&devlink_mutex);
return err;
}
@ -6747,7 +6696,6 @@ static int devlink_nl_cmd_info_get_dumpit(struct sk_buff *msg,
int idx = 0;
int err = 0;
mutex_lock(&devlink_mutex);
devlinks_xa_for_each_registered_get(sock_net(msg->sk), index, devlink) {
if (idx < start || !devlink->ops->info_get)
goto inc;
@ -6768,7 +6716,6 @@ inc:
idx++;
devlink_put(devlink);
}
mutex_unlock(&devlink_mutex);
if (err != -EMSGSIZE)
return err;
@ -7847,18 +7794,13 @@ devlink_health_reporter_get_from_cb(struct netlink_callback *cb)
struct nlattr **attrs = info->attrs;
struct devlink *devlink;
mutex_lock(&devlink_mutex);
devlink = devlink_get_from_attrs(sock_net(cb->skb->sk), attrs);
if (IS_ERR(devlink))
goto unlock;
return NULL;
reporter = devlink_health_reporter_get_from_attrs(devlink, attrs);
devlink_put(devlink);
mutex_unlock(&devlink_mutex);
return reporter;
unlock:
mutex_unlock(&devlink_mutex);
return NULL;
}
void
@ -7924,7 +7866,6 @@ devlink_nl_cmd_health_reporter_get_dumpit(struct sk_buff *msg,
int idx = 0;
int err;
mutex_lock(&devlink_mutex);
devlinks_xa_for_each_registered_get(sock_net(msg->sk), index, devlink) {
mutex_lock(&devlink->reporters_lock);
list_for_each_entry(reporter, &devlink->reporter_list,
@ -7976,8 +7917,6 @@ devlink_nl_cmd_health_reporter_get_dumpit(struct sk_buff *msg,
devlink_put(devlink);
}
out:
mutex_unlock(&devlink_mutex);
cb->args[0] = idx;
return msg->len;
}
@ -8510,7 +8449,6 @@ static int devlink_nl_cmd_trap_get_dumpit(struct sk_buff *msg,
int idx = 0;
int err;
mutex_lock(&devlink_mutex);
devlinks_xa_for_each_registered_get(sock_net(msg->sk), index, devlink) {
devl_lock(devlink);
list_for_each_entry(trap_item, &devlink->trap_list, list) {
@ -8534,8 +8472,6 @@ static int devlink_nl_cmd_trap_get_dumpit(struct sk_buff *msg,
devlink_put(devlink);
}
out:
mutex_unlock(&devlink_mutex);
cb->args[0] = idx;
return msg->len;
}
@ -8730,7 +8666,6 @@ static int devlink_nl_cmd_trap_group_get_dumpit(struct sk_buff *msg,
int idx = 0;
int err;
mutex_lock(&devlink_mutex);
devlinks_xa_for_each_registered_get(sock_net(msg->sk), index, devlink) {
devl_lock(devlink);
list_for_each_entry(group_item, &devlink->trap_group_list,
@ -8755,8 +8690,6 @@ static int devlink_nl_cmd_trap_group_get_dumpit(struct sk_buff *msg,
devlink_put(devlink);
}
out:
mutex_unlock(&devlink_mutex);
cb->args[0] = idx;
return msg->len;
}
@ -9037,7 +8970,6 @@ static int devlink_nl_cmd_trap_policer_get_dumpit(struct sk_buff *msg,
int idx = 0;
int err;
mutex_lock(&devlink_mutex);
devlinks_xa_for_each_registered_get(sock_net(msg->sk), index, devlink) {
devl_lock(devlink);
list_for_each_entry(policer_item, &devlink->trap_policer_list,
@ -9062,8 +8994,6 @@ static int devlink_nl_cmd_trap_policer_get_dumpit(struct sk_buff *msg,
devlink_put(devlink);
}
out:
mutex_unlock(&devlink_mutex);
cb->args[0] = idx;
return msg->len;
}
@ -9405,7 +9335,6 @@ static const struct genl_small_ops devlink_nl_ops[] = {
.validate = GENL_DONT_VALIDATE_STRICT | GENL_DONT_VALIDATE_DUMP,
.doit = devlink_nl_cmd_reload,
.flags = GENL_ADMIN_PERM,
.internal_flags = DEVLINK_NL_FLAG_NO_LOCK,
},
{
.cmd = DEVLINK_CMD_PARAM_GET,
@ -9576,6 +9505,7 @@ static struct genl_family devlink_nl_family __ro_after_init = {
.maxattr = DEVLINK_ATTR_MAX,
.policy = devlink_nl_policy,
.netnsok = true,
.parallel_ops = true,
.pre_doit = devlink_nl_pre_doit,
.post_doit = devlink_nl_post_doit,
.module = THIS_MODULE,
@ -9809,11 +9739,13 @@ void devlink_unregister(struct devlink *devlink)
ASSERT_DEVLINK_REGISTERED(devlink);
/* Make sure that we are in .remove() routine */
xa_set_mark(&devlinks, devlink->index, DEVLINK_UNREGISTERING);
devlink_put(devlink);
wait_for_completion(&devlink->comp);
devlink_notify_unregister(devlink);
xa_clear_mark(&devlinks, devlink->index, DEVLINK_REGISTERED);
xa_clear_mark(&devlinks, devlink->index, DEVLINK_UNREGISTERING);
}
EXPORT_SYMBOL_GPL(devlink_unregister);
@ -12493,18 +12425,18 @@ static void __net_exit devlink_pernet_pre_exit(struct net *net)
/* In case network namespace is getting destroyed, reload
* all devlink instances from this namespace into init_net.
*/
mutex_lock(&devlink_mutex);
devlinks_xa_for_each_registered_get(net, index, devlink) {
WARN_ON(!(devlink->features & DEVLINK_F_RELOAD));
mutex_lock(&devlink->lock);
err = devlink_reload(devlink, &init_net,
DEVLINK_RELOAD_ACTION_DRIVER_REINIT,
DEVLINK_RELOAD_LIMIT_UNSPEC,
&actions_performed, NULL);
mutex_unlock(&devlink->lock);
if (err && err != -EOPNOTSUPP)
pr_warn("Failed to reload devlink instance into init_net\n");
devlink_put(devlink);
}
mutex_unlock(&devlink_mutex);
}
static struct pernet_operations devlink_pernet_ops __net_initdata = {