Groups > linux.kernel > #1388152 > unrolled thread

[PATCH V4 00/18] coresight: tmc: make driver usable by Perf

Started by	Mathieu Poirier <mathieu.poirier@linaro.org>
First post	2016-04-27 00:20 +0200
Last post	2016-04-27 00:20 +0200
Articles	18 — 2 participants

Back to article view | Back to linux.kernel

  [PATCH V4 00/18] coresight: tmc: make driver usable by Perf Mathieu Poirier <mathieu.poirier@linaro.org> - 2016-04-27 00:20 +0200
    [PATCH V4 09/18] coresight: tmc: allocating memory when needed Mathieu Poirier <mathieu.poirier@linaro.org> - 2016-04-27 00:20 +0200
    [PATCH V4 14/18] coresight: tmc: keep track of memory width Mathieu Poirier <mathieu.poirier@linaro.org> - 2016-04-27 00:20 +0200
    [PATCH V4 15/18] coresight: moving struct cs_buffers to header file Mathieu Poirier <mathieu.poirier@linaro.org> - 2016-04-27 00:20 +0200
    [PATCH V4 04/18] coresight: tmc: clearly define number of transfers per burst Mathieu Poirier <mathieu.poirier@linaro.org> - 2016-04-27 00:20 +0200
    [PATCH V4 16/18] coresight: tmc: implementing TMC-ETF AUX space API Mathieu Poirier <mathieu.poirier@linaro.org> - 2016-04-27 00:20 +0200
      Re: [PATCH V4 16/18] coresight: tmc: implementing TMC-ETF AUX space  API Suzuki K Poulose <Suzuki.Poulose@arm.com> - 2016-04-27 13:30 +0200
        Re: [PATCH V4 16/18] coresight: tmc: implementing TMC-ETF AUX space  API Suzuki K Poulose <Suzuki.Poulose@arm.com> - 2016-04-27 19:30 +0200
        Re: [PATCH V4 16/18] coresight: tmc: implementing TMC-ETF AUX space API Mathieu Poirier <mathieu.poirier@linaro.org> - 2016-04-27 19:30 +0200
    [PATCH V4 11/18] coresight: tmc: adding mode of operation for link/sinks Mathieu Poirier <mathieu.poirier@linaro.org> - 2016-04-27 00:20 +0200
      Re: [PATCH V4 11/18] coresight: tmc: adding mode of operation for  link/sinks Suzuki K Poulose <Suzuki.Poulose@arm.com> - 2016-04-27 11:10 +0200
    [PATCH V4 02/18] coresight: tmc: waiting for TMCReady bit before programming Mathieu Poirier <mathieu.poirier@linaro.org> - 2016-04-27 00:20 +0200
    [PATCH V4 10/18] coresight: tmc: getting rid of multiple read access Mathieu Poirier <mathieu.poirier@linaro.org> - 2016-04-27 00:20 +0200
      Re: [PATCH V4 10/18] coresight: tmc: getting rid of multiple read  access Suzuki K Poulose <Suzuki.Poulose@arm.com> - 2016-04-27 11:00 +0200
    [PATCH V4 05/18] coresight: tmc: introducing new header file Mathieu Poirier <mathieu.poirier@linaro.org> - 2016-04-27 00:20 +0200
    [PATCH V4 08/18] coresight: tmc: making prepare/unprepare functions generic Mathieu Poirier <mathieu.poirier@linaro.org> - 2016-04-27 00:20 +0200
    [PATCH V4 18/18] coresight: configuring ETF in FIFO mode when acting as link Mathieu Poirier <mathieu.poirier@linaro.org> - 2016-04-27 00:20 +0200
    [PATCH V4 06/18] coresight: tmc: cleaning up header file Mathieu Poirier <mathieu.poirier@linaro.org> - 2016-04-27 00:20 +0200

#1388152 — [PATCH V4 00/18] coresight: tmc: make driver usable by Perf

From	Mathieu Poirier <mathieu.poirier@linaro.org>
Date	2016-04-27 00:20 +0200
Subject	[PATCH V4 00/18] coresight: tmc: make driver usable by Perf
Message-ID	<rsjuN-75I-3@gated-at.bofh.it>

Not sure if another revision was needed but to be on the safe side here
it is. All patches are pretty much the same except for 9/18 and 10/18.
In the former the code has been made clearer by removing the "fast_path"
label while the latter serialize access to the trace trace data from
sysFS.

The patchset makes the TMC driver usable from Perf by way of the
recently added AUX area functionality. Note that only the kernel
side of the operation is presented here. The user space perf tool
enhancement are kept here[1]. Upstreaming of that portion will
follow when the kernel side has been addressed.

The first 15 patches remodel the driver so that the functionality
is compartmented enough to be used as building blocks when interfacing
with the Perf subsystem.

Patch 16 and 17 implement the AUX area API, taking heavily on the existing
ETB10 implementation.

Finally patch 18 allows the TMC to be used as a link rather than a sink,
something that is required when dealing with an ETF component.

Best regards,
Mathieu

[1]. https://git.linaro.org/people/mathieu.poirier/coresight.git/shortlog/refs/heads/perf-opencsd-4.6-rc2

Changes for V4:
- Made functions tmc_enable_etf/etr_sink_sysfs() more readable by
removing the "fast_path" label.
- Consolidated the return path in functions tmc_read_prepare_etf/etr().
- Dealt with race condition nightmares in tmc_open/release() by
serializing access to the trace data buffer.

Changes for V3:
- Reworked drvdata::memwidth assignment.
- cleaned up coresight-tmc.h header file.
- Added WARN_ON[_ONCE]() on error paths.
- Added missing spinlock release.
- Using a type 'long' when dealing with local_[cmp]xchg()
- Added Suzuki K Poulose's Reviewed-by.

Changes for V2:
- Renamed functions tmc_read_prepare/unprepare_etf() to
tmc_read_prepare/unprepare_etb().
- Consolidated return points in functions tmc_enable_etf/etr_sink().
- Re-wrote allocation schemed in functions tmc_enable_etf/etr_sink().
- Fixed description typos in patch 7/15.
- Fixed problem with linksink devices when disabling a path in patch 15/15.

Mathieu Poirier (18):
coresight: tmc: modifying naming convention
coresight: tmc: waiting for TMCReady bit before programming
coresight: tmc: re-implementing tmc_read_prepare/unprepare() functions
coresight: tmc: clearly define number of transfers per burst
coresight: tmc: introducing new header file
coresight: tmc: cleaning up header file
coresight: tmc: splitting driver in ETB/ETF and ETR components
coresight: tmc: making prepare/unprepare functions generic
coresight: tmc: allocating memory when needed
coresight: tmc: getting rid of multiple read access
coresight: tmc: adding mode of operation for link/sinks
coresight: tmc: dump system memory content only when needed
coresight: tmc: make sysFS and Perf mode mutually exclusive
coresight: tmc: keep track of memory width
coresight: moving struct cs_buffers to header file
coresight: tmc: implementing TMC-ETF AUX space API
coresight: tmc: implementing TMC-ETR AUX space API
coresight: configuring ETF in FIFO mode when acting as link

drivers/hwtracing/coresight/Makefile | 4 +-
drivers/hwtracing/coresight/coresight-etb10.c | 20 -
drivers/hwtracing/coresight/coresight-priv.h | 20 +
drivers/hwtracing/coresight/coresight-tmc-etf.c | 603 ++++++++++++++++++++++++
drivers/hwtracing/coresight/coresight-tmc-etr.c | 471 ++++++++++++++++++
drivers/hwtracing/coresight/coresight-tmc.c | 504 +++-----------------
drivers/hwtracing/coresight/coresight-tmc.h | 143 ++++++
drivers/hwtracing/coresight/coresight.c | 32 +-
8 files changed, 1345 insertions(+), 452 deletions(-)
create mode 100644 drivers/hwtracing/coresight/coresight-tmc-etf.c
create mode 100644 drivers/hwtracing/coresight/coresight-tmc-etr.c
create mode 100644 drivers/hwtracing/coresight/coresight-tmc.h

--
2.5.0

[toc] | [next] | [standalone]

#1388153 — [PATCH V4 09/18] coresight: tmc: allocating memory when needed

From	Mathieu Poirier <mathieu.poirier@linaro.org>
Date	2016-04-27 00:20 +0200
Subject	[PATCH V4 09/18] coresight: tmc: allocating memory when needed
Message-ID	<rsjuO-75I-27@gated-at.bofh.it>
In reply to	#1388152

In it's current form the TMC probe() function allocates
trace buffer memory at boot time, event if coresight isn't
used.  This is highly inefficient since trace buffers can
occupy a lot of memory that could be used otherwised.

This patch allocates trace buffers on the fly, when the
coresight subsystem is solicited.  Allocated buffers are
released when traces are read using the device descriptors
under /dev.

Signed-off-by: Mathieu Poirier <mathieu.poirier@linaro.org>
Reviewed-by: Suzuki K Poulose <suzuki.poulose@arm.com>
---
 drivers/hwtracing/coresight/coresight-tmc-etf.c | 90 +++++++++++++++++++++---
 drivers/hwtracing/coresight/coresight-tmc-etr.c | 93 +++++++++++++++++++++++--
 drivers/hwtracing/coresight/coresight-tmc.c     | 14 ----
 3 files changed, 170 insertions(+), 27 deletions(-)

diff --git a/drivers/hwtracing/coresight/coresight-tmc-etf.c b/drivers/hwtracing/coresight/coresight-tmc-etf.c
index 91e43572ce9f..6eb1665cfc9c 100644
--- a/drivers/hwtracing/coresight/coresight-tmc-etf.c
+++ b/drivers/hwtracing/coresight/coresight-tmc-etf.c
@@ -16,14 +16,12 @@
  */
 
 #include <linux/coresight.h>
+#include <linux/slab.h>
 #include "coresight-priv.h"
 #include "coresight-tmc.h"
 
 void tmc_etb_enable_hw(struct tmc_drvdata *drvdata)
 {
-	/* Zero out the memory to help with debug */
-	memset(drvdata->buf, 0, drvdata->size);
-
 	CS_UNLOCK(drvdata->base);
 
 	/* Wait for TMCSReady bit to be set */
@@ -110,21 +108,67 @@ static void tmc_etf_disable_hw(struct tmc_drvdata *drvdata)
 
 static int tmc_enable_etf_sink(struct coresight_device *csdev, u32 mode)
 {
+	int ret = 0;
+	bool used = false;
+	char *buf = NULL;
 	unsigned long flags;
 	struct tmc_drvdata *drvdata = dev_get_drvdata(csdev->dev.parent);
 
+	 /* This shouldn't be happening */
+	if (WARN_ON(mode != CS_MODE_SYSFS))
+		return -EINVAL;
+
+	/*
+	 * If we don't have a buffer release the lock and allocate memory.
+	 * Otherwise keep the lock and move along.
+	 */
 	spin_lock_irqsave(&drvdata->spinlock, flags);
-	if (drvdata->reading) {
+	if (!drvdata->buf) {
 		spin_unlock_irqrestore(&drvdata->spinlock, flags);
-		return -EBUSY;
+
+		/* Allocating the memory here while outside of the spinlock */
+		buf = kzalloc(drvdata->size, GFP_KERNEL);
+		if (!buf)
+			return -ENOMEM;
+
+		/* Let's try again */
+		spin_lock_irqsave(&drvdata->spinlock, flags);
+	}
+
+	if (drvdata->reading) {
+		ret = -EBUSY;
+		goto out;
+	}
+
+	/*
+	 * If drvdata::buf isn't NULL, memory was allocated for a previous
+	 * trace run but wasn't read.  If so simply zero-out the memory.
+	 * Otherwise use the memory allocated above.
+	 *
+	 * The memory is freed when users read the buffer using the
+	 * /dev/xyz.{etf|etb} interface.  See tmc_read_unprepare_etf() for
+	 * details.
+	 */
+	if (drvdata->buf) {
+		memset(drvdata->buf, 0, drvdata->size);
+	} else {
+		used = true;
+		drvdata->buf = buf;
 	}
 
 	tmc_etb_enable_hw(drvdata);
 	drvdata->enable = true;
+out:
 	spin_unlock_irqrestore(&drvdata->spinlock, flags);
 
-	dev_info(drvdata->dev, "TMC-ETB/ETF enabled\n");
-	return 0;
+	/* Free memory outside the spinlock if need be */
+	if (!used && buf)
+		kfree(buf);
+
+	if (!ret)
+		dev_info(drvdata->dev, "TMC-ETB/ETF enabled\n");
+
+	return ret;
 }
 
 static void tmc_disable_etf_sink(struct coresight_device *csdev)
@@ -223,6 +267,12 @@ int tmc_read_prepare_etb(struct tmc_drvdata *drvdata)
 		goto out;
 	}
 
+	/* If drvdata::buf is NULL the trace data has been read already */
+	if (drvdata->buf == NULL) {
+		ret = -EINVAL;
+		goto out;
+	}
+
 	/* Disable the TMC if need be */
 	if (drvdata->enable)
 		tmc_etb_disable_hw(drvdata);
@@ -236,6 +286,7 @@ out:
 
 int tmc_read_unprepare_etb(struct tmc_drvdata *drvdata)
 {
+	char *buf = NULL;
 	enum tmc_mode mode;
 	unsigned long flags;
 
@@ -254,11 +305,34 @@ int tmc_read_unprepare_etb(struct tmc_drvdata *drvdata)
 	}
 
 	/* Re-enable the TMC if need be */
-	if (drvdata->enable)
+	if (drvdata->enable) {
+		/*
+		 * The trace run will continue with the same allocated trace
+		 * buffer. As such zero-out the buffer so that we don't end
+		 * up with stale data.
+		 *
+		 * Since the tracer is still enabled drvdata::buf
+		 * can't be NULL.
+		 */
+		memset(drvdata->buf, 0, drvdata->size);
 		tmc_etb_enable_hw(drvdata);
+	} else {
+		/*
+		 * The ETB/ETF is not tracing and the buffer was just read.
+		 * As such prepare to free the trace buffer.
+		 */
+		buf = drvdata->buf;
+		drvdata->buf = NULL;
+	}
 
 	drvdata->reading = false;
 	spin_unlock_irqrestore(&drvdata->spinlock, flags);
 
+	/*
+	 * Free allocated memory outside of the spinlock.  There is no need
+	 * to assert the validity of 'buf' since calling kfree(NULL) is safe.
+	 */
+	kfree(buf);
+
 	return 0;
 }
diff --git a/drivers/hwtracing/coresight/coresight-tmc-etr.c b/drivers/hwtracing/coresight/coresight-tmc-etr.c
index 3483d139a4ac..ac37bf904fb7 100644
--- a/drivers/hwtracing/coresight/coresight-tmc-etr.c
+++ b/drivers/hwtracing/coresight/coresight-tmc-etr.c
@@ -16,6 +16,7 @@
  */
 
 #include <linux/coresight.h>
+#include <linux/dma-mapping.h>
 #include "coresight-priv.h"
 #include "coresight-tmc.h"
 
@@ -83,21 +84,71 @@ static void tmc_etr_disable_hw(struct tmc_drvdata *drvdata)
 
 static int tmc_enable_etr_sink(struct coresight_device *csdev, u32 mode)
 {
+	int ret = 0;
+	bool used = false;
 	unsigned long flags;
+	void __iomem *vaddr = NULL;
+	dma_addr_t paddr;
 	struct tmc_drvdata *drvdata = dev_get_drvdata(csdev->dev.parent);
 
+	 /* This shouldn't be happening */
+	if (WARN_ON(mode != CS_MODE_SYSFS))
+		return -EINVAL;
+
+	/*
+	 * If we don't have a buffer release the lock and allocate memory.
+	 * Otherwise keep the lock and move along.
+	 */
 	spin_lock_irqsave(&drvdata->spinlock, flags);
-	if (drvdata->reading) {
+	if (!drvdata->vaddr) {
 		spin_unlock_irqrestore(&drvdata->spinlock, flags);
-		return -EBUSY;
+
+		/*
+		 * Contiguous  memory can't be allocated while a spinlock is
+		 * held.  As such allocate memory here and free it if a buffer
+		 * has already been allocated (from a previous session).
+		 */
+		vaddr = dma_alloc_coherent(drvdata->dev, drvdata->size,
+					   &paddr, GFP_KERNEL);
+		if (!vaddr)
+			return -ENOMEM;
+
+		/* Let's try again */
+		spin_lock_irqsave(&drvdata->spinlock, flags);
+	}
+
+	if (drvdata->reading) {
+		ret = -EBUSY;
+		goto out;
+	}
+
+	/*
+	 * If drvdata::buf == NULL, use the memory allocated above.
+	 * Otherwise a buffer still exists from a previous session, so
+	 * simply use that.
+	 */
+	if (drvdata->buf == NULL) {
+		used = true;
+		drvdata->vaddr = vaddr;
+		drvdata->paddr = paddr;
+		drvdata->buf = drvdata->vaddr;
 	}
 
+	memset(drvdata->vaddr, 0, drvdata->size);
+
 	tmc_etr_enable_hw(drvdata);
 	drvdata->enable = true;
+out:
 	spin_unlock_irqrestore(&drvdata->spinlock, flags);
 
-	dev_info(drvdata->dev, "TMC-ETR enabled\n");
-	return 0;
+	/* Free memory outside the spinlock if need be */
+	if (!used && vaddr)
+		dma_free_coherent(drvdata->dev, drvdata->size, vaddr, paddr);
+
+	if (!ret)
+		dev_info(drvdata->dev, "TMC-ETR enabled\n");
+
+	return ret;
 }
 
 static void tmc_disable_etr_sink(struct coresight_device *csdev)
@@ -129,6 +180,7 @@ const struct coresight_ops tmc_etr_cs_ops = {
 
 int tmc_read_prepare_etr(struct tmc_drvdata *drvdata)
 {
+	int ret = 0;
 	unsigned long flags;
 
 	/* config types are set a boot time and never change */
@@ -137,11 +189,18 @@ int tmc_read_prepare_etr(struct tmc_drvdata *drvdata)
 
 	spin_lock_irqsave(&drvdata->spinlock, flags);
 
+	/* If drvdata::buf is NULL the trace data has been read already */
+	if (drvdata->buf == NULL) {
+		ret = -EINVAL;
+		goto out;
+	}
+
 	/* Disable the TMC if need be */
 	if (drvdata->enable)
 		tmc_etr_disable_hw(drvdata);
 
 	drvdata->reading = true;
+out:
 	spin_unlock_irqrestore(&drvdata->spinlock, flags);
 
 	return 0;
@@ -150,6 +209,8 @@ int tmc_read_prepare_etr(struct tmc_drvdata *drvdata)
 int tmc_read_unprepare_etr(struct tmc_drvdata *drvdata)
 {
 	unsigned long flags;
+	dma_addr_t paddr;
+	void __iomem *vaddr = NULL;
 
 	/* config types are set a boot time and never change */
 	if (WARN_ON_ONCE(drvdata->config_type != TMC_CONFIG_TYPE_ETR))
@@ -158,11 +219,33 @@ int tmc_read_unprepare_etr(struct tmc_drvdata *drvdata)
 	spin_lock_irqsave(&drvdata->spinlock, flags);
 
 	/* RE-enable the TMC if need be */
-	if (drvdata->enable)
+	if (drvdata->enable) {
+		/*
+		 * The trace run will continue with the same allocated trace
+		 * buffer. As such zero-out the buffer so that we don't end
+		 * up with stale data.
+		 *
+		 * Since the tracer is still enabled drvdata::buf
+		 * can't be NULL.
+		 */
+		memset(drvdata->buf, 0, drvdata->size);
 		tmc_etr_enable_hw(drvdata);
+	} else {
+		/*
+		 * The ETR is not tracing and the buffer was just read.
+		 * As such prepare to free the trace buffer.
+		 */
+		vaddr = drvdata->vaddr;
+		paddr = drvdata->paddr;
+		drvdata->buf = NULL;
+	}
 
 	drvdata->reading = false;
 	spin_unlock_irqrestore(&drvdata->spinlock, flags);
 
+	/* Free allocated memory out side of the spinlock */
+	if (vaddr)
+		dma_free_coherent(drvdata->dev, drvdata->size, vaddr, paddr);
+
 	return 0;
 }
diff --git a/drivers/hwtracing/coresight/coresight-tmc.c b/drivers/hwtracing/coresight/coresight-tmc.c
index 63f8e55116a6..e8e12a9b917a 100644
--- a/drivers/hwtracing/coresight/coresight-tmc.c
+++ b/drivers/hwtracing/coresight/coresight-tmc.c
@@ -319,20 +319,6 @@ static int tmc_probe(struct amba_device *adev, const struct amba_id *id)
 
 	pm_runtime_put(&adev->dev);
 
-	if (drvdata->config_type == TMC_CONFIG_TYPE_ETR) {
-		drvdata->vaddr = dma_alloc_coherent(dev, drvdata->size,
-						&drvdata->paddr, GFP_KERNEL);
-		if (!drvdata->vaddr)
-			return -ENOMEM;
-
-		memset(drvdata->vaddr, 0, drvdata->size);
-		drvdata->buf = drvdata->vaddr;
-	} else {
-		drvdata->buf = devm_kzalloc(dev, drvdata->size, GFP_KERNEL);
-		if (!drvdata->buf)
-			return -ENOMEM;
-	}
-
 	desc = devm_kzalloc(dev, sizeof(*desc), GFP_KERNEL);
 	if (!desc) {
 		ret = -ENOMEM;
-- 
2.5.0

From	Suzuki K Poulose <Suzuki.Poulose@arm.com>
Date	2016-04-27 13:30 +0200
Subject	Re: [PATCH V4 16/18] coresight: tmc: implementing TMC-ETF AUX space API
Message-ID	<rsvPk-jK-17@gated-at.bofh.it>
In reply to	#1388159

[PATCH V4 00/18] coresight: tmc: make driver usable by Perf

Contents

#1388152 — [PATCH V4 00/18] coresight: tmc: make driver usable by Perf

#1388153 — [PATCH V4 09/18] coresight: tmc: allocating memory when needed

#1388154 — [PATCH V4 14/18] coresight: tmc: keep track of memory width

#1388157 — [PATCH V4 15/18] coresight: moving struct cs_buffers to header file

#1388158 — [PATCH V4 04/18] coresight: tmc: clearly define number of transfers per burst

#1388159 — [PATCH V4 16/18] coresight: tmc: implementing TMC-ETF AUX space API

#1388914 — Re: [PATCH V4 16/18] coresight: tmc: implementing TMC-ETF AUX space API

#1389397 — Re: [PATCH V4 16/18] coresight: tmc: implementing TMC-ETF AUX space API

#1389401 — Re: [PATCH V4 16/18] coresight: tmc: implementing TMC-ETF AUX space API

#1388161 — [PATCH V4 11/18] coresight: tmc: adding mode of operation for link/sinks

#1388818 — Re: [PATCH V4 11/18] coresight: tmc: adding mode of operation for link/sinks

#1388162 — [PATCH V4 02/18] coresight: tmc: waiting for TMCReady bit before programming

#1388164 — [PATCH V4 10/18] coresight: tmc: getting rid of multiple read access

#1388807 — Re: [PATCH V4 10/18] coresight: tmc: getting rid of multiple read access

#1388165 — [PATCH V4 05/18] coresight: tmc: introducing new header file

#1388166 — [PATCH V4 08/18] coresight: tmc: making prepare/unprepare functions generic

#1388168 — [PATCH V4 18/18] coresight: configuring ETF in FIFO mode when acting as link

#1388172 — [PATCH V4 06/18] coresight: tmc: cleaning up header file