| Message ID | 20221217011953.152487-7-kuba@kernel.org (mailing list archive) |
|---|---|
| State | RFC |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | devlink: remove the wait-for-references on unregister | expand |
| Context | Check | Description |
|---|---|---|
| netdev/tree_selection | success | Clearly marked for net-next |
| netdev/apply | fail | Patch does not apply to net-next |
Sat, Dec 17, 2022 at 02:19:49AM CET, kuba@kernel.org wrote: >Requiring devlink_set_features() to be run before devlink is >registered is overzealous. devlink_set_features() itself is >a leftover from old workarounds which were trying to prevent >initiating reload before probe was complete. Wouldn't it be better to remove this entirely? I don't think it is needed anymore. > >Signed-off-by: Jakub Kicinski <kuba@kernel.org> >--- > net/devlink/core.c | 2 -- > 1 file changed, 2 deletions(-) > >diff --git a/net/devlink/core.c b/net/devlink/core.c >index 413b92534ad6..f30fc167c8ad 100644 >--- a/net/devlink/core.c >+++ b/net/devlink/core.c >@@ -131,8 +131,6 @@ struct devlink *devlinks_xa_find_get(struct net *net, unsigned long *indexp) > */ > void devlink_set_features(struct devlink *devlink, u64 features) > { >- ASSERT_DEVLINK_NOT_REGISTERED(devlink); >- > WARN_ON(features & DEVLINK_F_RELOAD && > !devlink_reload_supported(devlink->ops)); > devlink->features = features; >-- >2.38.1 >
On Mon, 2 Jan 2023 16:25:18 +0100 Jiri Pirko wrote: > Sat, Dec 17, 2022 at 02:19:49AM CET, kuba@kernel.org wrote: > >Requiring devlink_set_features() to be run before devlink is > >registered is overzealous. devlink_set_features() itself is > >a leftover from old workarounds which were trying to prevent > >initiating reload before probe was complete. > > Wouldn't it be better to remove this entirely? I don't think it is > needed anymore. I think you're right. Since users don't have access to the instance before it's registered - this flag can have no impact.
On Mon, 2 Jan 2023 15:24:47 -0800 Jakub Kicinski wrote: > On Mon, 2 Jan 2023 16:25:18 +0100 Jiri Pirko wrote: > > Sat, Dec 17, 2022 at 02:19:49AM CET, kuba@kernel.org wrote: > > >Requiring devlink_set_features() to be run before devlink is > > >registered is overzealous. devlink_set_features() itself is > > >a leftover from old workarounds which were trying to prevent > > >initiating reload before probe was complete. > > > > Wouldn't it be better to remove this entirely? I don't think it is > > needed anymore. > > I think you're right. Since users don't have access to the instance > before it's registered - this flag can have no impact. Let's leave this for a separate follow up, mlx5 needs a bit more work. It sets the feature conditionally.
Tue, Jan 03, 2023 at 12:32:54AM CET, kuba@kernel.org wrote: >On Mon, 2 Jan 2023 15:24:47 -0800 Jakub Kicinski wrote: >> On Mon, 2 Jan 2023 16:25:18 +0100 Jiri Pirko wrote: >> > Sat, Dec 17, 2022 at 02:19:49AM CET, kuba@kernel.org wrote: >> > >Requiring devlink_set_features() to be run before devlink is >> > >registered is overzealous. devlink_set_features() itself is >> > >a leftover from old workarounds which were trying to prevent >> > >initiating reload before probe was complete. >> > >> > Wouldn't it be better to remove this entirely? I don't think it is >> > needed anymore. >> >> I think you're right. Since users don't have access to the instance >> before it's registered - this flag can have no impact. > >Let's leave this for a separate follow up, mlx5 needs a bit more work. >It sets the feature conditionally. Okay, let me take care of that.
diff --git a/net/devlink/core.c b/net/devlink/core.c index 413b92534ad6..f30fc167c8ad 100644 --- a/net/devlink/core.c +++ b/net/devlink/core.c @@ -131,8 +131,6 @@ struct devlink *devlinks_xa_find_get(struct net *net, unsigned long *indexp) */ void devlink_set_features(struct devlink *devlink, u64 features) { - ASSERT_DEVLINK_NOT_REGISTERED(devlink); - WARN_ON(features & DEVLINK_F_RELOAD && !devlink_reload_supported(devlink->ops)); devlink->features = features;
Requiring devlink_set_features() to be run before devlink is registered is overzealous. devlink_set_features() itself is a leftover from old workarounds which were trying to prevent initiating reload before probe was complete. Signed-off-by: Jakub Kicinski <kuba@kernel.org> --- net/devlink/core.c | 2 -- 1 file changed, 2 deletions(-)