Fri, Feb 22, 2019 at 11:07:53PM CET, jakub.kicinski@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 <jiri@resnulli.us>
Signed-off-by: Jakub Kicinski <redacted>
Reviewed-by: Florian Fainelli <f.fainelli@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