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 2/5] devlink: add API for explicit locking
Date: Sat, 30 Oct 2021 16:12:51 -0700	[thread overview]
Message-ID: <20211030231254.2477599-3-kuba@kernel.org> (raw)
In-Reply-To: <20211030231254.2477599-1-kuba@kernel.org>

Expose devlink instance lock and create the registration
helpers as needed. The new unregistration helper does
not wait for all refs to be gone, and free just gives
up the reference instead of actually freeing the memory.

Signed-off-by: Jakub Kicinski <kuba@kernel.org>
---
 include/net/devlink.h |  16 ++++++
 net/core/devlink.c    | 122 +++++++++++++++++++++++++++++++++++++++---
 2 files changed, 130 insertions(+), 8 deletions(-)

diff --git a/include/net/devlink.h b/include/net/devlink.h
index d8e4274e2af4..66c1951a6f0e 100644
--- a/include/net/devlink.h
+++ b/include/net/devlink.h
@@ -1191,6 +1191,7 @@ enum {
 };
 
 struct devlink_ops {
+	struct module *owner;
 	/**
 	 * @supported_flash_update_params:
 	 * mask of parameters supported by the driver's .flash_update
@@ -1490,6 +1491,8 @@ void *devlink_priv(struct devlink *devlink);
 struct devlink *priv_to_devlink(void *priv);
 struct device *devlink_to_dev(const struct devlink *devlink);
 
+int lockdep_devlink_is_held(struct devlink *devlink);
+
 struct ib_device;
 
 struct net *devlink_net(const struct devlink *devlink);
@@ -1507,10 +1510,23 @@ static inline struct devlink *devlink_alloc(const struct devlink_ops *ops,
 {
 	return devlink_alloc_ns(ops, priv_size, &init_net, dev);
 }
+
+void devlink_lock(struct devlink *devlink);
+void devlink_unlock(struct devlink *devlink);
+
+void devlink_lock_reg(struct devlink *devlink);
+void devlink_unlock_reg(struct devlink *devlink);
+
+void devlink_get(struct devlink *devlink);
+bool devlink_is_alive(const struct devlink *devlink);
+
 void devlink_set_features(struct devlink *devlink, u64 features);
 void devlink_register(struct devlink *devlink);
+void __devlink_register(struct devlink *devlink);
 void devlink_unregister(struct devlink *devlink);
+void __devlink_unregister(struct devlink *devlink);
 void devlink_free(struct devlink *devlink);
+void __devlink_free(struct devlink *devlink);
 int devlink_port_register(struct devlink *devlink,
 			  struct devlink_port *devlink_port,
 			  unsigned int port_index);
diff --git a/net/core/devlink.c b/net/core/devlink.c
index 9ea0c0bbc796..6783b066f9a7 100644
--- a/net/core/devlink.c
+++ b/net/core/devlink.c
@@ -182,11 +182,24 @@ struct net *devlink_net(const struct devlink *devlink)
 }
 EXPORT_SYMBOL_GPL(devlink_net);
 
+void devlink_free(struct devlink *devlink);
+
 void devlink_put(struct devlink *devlink)
 {
-	if (refcount_dec_and_test(&devlink->refcount))
-		complete(&devlink->comp);
+	if (refcount_dec_and_test(&devlink->refcount)) {
+		if (devlink->ops->lock_flags & DEVLINK_LOCK_REF_MODE)
+			devlink_free(devlink);
+		else
+			complete(&devlink->comp);
+	}
 }
+EXPORT_SYMBOL_GPL(devlink_put);
+
+void devlink_get(struct devlink *devlink)
+{
+	refcount_inc(&devlink->refcount);
+}
+EXPORT_SYMBOL_GPL(devlink_get);
 
 struct devlink *__must_check devlink_try_get(struct devlink *devlink)
 {
@@ -195,6 +208,60 @@ struct devlink *__must_check devlink_try_get(struct devlink *devlink)
 	return NULL;
 }
 
+bool devlink_is_alive(const struct devlink *devlink)
+{
+	return xa_get_mark(&devlinks, devlink->index, DEVLINK_REGISTERED);
+}
+EXPORT_SYMBOL_GPL(devlink_is_alive);
+
+/**
+ * devlink_lock_reg() - take devlink registration lock as well as instance lock
+ * @devlink: instance to lock
+ */
+void devlink_lock_reg(struct devlink *devlink)
+{
+	mutex_lock(&devlink_mutex);
+	mutex_lock(&devlink->lock);
+}
+EXPORT_SYMBOL_GPL(devlink_lock_reg);
+
+/**
+ * devlink_unlock_reg()
+ * @devlink: instance to lock
+ */
+void devlink_unlock_reg(struct devlink *devlink)
+{
+	mutex_unlock(&devlink->lock);
+	mutex_unlock(&devlink_mutex);
+}
+EXPORT_SYMBOL_GPL(devlink_unlock_reg);
+
+/**
+ * devlink_lock() - lock devlink instance
+ * @devlink: instance to lock
+ */
+void devlink_lock(struct devlink *devlink)
+{
+	mutex_lock(&devlink->lock);
+}
+EXPORT_SYMBOL_GPL(devlink_lock);
+
+/**
+ * devlink_unlock() - release devlink instance lock
+ * @devlink: instance to unlock
+ */
+void devlink_unlock(struct devlink *devlink)
+{
+	mutex_unlock(&devlink->lock);
+}
+EXPORT_SYMBOL_GPL(devlink_unlock);
+
+int lockdep_devlink_is_held(struct devlink *devlink)
+{
+	return lockdep_is_held(&devlink->lock);
+}
+EXPORT_SYMBOL_GPL(lockdep_devlink_is_held);
+
 static struct devlink *devlink_get_from_attrs(struct net *net,
 					      struct nlattr **attrs)
 {
@@ -9016,6 +9083,7 @@ struct devlink *devlink_alloc_ns(const struct devlink_ops *ops,
 	mutex_init(&devlink->reporters_lock);
 	refcount_set(&devlink->refcount, 1);
 	init_completion(&devlink->comp);
+	__module_get(devlink->ops->owner);
 
 	return devlink;
 }
@@ -9105,6 +9173,18 @@ static void devlink_notify_unregister(struct devlink *devlink)
 	devlink_notify(devlink, DEVLINK_CMD_DEL);
 }
 
+void __devlink_register(struct devlink *devlink)
+{
+	ASSERT_DEVLINK_NOT_REGISTERED(devlink);
+	lockdep_assert_held(&devlink->lock);
+	lockdep_assert_held(&devlink_mutex);
+	/* Make sure that we are in .probe() routine */
+
+	xa_set_mark(&devlinks, devlink->index, DEVLINK_REGISTERED);
+	devlink_notify_register(devlink);
+}
+EXPORT_SYMBOL_GPL(__devlink_register);
+
 /**
  *	devlink_register - Register devlink instance
  *
@@ -9112,16 +9192,27 @@ static void devlink_notify_unregister(struct devlink *devlink)
  */
 void devlink_register(struct devlink *devlink)
 {
-	ASSERT_DEVLINK_NOT_REGISTERED(devlink);
-	/* Make sure that we are in .probe() routine */
-
 	mutex_lock(&devlink_mutex);
-	xa_set_mark(&devlinks, devlink->index, DEVLINK_REGISTERED);
-	devlink_notify_register(devlink);
+	__devlink_register(devlink);
 	mutex_unlock(&devlink_mutex);
 }
 EXPORT_SYMBOL_GPL(devlink_register);
 
+/* Note: this is NOT an equivalent to devlink_unregister() sans locking
+ * it also accounts for the ability to take references on the instance.
+ */
+void __devlink_unregister(struct devlink *devlink)
+{
+	ASSERT_DEVLINK_REGISTERED(devlink);
+	/* Make sure that we are in .remove() routine */
+	WARN_ON(!(devlink->ops->lock_flags & DEVLINK_LOCK_REF_MODE));
+	lockdep_assert_held(&devlink->lock);
+
+	devlink_notify_unregister(devlink);
+	xa_clear_mark(&devlinks, devlink->index, DEVLINK_REGISTERED);
+}
+EXPORT_SYMBOL_GPL(__devlink_unregister);
+
 /**
  *	devlink_unregister - Unregister devlink instance
  *
@@ -9131,6 +9222,7 @@ void devlink_unregister(struct devlink *devlink)
 {
 	ASSERT_DEVLINK_REGISTERED(devlink);
 	/* Make sure that we are in .remove() routine */
+	WARN_ON(devlink->ops->lock_flags & DEVLINK_LOCK_REF_MODE);
 
 	devlink_put(devlink);
 	wait_for_completion(&devlink->comp);
@@ -9142,6 +9234,12 @@ void devlink_unregister(struct devlink *devlink)
 }
 EXPORT_SYMBOL_GPL(devlink_unregister);
 
+void __devlink_free(struct devlink *devlink)
+{
+	devlink_put(devlink);
+}
+EXPORT_SYMBOL_GPL(__devlink_free);
+
 /**
  *	devlink_free - Free devlink instance resources
  *
@@ -9168,6 +9266,8 @@ void devlink_free(struct devlink *devlink)
 	xa_destroy(&devlink->snapshot_ids);
 	xa_erase(&devlinks, devlink->index);
 
+	module_put(devlink->ops->owner);
+
 	kfree(devlink);
 }
 EXPORT_SYMBOL_GPL(devlink_free);
@@ -11502,7 +11602,8 @@ void devlink_compat_running_version(struct devlink *devlink,
 		return;
 
 	mutex_lock(&devlink->lock);
-	__devlink_compat_running_version(devlink, buf, len);
+	if (devlink_is_alive(devlink))
+		__devlink_compat_running_version(devlink, buf, len);
 	mutex_unlock(&devlink->lock);
 }
 
@@ -11519,9 +11620,14 @@ int devlink_compat_flash_update(struct devlink *devlink, const char *file_name)
 		return ret;
 
 	mutex_lock(&devlink->lock);
+	if (!devlink_is_alive(devlink)) {
+		ret = -ENODEV;
+		goto exit_unlock;
+	}
 	devlink_flash_update_begin_notify(devlink);
 	ret = devlink->ops->flash_update(devlink, &params, NULL);
 	devlink_flash_update_end_notify(devlink);
+exit_unlock:
 	mutex_unlock(&devlink->lock);
 
 	release_firmware(params.fw);
-- 
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 ` Jakub Kicinski [this message]
2021-10-30 23:12 ` [RFC 3/5] devlink: allow locking of all ops Jakub Kicinski
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-3-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.