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 08/16] devlink: Protect resource list with specific lock
Date: Mon, 8 Nov 2021 19:05:30 +0200 [thread overview]
Message-ID: <461d1e1b3d0399d6cab1117a796f0b02b634597f.1636390483.git.leonro@nvidia.com> (raw)
In-Reply-To: <cover.1636390483.git.leonro@nvidia.com>
From: Leon Romanovsky <leonro@nvidia.com>
Devlink resource flows relied on devlink instance lock to protect
from parallel addition and deletion from resource_list. So instead
of overloading devlink->lock, let's introduce new lock with much more
clear scope.
Signed-off-by: Leon Romanovsky <leonro@nvidia.com>
---
net/core/devlink.c | 100 +++++++++++++++++++++++++--------------------
1 file changed, 56 insertions(+), 44 deletions(-)
diff --git a/net/core/devlink.c b/net/core/devlink.c
index 86db7cf1f3ca..97154219fca2 100644
--- a/net/core/devlink.c
+++ b/net/core/devlink.c
@@ -46,6 +46,7 @@ struct devlink {
struct list_head sb_list;
struct list_head dpipe_table_list;
struct list_head resource_list;
+ struct mutex resource_list_lock; /* protects resource_list */
struct list_head param_list;
struct list_head region_list;
struct list_head reporter_list;
@@ -3587,12 +3588,15 @@ devlink_resource_find(struct devlink *devlink,
}
static void
-devlink_resource_validate_children(struct devlink_resource *resource)
+devlink_resource_validate_children(struct devlink *devlink,
+ struct devlink_resource *resource)
{
struct devlink_resource *child_resource;
bool size_valid = true;
u64 parts_size = 0;
+ lockdep_assert_held(&devlink->lock);
+
if (list_empty(&resource->resource_list))
goto out;
@@ -3645,20 +3649,25 @@ static int devlink_nl_cmd_resource_set(struct sk_buff *skb,
return -EINVAL;
resource_id = nla_get_u64(info->attrs[DEVLINK_ATTR_RESOURCE_ID]);
+ mutex_lock(&devlink->resource_list_lock);
resource = devlink_resource_find(devlink, NULL, resource_id);
- if (!resource)
- return -EINVAL;
+ if (!resource) {
+ err = -EINVAL;
+ goto out;
+ }
size = nla_get_u64(info->attrs[DEVLINK_ATTR_RESOURCE_SIZE]);
err = devlink_resource_validate_size(resource, size, info->extack);
if (err)
- return err;
+ goto out;
resource->size_new = size;
- devlink_resource_validate_children(resource);
+ devlink_resource_validate_children(devlink, resource);
if (resource->parent)
- devlink_resource_validate_children(resource->parent);
- return 0;
+ devlink_resource_validate_children(devlink, resource->parent);
+out:
+ mutex_unlock(&devlink->resource_list_lock);
+ return err;
}
static int
@@ -3742,31 +3751,36 @@ static int devlink_resource_put(struct devlink *devlink, struct sk_buff *skb,
return -EMSGSIZE;
}
-static int devlink_resource_fill(struct genl_info *info,
- enum devlink_command cmd, int flags)
+static int devlink_nl_cmd_resource_dump(struct sk_buff *sk,
+ struct genl_info *info)
{
struct devlink *devlink = info->user_ptr[0];
struct devlink_resource *resource;
struct nlattr *resources_attr;
struct sk_buff *skb = NULL;
+ int err = -EOPNOTSUPP;
struct nlmsghdr *nlh;
bool incomplete;
void *hdr;
int i;
- int err;
+
+ mutex_lock(&devlink->resource_list_lock);
+ if (list_empty(&devlink->resource_list))
+ goto out;
resource = list_first_entry(&devlink->resource_list,
struct devlink_resource, list);
start_again:
err = devlink_dpipe_send_and_alloc_skb(&skb, info);
if (err)
- return err;
+ goto out;
hdr = genlmsg_put(skb, info->snd_portid, info->snd_seq,
- &devlink_nl_family, NLM_F_MULTI, cmd);
+ &devlink_nl_family, NLM_F_MULTI,
+ DEVLINK_CMD_RESOURCE_DUMP);
if (!hdr) {
- nlmsg_free(skb);
- return -EMSGSIZE;
+ err = -EMSGSIZE;
+ goto err_resource_put;
}
if (devlink_nl_put_handle(skb, devlink))
@@ -3793,9 +3807,10 @@ static int devlink_resource_fill(struct genl_info *info,
genlmsg_end(skb, hdr);
if (incomplete)
goto start_again;
+ mutex_unlock(&devlink->resource_list_lock);
send_done:
- nlh = nlmsg_put(skb, info->snd_portid, info->snd_seq,
- NLMSG_DONE, 0, flags | NLM_F_MULTI);
+ nlh = nlmsg_put(skb, info->snd_portid, info->snd_seq, NLMSG_DONE, 0,
+ NLM_F_MULTI);
if (!nlh) {
err = devlink_dpipe_send_and_alloc_skb(&skb, info);
if (err)
@@ -3808,28 +3823,20 @@ static int devlink_resource_fill(struct genl_info *info,
err = -EMSGSIZE;
err_resource_put:
nlmsg_free(skb);
+out:
+ mutex_unlock(&devlink->resource_list_lock);
return err;
}
-static int devlink_nl_cmd_resource_dump(struct sk_buff *skb,
- struct genl_info *info)
-{
- struct devlink *devlink = info->user_ptr[0];
-
- if (list_empty(&devlink->resource_list))
- return -EOPNOTSUPP;
-
- return devlink_resource_fill(info, DEVLINK_CMD_RESOURCE_DUMP, 0);
-}
-
static int
devlink_resources_validate(struct devlink *devlink,
- struct devlink_resource *resource,
- struct genl_info *info)
+ struct devlink_resource *resource)
{
struct list_head *resource_list;
int err = 0;
+ lockdep_assert_held(&devlink->lock);
+
if (resource)
resource_list = &resource->resource_list;
else
@@ -3838,9 +3845,10 @@ devlink_resources_validate(struct devlink *devlink,
list_for_each_entry(resource, resource_list, list) {
if (!resource->size_valid)
return -EINVAL;
- err = devlink_resources_validate(devlink, resource, info);
+
+ err = devlink_resources_validate(devlink, resource);
if (err)
- return err;
+ break;
}
return err;
}
@@ -4064,7 +4072,9 @@ static int devlink_nl_cmd_reload(struct sk_buff *skb, struct genl_info *info)
if (!(devlink->features & DEVLINK_F_RELOAD))
return -EOPNOTSUPP;
- err = devlink_resources_validate(devlink, NULL, info);
+ mutex_lock(&devlink->resource_list_lock);
+ err = devlink_resources_validate(devlink, NULL);
+ mutex_unlock(&devlink->resource_list_lock);
if (err) {
NL_SET_ERR_MSG_MOD(info->extack, "resources size validation failed");
return err;
@@ -8955,7 +8965,10 @@ struct devlink *devlink_alloc_ns(const struct devlink_ops *ops,
INIT_LIST_HEAD(&devlink->rate_list);
INIT_LIST_HEAD(&devlink->sb_list);
INIT_LIST_HEAD_RCU(&devlink->dpipe_table_list);
+
INIT_LIST_HEAD(&devlink->resource_list);
+ mutex_init(&devlink->resource_list_lock);
+
INIT_LIST_HEAD(&devlink->param_list);
INIT_LIST_HEAD(&devlink->region_list);
INIT_LIST_HEAD(&devlink->reporter_list);
@@ -9108,6 +9121,7 @@ void devlink_free(struct devlink *devlink)
mutex_destroy(&devlink->reporters_lock);
mutex_destroy(&devlink->port_list_lock);
+ mutex_destroy(&devlink->resource_list_lock);
mutex_destroy(&devlink->lock);
WARN_ON(!list_empty(&devlink->trap_policer_list));
WARN_ON(!list_empty(&devlink->trap_group_list));
@@ -9825,7 +9839,7 @@ int devlink_resource_register(struct devlink *devlink,
sizeof(resource->size_params));
INIT_LIST_HEAD(&resource->resource_list);
- mutex_lock(&devlink->lock);
+ mutex_lock(&devlink->resource_list_lock);
if (parent_resource_id == DEVLINK_RESOURCE_ID_PARENT_TOP) {
resource_list = &devlink->resource_list;
} else {
@@ -9845,7 +9859,7 @@ int devlink_resource_register(struct devlink *devlink,
list_add_tail(&resource->list, resource_list);
out:
- mutex_unlock(&devlink->lock);
+ mutex_unlock(&devlink->resource_list_lock);
return err;
}
EXPORT_SYMBOL_GPL(devlink_resource_register);
@@ -9872,16 +9886,14 @@ void devlink_resources_unregister(struct devlink *devlink)
{
struct devlink_resource *tmp, *child_resource;
- mutex_lock(&devlink->lock);
-
+ mutex_lock(&devlink->resource_list_lock);
list_for_each_entry_safe(child_resource, tmp, &devlink->resource_list,
list) {
devlink_resource_unregister(devlink, child_resource);
list_del(&child_resource->list);
kfree(child_resource);
}
-
- mutex_unlock(&devlink->lock);
+ mutex_unlock(&devlink->resource_list_lock);
}
EXPORT_SYMBOL_GPL(devlink_resources_unregister);
@@ -9899,7 +9911,7 @@ int devlink_resource_size_get(struct devlink *devlink,
struct devlink_resource *resource;
int err = 0;
- mutex_lock(&devlink->lock);
+ mutex_lock(&devlink->resource_list_lock);
resource = devlink_resource_find(devlink, NULL, resource_id);
if (!resource) {
err = -EINVAL;
@@ -9908,7 +9920,7 @@ int devlink_resource_size_get(struct devlink *devlink,
*p_resource_size = resource->size_new;
resource->size = resource->size_new;
out:
- mutex_unlock(&devlink->lock);
+ mutex_unlock(&devlink->resource_list_lock);
return err;
}
EXPORT_SYMBOL_GPL(devlink_resource_size_get);
@@ -9959,7 +9971,7 @@ void devlink_resource_occ_get_register(struct devlink *devlink,
{
struct devlink_resource *resource;
- mutex_lock(&devlink->lock);
+ mutex_lock(&devlink->resource_list_lock);
resource = devlink_resource_find(devlink, NULL, resource_id);
if (WARN_ON(!resource))
goto out;
@@ -9968,7 +9980,7 @@ void devlink_resource_occ_get_register(struct devlink *devlink,
resource->occ_get = occ_get;
resource->occ_get_priv = occ_get_priv;
out:
- mutex_unlock(&devlink->lock);
+ mutex_unlock(&devlink->resource_list_lock);
}
EXPORT_SYMBOL_GPL(devlink_resource_occ_get_register);
@@ -9983,7 +9995,7 @@ void devlink_resource_occ_get_unregister(struct devlink *devlink,
{
struct devlink_resource *resource;
- mutex_lock(&devlink->lock);
+ mutex_lock(&devlink->resource_list_lock);
resource = devlink_resource_find(devlink, NULL, resource_id);
if (WARN_ON(!resource))
goto out;
@@ -9992,7 +10004,7 @@ void devlink_resource_occ_get_unregister(struct devlink *devlink,
resource->occ_get = NULL;
resource->occ_get_priv = NULL;
out:
- mutex_unlock(&devlink->lock);
+ mutex_unlock(&devlink->resource_list_lock);
}
EXPORT_SYMBOL_GPL(devlink_resource_occ_get_unregister);
--
2.33.1
next prev 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 ` Leon Romanovsky [this message]
2021-11-08 17:05 ` [RFC PATCH 09/16] devlink: Protect all traps lists with specialized lock Leon Romanovsky
2021-11-08 17:05 ` [RFC PATCH 10/16] devlink: Separate region list protection to be done " Leon Romanovsky
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=461d1e1b3d0399d6cab1117a796f0b02b634597f.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.