Fri, Feb 22, 2019 at 11:07:53PM CET, jakub.kicin...@netronome.com wrote:
>Instead of iterating over all devlink ports add a NDO which
>will return the devlink instance from the driver.
>
>v2: add the netdev_to_devlink() helper (Michal)
>v3: check that devlink has ops (Florian)
>
>Suggested-by: Jiri Pirko <j...@resnulli.us>
>Signed-off-by: Jakub Kicinski <jakub.kicin...@netronome.com>
>Reviewed-by: Florian Fainelli <f.faine...@gmail.com>
[...]
> void devlink_compat_running_version(struct net_device *dev,
> char *buf, size_t len)
> {
>- struct devlink_port *devlink_port;
> struct devlink *devlink;
>
>- mutex_lock(&devlink_mutex);
>- list_for_each_entry(devlink, &devlink_list, list) {
>- mutex_lock(&devlink->lock);
>- list_for_each_entry(devlink_port, &devlink->port_list, list) {
>- if (devlink_port->type == DEVLINK_PORT_TYPE_ETH &&
>- devlink_port->type_dev == dev) {
>- __devlink_compat_running_version(devlink,
>- buf, len);
>- mutex_unlock(&devlink->lock);
>- goto out;
>- }
>- }
>- mutex_unlock(&devlink->lock);
>- }
>-out:
>- mutex_unlock(&devlink_mutex);
>+ devlink = netdev_to_devlink(dev);
What prevents devlink from being unregistered now from another thread?
I think you should hold &devlink_mutex here to prevent it.
>+ if (!devlink || !devlink->ops || !devlink->ops->info_get)
>+ return;
>+
>+ mutex_lock(&devlink->lock);
>+ __devlink_compat_running_version(devlink, buf, len);
>+ mutex_unlock(&devlink->lock);
> }
>
> int devlink_compat_flash_update(struct net_device *dev, const char *file_name)
> {
>- struct devlink_port *devlink_port;
> struct devlink *devlink;
>+ int ret;
>
>- mutex_lock(&devlink_mutex);
>- list_for_each_entry(devlink, &devlink_list, list) {
>- mutex_lock(&devlink->lock);
>- list_for_each_entry(devlink_port, &devlink->port_list, list) {
>- int ret = -EOPNOTSUPP;
>-
>- if (devlink_port->type != DEVLINK_PORT_TYPE_ETH ||
>- devlink_port->type_dev != dev)
>- continue;
>-
>- mutex_unlock(&devlink_mutex);
>- if (devlink->ops->flash_update)
>- ret = devlink->ops->flash_update(devlink,
>- file_name,
>- NULL, NULL);
>- mutex_unlock(&devlink->lock);
>- return ret;
>- }
>- mutex_unlock(&devlink->lock);
>- }
>- mutex_unlock(&devlink_mutex);
>+ devlink = netdev_to_devlink(dev);
Same here.
>+ if (!devlink || !devlink->ops || !devlink->ops->flash_update)
>+ return -EOPNOTSUPP;
>
>- return -EOPNOTSUPP;
>+ mutex_lock(&devlink->lock);
>+ ret = devlink->ops->flash_update(devlink, file_name, NULL, NULL);
>+ mutex_unlock(&devlink->lock);
>+ return ret;
> }
>
> static int __init devlink_init(void)
>--
>2.19.2
>
