[SRU][N][PATCH v2 1/1] coresight: Remove atomic type from refcnt
Noah Wager
noah.wager at canonical.com
Wed Feb 25 23:11:53 UTC 2026
On Mon, Feb 23, 2026 at 11:33:59PM -0800, Noah Wager wrote:
> From: James Clark <james.clark at arm.com>
I forgot to add a buglink here. It should be:
>
> BugLink: https://bugs.launchpad.net/bugs/2142336
Let me know if this can be fixed when applying, or if I should send a v3
with it added.
Thanks!
- Noah
>
> 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: Context changes due to 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]
> 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..52d50545d5c9 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