| Message ID | 03cf077ca8d3a1a0a755866df1a99031e5badb14.1700047319.git.petrm@nvidia.com (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | bf6b200bc80d18480f8d0fb61e185bb0587e633c |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | mlxsw: Add support for new reset flow | expand |
On Wed, Nov 15, 2023 at 01:17:14PM +0100, Petr Machata wrote: > From: Ido Schimmel <idosch@nvidia.com> > > Device drivers register with devlink from their probe routines (under > the device lock) by acquiring the devlink instance lock and calling > devl_register(). > > Drivers that support a devlink reload usually implement the > reload_{down, up}() operations in a similar fashion to their remove and > probe routines, respectively. > > However, while the remove and probe routines are invoked with the device > lock held, the reload operations are only invoked with the devlink > instance lock held. It is therefore impossible for drivers to acquire > the device lock from their reload operations, as this would result in > lock inversion. > > The motivating use case for invoking the reload operations with the > device lock held is in mlxsw which needs to trigger a PCI reset as part > of the reload. The driver cannot call pci_reset_function() as this > function acquires the device lock. Instead, it needs to call > __pci_reset_function_locked which expects the device lock to be held. > > To that end, adjust devlink to always acquire the device lock before the > devlink instance lock when performing a reload. > > Do that when reload is explicitly triggered by user space by specifying > the 'DEVLINK_NL_FLAG_NEED_DEV_LOCK' flag in the pre_doit and post_doit > operations of the reload command. > > A previous patch already handled the case where reload is invoked as > part of netns dismantle. > > Signed-off-by: Ido Schimmel <idosch@nvidia.com> > Reviewed-by: Jiri Pirko <jiri@nvidia.com> > Signed-off-by: Petr Machata <petrm@nvidia.com> Reviewed-by: Simon Horman <horms@kernel.org>
diff --git a/Documentation/netlink/specs/devlink.yaml b/Documentation/netlink/specs/devlink.yaml index 572d83a414d0..43067e1f63aa 100644 --- a/Documentation/netlink/specs/devlink.yaml +++ b/Documentation/netlink/specs/devlink.yaml @@ -1484,8 +1484,8 @@ operations: dont-validate: [ strict ] flags: [ admin-perm ] do: - pre: devlink-nl-pre-doit - post: devlink-nl-post-doit + pre: devlink-nl-pre-doit-dev-lock + post: devlink-nl-post-doit-dev-lock request: attributes: - bus-name diff --git a/net/devlink/netlink.c b/net/devlink/netlink.c index 86f12531bf99..fa9afe3e6d9b 100644 --- a/net/devlink/netlink.c +++ b/net/devlink/netlink.c @@ -138,6 +138,12 @@ int devlink_nl_pre_doit_port(const struct genl_split_ops *ops, return __devlink_nl_pre_doit(skb, info, DEVLINK_NL_FLAG_NEED_PORT); } +int devlink_nl_pre_doit_dev_lock(const struct genl_split_ops *ops, + struct sk_buff *skb, struct genl_info *info) +{ + return __devlink_nl_pre_doit(skb, info, DEVLINK_NL_FLAG_NEED_DEV_LOCK); +} + int devlink_nl_pre_doit_port_optional(const struct genl_split_ops *ops, struct sk_buff *skb, struct genl_info *info) @@ -162,6 +168,13 @@ void devlink_nl_post_doit(const struct genl_split_ops *ops, __devlink_nl_post_doit(skb, info, 0); } +void +devlink_nl_post_doit_dev_lock(const struct genl_split_ops *ops, + struct sk_buff *skb, struct genl_info *info) +{ + __devlink_nl_post_doit(skb, info, DEVLINK_NL_FLAG_NEED_DEV_LOCK); +} + static int devlink_nl_inst_single_dumpit(struct sk_buff *msg, struct netlink_callback *cb, int flags, devlink_nl_dump_one_func_t *dump_one, diff --git a/net/devlink/netlink_gen.c b/net/devlink/netlink_gen.c index 788dfdc498a9..95f9b4350ab7 100644 --- a/net/devlink/netlink_gen.c +++ b/net/devlink/netlink_gen.c @@ -846,9 +846,9 @@ const struct genl_split_ops devlink_nl_ops[73] = { { .cmd = DEVLINK_CMD_RELOAD, .validate = GENL_DONT_VALIDATE_STRICT, - .pre_doit = devlink_nl_pre_doit, + .pre_doit = devlink_nl_pre_doit_dev_lock, .doit = devlink_nl_reload_doit, - .post_doit = devlink_nl_post_doit, + .post_doit = devlink_nl_post_doit_dev_lock, .policy = devlink_reload_nl_policy, .maxattr = DEVLINK_ATTR_RELOAD_LIMITS, .flags = GENL_ADMIN_PERM | GENL_CMD_CAP_DO, diff --git a/net/devlink/netlink_gen.h b/net/devlink/netlink_gen.h index 0e9e89c31c31..02f3c0bfae0e 100644 --- a/net/devlink/netlink_gen.h +++ b/net/devlink/netlink_gen.h @@ -22,12 +22,17 @@ int devlink_nl_pre_doit(const struct genl_split_ops *ops, struct sk_buff *skb, struct genl_info *info); int devlink_nl_pre_doit_port(const struct genl_split_ops *ops, struct sk_buff *skb, struct genl_info *info); +int devlink_nl_pre_doit_dev_lock(const struct genl_split_ops *ops, + struct sk_buff *skb, struct genl_info *info); int devlink_nl_pre_doit_port_optional(const struct genl_split_ops *ops, struct sk_buff *skb, struct genl_info *info); void devlink_nl_post_doit(const struct genl_split_ops *ops, struct sk_buff *skb, struct genl_info *info); +void +devlink_nl_post_doit_dev_lock(const struct genl_split_ops *ops, + struct sk_buff *skb, struct genl_info *info); int devlink_nl_get_doit(struct sk_buff *skb, struct genl_info *info); int devlink_nl_get_dumpit(struct sk_buff *skb, struct netlink_callback *cb);