[PATCH net-next 0/4] devlink small improvements

[PATCH net-next 0/4] devlink small improvements

[Parav Pandit]

Hi Jakub, Dave,

This short series improves the devlink code for lock commment,
simplifying checks and keeping the scope of mutex lock for necessary
fields.

Patch summary:
Patch-1 Keep the devlink_mutex for only for necessary changes.
Patch-2 Avoids duplicate check for reload flag
Patch-3 Adds missing comment for the scope of devlink instance lock
Patch-4 Constify devlink instance pointer


Parav Pandit (4):
  devlink: Do not hold devlink mutex when initializing devlink fields
  devlink: Avoid duplicate check for reload enabled flag
  devlink: Add comment for devlink instance lock
  devlink: Constify devlink instance pointer

 include/net/devlink.h | 4 +++-
 net/core/devlink.c    | 6 +++---
 2 files changed, 6 insertions(+), 4 deletions(-)

-- 
2.25.4

[PATCH net-next 1/4] devlink: Do not hold devlink mutex when initializing devlink fields

[Parav Pandit]

There is no need to hold a device global lock when initializing
devlink device fields of a devlink instance which is not yet part of the
devices list.

Signed-off-by: Parav Pandit <parav@mellanox.com>
Reviewed-by: Jiri Pirko <jiri@mellanox.com>
---
 net/core/devlink.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/net/core/devlink.c b/net/core/devlink.c
index 6335e1851088..7df918a5899e 100644
--- a/net/core/devlink.c
+++ b/net/core/devlink.c
@@ -7421,9 +7421,9 @@ EXPORT_SYMBOL_GPL(devlink_alloc);
  */
 int devlink_register(struct devlink *devlink, struct device *dev)
 {
-	mutex_lock(&devlink_mutex);
 	devlink->dev = dev;
 	devlink->registered = true;
+	mutex_lock(&devlink_mutex);
 	list_add_tail(&devlink->list, &devlink_list);
 	devlink_notify(devlink, DEVLINK_CMD_NEW);
 	mutex_unlock(&devlink_mutex);
-- 
2.25.4

[PATCH net-next 2/4] devlink: Avoid duplicate check for reload enabled flag

[Parav Pandit]

Reload operation is enabled or not is already checked by
devlink_reload(). Hence, remove the duplicate check from
devlink_nl_cmd_reload().

Signed-off-by: Parav Pandit <parav@mellanox.com>
Reviewed-by: Jiri Pirko <jiri@mellanox.com>
---
 net/core/devlink.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/net/core/devlink.c b/net/core/devlink.c
index 7df918a5899e..5c74e67f358c 100644
--- a/net/core/devlink.c
+++ b/net/core/devlink.c
@@ -2967,7 +2967,7 @@ static int devlink_nl_cmd_reload(struct sk_buff *skb, struct genl_info *info)
 	struct net *dest_net = NULL;
 	int err;
 
-	if (!devlink_reload_supported(devlink) || !devlink->reload_enabled)
+	if (!devlink_reload_supported(devlink))
 		return -EOPNOTSUPP;
 
 	err = devlink_resources_validate(devlink, NULL, info);
-- 
2.25.4

[PATCH net-next 3/4] devlink: Add comment for devlink instance lock

[Parav Pandit]

Add comment to describe the purpose of devlink instance lock.

Signed-off-by: Parav Pandit <parav@mellanox.com>
Reviewed-by: Jiri Pirko <jiri@mellanox.com>
---
 include/net/devlink.h | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/include/net/devlink.h b/include/net/devlink.h
index 913e8679ae35..19d990c8edcc 100644
--- a/include/net/devlink.h
+++ b/include/net/devlink.h
@@ -40,7 +40,9 @@ struct devlink {
 	struct xarray snapshot_ids;
 	struct device *dev;
 	possible_net_t _net;
-	struct mutex lock;
+	struct mutex lock; /* Serializes access to devlink instance specific objects such as
+			    * port, sb, dpipe, resource, params, region, traps and more.
+			    */
 	u8 reload_failed:1,
 	   reload_enabled:1,
 	   registered:1;
-- 
2.25.4

[PATCH net-next 4/4] devlink: Constify devlink instance pointer

[Parav Pandit]

Constify devlink instance pointer while checking if reload operation is
supported or not.

This helps to review the scope of checks done in reload.

Signed-off-by: Parav Pandit <parav@mellanox.com>
Reviewed-by: Jiri Pirko <jiri@mellanox.com>
---
 net/core/devlink.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/net/core/devlink.c b/net/core/devlink.c
index 5c74e67f358c..8b7bb4bfb6d0 100644
--- a/net/core/devlink.c
+++ b/net/core/devlink.c
@@ -2921,7 +2921,7 @@ static void devlink_reload_netns_change(struct devlink *devlink,
 				     DEVLINK_CMD_PARAM_NEW);
 }
 
-static bool devlink_reload_supported(struct devlink *devlink)
+static bool devlink_reload_supported(const struct devlink *devlink)
 {
 	return devlink->ops->reload_down && devlink->ops->reload_up;
 }
-- 
2.25.4

Re: [PATCH net-next 0/4] devlink small improvements

[Jakub Kicinski]

On Tue, 21 Jul 2020 19:53:50 +0300 Parav Pandit wrote:
> Hi Jakub, Dave,
> 
> This short series improves the devlink code for lock commment,
> simplifying checks and keeping the scope of mutex lock for necessary
> fields.

Reviewed-by: Jakub Kicinski <kuba@kernel.org>

Re: [PATCH net-next 0/4] devlink small improvements

[David Miller]

From: Parav Pandit <parav@mellanox.com>
Date: Tue, 21 Jul 2020 19:53:50 +0300

> This short series improves the devlink code for lock commment,
> simplifying checks and keeping the scope of mutex lock for necessary
> fields.
> 
> Patch summary:
> Patch-1 Keep the devlink_mutex for only for necessary changes.
> Patch-2 Avoids duplicate check for reload flag
> Patch-3 Adds missing comment for the scope of devlink instance lock
> Patch-4 Constify devlink instance pointer

Series applied, thank you.