All of lore.kernel.org
 help / color / mirror / Atom feed
From: Alexander Duyck <alexander.h.duyck@linux.intel.com>
To: linux-kernel@vger.kernel.org, gregkh@linuxfoundation.org
Cc: mcgrof@kernel.org, linux-nvdimm@lists.01.org, tj@kernel.org,
	akpm@linux-foundation.org, linux-pm@vger.kernel.org,
	jiangshanlai@gmail.com, rafael@kernel.org, len.brown@intel.com,
	pavel@ucw.cz, zwisler@kernel.org, dan.j.williams@intel.com,
	dave.jiang@intel.com, bvanassche@acm.org,
	alexander.h.duyck@linux.intel.com
Subject: [driver-core PATCH v10 1/9] driver core: Establish order of operations for device_add and device_del via bitflag
Date: Tue, 22 Jan 2019 10:39:10 -0800	[thread overview]
Message-ID: <154818235081.18753.1278409711806099880.stgit@ahduyck-desk1.amr.corp.intel.com> (raw)
In-Reply-To: <154818223154.18753.12374915684623789884.stgit@ahduyck-desk1.amr.corp.intel.com>

Add an additional bit flag to the device_private struct named "dead".

This additional flag provides a guarantee that when a device_del is
executed on a given interface an async worker will not attempt to attach
the driver following the earlier device_del call. Previously this
guarantee was not present and could result in the device_del call
attempting to remove a driver from an interface only to have the async
worker attempt to probe the driver later when it finally completes the
asynchronous probe call.

One additional change added was that I pulled the check for dev->driver
out of the __device_attach_driver call and instead placed it in the
__device_attach_async_helper call. This was motivated by the fact that the
only other caller of this, __device_attach, had already taken the
device_lock() and checked for dev->driver. Instead of testing for this
twice in this path it makes more sense to just consolidate the dev->dead
and dev->driver checks together into one set of checks.

Reviewed-by: Dan Williams <dan.j.williams@intel.com>
Reviewed-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
Signed-off-by: Alexander Duyck <alexander.h.duyck@linux.intel.com>
---
 drivers/base/base.h |    4 ++++
 drivers/base/core.c |   11 +++++++++++
 drivers/base/dd.c   |   22 +++++++++++-----------
 3 files changed, 26 insertions(+), 11 deletions(-)

diff --git a/drivers/base/base.h b/drivers/base/base.h
index 37329a668935..7ca475af8953 100644
--- a/drivers/base/base.h
+++ b/drivers/base/base.h
@@ -67,6 +67,9 @@ struct driver_private {
  *	probed first.
  * @device - pointer back to the struct device that this structure is
  * associated with.
+ * @dead - This device is currently either in the process of or has been
+ *	removed from the system. Any asynchronous events scheduled for this
+ *	device should exit without taking any action.
  *
  * Nothing outside of the driver core should ever touch these fields.
  */
@@ -78,6 +81,7 @@ struct device_private {
 	struct klist_node knode_class;
 	struct list_head deferred_probe;
 	struct device *device;
+	u8 dead:1;
 };
 #define to_device_private_parent(obj)	\
 	container_of(obj, struct device_private, knode_parent)
diff --git a/drivers/base/core.c b/drivers/base/core.c
index 4a4b6f8cbc4f..c9a82e367480 100644
--- a/drivers/base/core.c
+++ b/drivers/base/core.c
@@ -2080,6 +2080,17 @@ void device_del(struct device *dev)
 	struct kobject *glue_dir = NULL;
 	struct class_interface *class_intf;
 
+	/*
+	 * Hold the device lock and set the "dead" flag to guarantee that
+	 * the update behavior is consistent with the other bitfields near
+	 * it and that we cannot have an asynchronous probe routine trying
+	 * to run while we are tearing out the bus/class/sysfs from
+	 * underneath the device.
+	 */
+	device_lock(dev);
+	dev->p->dead = true;
+	device_unlock(dev);
+
 	/* Notify clients of device removal.  This call must come
 	 * before dpm_sysfs_remove().
 	 */
diff --git a/drivers/base/dd.c b/drivers/base/dd.c
index 8ac10af17c00..636cd16b1b62 100644
--- a/drivers/base/dd.c
+++ b/drivers/base/dd.c
@@ -731,15 +731,6 @@ static int __device_attach_driver(struct device_driver *drv, void *_data)
 	bool async_allowed;
 	int ret;
 
-	/*
-	 * Check if device has already been claimed. This may
-	 * happen with driver loading, device discovery/registration,
-	 * and deferred probe processing happens all at once with
-	 * multiple threads.
-	 */
-	if (dev->driver)
-		return -EBUSY;
-
 	ret = driver_match_device(drv, dev);
 	if (ret == 0) {
 		/* no match */
@@ -774,6 +765,15 @@ static void __device_attach_async_helper(void *_dev, async_cookie_t cookie)
 
 	device_lock(dev);
 
+	/*
+	 * Check if device has already been removed or claimed. This may
+	 * happen with driver loading, device discovery/registration,
+	 * and deferred probe processing happens all at once with
+	 * multiple threads.
+	 */
+	if (dev->p->dead || dev->driver)
+		goto out_unlock;
+
 	if (dev->parent)
 		pm_runtime_get_sync(dev->parent);
 
@@ -784,7 +784,7 @@ static void __device_attach_async_helper(void *_dev, async_cookie_t cookie)
 
 	if (dev->parent)
 		pm_runtime_put(dev->parent);
-
+out_unlock:
 	device_unlock(dev);
 
 	put_device(dev);
@@ -897,7 +897,7 @@ static int __driver_attach(struct device *dev, void *data)
 	if (dev->parent && dev->bus->need_parent_lock)
 		device_lock(dev->parent);
 	device_lock(dev);
-	if (!dev->driver)
+	if (!dev->p->dead && !dev->driver)
 		driver_probe_device(drv, dev);
 	device_unlock(dev);
 	if (dev->parent && dev->bus->need_parent_lock)

  reply	other threads:[~2019-01-22 18:39 UTC|newest]

Thread overview: 16+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-01-22 18:39 [driver-core PATCH v10 0/9] Add NUMA aware async_schedule calls Alexander Duyck
2019-01-22 18:39 ` Alexander Duyck [this message]
2019-01-22 18:39 ` [driver-core PATCH v10 2/9] device core: Consolidate locking and unlocking of parent and device Alexander Duyck
2019-01-22 18:39 ` [driver-core PATCH v10 3/9] driver core: Probe devices asynchronously instead of the driver Alexander Duyck
2019-01-30 23:44   ` Rafael J. Wysocki
2019-01-30 23:44     ` Rafael J. Wysocki
2019-01-22 18:39 ` [driver-core PATCH v10 4/9] workqueue: Provide queue_work_node to queue work near a given NUMA node Alexander Duyck
2019-01-22 18:39 ` [driver-core PATCH v10 5/9] async: Add support for queueing on specific " Alexander Duyck
2019-01-22 18:39 ` [driver-core PATCH v10 6/9] driver core: Attach devices on CPU local to device node Alexander Duyck
2019-01-30 23:45   ` Rafael J. Wysocki
2019-01-30 23:45     ` Rafael J. Wysocki
2019-01-22 18:39 ` [driver-core PATCH v10 7/9] PM core: Use new async_schedule_dev command Alexander Duyck
2019-01-22 18:39 ` [driver-core PATCH v10 8/9] libnvdimm: Schedule device registration on node local to the device Alexander Duyck
2019-01-22 18:39 ` [driver-core PATCH v10 9/9] driver core: Rewrite test_async_driver_probe to cover serialization and NUMA affinity Alexander Duyck
     [not found] ` <154818223154.18753.12374915684623789884.stgit-+uVpp3jiz/SWyQ3uPIV3rPooFf0ArEBIu+b9c/7xato@public.gmane.org>
2019-01-31 15:17   ` [driver-core PATCH v10 0/9] Add NUMA aware async_schedule calls Greg KH
2019-01-31 15:17     ` 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=154818235081.18753.1278409711806099880.stgit@ahduyck-desk1.amr.corp.intel.com \
    --to=alexander.h.duyck@linux.intel.com \
    --cc=akpm@linux-foundation.org \
    --cc=bvanassche@acm.org \
    --cc=dan.j.williams@intel.com \
    --cc=dave.jiang@intel.com \
    --cc=gregkh@linuxfoundation.org \
    --cc=jiangshanlai@gmail.com \
    --cc=len.brown@intel.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-nvdimm@lists.01.org \
    --cc=linux-pm@vger.kernel.org \
    --cc=mcgrof@kernel.org \
    --cc=pavel@ucw.cz \
    --cc=rafael@kernel.org \
    --cc=tj@kernel.org \
    --cc=zwisler@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.