All of lore.kernel.org
 help / color / mirror / Atom feed
From: Leon Romanovsky <leon@kernel.org>
To: "David S . Miller" <davem@davemloft.net>,
	Jakub Kicinski <kuba@kernel.org>
Cc: Leon Romanovsky <leonro@nvidia.com>,
	Ido Schimmel <idosch@nvidia.com>, Jiri Pirko <jiri@nvidia.com>,
	netdev <netdev@vger.kernel.org>
Subject: [RFC PATCH 10/16] devlink: Separate region list protection to be done with specialized lock
Date: Mon,  8 Nov 2021 19:05:32 +0200	[thread overview]
Message-ID: <a84ddeda0fb9f6a4b895bb0a8a92b22773d602f3.1636390483.git.leonro@nvidia.com> (raw)
In-Reply-To: <cover.1636390483.git.leonro@nvidia.com>

From: Leon Romanovsky <leonro@nvidia.com>

Reduce scope of devlink->lock and use dedicated lock to protect region_list.

Signed-off-by: Leon Romanovsky <leonro@nvidia.com>
---
 net/core/devlink.c | 66 +++++++++++++++++++++++++++-------------------
 1 file changed, 39 insertions(+), 27 deletions(-)

diff --git a/net/core/devlink.c b/net/core/devlink.c
index e89eaaba653d..52a1255a0917 100644
--- a/net/core/devlink.c
+++ b/net/core/devlink.c
@@ -49,6 +49,7 @@ struct devlink {
 	struct mutex resource_list_lock; /* protects resource_list */
 	struct list_head param_list;
 	struct list_head region_list;
+	struct mutex region_list_lock; /* protects region_list, including port */
 	struct list_head reporter_list;
 	struct mutex reporters_lock; /* protects reporter_list */
 	struct devlink_dpipe_headers *dpipe_headers;
@@ -5412,25 +5413,28 @@ static int devlink_nl_cmd_region_get_port_dumpit(struct sk_buff *msg,
 						 int *idx,
 						 int start)
 {
+	struct devlink *devlink = port->devlink;
 	struct devlink_region *region;
 	int err = 0;
 
+	mutex_lock(&devlink->region_list_lock);
 	list_for_each_entry(region, &port->region_list, list) {
 		if (*idx < start) {
 			(*idx)++;
 			continue;
 		}
-		err = devlink_nl_region_fill(msg, port->devlink,
+		err = devlink_nl_region_fill(msg, devlink,
 					     DEVLINK_CMD_REGION_GET,
 					     NETLINK_CB(cb->skb).portid,
-					     cb->nlh->nlmsg_seq,
-					     NLM_F_MULTI, region);
+					     cb->nlh->nlmsg_seq, NLM_F_MULTI,
+					     region);
 		if (err)
 			goto out;
 		(*idx)++;
 	}
 
 out:
+	mutex_unlock(&devlink->region_list_lock);
 	return err;
 }
 
@@ -5444,7 +5448,7 @@ static int devlink_nl_cmd_region_get_devlink_dumpit(struct sk_buff *msg,
 	struct devlink_port *port;
 	int err = 0;
 
-	mutex_lock(&devlink->lock);
+	mutex_lock(&devlink->region_list_lock);
 	list_for_each_entry(region, &devlink->region_list, list) {
 		if (*idx < start) {
 			(*idx)++;
@@ -5455,22 +5459,22 @@ static int devlink_nl_cmd_region_get_devlink_dumpit(struct sk_buff *msg,
 					     NETLINK_CB(cb->skb).portid,
 					     cb->nlh->nlmsg_seq,
 					     NLM_F_MULTI, region);
-		if (err)
-			goto out;
+		if (err) {
+			mutex_unlock(&devlink->region_list_lock);
+			return err;
+		}
 		(*idx)++;
 	}
+	mutex_unlock(&devlink->region_list_lock);
 
 	mutex_lock(&devlink->port_list_lock);
 	list_for_each_entry(port, &devlink->port_list, list) {
 		err = devlink_nl_cmd_region_get_port_dumpit(msg, cb, port, idx,
 							    start);
 		if (err)
-			goto out;
+			break;
 	}
-
-out:
 	mutex_unlock(&devlink->port_list_lock);
-	mutex_unlock(&devlink->lock);
 	return err;
 }
 
@@ -5760,7 +5764,7 @@ static int devlink_nl_cmd_region_read_dumpit(struct sk_buff *skb,
 		goto out_dev;
 	}
 
-	mutex_lock(&devlink->lock);
+	mutex_lock(&devlink->region_list_lock);
 
 	if (!attrs[DEVLINK_ATTR_REGION_NAME] ||
 	    !attrs[DEVLINK_ATTR_REGION_SNAPSHOT_ID]) {
@@ -5856,7 +5860,7 @@ static int devlink_nl_cmd_region_read_dumpit(struct sk_buff *skb,
 
 	nla_nest_end(skb, chunks_attr);
 	genlmsg_end(skb, hdr);
-	mutex_unlock(&devlink->lock);
+	mutex_unlock(&devlink->region_list_lock);
 	devlink_put(devlink);
 	mutex_unlock(&devlink_mutex);
 
@@ -5865,7 +5869,7 @@ static int devlink_nl_cmd_region_read_dumpit(struct sk_buff *skb,
 nla_put_failure:
 	genlmsg_cancel(skb, hdr);
 out_unlock:
-	mutex_unlock(&devlink->lock);
+	mutex_unlock(&devlink->region_list_lock);
 	devlink_put(devlink);
 out_dev:
 	mutex_unlock(&devlink_mutex);
@@ -9012,7 +9016,10 @@ struct devlink *devlink_alloc_ns(const struct devlink_ops *ops,
 	mutex_init(&devlink->resource_list_lock);
 
 	INIT_LIST_HEAD(&devlink->param_list);
+
 	INIT_LIST_HEAD(&devlink->region_list);
+	mutex_init(&devlink->region_list_lock);
+
 	INIT_LIST_HEAD(&devlink->reporter_list);
 
 	INIT_LIST_HEAD(&devlink->trap_list);
@@ -9073,8 +9080,10 @@ static void devlink_notify_register(struct devlink *devlink)
 	list_for_each_entry(rate_node, &devlink->rate_list, list)
 		devlink_rate_notify(rate_node, DEVLINK_CMD_RATE_NEW);
 
+	mutex_lock(&devlink->region_list_lock);
 	list_for_each_entry(region, &devlink->region_list, list)
 		devlink_nl_region_notify(region, NULL, DEVLINK_CMD_REGION_NEW);
+	mutex_unlock(&devlink->region_list_lock);
 
 	list_for_each_entry(param_item, &devlink->param_list, list)
 		devlink_param_notify(devlink, 0, param_item,
@@ -9095,8 +9104,10 @@ static void devlink_notify_unregister(struct devlink *devlink)
 		devlink_param_notify(devlink, 0, param_item,
 				     DEVLINK_CMD_PARAM_DEL);
 
+	mutex_lock(&devlink->region_list_lock);
 	list_for_each_entry_reverse(region, &devlink->region_list, list)
 		devlink_nl_region_notify(region, NULL, DEVLINK_CMD_REGION_DEL);
+	mutex_unlock(&devlink->region_list_lock);
 
 	list_for_each_entry_reverse(rate_node, &devlink->rate_list, list)
 		devlink_rate_notify(rate_node, DEVLINK_CMD_RATE_DEL);
@@ -9172,6 +9183,7 @@ void devlink_free(struct devlink *devlink)
 	mutex_destroy(&devlink->port_list_lock);
 	mutex_destroy(&devlink->resource_list_lock);
 	mutex_destroy(&devlink->traps_lock);
+	mutex_destroy(&devlink->region_list_lock);
 	mutex_destroy(&devlink->lock);
 	WARN_ON(!list_empty(&devlink->trap_policer_list));
 	WARN_ON(!list_empty(&devlink->trap_group_list));
@@ -10293,7 +10305,7 @@ devlink_region_create(struct devlink *devlink,
 	if (WARN_ON(!ops) || WARN_ON(!ops->destructor))
 		return ERR_PTR(-EINVAL);
 
-	mutex_lock(&devlink->lock);
+	mutex_lock(&devlink->region_list_lock);
 
 	if (devlink_region_get_by_name(devlink, ops->name)) {
 		err = -EEXIST;
@@ -10314,11 +10326,11 @@ devlink_region_create(struct devlink *devlink,
 	list_add_tail(&region->list, &devlink->region_list);
 	devlink_nl_region_notify(region, NULL, DEVLINK_CMD_REGION_NEW);
 
-	mutex_unlock(&devlink->lock);
+	mutex_unlock(&devlink->region_list_lock);
 	return region;
 
 unlock:
-	mutex_unlock(&devlink->lock);
+	mutex_unlock(&devlink->region_list_lock);
 	return ERR_PTR(err);
 }
 EXPORT_SYMBOL_GPL(devlink_region_create);
@@ -10343,7 +10355,7 @@ devlink_port_region_create(struct devlink_port *port,
 	if (WARN_ON(!ops) || WARN_ON(!ops->destructor))
 		return ERR_PTR(-EINVAL);
 
-	mutex_lock(&devlink->lock);
+	mutex_lock(&devlink->region_list_lock);
 
 	if (devlink_port_region_get_by_name(port, ops->name)) {
 		err = -EEXIST;
@@ -10365,11 +10377,11 @@ devlink_port_region_create(struct devlink_port *port,
 	list_add_tail(&region->list, &port->region_list);
 	devlink_nl_region_notify(region, NULL, DEVLINK_CMD_REGION_NEW);
 
-	mutex_unlock(&devlink->lock);
+	mutex_unlock(&devlink->region_list_lock);
 	return region;
 
 unlock:
-	mutex_unlock(&devlink->lock);
+	mutex_unlock(&devlink->region_list_lock);
 	return ERR_PTR(err);
 }
 EXPORT_SYMBOL_GPL(devlink_port_region_create);
@@ -10384,7 +10396,7 @@ void devlink_region_destroy(struct devlink_region *region)
 	struct devlink *devlink = region->devlink;
 	struct devlink_snapshot *snapshot, *ts;
 
-	mutex_lock(&devlink->lock);
+	mutex_lock(&devlink->region_list_lock);
 
 	/* Free all snapshots of region */
 	list_for_each_entry_safe(snapshot, ts, &region->snapshot_list, list)
@@ -10393,7 +10405,7 @@ void devlink_region_destroy(struct devlink_region *region)
 	list_del(&region->list);
 
 	devlink_nl_region_notify(region, NULL, DEVLINK_CMD_REGION_DEL);
-	mutex_unlock(&devlink->lock);
+	mutex_unlock(&devlink->region_list_lock);
 	kfree(region);
 }
 EXPORT_SYMBOL_GPL(devlink_region_destroy);
@@ -10417,9 +10429,9 @@ int devlink_region_snapshot_id_get(struct devlink *devlink, u32 *id)
 {
 	int err;
 
-	mutex_lock(&devlink->lock);
+	mutex_lock(&devlink->region_list_lock);
 	err = __devlink_region_snapshot_id_get(devlink, id);
-	mutex_unlock(&devlink->lock);
+	mutex_unlock(&devlink->region_list_lock);
 
 	return err;
 }
@@ -10437,9 +10449,9 @@ EXPORT_SYMBOL_GPL(devlink_region_snapshot_id_get);
  */
 void devlink_region_snapshot_id_put(struct devlink *devlink, u32 id)
 {
-	mutex_lock(&devlink->lock);
+	mutex_lock(&devlink->region_list_lock);
 	__devlink_snapshot_id_decrement(devlink, id);
-	mutex_unlock(&devlink->lock);
+	mutex_unlock(&devlink->region_list_lock);
 }
 EXPORT_SYMBOL_GPL(devlink_region_snapshot_id_put);
 
@@ -10461,9 +10473,9 @@ int devlink_region_snapshot_create(struct devlink_region *region,
 	struct devlink *devlink = region->devlink;
 	int err;
 
-	mutex_lock(&devlink->lock);
+	mutex_lock(&devlink->region_list_lock);
 	err = __devlink_region_snapshot_create(region, data, snapshot_id);
-	mutex_unlock(&devlink->lock);
+	mutex_unlock(&devlink->region_list_lock);
 
 	return err;
 }
-- 
2.33.1


  parent reply	other threads:[~2021-11-08 17:06 UTC|newest]

Thread overview: 17+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-11-08 17:05 [RFC PATCH 00/16] Allow parallel devlink execution Leon Romanovsky
2021-11-08 17:05 ` [RFC PATCH 01/16] devlink: Remove misleading internal_flags from health reporter dump Leon Romanovsky
2021-11-08 17:05 ` [RFC PATCH 02/16] devlink: Delete useless checks of holding devlink lock Leon Romanovsky
2021-11-08 17:05 ` [RFC PATCH 03/16] devlink: Simplify devlink resources unregister call Leon Romanovsky
2021-11-08 17:05 ` [RFC PATCH 04/16] devlink: Clean registration of devlink port Leon Romanovsky
2021-11-08 17:05 ` [RFC PATCH 05/16] devlink: Be explicit with devlink port protection Leon Romanovsky
2021-11-08 17:05 ` [RFC PATCH 06/16] devlink: Reshuffle resource registration logic Leon Romanovsky
2021-11-08 17:05 ` [RFC PATCH 07/16] devlink: Inline sb related functions Leon Romanovsky
2021-11-08 17:05 ` [RFC PATCH 08/16] devlink: Protect resource list with specific lock Leon Romanovsky
2021-11-08 17:05 ` [RFC PATCH 09/16] devlink: Protect all traps lists with specialized lock Leon Romanovsky
2021-11-08 17:05 ` Leon Romanovsky [this message]
2021-11-08 17:05 ` [RFC PATCH 11/16] devlink: Protect all rate operations " Leon Romanovsky
2021-11-08 17:05 ` [RFC PATCH 12/16] devlink: Protect all sb " Leon Romanovsky
2021-11-08 17:05 ` [RFC PATCH 13/16] devlink: Convert dpipe to use dpipe_lock Leon Romanovsky
2021-11-08 17:05 ` [RFC PATCH 14/16] devlink: Require devlink lock during device reload Leon Romanovsky
2021-11-08 17:05 ` [RFC PATCH 15/16] devlink: Use xarray locking mechanism instead big devlink lock Leon Romanovsky
2021-11-08 17:05 ` [RFC PATCH 16/16] devlink: Open devlink to parallel operations Leon Romanovsky

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=a84ddeda0fb9f6a4b895bb0a8a92b22773d602f3.1636390483.git.leonro@nvidia.com \
    --to=leon@kernel.org \
    --cc=davem@davemloft.net \
    --cc=idosch@nvidia.com \
    --cc=jiri@nvidia.com \
    --cc=kuba@kernel.org \
    --cc=leonro@nvidia.com \
    --cc=netdev@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.