[SRU][N][PATCH v2 1/1] coresight: Remove atomic type from refcnt
Stefan Bader
stefan.bader at canonical.com
Thu Feb 26 11:33:35 UTC 2026
On 26/02/2026 00:11, Noah Wager wrote:
> 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.
We can adjust when applying.
-Stefan
>
> 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
>>
>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: OpenPGP_0xE8675DEECBEECEA3.asc
Type: application/pgp-keys
Size: 52669 bytes
Desc: OpenPGP public key
URL: <https://lists.ubuntu.com/archives/kernel-team/attachments/20260226/193b977a/attachment-0001.key>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: OpenPGP_signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: OpenPGP digital signature
URL: <https://lists.ubuntu.com/archives/kernel-team/attachments/20260226/193b977a/attachment-0001.sig>
More information about the kernel-team
mailing list