Thu, Mar 21, 2019 at 08:08:24PM CET, jakub.kicin...@netronome.com wrote: >On Thu, 21 Mar 2019 14:20:14 +0100, Jiri Pirko wrote: >> From: Jiri Pirko <j...@mellanox.com> >> >> The netdevice is guaranteed to not disappear so we can rely that >> devlink_port and devlink won't disappear as well. No need to take >> devlink_mutex so don't take it here. > >I'm not sure the port can't disappear, just looking at this series it >seems bnxt registers the port after netdev (maybe it unregisters in >the other order). Not that it matters here, we use the main devlink >instance for the compat helpers, and devlink instance should >definitely exist.
You are right. Btw, the devlink_port-netdev registration order should be fixed as well. Niting it down to my todo. > >So FWIW the entire series looks good to me. > >I've also pushed my port patches out: > >https://git.kernel.org/pub/scm/linux/kernel/git/kuba/linux.git/log/?h=devlink-pci-ports > >if that's of any use to you (e.g. the patch which changes the order of >port vs netdev registration). Thanks. > >> Signed-off-by: Jiri Pirko <j...@mellanox.com> > >> diff --git a/net/core/devlink.c b/net/core/devlink.c >> index 3dc51ddf7451..1e125c3b890c 100644 >> --- a/net/core/devlink.c >> +++ b/net/core/devlink.c >> @@ -6444,17 +6444,15 @@ void devlink_compat_running_version(struct >> net_device *dev, >> dev_hold(dev); >> rtnl_unlock(); >> >> - mutex_lock(&devlink_mutex); >> devlink = netdev_to_devlink(dev); >> if (!devlink || !devlink->ops->info_get) >> - goto unlock_list; >> + goto out; >> >> mutex_lock(&devlink->lock); >> __devlink_compat_running_version(devlink, buf, len); >> mutex_unlock(&devlink->lock); >> -unlock_list: >> - mutex_unlock(&devlink_mutex); >> >> +out: >> rtnl_lock(); >> dev_put(dev); >> } >> @@ -6462,22 +6460,22 @@ void devlink_compat_running_version(struct >> net_device *dev, >> int devlink_compat_flash_update(struct net_device *dev, const char >> *file_name) >> { >> struct devlink *devlink; >> - int ret = -EOPNOTSUPP; >> + int ret; >> >> dev_hold(dev); >> rtnl_unlock(); >> >> - mutex_lock(&devlink_mutex); >> devlink = netdev_to_devlink(dev); >> - if (!devlink || !devlink->ops->flash_update) >> - goto unlock_list; >> + if (!devlink || !devlink->ops->flash_update) { >> + ret = -EOPNOTSUPP; >> + goto out; >> + } >> >> mutex_lock(&devlink->lock); >> ret = devlink->ops->flash_update(devlink, file_name, NULL, NULL); >> mutex_unlock(&devlink->lock); >> -unlock_list: >> - mutex_unlock(&devlink_mutex); >> >> +out: >> rtnl_lock(); >> dev_put(dev); >> >
