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

Groups > linux.kernel > #1296832 > unrolled thread

[PATCH v3 0/9] stm/intel_th: Updates for 4.4

Back to article view | Back to linux.kernel

Contents

  [PATCH v3 0/9] stm/intel_th: Updates for 4.4 Alexander Shishkin <alexander.shishkin@linux.intel.com> - 2015-12-22 16:30 +0100
    [PATCH v3 4/9] stm class: Fix locking in unbinding policy path Alexander Shishkin <alexander.shishkin@linux.intel.com> - 2015-12-22 16:30 +0100
    [PATCH v3 1/9] stm class: Hide STM-specific options if STM is disabled Alexander Shishkin <alexander.shishkin@linux.intel.com> - 2015-12-22 16:30 +0100
    [PATCH v3 8/9] intel_th: pci: Add Apollo Lake SOC support Alexander Shishkin <alexander.shishkin@linux.intel.com> - 2015-12-22 16:30 +0100
    [PATCH v3 5/9] stm class: Fix link list locking Alexander Shishkin <alexander.shishkin@linux.intel.com> - 2015-12-22 16:30 +0100
    [PATCH v3 9/9] intel_th: pci: Add Broxton SOC support Alexander Shishkin <alexander.shishkin@linux.intel.com> - 2015-12-22 16:40 +0100

#1296832 — [PATCH v3 0/9] stm/intel_th: Updates for 4.4

Hi Greg,

Since the previous versions ([1], [2]) didn't land anywhere yet, I'm
resending the whole thing with a few additions: Sasha reported a
user-controllable allocation with a possible overflow, Chunyan found
an off-by-one in another allocation path and I also found a couple of
locking issues that are better resolved sooner than later. Please
consider this for 4.4 still if possible.

[1] http://marc.info/?l=linux-kernel&m=144845503811729
[2] http://marc.info/?l=linux-kernel&m=144706387705997

Alexander Shishkin (5):
  stm class: Fix locking in unbinding policy path
  stm class: Fix link list locking
  stm class: Prevent user-controllable allocations
  intel_th: pci: Add Apollo Lake SOC support
  intel_th: pci: Add Broxton SOC support

Arnd Bergmann (1):
  stm class: Select CONFIG_SRCU

Chunyan Zhang (1):
  stm class: Fix an off-by-one in master array allocation

Geert Uytterhoeven (2):
  stm class: Hide STM-specific options if STM is disabled
  intel_th: INTEL_TH should depend on HAS_DMA

 drivers/hwtracing/intel_th/Kconfig |  1 +
 drivers/hwtracing/intel_th/pci.c   | 10 +++++++++
 drivers/hwtracing/stm/Kconfig      |  5 +++++
 drivers/hwtracing/stm/core.c       | 43 +++++++++++++++++++++++++++++---------
 drivers/hwtracing/stm/policy.c     | 18 +++++++++++++---
 drivers/hwtracing/stm/stm.h        |  1 +
 6 files changed, 65 insertions(+), 13 deletions(-)

-- 
2.6.4

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [next] | [standalone]

#1296833 — [PATCH v3 4/9] stm class: Fix locking in unbinding policy path

Right now, if stm device removal has to unbind from a policy (that is,
an stm device that has STP policy, gets removed), it will trigger a
nested lock on the stm device's policy mutex.

This patch fixes the problem by moving the locking from the policy
unbinding to policy removal (configfs path), where it's actually needed;
the other caller of the policy unbinding function already takes the
mutex around the call.

Signed-off-by: Alexander Shishkin <alexander.shishkin@linux.intel.com>
---
 drivers/hwtracing/stm/policy.c | 18 +++++++++++++++---
 1 file changed, 15 insertions(+), 3 deletions(-)

diff --git a/drivers/hwtracing/stm/policy.c b/drivers/hwtracing/stm/policy.c
index 11ab6d01ad..94d3abfb73 100644
--- a/drivers/hwtracing/stm/policy.c
+++ b/drivers/hwtracing/stm/policy.c
@@ -272,13 +272,17 @@ void stp_policy_unbind(struct stp_policy *policy)
 {
 	struct stm_device *stm = policy->stm;
 
+	/*
+	 * stp_policy_release() will not call here if the policy is already
+	 * unbound; other users should not either, as no link exists between
+	 * this policy and anything else in that case
+	 */
 	if (WARN_ON_ONCE(!policy->stm))
 		return;
 
-	mutex_lock(&stm->policy_mutex);
-	stm->policy = NULL;
-	mutex_unlock(&stm->policy_mutex);
+	lockdep_assert_held(&stm->policy_mutex);
 
+	stm->policy = NULL;
 	policy->stm = NULL;
 
 	stm_put_device(stm);
@@ -287,8 +291,16 @@ void stp_policy_unbind(struct stp_policy *policy)
 static void stp_policy_release(struct config_item *item)
 {
 	struct stp_policy *policy = to_stp_policy(item);
+	struct stm_device *stm = policy->stm;
 
+	/* a policy *can* be unbound and still exist in configfs tree */
+	if (!stm)
+		return;
+
+	mutex_lock(&stm->policy_mutex);
 	stp_policy_unbind(policy);
+	mutex_unlock(&stm->policy_mutex);
+
 	kfree(policy);
 }
 
-- 
2.6.4

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1296834 — [PATCH v3 1/9] stm class: Hide STM-specific options if STM is disabled

From: Geert Uytterhoeven <geert@linux-m68k.org>

If STM=n, it doesn't make sense to ask about STM_DUMMY and
STM_SOURCE_CONSOLE support, which are not even built when enabled
anyway. Hence hide these options if STM=n.

Reported-by: Linus Torvalds <torvalds@linux-foundation.org>
Signed-off-by: Geert Uytterhoeven <geert@linux-m68k.org>
Signed-off-by: Alexander Shishkin <alexander.shishkin@linux.intel.com>
---
 drivers/hwtracing/stm/Kconfig | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/hwtracing/stm/Kconfig b/drivers/hwtracing/stm/Kconfig
index 83e9f591a5..4c13762f2b 100644
--- a/drivers/hwtracing/stm/Kconfig
+++ b/drivers/hwtracing/stm/Kconfig
@@ -8,6 +8,8 @@ config STM
 
 	  Say Y here to enable System Trace Module device support.
 
+if STM
+
 config STM_DUMMY
 	tristate "Dummy STM driver"
 	help
@@ -24,3 +26,5 @@ config STM_SOURCE_CONSOLE
 
 	  If you want to send kernel console messages over STM devices,
 	  say Y.
+
+endif
-- 
2.6.4

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1296835 — [PATCH v3 8/9] intel_th: pci: Add Apollo Lake SOC support

This adds Intel(R) Trace Hub PCI ID for Apollo Lake SOC.

Signed-off-by: Alexander Shishkin <alexander.shishkin@linux.intel.com>
---
 drivers/hwtracing/intel_th/pci.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/drivers/hwtracing/intel_th/pci.c b/drivers/hwtracing/intel_th/pci.c
index 641e879360..b5760730e1 100644
--- a/drivers/hwtracing/intel_th/pci.c
+++ b/drivers/hwtracing/intel_th/pci.c
@@ -67,6 +67,11 @@ static const struct pci_device_id intel_th_pci_id_table[] = {
 		PCI_DEVICE(PCI_VENDOR_ID_INTEL, 0xa126),
 		.driver_data = (kernel_ulong_t)0,
 	},
+	{
+		/* Apollo Lake */
+		PCI_DEVICE(PCI_VENDOR_ID_INTEL, 0x5a8e),
+		.driver_data = (kernel_ulong_t)0,
+	},
 	{ 0 },
 };
 
-- 
2.6.4

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1296836 — [PATCH v3 5/9] stm class: Fix link list locking

Currently, the list of stm_sources linked to an stm device is protected by
a spinlock, which also means that sources' .unlink() method is called under
this spinlock. However, this method may (and does) sleep, which means
trouble.

This patch slightly reworks locking around stm::link_list so that bits that
might_sleep() are called with a mutex held instead. Modification of this
list requires both mutex and spinlock to be held, while looking at the list
can be done under either mutex or spinlock.

Signed-off-by: Alexander Shishkin <alexander.shishkin@linux.intel.com>
---
 drivers/hwtracing/stm/core.c | 38 +++++++++++++++++++++++++++++---------
 drivers/hwtracing/stm/stm.h  |  1 +
 2 files changed, 30 insertions(+), 9 deletions(-)

diff --git a/drivers/hwtracing/stm/core.c b/drivers/hwtracing/stm/core.c
index b6445d9e54..ddcb606ace 100644
--- a/drivers/hwtracing/stm/core.c
+++ b/drivers/hwtracing/stm/core.c
@@ -641,6 +641,7 @@ int stm_register_device(struct device *parent, struct stm_data *stm_data,
 	if (err)
 		goto err_device;
 
+	mutex_init(&stm->link_mutex);
 	spin_lock_init(&stm->link_lock);
 	INIT_LIST_HEAD(&stm->link_list);
 
@@ -671,11 +672,11 @@ void stm_unregister_device(struct stm_data *stm_data)
 	struct stm_source_device *src, *iter;
 	int i;
 
-	spin_lock(&stm->link_lock);
+	mutex_lock(&stm->link_mutex);
 	list_for_each_entry_safe(src, iter, &stm->link_list, link_entry) {
 		__stm_source_link_drop(src, stm);
 	}
-	spin_unlock(&stm->link_lock);
+	mutex_unlock(&stm->link_mutex);
 
 	synchronize_srcu(&stm_source_srcu);
 
@@ -694,6 +695,17 @@ void stm_unregister_device(struct stm_data *stm_data)
 }
 EXPORT_SYMBOL_GPL(stm_unregister_device);
 
+/*
+ * stm::link_list access serialization uses a spinlock and a mutex; holding
+ * either of them guarantees that the list is stable; modification requires
+ * holding both of them.
+ *
+ * Lock ordering is as follows:
+ *   stm::link_mutex
+ *     stm::link_lock
+ *       src::link_lock
+ */
+
 /**
  * stm_source_link_add() - connect an stm_source device to an stm device
  * @src:	stm_source device
@@ -710,6 +722,7 @@ static int stm_source_link_add(struct stm_source_device *src,
 	char *id;
 	int err;
 
+	mutex_lock(&stm->link_mutex);
 	spin_lock(&stm->link_lock);
 	spin_lock(&src->link_lock);
 
@@ -719,6 +732,7 @@ static int stm_source_link_add(struct stm_source_device *src,
 
 	spin_unlock(&src->link_lock);
 	spin_unlock(&stm->link_lock);
+	mutex_unlock(&stm->link_mutex);
 
 	id = kstrdup(src->data->name, GFP_KERNEL);
 	if (id) {
@@ -756,6 +770,7 @@ fail_free_output:
 	stm_put_device(stm);
 
 fail_detach:
+	mutex_lock(&stm->link_mutex);
 	spin_lock(&stm->link_lock);
 	spin_lock(&src->link_lock);
 
@@ -764,6 +779,7 @@ fail_detach:
 
 	spin_unlock(&src->link_lock);
 	spin_unlock(&stm->link_lock);
+	mutex_unlock(&stm->link_mutex);
 
 	return err;
 }
@@ -776,13 +792,20 @@ fail_detach:
  * If @stm is @src::link, disconnect them from one another and put the
  * reference on the @stm device.
  *
- * Caller must hold stm::link_lock.
+ * Caller must hold stm::link_mutex.
  */
 static void __stm_source_link_drop(struct stm_source_device *src,
 				   struct stm_device *stm)
 {
 	struct stm_device *link;
 
+	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)) {
@@ -791,13 +814,13 @@ static void __stm_source_link_drop(struct stm_source_device *src,
 	}
 
 	stm_output_free(link, &src->output);
-	/* caller must hold stm::link_lock */
 	list_del_init(&src->link_entry);
 	/* matches stm_find_device() from stm_source_link_store() */
 	stm_put_device(link);
 	rcu_assign_pointer(src->link, NULL);
 
 	spin_unlock(&src->link_lock);
+	spin_unlock(&stm->link_lock);
 }
 
 /**
@@ -819,12 +842,9 @@ static void stm_source_link_drop(struct stm_source_device *src)
 	stm = srcu_dereference(src->link, &stm_source_srcu);
 
 	if (stm) {
-		if (src->data->unlink)
-			src->data->unlink(src->data);
-
-		spin_lock(&stm->link_lock);
+		mutex_lock(&stm->link_mutex);
 		__stm_source_link_drop(src, stm);
-		spin_unlock(&stm->link_lock);
+		mutex_unlock(&stm->link_mutex);
 	}
 
 	srcu_read_unlock(&stm_source_srcu, idx);
diff --git a/drivers/hwtracing/stm/stm.h b/drivers/hwtracing/stm/stm.h
index 95ece0292c..97ee022414 100644
--- a/drivers/hwtracing/stm/stm.h
+++ b/drivers/hwtracing/stm/stm.h
@@ -45,6 +45,7 @@ struct stm_device {
 	int			major;
 	unsigned int		sw_nmasters;
 	struct stm_data		*data;
+	struct mutex		link_mutex;
 	spinlock_t		link_lock;
 	struct list_head	link_list;
 	/* master allocation */
-- 
2.6.4

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1296838 — [PATCH v3 9/9] intel_th: pci: Add Broxton SOC support

This adds Intel(R) Trace Hub PCI ID for Broxton SOC.

Signed-off-by: Alexander Shishkin <alexander.shishkin@linux.intel.com>
---
 drivers/hwtracing/intel_th/pci.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/drivers/hwtracing/intel_th/pci.c b/drivers/hwtracing/intel_th/pci.c
index b5760730e1..09017073d7 100644
--- a/drivers/hwtracing/intel_th/pci.c
+++ b/drivers/hwtracing/intel_th/pci.c
@@ -72,6 +72,11 @@ static const struct pci_device_id intel_th_pci_id_table[] = {
 		PCI_DEVICE(PCI_VENDOR_ID_INTEL, 0x5a8e),
 		.driver_data = (kernel_ulong_t)0,
 	},
+	{
+		/* Broxton */
+		PCI_DEVICE(PCI_VENDOR_ID_INTEL, 0x0a80),
+		.driver_data = (kernel_ulong_t)0,
+	},
 	{ 0 },
 };
 
-- 
2.6.4

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web