From: Ido Schimmel <idosch@nvidia.com>
To: <netdev@vger.kernel.org>, <linux-pci@vger.kernel.org>
Cc: <davem@davemloft.net>, <kuba@kernel.org>, <pabeni@redhat.com>,
<edumazet@google.com>, <bhelgaas@google.com>,
<alex.williamson@redhat.com>, <lukas@wunner.de>,
<petrm@nvidia.com>, <jiri@nvidia.com>, <mlxsw@nvidia.com>,
Ido Schimmel <idosch@nvidia.com>
Subject: [RFC PATCH net-next 01/12] netdevsim: Block until all devices are released
Date: Tue, 17 Oct 2023 10:42:46 +0300 [thread overview]
Message-ID: <20231017074257.3389177-2-idosch@nvidia.com> (raw)
In-Reply-To: <20231017074257.3389177-1-idosch@nvidia.com>
Like other buses, devices on the netdevsim bus have a release callback
that is invoked when the reference count of the device drops to zero.
However, unlike other buses such as PCI, the release callback is not
necessarily built into the kernel, as netdevsim can be built as a
module.
This above is problematic as nothing prevents the module from being
unloaded before the release callback has been invoked, which can happen
asynchronously. One such example is going to be added in subsequent
patches where devlink will call put_device() from an RCU callback.
The issue is not theoretical and the reproducer in [1] can reliably
crash the kernel. The conclusion of this discussion was that the issue
should be solved in netdevsim, which is what this patch is trying to do.
Add a reference count that is increased when a device is added to the
bus and decreased when a device is released. Signal a completion when
the reference count drops to zero and wait for the completion when
unloading the module so that the module will not be unloaded before all
the devices were released. The reference count is initialized to one so
that completion is only signaled when unloading the module.
With this patch, the reproducer in [1] no longer crashes the kernel.
[1] https://lore.kernel.org/netdev/20230619125015.1541143-2-idosch@nvidia.com/
Signed-off-by: Ido Schimmel <idosch@nvidia.com>
---
drivers/net/netdevsim/bus.c | 12 ++++++++++++
1 file changed, 12 insertions(+)
diff --git a/drivers/net/netdevsim/bus.c b/drivers/net/netdevsim/bus.c
index 0787ad252dd9..bcbc1e19edde 100644
--- a/drivers/net/netdevsim/bus.c
+++ b/drivers/net/netdevsim/bus.c
@@ -3,11 +3,13 @@
* Copyright (C) 2019 Mellanox Technologies. All rights reserved
*/
+#include <linux/completion.h>
#include <linux/device.h>
#include <linux/idr.h>
#include <linux/kernel.h>
#include <linux/list.h>
#include <linux/mutex.h>
+#include <linux/refcount.h>
#include <linux/slab.h>
#include <linux/sysfs.h>
@@ -17,6 +19,8 @@ static DEFINE_IDA(nsim_bus_dev_ids);
static LIST_HEAD(nsim_bus_dev_list);
static DEFINE_MUTEX(nsim_bus_dev_list_lock);
static bool nsim_bus_enable;
+static refcount_t nsim_bus_devs; /* Including the bus itself. */
+static DECLARE_COMPLETION(nsim_bus_devs_released);
static struct nsim_bus_dev *to_nsim_bus_dev(struct device *dev)
{
@@ -121,6 +125,8 @@ static void nsim_bus_dev_release(struct device *dev)
nsim_bus_dev = container_of(dev, struct nsim_bus_dev, dev);
kfree(nsim_bus_dev);
+ if (refcount_dec_and_test(&nsim_bus_devs))
+ complete(&nsim_bus_devs_released);
}
static struct device_type nsim_bus_dev_type = {
@@ -170,6 +176,7 @@ new_device_store(const struct bus_type *bus, const char *buf, size_t count)
goto err;
}
+ refcount_inc(&nsim_bus_devs);
/* Allow using nsim_bus_dev */
smp_store_release(&nsim_bus_dev->init, true);
@@ -326,6 +333,7 @@ int nsim_bus_init(void)
err = driver_register(&nsim_driver);
if (err)
goto err_bus_unregister;
+ refcount_set(&nsim_bus_devs, 1);
/* Allow using resources */
smp_store_release(&nsim_bus_enable, true);
return 0;
@@ -341,6 +349,8 @@ void nsim_bus_exit(void)
/* Disallow using resources */
smp_store_release(&nsim_bus_enable, false);
+ if (refcount_dec_and_test(&nsim_bus_devs))
+ complete(&nsim_bus_devs_released);
mutex_lock(&nsim_bus_dev_list_lock);
list_for_each_entry_safe(nsim_bus_dev, tmp, &nsim_bus_dev_list, list) {
@@ -349,6 +359,8 @@ void nsim_bus_exit(void)
}
mutex_unlock(&nsim_bus_dev_list_lock);
+ wait_for_completion(&nsim_bus_devs_released);
+
driver_unregister(&nsim_driver);
bus_unregister(&nsim_bus);
}
--
2.40.1
next prev parent reply other threads:[~2023-10-17 7:44 UTC|newest]
Thread overview: 24+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-10-17 7:42 [RFC PATCH net-next 00/12] mlxsw: Add support for new reset flow Ido Schimmel
2023-10-17 7:42 ` Ido Schimmel [this message]
2023-10-19 0:53 ` [RFC PATCH net-next 01/12] netdevsim: Block until all devices are released Jakub Kicinski
2023-10-17 7:42 ` [RFC PATCH net-next 02/12] devlink: Hold a reference on parent device Ido Schimmel
2023-10-17 7:56 ` Jiri Pirko
2023-10-17 8:11 ` Ido Schimmel
2023-10-17 9:01 ` Jiri Pirko
2023-10-17 7:42 ` [RFC PATCH net-next 03/12] devlink: Acquire device lock during reload Ido Schimmel
2023-10-17 8:04 ` Jiri Pirko
2023-10-17 7:42 ` [RFC PATCH net-next 04/12] PCI: Add no PM reset quirk for NVIDIA Spectrum devices Ido Schimmel
2023-10-18 19:40 ` Bjorn Helgaas
2023-10-22 8:23 ` Ido Schimmel
2023-10-17 7:42 ` [RFC PATCH net-next 05/12] PCI: Add device-specific reset " Ido Schimmel
2023-10-17 10:00 ` Lukas Wunner
2023-10-18 20:08 ` Bjorn Helgaas
2023-10-25 11:05 ` Ido Schimmel
2023-10-17 7:42 ` [RFC PATCH net-next 06/12] PCI: Add debug print for device ready delay Ido Schimmel
2023-10-18 19:41 ` Bjorn Helgaas
2023-10-17 7:42 ` [RFC PATCH net-next 07/12] mlxsw: Extend MRSR pack() function to support new commands Ido Schimmel
2023-10-17 7:42 ` [RFC PATCH net-next 08/12] mlxsw: pci: Rename mlxsw_pci_sw_reset() Ido Schimmel
2023-10-17 7:42 ` [RFC PATCH net-next 09/12] mlxsw: pci: Move software reset code to a separate function Ido Schimmel
2023-10-17 7:42 ` [RFC PATCH net-next 10/12] mlxsw: pci: Add support for new reset flow Ido Schimmel
2023-10-17 7:42 ` [RFC PATCH net-next 11/12] mlxsw: pci: Implement PCI reset handlers Ido Schimmel
2023-10-17 7:42 ` [RFC PATCH net-next 12/12] selftests: mlxsw: Add PCI reset test Ido Schimmel
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=20231017074257.3389177-2-idosch@nvidia.com \
--to=idosch@nvidia.com \
--cc=alex.williamson@redhat.com \
--cc=bhelgaas@google.com \
--cc=davem@davemloft.net \
--cc=edumazet@google.com \
--cc=jiri@nvidia.com \
--cc=kuba@kernel.org \
--cc=linux-pci@vger.kernel.org \
--cc=lukas@wunner.de \
--cc=mlxsw@nvidia.com \
--cc=netdev@vger.kernel.org \
--cc=pabeni@redhat.com \
--cc=petrm@nvidia.com \
/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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox