All of lore.kernel.org
 help / color / mirror / Atom feed
From: Jakub Kicinski <kuba@kernel.org>
To: leon@kernel.org, idosch@idosch.org
Cc: edwin.peer@broadcom.com, jiri@resnulli.us,
	netdev@vger.kernel.org, Jakub Kicinski <kuba@kernel.org>
Subject: [RFC 3/5] devlink: allow locking of all ops
Date: Sat, 30 Oct 2021 16:12:52 -0700	[thread overview]
Message-ID: <20211030231254.2477599-4-kuba@kernel.org> (raw)
In-Reply-To: <20211030231254.2477599-1-kuba@kernel.org>

Drivers using the "explicit locking" API can opt-in
to have all callbacks locked by the devlink instance
lock.

Signed-off-by: Jakub Kicinski <kuba@kernel.org>
---
 include/net/devlink.h | 12 +++++++++
 net/core/devlink.c    | 63 ++++++++++++++++++++++++++++++++++++++-----
 2 files changed, 68 insertions(+), 7 deletions(-)

diff --git a/include/net/devlink.h b/include/net/devlink.h
index 66c1951a6f0e..41ab6a2b63f0 100644
--- a/include/net/devlink.h
+++ b/include/net/devlink.h
@@ -1190,7 +1190,19 @@ enum {
 	DEVLINK_F_RELOAD = 1UL << 0,
 };
 
+enum {
+	DEVLINK_LOCK_REF_MODE	= 1UL << 0,
+	DEVLINK_LOCK_ESWITCH	= 1UL << 1,
+	DEVLINK_LOCK_PORT_OPS	= 1UL << 2,
+	DEVLINK_LOCK_HEALTH_OPS	= 1UL << 3,
+};
+#define DEVLINK_LOCK_ALL_OPS (DEVLINK_LOCK_REF_MODE |	\
+			      DEVLINK_LOCK_ESWITCH |	\
+			      DEVLINK_LOCK_PORT_OPS |	\
+			      DEVLINK_LOCK_HEALTH_OPS)
+
 struct devlink_ops {
+	u8 lock_flags;
 	struct module *owner;
 	/**
 	 * @supported_flash_update_params:
diff --git a/net/core/devlink.c b/net/core/devlink.c
index 6783b066f9a7..70b3f725cb53 100644
--- a/net/core/devlink.c
+++ b/net/core/devlink.c
@@ -256,6 +256,18 @@ void devlink_unlock(struct devlink *devlink)
 }
 EXPORT_SYMBOL_GPL(devlink_unlock);
 
+static void devlink_lock_cond(struct devlink *devlink, unsigned long flag)
+{
+	if (devlink->ops->lock_flags & flag)
+		mutex_lock(&devlink->lock);
+}
+
+static void devlink_unlock_cond(struct devlink *devlink, unsigned long flag)
+{
+	if (devlink->ops->lock_flags & flag)
+		mutex_unlock(&devlink->lock);
+}
+
 int lockdep_devlink_is_held(struct devlink *devlink)
 {
 	return lockdep_is_held(&devlink->lock);
@@ -1595,6 +1607,7 @@ static int devlink_nl_cmd_port_split_doit(struct sk_buff *skb,
 	struct devlink_port *devlink_port;
 	u32 port_index;
 	u32 count;
+	int ret;
 
 	if (!info->attrs[DEVLINK_ATTR_PORT_INDEX] ||
 	    !info->attrs[DEVLINK_ATTR_PORT_SPLIT_COUNT])
@@ -1621,7 +1634,10 @@ static int devlink_nl_cmd_port_split_doit(struct sk_buff *skb,
 		return -EINVAL;
 	}
 
-	return devlink_port_split(devlink, port_index, count, info->extack);
+	devlink_lock_cond(devlink, DEVLINK_LOCK_PORT_OPS);
+	ret = devlink_port_split(devlink, port_index, count, info->extack);
+	devlink_unlock_cond(devlink, DEVLINK_LOCK_PORT_OPS);
+	return ret;
 }
 
 static int devlink_port_unsplit(struct devlink *devlink, u32 port_index,
@@ -1638,12 +1654,17 @@ static int devlink_nl_cmd_port_unsplit_doit(struct sk_buff *skb,
 {
 	struct devlink *devlink = info->user_ptr[0];
 	u32 port_index;
+	int ret;
 
 	if (!info->attrs[DEVLINK_ATTR_PORT_INDEX])
 		return -EINVAL;
 
 	port_index = nla_get_u32(info->attrs[DEVLINK_ATTR_PORT_INDEX]);
-	return devlink_port_unsplit(devlink, port_index, info->extack);
+
+	devlink_lock_cond(devlink, DEVLINK_LOCK_PORT_OPS);
+	ret = devlink_port_unsplit(devlink, port_index, info->extack);
+	devlink_unlock_cond(devlink, DEVLINK_LOCK_PORT_OPS);
+	return ret;
 }
 
 static int devlink_port_new_notifiy(struct devlink *devlink,
@@ -1718,15 +1739,19 @@ static int devlink_nl_cmd_port_new_doit(struct sk_buff *skb,
 		new_attrs.sfnum_valid = true;
 	}
 
+	devlink_lock_cond(devlink, DEVLINK_LOCK_PORT_OPS);
 	err = devlink->ops->port_new(devlink, &new_attrs, extack,
 				     &new_port_index);
+	devlink_unlock_cond(devlink, DEVLINK_LOCK_PORT_OPS);
 	if (err)
 		return err;
 
 	err = devlink_port_new_notifiy(devlink, new_port_index, info);
 	if (err && err != -ENODEV) {
 		/* Fail to send the response; destroy newly created port. */
+		devlink_lock_cond(devlink, DEVLINK_LOCK_PORT_OPS);
 		devlink->ops->port_del(devlink, new_port_index, extack);
+		devlink_unlock_cond(devlink, DEVLINK_LOCK_PORT_OPS);
 	}
 	return err;
 }
@@ -1737,6 +1762,7 @@ static int devlink_nl_cmd_port_del_doit(struct sk_buff *skb,
 	struct netlink_ext_ack *extack = info->extack;
 	struct devlink *devlink = info->user_ptr[0];
 	unsigned int port_index;
+	int ret;
 
 	if (!devlink->ops->port_del)
 		return -EOPNOTSUPP;
@@ -1747,7 +1773,10 @@ static int devlink_nl_cmd_port_del_doit(struct sk_buff *skb,
 	}
 	port_index = nla_get_u32(info->attrs[DEVLINK_ATTR_PORT_INDEX]);
 
-	return devlink->ops->port_del(devlink, port_index, extack);
+	devlink_lock_cond(devlink, DEVLINK_LOCK_PORT_OPS);
+	ret = devlink->ops->port_del(devlink, port_index, extack);
+	devlink_unlock_cond(devlink, DEVLINK_LOCK_PORT_OPS);
+	return ret;
 }
 
 static int
@@ -2841,7 +2870,9 @@ static int devlink_nl_eswitch_fill(struct sk_buff *msg, struct devlink *devlink,
 		goto nla_put_failure;
 
 	if (ops->eswitch_mode_get) {
+		devlink_lock_cond(devlink, DEVLINK_LOCK_ESWITCH);
 		err = ops->eswitch_mode_get(devlink, &mode);
+		devlink_unlock_cond(devlink, DEVLINK_LOCK_ESWITCH);
 		if (err)
 			goto nla_put_failure;
 		err = nla_put_u16(msg, DEVLINK_ATTR_ESWITCH_MODE, mode);
@@ -2932,7 +2963,9 @@ static int devlink_nl_cmd_eswitch_set_doit(struct sk_buff *skb,
 		err = devlink_rate_nodes_check(devlink, mode, info->extack);
 		if (err)
 			return err;
+		devlink_lock_cond(devlink, DEVLINK_LOCK_ESWITCH);
 		err = ops->eswitch_mode_set(devlink, mode, info->extack);
+		devlink_unlock_cond(devlink, DEVLINK_LOCK_ESWITCH);
 		if (err)
 			return err;
 	}
@@ -4227,7 +4260,9 @@ static int devlink_nl_cmd_reload(struct sk_buff *skb, struct genl_info *info)
 			return -EINVAL;
 		}
 	}
+	devlink_lock_cond(devlink, DEVLINK_LOCK_REF_MODE);
 	err = devlink_reload(devlink, dest_net, action, limit, &actions_performed, info->extack);
+	devlink_unlock_cond(devlink, DEVLINK_LOCK_REF_MODE);
 
 	if (dest_net)
 		put_net(dest_net);
@@ -7116,8 +7151,10 @@ static int devlink_health_do_dump(struct devlink_health_reporter *reporter,
 	if (err)
 		goto dump_err;
 
+	devlink_lock_cond(reporter->devlink, DEVLINK_LOCK_HEALTH_OPS);
 	err = reporter->ops->dump(reporter, reporter->dump_fmsg,
 				  priv_ctx, extack);
+	devlink_unlock_cond(reporter->devlink, DEVLINK_LOCK_HEALTH_OPS);
 	if (err)
 		goto dump_err;
 
@@ -7141,6 +7178,7 @@ int devlink_health_report(struct devlink_health_reporter *reporter,
 	enum devlink_health_reporter_state prev_health_state;
 	struct devlink *devlink = reporter->devlink;
 	unsigned long recover_ts_threshold;
+	int ret;
 
 	/* write a log message of the current error */
 	WARN_ON(!msg);
@@ -7174,11 +7212,14 @@ int devlink_health_report(struct devlink_health_reporter *reporter,
 		mutex_unlock(&reporter->dump_lock);
 	}
 
-	if (reporter->auto_recover)
-		return devlink_health_reporter_recover(reporter,
-						       priv_ctx, NULL);
+	ret = 0;
+	if (reporter->auto_recover) {
+		devlink_lock_cond(devlink, DEVLINK_LOCK_HEALTH_OPS);
+		ret = devlink_health_reporter_recover(reporter, priv_ctx, NULL);
+		devlink_unlock_cond(devlink, DEVLINK_LOCK_HEALTH_OPS);
+	}
 
-	return 0;
+	return ret;
 }
 EXPORT_SYMBOL_GPL(devlink_health_report);
 
@@ -7430,7 +7471,9 @@ static int devlink_nl_cmd_health_reporter_recover_doit(struct sk_buff *skb,
 	if (!reporter)
 		return -EINVAL;
 
+	devlink_lock_cond(devlink, DEVLINK_LOCK_HEALTH_OPS);
 	err = devlink_health_reporter_recover(reporter, NULL, info->extack);
+	devlink_unlock_cond(devlink, DEVLINK_LOCK_HEALTH_OPS);
 
 	devlink_health_reporter_put(reporter);
 	return err;
@@ -7463,7 +7506,9 @@ static int devlink_nl_cmd_health_reporter_diagnose_doit(struct sk_buff *skb,
 	if (err)
 		goto out;
 
+	devlink_lock_cond(devlink, DEVLINK_LOCK_HEALTH_OPS);
 	err = reporter->ops->diagnose(reporter, fmsg, info->extack);
+	devlink_unlock_cond(devlink, DEVLINK_LOCK_HEALTH_OPS);
 	if (err)
 		goto out;
 
@@ -7557,7 +7602,9 @@ static int devlink_nl_cmd_health_reporter_test_doit(struct sk_buff *skb,
 		return -EOPNOTSUPP;
 	}
 
+	devlink_lock_cond(devlink, DEVLINK_LOCK_HEALTH_OPS);
 	err = reporter->ops->test(reporter, info->extack);
+	devlink_unlock_cond(devlink, DEVLINK_LOCK_HEALTH_OPS);
 
 	devlink_health_reporter_put(reporter);
 	return err;
@@ -11690,10 +11737,12 @@ static void __net_exit devlink_pernet_pre_exit(struct net *net)
 			goto retry;
 
 		WARN_ON(!(devlink->features & DEVLINK_F_RELOAD));
+		devlink_lock_cond(devlink, DEVLINK_LOCK_REF_MODE);
 		err = devlink_reload(devlink, &init_net,
 				     DEVLINK_RELOAD_ACTION_DRIVER_REINIT,
 				     DEVLINK_RELOAD_LIMIT_UNSPEC,
 				     &actions_performed, NULL);
+		devlink_unlock_cond(devlink, DEVLINK_LOCK_REF_MODE);
 		if (err && err != -EOPNOTSUPP)
 			pr_warn("Failed to reload devlink instance into init_net\n");
 retry:
-- 
2.31.1


  parent reply	other threads:[~2021-10-30 23:13 UTC|newest]

Thread overview: 23+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-10-30 23:12 [RFC 0/5] devlink: add an explicit locking API Jakub Kicinski
2021-10-30 23:12 ` [RFC 1/5] devlink: add unlocked APIs Jakub Kicinski
2021-10-30 23:12 ` [RFC 2/5] devlink: add API for explicit locking Jakub Kicinski
2021-10-30 23:12 ` Jakub Kicinski [this message]
2021-10-30 23:12 ` [RFC 4/5] netdevsim: minor code move Jakub Kicinski
2021-10-30 23:12 ` [RFC 5/5] netdevsim: use devlink locking Jakub Kicinski
2021-10-31  7:23 ` [RFC 0/5] devlink: add an explicit locking API Leon Romanovsky
2021-11-01 14:32   ` Jakub Kicinski
2021-11-01 18:36     ` Leon Romanovsky
2021-11-01 21:16       ` Jakub Kicinski
2021-11-02  8:08         ` Leon Romanovsky
2021-11-02 15:14           ` Jakub Kicinski
2021-11-02 18:14             ` Leon Romanovsky
2021-11-03  0:05               ` Jakub Kicinski
2021-11-03  7:23                 ` Leon Romanovsky
2021-11-03 14:12                   ` Jakub Kicinski
2021-11-01 20:04   ` Edwin Peer
2021-11-02  7:44     ` Leon Romanovsky
2021-11-02 15:16       ` Jakub Kicinski
2021-11-02 17:50         ` Leon Romanovsky
2021-11-03  9:03 ` Jiri Pirko
2021-11-03 14:52   ` Jakub Kicinski
2021-11-03 19:19     ` Jiri Pirko

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=20211030231254.2477599-4-kuba@kernel.org \
    --to=kuba@kernel.org \
    --cc=edwin.peer@broadcom.com \
    --cc=idosch@idosch.org \
    --cc=jiri@resnulli.us \
    --cc=leon@kernel.org \
    --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.