public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: Dmitry Torokhov <dtor_core@ameritech.net>
To: Greg KH <greg@kroah.com>
Cc: linux-kernel@vger.kernel.org
Subject: [PATCH 4/3] Allow registering device without taking bus lock
Date: Thu, 10 Jun 2004 01:46:23 -0500	[thread overview]
Message-ID: <200406100146.25471.dtor_core@ameritech.net> (raw)
In-Reply-To: <200406100145.01599.dtor_core@ameritech.net>


===================================================================


ChangeSet@1.1769, 2004-06-10 00:10:02-05:00, dtor_core@ameritech.net
  sysfs: provide means for adding and removing devices to a bus without
         taking bus' semaphore so devices can be added/removed from
         driver's probe() and remove() methods.
  
  Signed-off-by: Dmitry Torokhov <dtor@mail.ru>


 drivers/base/base.h    |    4 ++--
 drivers/base/bus.c     |   48 +++++++++++++++++++++++++++++++++++++++++-------
 drivers/base/core.c    |   46 ++++++++++++++++++++++++++++++++--------------
 include/linux/device.h |   31 ++++++++++++++++++++++++++-----
 4 files changed, 101 insertions(+), 28 deletions(-)


===================================================================



diff -Nru a/drivers/base/base.h b/drivers/base/base.h
--- a/drivers/base/base.h	2004-06-10 01:34:48 -05:00
+++ b/drivers/base/base.h	2004-06-10 01:34:48 -05:00
@@ -1,5 +1,5 @@
-extern int bus_add_device(struct device * dev);
-extern void bus_remove_device(struct device * dev);
+extern int bus_add_device(struct device * dev, int lock);
+extern void bus_remove_device(struct device * dev, int lock);
 
 extern int bus_add_driver(struct device_driver *);
 extern void bus_remove_driver(struct device_driver *);
diff -Nru a/drivers/base/bus.c b/drivers/base/bus.c
--- a/drivers/base/bus.c	2004-06-10 01:34:48 -05:00
+++ b/drivers/base/bus.c	2004-06-10 01:34:48 -05:00
@@ -394,22 +394,39 @@
 /**
  *	bus_add_device - add device to bus
  *	@dev:	device being added
+ *	@lock:	flag instructing the function to acquire bus' semaphore
  *
  *	- Add the device to its bus's list of devices.
  *	- Try to attach to driver.
  *	- Create link to device's physical location.
+ *
+ *	Note that only time when bus semaphore should not be taken is
+ *	when bus_add_device is called from driver's probe function
+ *	when it needs to add a child device sitting on the same bus
+ *	with the parent.
  */
-int bus_add_device(struct device * dev)
+int bus_add_device(struct device * dev, int lock)
 {
 	struct bus_type * bus = get_bus(dev->bus);
 	int error = 0;
 
 	if (bus) {
-		down_write(&dev->bus->subsys.rwsem);
+		if (lock)
+			down_write(&bus->subsys.rwsem);
+		else {
+			/*
+			 * Attempt to catch invalid usage - semaphore should
+			 * already be taken and trylock should fail.
+			 * Unfortunately the check will miss scenario where
+			 * semaphore is taken by some other thread
+			 */
+			BUG_ON(down_write_trylock(&bus->subsys.rwsem));
+		}
 		pr_debug("bus %s: add device %s\n", bus->name, dev->bus_id);
-		list_add_tail(&dev->bus_list, &dev->bus->devices.list);
+		list_add_tail(&dev->bus_list, &bus->devices.list);
 		device_attach(dev);
-		up_write(&dev->bus->subsys.rwsem);
+		if (lock)
+			up_write(&bus->subsys.rwsem);
 		sysfs_create_link(&bus->devices.kobj, &dev->kobj, dev->bus_id);
 	}
 	return error;
@@ -418,21 +435,38 @@
 /**
  *	bus_remove_device - remove device from bus
  *	@dev:	device to be removed
+ *	@lock:	flag instructing the function to acquire bus' semaphore
  *
  *	- Remove symlink from bus's directory.
  *	- Delete device from bus's list.
  *	- Detach from its driver.
  *	- Drop reference taken in bus_add_device().
+ *
+ *	Note that only time when bus semaphore should not be taken is
+ *	when bus_remove_device is called from driver's remove function
+ *	when it needs to remove a child device sitting on the same bus
+ *	with the parent.
  */
-void bus_remove_device(struct device * dev)
+void bus_remove_device(struct device * dev, int lock)
 {
 	if (dev->bus) {
 		sysfs_remove_link(&dev->bus->devices.kobj, dev->bus_id);
-		down_write(&dev->bus->subsys.rwsem);
+		if (lock)
+			down_write(&dev->bus->subsys.rwsem);
+		else {
+			/*
+			 * Attempt to catch invalid usage - semaphore should
+			 * already be taken and trylock should fail.
+			 * Unfortunately the check will miss scenario where
+			 * semaphore is taken by some other thread
+			 */
+			BUG_ON(down_write_trylock(&dev->bus->subsys.rwsem));
+		}
 		pr_debug("bus %s: remove device %s\n", dev->bus->name, dev->bus_id);
 		device_release_driver(dev);
 		list_del_init(&dev->bus_list);
-		up_write(&dev->bus->subsys.rwsem);
+		if (lock)
+			up_write(&dev->bus->subsys.rwsem);
 		put_bus(dev->bus);
 	}
 }
diff -Nru a/drivers/base/core.c b/drivers/base/core.c
--- a/drivers/base/core.c	2004-06-10 01:34:48 -05:00
+++ b/drivers/base/core.c	2004-06-10 01:34:48 -05:00
@@ -197,17 +197,19 @@
 }
 
 /**
- *	device_add - add device to device hierarchy.
+ *	__device_add - add device to device hierarchy.
  *	@dev:	device.
+ *	@lock:	flag indicating whether bus_add_device() should acquire
+ *		bus' semaphore or not.
  *
- *	This is part 2 of device_register(), though may be called
+ *	This is part 2 of __device_register(), though may be called
  *	separately _iff_ device_initialize() has been called separately.
  *
  *	This adds it to the kobject hierarchy via kobject_add(), adds it
  *	to the global and sibling lists for the device, then
  *	adds it to the other relevant subsystems of the driver model.
  */
-int device_add(struct device *dev)
+int __device_add(struct device *dev, int lock)
 {
 	struct device * parent;
 	int error;
@@ -229,7 +231,7 @@
 		goto Error;
 	if ((error = device_pm_add(dev)))
 		goto PMError;
-	if ((error = bus_add_device(dev)))
+	if ((error = bus_add_device(dev, lock)))
 		goto BusError;
 	down_write(&devices_subsys.rwsem);
 	if (parent)
@@ -256,6 +258,8 @@
 /**
  *	device_register - register a device with the system.
  *	@dev:	pointer to the device structure
+ *	@lock:	flag indicating whether bus' semaphore should be taken
+ *		before adding the device to its bus
  *
  *	This happens in two clean steps - initialize the device
  *	and add it to the system. The two steps can be called
@@ -263,12 +267,16 @@
  *	I.e. you should only call the two helpers separately if
  *	have a clearly defined need to use and refcount the device
  *	before it is added to the hierarchy.
+ *
+ *	Note that only time when bus' semaphore should not be taken is
+ *	when __device_register() is called from driver's probe routine.
+ *	Regular users users should just call device_register().
  */
 
-int device_register(struct device *dev)
+int __device_register(struct device *dev, int lock)
 {
 	device_initialize(dev);
-	return device_add(dev);
+	return __device_add(dev, lock);
 }
 
 
@@ -300,6 +308,8 @@
 /**
  *	device_del - delete device from system.
  *	@dev:	device.
+ *	@lock:	flag indicating whether bus' semaphore should be taken
+ *		before removing the device from its bus
  *
  *	This is the first part of the device unregistration
  *	sequence. This removes the device from the lists we control
@@ -309,9 +319,11 @@
  *
  *	NOTE: this should be called manually _iff_ device_add() was
  *	also called manually.
+ *
+ *	Regular users users should just call device_del().
  */
 
-void device_del(struct device * dev)
+void __device_del(struct device * dev, int lock)
 {
 	struct device * parent = dev->parent;
 
@@ -325,7 +337,7 @@
 	 */
 	if (platform_notify_remove)
 		platform_notify_remove(dev);
-	bus_remove_device(dev);
+	bus_remove_device(dev, lock);
 	device_pm_remove(dev);
 	kobject_del(&dev->kobj);
 	if (parent)
@@ -335,6 +347,8 @@
 /**
  *	device_unregister - unregister device from system.
  *	@dev:	device going away.
+ *	@lock:	flag indicating whether bus' semaphore should be taken
+ *		before removing the device from its bus
  *
  *	We do this in two parts, like we do device_register(). First,
  *	we remove it from all the subsystems with device_del(), then
@@ -342,11 +356,15 @@
  *	is the final reference count, the device will be cleaned up
  *	via device_release() above. Otherwise, the structure will
  *	stick around until the final reference to the device is dropped.
+ *
+ *	Note that only time when bus' semaphore should not be taken is
+ *	when __device_unregister() is called from driver's remove routine.
+ *	Regular users users should just call device_unregister().
  */
-void device_unregister(struct device * dev)
+void __device_unregister(struct device * dev, int lock)
 {
 	pr_debug("DEV: Unregistering device. ID = '%s'\n", dev->bus_id);
-	device_del(dev);
+	__device_del(dev, lock);
 	put_device(dev);
 }
 
@@ -394,11 +412,11 @@
 EXPORT_SYMBOL(device_for_each_child);
 
 EXPORT_SYMBOL(device_initialize);
-EXPORT_SYMBOL(device_add);
-EXPORT_SYMBOL(device_register);
+EXPORT_SYMBOL(__device_add);
+EXPORT_SYMBOL(__device_register);
 
-EXPORT_SYMBOL(device_del);
-EXPORT_SYMBOL(device_unregister);
+EXPORT_SYMBOL(__device_del);
+EXPORT_SYMBOL(__device_unregister);
 EXPORT_SYMBOL(get_device);
 EXPORT_SYMBOL(put_device);
 EXPORT_SYMBOL(device_find);
diff -Nru a/include/linux/device.h b/include/linux/device.h
--- a/include/linux/device.h	2004-06-10 01:34:48 -05:00
+++ b/include/linux/device.h	2004-06-10 01:34:48 -05:00
@@ -316,16 +316,37 @@
 /*
  * High level routines for use by the bus drivers
  */
-extern int device_register(struct device * dev);
-extern void device_unregister(struct device * dev);
+extern int __device_register(struct device * dev, int lock);
+static inline int device_register(struct device * dev)
+{
+	return __device_register(dev, 1);
+}
+
+extern void __device_unregister(struct device * dev, int lock);
+static inline void device_unregister(struct device * dev)
+{
+	__device_unregister(dev, 1);
+}
+
 extern void device_initialize(struct device * dev);
-extern int device_add(struct device * dev);
-extern void device_del(struct device * dev);
+
+extern int __device_add(struct device * dev, int lock);
+static inline int device_add(struct device * dev)
+{
+	return __device_add(dev, 1);
+}
+
+extern void __device_del(struct device * dev, int lock);
+static inline void device_del(struct device * dev)
+{
+	__device_del(dev, 1);
+}
+
 extern int device_for_each_child(struct device *, void *,
 		     int (*fn)(struct device *, void *));
 
 /*
- * Manual binding of a device to driver. See drivers/base/bus.c 
+ * Manual binding of a device to driver. See drivers/base/bus.c
  * for information on use.
  */
 extern void device_bind_driver(struct device * dev);

  reply	other threads:[~2004-06-10  6:55 UTC|newest]

Thread overview: 19+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2004-06-09  7:21 [PATCH 0/3] Couple of sysfs patches Dmitry Torokhov
2004-06-09  7:22 ` [PATCH 1/3] whitespace fixes in drivers/base Dmitry Torokhov
2004-06-09  7:24   ` [PATCH 2/3] Suppress platform device suffixes Dmitry Torokhov
2004-06-09  7:25     ` [PATCH 3/3] Add platform_device_simple_release Dmitry Torokhov
2004-06-09 22:13 ` [PATCH 0/3] Couple of sysfs patches Greg KH
     [not found]   ` <200406091732.28684.dtor_core@ameritech.net>
2004-06-09 22:45     ` Greg KH
     [not found]       ` <200406091754.23303.dtor_core@ameritech.net>
2004-06-09 23:19         ` Greg KH
2004-06-10  6:40           ` Dmitry Torokhov
2004-06-10  6:42             ` [PATCH 1/3] Suppress platform device suffixes - take 2 Dmitry Torokhov
2004-06-10  6:43               ` [PATCH 2/3] Add platform_device_register_simple Dmitry Torokhov
2004-06-10  6:44                 ` [PATCH 3/3] Whitespace fixes Dmitry Torokhov
2004-06-10  6:46                   ` Dmitry Torokhov [this message]
2004-06-10 10:14                     ` [PATCH 4/3] Allow registering device without taking bus lock Russell King
2004-06-10 16:02                     ` Greg KH
2004-06-10 16:26                   ` [PATCH 3/3] Whitespace fixes Greg KH
2004-06-10 10:16                 ` [PATCH 2/3] Add platform_device_register_simple Russell King
     [not found]                   ` <200406100755.59943.dtor_core@ameritech.net>
2004-06-10 13:56                     ` Russell King
2004-06-10 16:04                 ` Greg KH
2004-06-10 16:19               ` [PATCH 1/3] Suppress platform device suffixes - take 2 Greg KH

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=200406100146.25471.dtor_core@ameritech.net \
    --to=dtor_core@ameritech.net \
    --cc=greg@kroah.com \
    --cc=linux-kernel@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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox