[SRU][N][PATCH v3 1/1] coresight: Remove atomic type from refcnt
Noah Wager
noah.wager at canonical.com
Sun Mar 1 02:08:48 UTC 2026
From: James Clark <james.clark at arm.com>
BugLink: https://bugs.launchpad.net/bugs/2142336
Refcnt is only ever accessed from either inside the coresight_mutex, or
the device's spinlock, making the atomic type and atomic_dec_return()
calls confusing and unnecessary. The only point of synchronisation
outside of these two types of locks is already done with a compare and
swap on 'mode', which a comment has been added for.
There was one instance of refcnt being used outside of a lock in TPIU,
but that can easily be fixed by making it the same as all the other
devices and adding a spinlock. Potentially in the future all the
refcounting and locking can be moved up into the core code, and all the
mostly duplicate code from the individual devices can be removed.
Signed-off-by: James Clark <james.clark at arm.com>
Link: https://lore.kernel.org/r/20240129154050.569566-8-james.clark@arm.com
Signed-off-by: Suzuki K Poulose <suzuki.poulose at arm.com>
(backported from commit 4545b38ef004a586295750ea49a505b6396a7c90)
[nwager: This patch fixes compile issues introduced by
"coresight: catu: Introduce refcount and spinlock for enabling/disabling".
Backported due to context changes because of missing patchset:
"coresight: Separate sysfs and Perf usage and some other cleanups".
Only this commit was a missing dependency of "coresight: catu: Introduce
refcount and spinlock for enabling/disabling", and since this change is
simple (basically a find/replace of refcnt atomic_read/inc to int accesses),
I just fixed up the context changes instead of porting the five previous
patches from the series. Here is the gist of the changes I made:
- previous line context changes due to different way of checking CS_MODE_SYSFS
- some occurrences were removed before this patch arrived upstream, so
replace those occurrences in my fixup
- upstream moved some hunks to coresight-sysfs.c, so I needed to
replace the occurrences in the original locations
- drop the 'mode' comment due to missing commit from series
- the upstream commit has some different refcnt references due to other
commits from the patchset that we are missing, like in
drivers/hwtracing/coresight/coresight-tpda.c, so those are not
present in this backport.
Finally, if a downstream kernel backports the whole missing patchset,
this can be reverted in favor of a clean upstream cherrypick.]
Signed-off-by: Noah Wager <noah.wager at canonical.com>
Signed-off-by: Noah Wager <noah.wager at canonical.com>
---
drivers/hwtracing/coresight/coresight-core.c | 9 +++++----
drivers/hwtracing/coresight/coresight-etb10.c | 11 +++++-----
.../hwtracing/coresight/coresight-tmc-etf.c | 20 ++++++++++---------
.../hwtracing/coresight/coresight-tmc-etr.c | 13 ++++++------
drivers/hwtracing/coresight/coresight-tpiu.c | 14 +++++++++++--
drivers/hwtracing/coresight/ultrasoc-smb.c | 9 +++++----
include/linux/coresight.h | 7 +++++--
7 files changed, 51 insertions(+), 32 deletions(-)
diff --git a/drivers/hwtracing/coresight/coresight-core.c b/drivers/hwtracing/coresight/coresight-core.c
index fc0e7f18d95b..76abcea8cfb1 100644
--- a/drivers/hwtracing/coresight/coresight-core.c
+++ b/drivers/hwtracing/coresight/coresight-core.c
@@ -370,7 +370,7 @@ static void coresight_disable_link(struct coresight_device *csdev,
0)
return;
} else {
- if (atomic_read(&csdev->refcnt) != 0)
+ if (csdev->refcnt != 0)
return;
}
@@ -391,7 +391,7 @@ int coresight_enable_source(struct coresight_device *csdev, enum cs_mode mode,
csdev->enable = true;
}
- atomic_inc(&csdev->refcnt);
+ csdev->refcnt++;
return 0;
}
@@ -473,7 +473,8 @@ EXPORT_SYMBOL_GPL(coresight_disable_source);
static bool coresight_disable_source_sysfs(struct coresight_device *csdev,
void *data)
{
- if (atomic_dec_return(&csdev->refcnt) == 0) {
+ csdev->refcnt--;
+ if (csdev->refcnt == 0) {
coresight_disable_source(csdev, data);
csdev->enable = false;
}
@@ -1142,7 +1143,7 @@ int coresight_enable(struct coresight_device *csdev)
* source is already enabled.
*/
if (subtype == CORESIGHT_DEV_SUBTYPE_SOURCE_SOFTWARE)
- atomic_inc(&csdev->refcnt);
+ csdev->refcnt++;
goto out;
}
diff --git a/drivers/hwtracing/coresight/coresight-etb10.c b/drivers/hwtracing/coresight/coresight-etb10.c
index d83d52dbd1a2..b2d0069bd21c 100644
--- a/drivers/hwtracing/coresight/coresight-etb10.c
+++ b/drivers/hwtracing/coresight/coresight-etb10.c
@@ -163,7 +163,7 @@ static int etb_enable_sysfs(struct coresight_device *csdev)
drvdata->mode = CS_MODE_SYSFS;
}
- atomic_inc(&csdev->refcnt);
+ csdev->refcnt++;
out:
raw_spin_unlock_irqrestore(&drvdata->spinlock, flags);
return ret;
@@ -199,7 +199,7 @@ static int etb_enable_perf(struct coresight_device *csdev, void *data)
* use for this session.
*/
if (drvdata->pid == pid) {
- atomic_inc(&csdev->refcnt);
+ csdev->refcnt++;
goto out;
}
@@ -217,7 +217,7 @@ static int etb_enable_perf(struct coresight_device *csdev, void *data)
/* Associate with monitored process. */
drvdata->pid = pid;
drvdata->mode = CS_MODE_PERF;
- atomic_inc(&csdev->refcnt);
+ csdev->refcnt++;
}
out:
@@ -356,7 +356,8 @@ static int etb_disable(struct coresight_device *csdev)
raw_spin_lock_irqsave(&drvdata->spinlock, flags);
- if (atomic_dec_return(&csdev->refcnt)) {
+ csdev->refcnt--;
+ if (csdev->refcnt) {
raw_spin_unlock_irqrestore(&drvdata->spinlock, flags);
return -EBUSY;
}
@@ -447,7 +448,7 @@ static unsigned long etb_update_buffer(struct coresight_device *csdev,
raw_spin_lock_irqsave(&drvdata->spinlock, flags);
/* Don't do anything if another tracer is using this sink */
- if (atomic_read(&csdev->refcnt) != 1)
+ if (csdev->refcnt != 1)
goto out;
__etb_disable_hw(drvdata);
diff --git a/drivers/hwtracing/coresight/coresight-tmc-etf.c b/drivers/hwtracing/coresight/coresight-tmc-etf.c
index 7406b65e2cdd..b7ecf660309a 100644
--- a/drivers/hwtracing/coresight/coresight-tmc-etf.c
+++ b/drivers/hwtracing/coresight/coresight-tmc-etf.c
@@ -206,7 +206,7 @@ static int tmc_enable_etf_sink_sysfs(struct coresight_device *csdev)
* touched.
*/
if (drvdata->mode == CS_MODE_SYSFS) {
- atomic_inc(&csdev->refcnt);
+ csdev->refcnt++;
goto out;
}
@@ -229,7 +229,7 @@ static int tmc_enable_etf_sink_sysfs(struct coresight_device *csdev)
ret = tmc_etb_enable_hw(drvdata);
if (!ret) {
drvdata->mode = CS_MODE_SYSFS;
- atomic_inc(&csdev->refcnt);
+ csdev->refcnt++;
} else {
/* Free up the buffer if we failed to enable */
used = false;
@@ -284,7 +284,7 @@ static int tmc_enable_etf_sink_perf(struct coresight_device *csdev, void *data)
* use for this session.
*/
if (drvdata->pid == pid) {
- atomic_inc(&csdev->refcnt);
+ csdev->refcnt++;
break;
}
@@ -293,7 +293,7 @@ static int tmc_enable_etf_sink_perf(struct coresight_device *csdev, void *data)
/* Associate with monitored process. */
drvdata->pid = pid;
drvdata->mode = CS_MODE_PERF;
- atomic_inc(&csdev->refcnt);
+ csdev->refcnt++;
}
} while (0);
spin_unlock_irqrestore(&drvdata->spinlock, flags);
@@ -338,7 +338,8 @@ static int tmc_disable_etf_sink(struct coresight_device *csdev)
return -EBUSY;
}
- if (atomic_dec_return(&csdev->refcnt)) {
+ csdev->refcnt--;
+ if (csdev->refcnt) {
spin_unlock_irqrestore(&drvdata->spinlock, flags);
return -EBUSY;
}
@@ -371,7 +372,7 @@ static int tmc_enable_etf_link(struct coresight_device *csdev,
return -EBUSY;
}
- if (atomic_read(&csdev->refcnt) == 0) {
+ if (csdev->refcnt == 0) {
ret = tmc_etf_enable_hw(drvdata);
if (!ret) {
drvdata->mode = CS_MODE_SYSFS;
@@ -379,7 +380,7 @@ static int tmc_enable_etf_link(struct coresight_device *csdev,
}
}
if (!ret)
- atomic_inc(&csdev->refcnt);
+ csdev->refcnt++;
spin_unlock_irqrestore(&drvdata->spinlock, flags);
if (first_enable)
@@ -401,7 +402,8 @@ static void tmc_disable_etf_link(struct coresight_device *csdev,
return;
}
- if (atomic_dec_return(&csdev->refcnt) == 0) {
+ csdev->refcnt--;
+ if (csdev->refcnt == 0) {
tmc_etf_disable_hw(drvdata);
drvdata->mode = CS_MODE_DISABLED;
last_disable = true;
@@ -489,7 +491,7 @@ static unsigned long tmc_update_etf_buffer(struct coresight_device *csdev,
spin_lock_irqsave(&drvdata->spinlock, flags);
/* Don't do anything if another tracer is using this sink */
- if (atomic_read(&csdev->refcnt) != 1)
+ if (csdev->refcnt != 1)
goto out;
CS_UNLOCK(drvdata->base);
diff --git a/drivers/hwtracing/coresight/coresight-tmc-etr.c b/drivers/hwtracing/coresight/coresight-tmc-etr.c
index 52c55e8172ee..b1c55e7d3d3b 100644
--- a/drivers/hwtracing/coresight/coresight-tmc-etr.c
+++ b/drivers/hwtracing/coresight/coresight-tmc-etr.c
@@ -1231,14 +1231,14 @@ static int tmc_enable_etr_sink_sysfs(struct coresight_device *csdev)
* touched, even if the buffer size has changed.
*/
if (drvdata->mode == CS_MODE_SYSFS) {
- atomic_inc(&csdev->refcnt);
+ csdev->refcnt++;
goto out;
}
ret = tmc_etr_enable_hw(drvdata, sysfs_buf);
if (!ret) {
drvdata->mode = CS_MODE_SYSFS;
- atomic_inc(&csdev->refcnt);
+ csdev->refcnt++;
}
out:
@@ -1564,7 +1564,7 @@ tmc_update_etr_buffer(struct coresight_device *csdev,
spin_lock_irqsave(&drvdata->spinlock, flags);
/* Don't do anything if another tracer is using this sink */
- if (atomic_read(&csdev->refcnt) != 1) {
+ if (csdev->refcnt != 1) {
spin_unlock_irqrestore(&drvdata->spinlock, flags);
goto out;
}
@@ -1676,7 +1676,7 @@ static int tmc_enable_etr_sink_perf(struct coresight_device *csdev, void *data)
* use for this session.
*/
if (drvdata->pid == pid) {
- atomic_inc(&csdev->refcnt);
+ csdev->refcnt++;
goto unlock_out;
}
@@ -1686,7 +1686,7 @@ static int tmc_enable_etr_sink_perf(struct coresight_device *csdev, void *data)
drvdata->pid = pid;
drvdata->mode = CS_MODE_PERF;
drvdata->perf_buf = etr_perf->etr_buf;
- atomic_inc(&csdev->refcnt);
+ csdev->refcnt++;
}
unlock_out:
@@ -1719,7 +1719,8 @@ static int tmc_disable_etr_sink(struct coresight_device *csdev)
return -EBUSY;
}
- if (atomic_dec_return(&csdev->refcnt)) {
+ csdev->refcnt--;
+ if (csdev->refcnt) {
spin_unlock_irqrestore(&drvdata->spinlock, flags);
return -EBUSY;
}
diff --git a/drivers/hwtracing/coresight/coresight-tpiu.c b/drivers/hwtracing/coresight/coresight-tpiu.c
index 59eac93fd6bb..c23a6b9b41fe 100644
--- a/drivers/hwtracing/coresight/coresight-tpiu.c
+++ b/drivers/hwtracing/coresight/coresight-tpiu.c
@@ -58,6 +58,7 @@ struct tpiu_drvdata {
void __iomem *base;
struct clk *atclk;
struct coresight_device *csdev;
+ spinlock_t spinlock;
};
static void tpiu_enable_hw(struct csdev_access *csa)
@@ -72,8 +73,11 @@ static void tpiu_enable_hw(struct csdev_access *csa)
static int tpiu_enable(struct coresight_device *csdev, enum cs_mode mode,
void *__unused)
{
+ struct tpiu_drvdata *drvdata = dev_get_drvdata(csdev->dev.parent);
+
+ guard(spinlock)(&drvdata->spinlock);
tpiu_enable_hw(&csdev->access);
- atomic_inc(&csdev->refcnt);
+ csdev->refcnt++;
dev_dbg(&csdev->dev, "TPIU enabled\n");
return 0;
}
@@ -96,7 +100,11 @@ static void tpiu_disable_hw(struct csdev_access *csa)
static int tpiu_disable(struct coresight_device *csdev)
{
- if (atomic_dec_return(&csdev->refcnt))
+ struct tpiu_drvdata *drvdata = dev_get_drvdata(csdev->dev.parent);
+
+ guard(spinlock)(&drvdata->spinlock);
+ csdev->refcnt--;
+ if (csdev->refcnt)
return -EBUSY;
tpiu_disable_hw(&csdev->access);
@@ -132,6 +140,8 @@ static int tpiu_probe(struct amba_device *adev, const struct amba_id *id)
if (!drvdata)
return -ENOMEM;
+ spin_lock_init(&drvdata->spinlock);
+
drvdata->atclk = devm_clk_get(&adev->dev, "atclk"); /* optional */
if (!IS_ERR(drvdata->atclk)) {
ret = clk_prepare_enable(drvdata->atclk);
diff --git a/drivers/hwtracing/coresight/ultrasoc-smb.c b/drivers/hwtracing/coresight/ultrasoc-smb.c
index 10e886455b8b..2c1227fd701b 100644
--- a/drivers/hwtracing/coresight/ultrasoc-smb.c
+++ b/drivers/hwtracing/coresight/ultrasoc-smb.c
@@ -103,7 +103,7 @@ static int smb_open(struct inode *inode, struct file *file)
if (drvdata->reading)
return -EBUSY;
- if (atomic_read(&drvdata->csdev->refcnt))
+ if (drvdata->csdev->refcnt)
return -EBUSY;
smb_update_data_size(drvdata);
@@ -270,7 +270,7 @@ static int smb_enable(struct coresight_device *csdev, enum cs_mode mode,
if (ret)
return ret;
- atomic_inc(&csdev->refcnt);
+ csdev->refcnt++;
dev_dbg(&csdev->dev, "Ultrasoc SMB enabled\n");
return ret;
@@ -285,7 +285,8 @@ static int smb_disable(struct coresight_device *csdev)
if (drvdata->reading)
return -EBUSY;
- if (atomic_dec_return(&csdev->refcnt))
+ csdev->refcnt--;
+ if (csdev->refcnt)
return -EBUSY;
/* Complain if we (somehow) got out of sync */
@@ -380,7 +381,7 @@ static unsigned long smb_update_buffer(struct coresight_device *csdev,
guard(spinlock)(&drvdata->spinlock);
/* Don't do anything if another tracer is using this sink. */
- if (atomic_read(&csdev->refcnt) != 1)
+ if (csdev->refcnt != 1)
return 0;
smb_disable_hw(drvdata);
diff --git a/include/linux/coresight.h b/include/linux/coresight.h
index 4c9ff95c2c87..7a908907a652 100644
--- a/include/linux/coresight.h
+++ b/include/linux/coresight.h
@@ -226,7 +226,10 @@ struct coresight_sysfs_link {
* by @coresight_ops.
* @access: Device i/o access abstraction for this device.
* @dev: The device entity associated to this component.
- * @refcnt: keep track of what is in use.
+ * @refcnt: keep track of what is in use. Only access this outside of the
+ * device's spinlock when the coresight_mutex held and mode ==
+ * CS_MODE_SYSFS. Otherwise it must be accessed from inside the
+ * spinlock.
* @orphan: true if the component has connections that haven't been linked.
* @enable: 'true' if component is currently part of an active path.
* @activated: 'true' only if a _sink_ has been activated. A sink can be
@@ -250,7 +253,7 @@ struct coresight_device {
const struct coresight_ops *ops;
struct csdev_access access;
struct device dev;
- atomic_t refcnt;
+ int refcnt;
bool orphan;
bool enable; /* true only if configured as part of a path */
/* sink specific fields */
--
2.43.0
More information about the kernel-team
mailing list