Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1332939

[QUEUED v20160212 17/19] stm class: Fix a race in unlinking

Show all headers | View raw

There is a window in stm_source_link_drop(), during which the source's
link may change before locks are acquired. When this happens, it throws
a warning, since this is not an expected scenario.

This patch handles the race in such a way that if the link appears to
have changed by the time we took the locks, it will release them and
repeat the whole unlinking procedure from the beginning, unless the
other contender beat us to it.

Signed-off-by: Alexander Shishkin <alexander.shishkin@linux.intel.com>
---
 drivers/hwtracing/stm/core.c | 54 ++++++++++++++++++++++++++++++++++----------
 1 file changed, 42 insertions(+), 12 deletions(-)

diff --git a/drivers/hwtracing/stm/core.c b/drivers/hwtracing/stm/core.c
index 0144ded45b..43a4761695 100644
--- a/drivers/hwtracing/stm/core.c
+++ b/drivers/hwtracing/stm/core.c
@@ -694,18 +694,26 @@ err_free:
 }
 EXPORT_SYMBOL_GPL(stm_register_device);
 
-static void __stm_source_link_drop(struct stm_source_device *src,
-				   struct stm_device *stm);
+static int __stm_source_link_drop(struct stm_source_device *src,
+				  struct stm_device *stm);
 
 void stm_unregister_device(struct stm_data *stm_data)
 {
 	struct stm_device *stm = stm_data->stm;
 	struct stm_source_device *src, *iter;
-	int i;
+	int i, ret;
 
 	mutex_lock(&stm->link_mutex);
 	list_for_each_entry_safe(src, iter, &stm->link_list, link_entry) {
-		__stm_source_link_drop(src, stm);
+		ret = __stm_source_link_drop(src, stm);
+		/*
+		 * src <-> stm link must not change under the same
+		 * stm::link_mutex, so complain loudly if it has;
+		 * also in this situation ret!=0 means this src is
+		 * not connected to this stm and it should be otherwise
+		 * safe to proceed with the tear-down of stm.
+		 */
+		WARN_ON_ONCE(ret);
 	}
 	mutex_unlock(&stm->link_mutex);
 
@@ -824,22 +832,28 @@ fail_detach:
  *
  * Caller must hold stm::link_mutex.
  */
-static void __stm_source_link_drop(struct stm_source_device *src,
-				   struct stm_device *stm)
+static int __stm_source_link_drop(struct stm_source_device *src,
+				  struct stm_device *stm)
 {
 	struct stm_device *link;
+	int ret = 0;
 
 	lockdep_assert_held(&stm->link_mutex);
 
-	if (src->data->unlink)
-		src->data->unlink(src->data);
-
 	/* for stm::link_list modification, we hold both mutex and spinlock */
 	spin_lock(&stm->link_lock);
 	spin_lock(&src->link_lock);
 	link = srcu_dereference_check(src->link, &stm_source_srcu, 1);
-	if (WARN_ON_ONCE(link != stm))
+
+	/*
+	 * The linked device may have changed since we last looked, because
+	 * we weren't holding the src::link_lock back then; if this is the
+	 * case, tell the caller to retry.
+	 */
+	if (link != stm) {
+		ret = -EAGAIN;
 		goto unlock;
+	}
 
 	stm_output_free(link, &src->output);
 	list_del_init(&src->link_entry);
@@ -850,6 +864,11 @@ static void __stm_source_link_drop(struct stm_source_device *src,
 unlock:
 	spin_unlock(&src->link_lock);
 	spin_unlock(&stm->link_lock);
+
+	if (!ret && src->data->unlink)
+		src->data->unlink(src->data);
+
+	return ret;
 }
 
 /**
@@ -865,18 +884,29 @@ unlock:
 static void stm_source_link_drop(struct stm_source_device *src)
 {
 	struct stm_device *stm;
-	int idx;
+	int idx, ret;
 
+retry:
 	idx = srcu_read_lock(&stm_source_srcu);
+	/*
+	 * The stm device will be valid for the duration of this
+	 * read section, but the link may change before we grab
+	 * the src::link_lock in __stm_source_link_drop().
+	 */
 	stm = srcu_dereference(src->link, &stm_source_srcu);
 
+	ret = 0;
 	if (stm) {
 		mutex_lock(&stm->link_mutex);
-		__stm_source_link_drop(src, stm);
+		ret = __stm_source_link_drop(src, stm);
 		mutex_unlock(&stm->link_mutex);
 	}
 
 	srcu_read_unlock(&stm_source_srcu, idx);
+
+	/* if it did change, retry */
+	if (ret == -EAGAIN)
+		goto retry;
 }
 
 static ssize_t stm_source_link_show(struct device *dev,
-- 
2.7.0

Back to linux.kernel | Previous | Next — Previous in thread | Next in thread | Find similar | Unroll thread

Thread

[QUEUED v20160212 00/19] stm class/intel_th: Patches in my queue Alexander Shishkin <alexander.shishkin@linux.intel.com> - 2016-02-12 18:10 +0100
  [QUEUED v20160212 01/19] stm class: Use a signed return type for stm_find_master_chan Alexander Shishkin <alexander.shishkin@linux.intel.com> - 2016-02-12 18:10 +0100
  [QUEUED v20160212 04/19] intel_th: Update scratchpad bits according to enabled output activity Alexander Shishkin <alexander.shishkin@linux.intel.com> - 2016-02-12 18:10 +0100
  [QUEUED v20160212 02/19] intel_th: Depend on HAS_IOMEM Alexander Shishkin <alexander.shishkin@linux.intel.com> - 2016-02-12 18:10 +0100
  [QUEUED v20160212 07/19] intel_th: sth: Sanitize packet callback's return values Alexander Shishkin <alexander.shishkin@linux.intel.com> - 2016-02-12 18:10 +0100
  [QUEUED v20160212 08/19] intel_th: Set root device's drvdata early Alexander Shishkin <alexander.shishkin@linux.intel.com> - 2016-02-12 18:10 +0100
  [QUEUED v20160212 11/19] stm class: Support devices with multiple instances Alexander Shishkin <alexander.shishkin@linux.intel.com> - 2016-02-12 18:10 +0100
  [QUEUED v20160212 03/19] intel_th: gth: Remove commented-out code Alexander Shishkin <alexander.shishkin@linux.intel.com> - 2016-02-12 18:10 +0100
  [QUEUED v20160212 18/19] stm class: Plug stm device's unlink callback Alexander Shishkin <alexander.shishkin@linux.intel.com> - 2016-02-12 18:20 +0100
  [QUEUED v20160212 16/19] stm class: Fix unbalanced module/device refcounting Alexander Shishkin <alexander.shishkin@linux.intel.com> - 2016-02-12 18:20 +0100
  [QUEUED v20160212 19/19] stm class: dummy_stm: Add link callback for fault injection Alexander Shishkin <alexander.shishkin@linux.intel.com> - 2016-02-12 18:20 +0100
  [QUEUED v20160212 12/19] stm class: dummy_stm: Create multiple devices Alexander Shishkin <alexander.shishkin@linux.intel.com> - 2016-02-12 18:20 +0100
  [QUEUED v20160212 17/19] stm class: Fix a race in unlinking Alexander Shishkin <alexander.shishkin@linux.intel.com> - 2016-02-12 18:20 +0100
  [QUEUED v20160212 15/19] stm class: Guard output assignment against concurrency Alexander Shishkin <alexander.shishkin@linux.intel.com> - 2016-02-12 18:20 +0100
  [QUEUED v20160212 14/19] stm class: Fix unlocking braino in the error path Alexander Shishkin <alexander.shishkin@linux.intel.com> - 2016-02-12 18:20 +0100
  [QUEUED v20160212 13/19] stm class: Add heartbeat stm source device Alexander Shishkin <alexander.shishkin@linux.intel.com> - 2016-02-12 18:20 +0100

csiph-web