* [PATCH] coresight: Drop atomics in connection refcounts
@ 2024-11-28 12:14 James Clark
2024-12-05 10:10 ` Yeoreum Yun
2024-12-11 10:20 ` Suzuki K Poulose
0 siblings, 2 replies; 3+ messages in thread
From: James Clark @ 2024-11-28 12:14 UTC (permalink / raw)
To: suzuki.poulose, coresight, mike.leach, yeoreum.yun
Cc: James Clark, Alexander Shishkin, linux-arm-kernel, linux-kernel
These belong to the device being enabled or disabled and are only ever
used inside the device's spinlock. Remove the atomics to not imply that
there are any other concurrent accesses.
If atomics were necessary I don't think they would have been enough
anyway. There would be nothing to prevent an enable or disable running
concurrently if not for the spinlock.
Signed-off-by: James Clark <james.clark@linaro.org>
---
drivers/hwtracing/coresight/coresight-funnel.c | 6 +++---
drivers/hwtracing/coresight/coresight-replicator.c | 6 +++---
drivers/hwtracing/coresight/coresight-tpda.c | 6 +++---
include/linux/coresight.h | 4 ++--
4 files changed, 11 insertions(+), 11 deletions(-)
diff --git a/drivers/hwtracing/coresight/coresight-funnel.c b/drivers/hwtracing/coresight/coresight-funnel.c
index 5a819c8970fb..bd32f74cbdae 100644
--- a/drivers/hwtracing/coresight/coresight-funnel.c
+++ b/drivers/hwtracing/coresight/coresight-funnel.c
@@ -86,14 +86,14 @@ static int funnel_enable(struct coresight_device *csdev,
bool first_enable = false;
spin_lock_irqsave(&drvdata->spinlock, flags);
- if (atomic_read(&in->dest_refcnt) == 0) {
+ if (in->dest_refcnt == 0) {
if (drvdata->base)
rc = dynamic_funnel_enable_hw(drvdata, in->dest_port);
if (!rc)
first_enable = true;
}
if (!rc)
- atomic_inc(&in->dest_refcnt);
+ in->dest_refcnt++;
spin_unlock_irqrestore(&drvdata->spinlock, flags);
if (first_enable)
@@ -130,7 +130,7 @@ static void funnel_disable(struct coresight_device *csdev,
bool last_disable = false;
spin_lock_irqsave(&drvdata->spinlock, flags);
- if (atomic_dec_return(&in->dest_refcnt) == 0) {
+ if (--in->dest_refcnt == 0) {
if (drvdata->base)
dynamic_funnel_disable_hw(drvdata, in->dest_port);
last_disable = true;
diff --git a/drivers/hwtracing/coresight/coresight-replicator.c b/drivers/hwtracing/coresight/coresight-replicator.c
index 3e55be9c8418..31322aea19f2 100644
--- a/drivers/hwtracing/coresight/coresight-replicator.c
+++ b/drivers/hwtracing/coresight/coresight-replicator.c
@@ -126,7 +126,7 @@ static int replicator_enable(struct coresight_device *csdev,
bool first_enable = false;
spin_lock_irqsave(&drvdata->spinlock, flags);
- if (atomic_read(&out->src_refcnt) == 0) {
+ if (out->src_refcnt == 0) {
if (drvdata->base)
rc = dynamic_replicator_enable(drvdata, in->dest_port,
out->src_port);
@@ -134,7 +134,7 @@ static int replicator_enable(struct coresight_device *csdev,
first_enable = true;
}
if (!rc)
- atomic_inc(&out->src_refcnt);
+ out->src_refcnt++;
spin_unlock_irqrestore(&drvdata->spinlock, flags);
if (first_enable)
@@ -180,7 +180,7 @@ static void replicator_disable(struct coresight_device *csdev,
bool last_disable = false;
spin_lock_irqsave(&drvdata->spinlock, flags);
- if (atomic_dec_return(&out->src_refcnt) == 0) {
+ if (--out->src_refcnt == 0) {
if (drvdata->base)
dynamic_replicator_disable(drvdata, in->dest_port,
out->src_port);
diff --git a/drivers/hwtracing/coresight/coresight-tpda.c b/drivers/hwtracing/coresight/coresight-tpda.c
index bfca103f9f84..4ec676bea1ce 100644
--- a/drivers/hwtracing/coresight/coresight-tpda.c
+++ b/drivers/hwtracing/coresight/coresight-tpda.c
@@ -190,10 +190,10 @@ static int tpda_enable(struct coresight_device *csdev,
int ret = 0;
spin_lock(&drvdata->spinlock);
- if (atomic_read(&in->dest_refcnt) == 0) {
+ if (in->dest_refcnt == 0) {
ret = __tpda_enable(drvdata, in->dest_port);
if (!ret) {
- atomic_inc(&in->dest_refcnt);
+ in->dest_refcnt++;
csdev->refcnt++;
dev_dbg(drvdata->dev, "TPDA inport %d enabled.\n", in->dest_port);
}
@@ -223,7 +223,7 @@ static void tpda_disable(struct coresight_device *csdev,
struct tpda_drvdata *drvdata = dev_get_drvdata(csdev->dev.parent);
spin_lock(&drvdata->spinlock);
- if (atomic_dec_return(&in->dest_refcnt) == 0) {
+ if (--in->dest_refcnt == 0) {
__tpda_disable(drvdata, in->dest_port);
csdev->refcnt--;
}
diff --git a/include/linux/coresight.h b/include/linux/coresight.h
index c13342594278..834029cb9ba2 100644
--- a/include/linux/coresight.h
+++ b/include/linux/coresight.h
@@ -200,8 +200,8 @@ struct coresight_connection {
struct coresight_device *dest_dev;
struct coresight_sysfs_link *link;
struct coresight_device *src_dev;
- atomic_t src_refcnt;
- atomic_t dest_refcnt;
+ int src_refcnt;
+ int dest_refcnt;
};
/**
--
2.34.1
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] coresight: Drop atomics in connection refcounts
2024-11-28 12:14 [PATCH] coresight: Drop atomics in connection refcounts James Clark
@ 2024-12-05 10:10 ` Yeoreum Yun
2024-12-11 10:20 ` Suzuki K Poulose
1 sibling, 0 replies; 3+ messages in thread
From: Yeoreum Yun @ 2024-12-05 10:10 UTC (permalink / raw)
To: James Clark
Cc: suzuki.poulose, coresight, mike.leach, Alexander Shishkin,
linux-arm-kernel, linux-kernel
This patch looks good to me.
Reviewed-by: Yeoreum Yun <yeoreum.yun@arm.com>
On Thu, Nov 28, 2024 at 12:14:14PM +0000, James Clark wrote:
> These belong to the device being enabled or disabled and are only ever
> used inside the device's spinlock. Remove the atomics to not imply that
> there are any other concurrent accesses.
>
> If atomics were necessary I don't think they would have been enough
> anyway. There would be nothing to prevent an enable or disable running
> concurrently if not for the spinlock.
>
> Signed-off-by: James Clark <james.clark@linaro.org>
> ---
> drivers/hwtracing/coresight/coresight-funnel.c | 6 +++---
> drivers/hwtracing/coresight/coresight-replicator.c | 6 +++---
> drivers/hwtracing/coresight/coresight-tpda.c | 6 +++---
> include/linux/coresight.h | 4 ++--
> 4 files changed, 11 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/hwtracing/coresight/coresight-funnel.c b/drivers/hwtracing/coresight/coresight-funnel.c
> index 5a819c8970fb..bd32f74cbdae 100644
> --- a/drivers/hwtracing/coresight/coresight-funnel.c
> +++ b/drivers/hwtracing/coresight/coresight-funnel.c
> @@ -86,14 +86,14 @@ static int funnel_enable(struct coresight_device *csdev,
> bool first_enable = false;
>
> spin_lock_irqsave(&drvdata->spinlock, flags);
> - if (atomic_read(&in->dest_refcnt) == 0) {
> + if (in->dest_refcnt == 0) {
> if (drvdata->base)
> rc = dynamic_funnel_enable_hw(drvdata, in->dest_port);
> if (!rc)
> first_enable = true;
> }
> if (!rc)
> - atomic_inc(&in->dest_refcnt);
> + in->dest_refcnt++;
> spin_unlock_irqrestore(&drvdata->spinlock, flags);
>
> if (first_enable)
> @@ -130,7 +130,7 @@ static void funnel_disable(struct coresight_device *csdev,
> bool last_disable = false;
>
> spin_lock_irqsave(&drvdata->spinlock, flags);
> - if (atomic_dec_return(&in->dest_refcnt) == 0) {
> + if (--in->dest_refcnt == 0) {
> if (drvdata->base)
> dynamic_funnel_disable_hw(drvdata, in->dest_port);
> last_disable = true;
> diff --git a/drivers/hwtracing/coresight/coresight-replicator.c b/drivers/hwtracing/coresight/coresight-replicator.c
> index 3e55be9c8418..31322aea19f2 100644
> --- a/drivers/hwtracing/coresight/coresight-replicator.c
> +++ b/drivers/hwtracing/coresight/coresight-replicator.c
> @@ -126,7 +126,7 @@ static int replicator_enable(struct coresight_device *csdev,
> bool first_enable = false;
>
> spin_lock_irqsave(&drvdata->spinlock, flags);
> - if (atomic_read(&out->src_refcnt) == 0) {
> + if (out->src_refcnt == 0) {
> if (drvdata->base)
> rc = dynamic_replicator_enable(drvdata, in->dest_port,
> out->src_port);
> @@ -134,7 +134,7 @@ static int replicator_enable(struct coresight_device *csdev,
> first_enable = true;
> }
> if (!rc)
> - atomic_inc(&out->src_refcnt);
> + out->src_refcnt++;
> spin_unlock_irqrestore(&drvdata->spinlock, flags);
>
> if (first_enable)
> @@ -180,7 +180,7 @@ static void replicator_disable(struct coresight_device *csdev,
> bool last_disable = false;
>
> spin_lock_irqsave(&drvdata->spinlock, flags);
> - if (atomic_dec_return(&out->src_refcnt) == 0) {
> + if (--out->src_refcnt == 0) {
> if (drvdata->base)
> dynamic_replicator_disable(drvdata, in->dest_port,
> out->src_port);
> diff --git a/drivers/hwtracing/coresight/coresight-tpda.c b/drivers/hwtracing/coresight/coresight-tpda.c
> index bfca103f9f84..4ec676bea1ce 100644
> --- a/drivers/hwtracing/coresight/coresight-tpda.c
> +++ b/drivers/hwtracing/coresight/coresight-tpda.c
> @@ -190,10 +190,10 @@ static int tpda_enable(struct coresight_device *csdev,
> int ret = 0;
>
> spin_lock(&drvdata->spinlock);
> - if (atomic_read(&in->dest_refcnt) == 0) {
> + if (in->dest_refcnt == 0) {
> ret = __tpda_enable(drvdata, in->dest_port);
> if (!ret) {
> - atomic_inc(&in->dest_refcnt);
> + in->dest_refcnt++;
> csdev->refcnt++;
> dev_dbg(drvdata->dev, "TPDA inport %d enabled.\n", in->dest_port);
> }
> @@ -223,7 +223,7 @@ static void tpda_disable(struct coresight_device *csdev,
> struct tpda_drvdata *drvdata = dev_get_drvdata(csdev->dev.parent);
>
> spin_lock(&drvdata->spinlock);
> - if (atomic_dec_return(&in->dest_refcnt) == 0) {
> + if (--in->dest_refcnt == 0) {
> __tpda_disable(drvdata, in->dest_port);
> csdev->refcnt--;
> }
> diff --git a/include/linux/coresight.h b/include/linux/coresight.h
> index c13342594278..834029cb9ba2 100644
> --- a/include/linux/coresight.h
> +++ b/include/linux/coresight.h
> @@ -200,8 +200,8 @@ struct coresight_connection {
> struct coresight_device *dest_dev;
> struct coresight_sysfs_link *link;
> struct coresight_device *src_dev;
> - atomic_t src_refcnt;
> - atomic_t dest_refcnt;
> + int src_refcnt;
> + int dest_refcnt;
> };
>
> /**
> --
> 2.34.1
>
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] coresight: Drop atomics in connection refcounts
2024-11-28 12:14 [PATCH] coresight: Drop atomics in connection refcounts James Clark
2024-12-05 10:10 ` Yeoreum Yun
@ 2024-12-11 10:20 ` Suzuki K Poulose
1 sibling, 0 replies; 3+ messages in thread
From: Suzuki K Poulose @ 2024-12-11 10:20 UTC (permalink / raw)
To: James Clark, mike.leach, coresight, yeoreum.yun
Cc: Suzuki K Poulose, Alexander Shishkin, linux-kernel, linux-arm-kernel
On Thu, 28 Nov 2024 12:14:14 +0000, James Clark wrote:
> These belong to the device being enabled or disabled and are only ever
> used inside the device's spinlock. Remove the atomics to not imply that
> there are any other concurrent accesses.
>
> If atomics were necessary I don't think they would have been enough
> anyway. There would be nothing to prevent an enable or disable running
> concurrently if not for the spinlock.
>
> [...]
Applied, thanks!
[1/1] coresight: Drop atomics in connection refcounts
https://git.kernel.org/coresight/c/5aec7c065fba0c56d6c1ea5d629395210f174be8
Best regards,
--
Suzuki K Poulose <suzuki.poulose@arm.com>
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2024-12-11 10:20 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2024-11-28 12:14 [PATCH] coresight: Drop atomics in connection refcounts James Clark
2024-12-05 10:10 ` Yeoreum Yun
2024-12-11 10:20 ` Suzuki K Poulose
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®