linux-arm-kernel.lists.infradead.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] coresight: Fix run time warnings while reusing ETR buffer
@ 2023-08-23  4:29 Linu Cherian
  2023-09-15 13:24 ` James Clark
  2023-09-18 10:34 ` Suzuki K Poulose
  0 siblings, 2 replies; 3+ messages in thread
From: Linu Cherian @ 2023-08-23  4:29 UTC (permalink / raw)
  To: suzuki.poulose, mike.leach, james.clark, leo.yan
  Cc: linux-arm-kernel, coresight, sgoutham, Linu Cherian

Fix the below warning by avoding calls to tmc_etr_enable_hw,
if we are reusing the ETR buffer for multiple sources in sysfs mode.

echo 1 > /sys/bus/coresight/devices/tmc_etr0/enable_sink
echo 1 > /sys/bus/coresight/devices/ete1/enable_source
echo 1 > /sys/bus/coresight/devices/ete2/enable_source
[  166.918290] ------------[ cut here ]------------
[  166.922905] WARNING: CPU: 4 PID: 2288 at
drivers/hwtracing/coresight/coresight-tmc-etr.c:1037
tmc_etr_enable_hw+0xb0/0xc8
[  166.933862] Modules linked in:
[  166.936911] CPU: 4 PID: 2288 Comm: bash Not tainted 6.5.0-rc7 #132
[  166.943084] Hardware name: Marvell CN106XX board (DT)
[  166.948127] pstate: 834000c9 (Nzcv daIF +PAN -UAO +TCO +DIT -SSBS
BTYPE=--)
[  166.955083] pc : tmc_etr_enable_hw+0xb0/0xc8
[  166.959345] lr : tmc_enable_etr_sink+0x134/0x210
snip..
  167.038545] Call trace:
[  167.040982]  tmc_etr_enable_hw+0xb0/0xc8
[  167.044897]  tmc_enable_etr_sink+0x134/0x210
[  167.049160]  coresight_enable_path+0x160/0x278
[  167.053596]  coresight_enable+0xd4/0x298
[  167.057510]  enable_source_store+0x54/0xa0
[  167.061598]  dev_attr_store+0x20/0x40
[  167.065254]  sysfs_kf_write+0x4c/0x68
[  167.068909]  kernfs_fop_write_iter+0x128/0x200
[  167.073345]  vfs_write+0x1ac/0x2f8
[  167.076739]  ksys_write+0x74/0x110
[  167.080132]  __arm64_sys_write+0x24/0x38
[  167.084045]  invoke_syscall.constprop.0+0x58/0xf8
[  167.088744]  do_el0_svc+0x60/0x160
[  167.092137]  el0_svc+0x40/0x170
[  167.095273]  el0t_64_sync_handler+0x100/0x130
[  167.099621]  el0t_64_sync+0x190/0x198
[  167.103277] ---[ end trace 0000000000000000 ]---
-bash: echo: write error: Device or resource busy

Fixes: 296b01fd106e ("coresight: Refactor out buffer allocation function for ETR")
Signed-off-by: Linu Cherian <lcherian@marvell.com>
---
 .../hwtracing/coresight/coresight-tmc-etr.c   | 24 ++++++++++---------
 1 file changed, 13 insertions(+), 11 deletions(-)

diff --git a/drivers/hwtracing/coresight/coresight-tmc-etr.c b/drivers/hwtracing/coresight/coresight-tmc-etr.c
index 766325de0e29..6a79c4dc09f2 100644
--- a/drivers/hwtracing/coresight/coresight-tmc-etr.c
+++ b/drivers/hwtracing/coresight/coresight-tmc-etr.c
@@ -1172,16 +1172,6 @@ static struct etr_buf *tmc_etr_get_sysfs_buffer(struct coresight_device *csdev)
 		goto out;
 	}
 
-	/*
-	 * In sysFS mode we can have multiple writers per sink.  Since this
-	 * sink is already enabled no memory is needed and the HW need not be
-	 * touched, even if the buffer size has changed.
-	 */
-	if (drvdata->mode == CS_MODE_SYSFS) {
-		atomic_inc(&csdev->refcnt);
-		goto out;
-	}
-
 	/*
 	 * If we don't have a buffer or it doesn't match the requested size,
 	 * use the buffer allocated above. Otherwise reuse the existing buffer.
@@ -1203,7 +1193,7 @@ static struct etr_buf *tmc_etr_get_sysfs_buffer(struct coresight_device *csdev)
 
 static int tmc_enable_etr_sink_sysfs(struct coresight_device *csdev)
 {
-	int ret;
+	int ret = 0;
 	unsigned long flags;
 	struct tmc_drvdata *drvdata = dev_get_drvdata(csdev->dev.parent);
 	struct etr_buf *sysfs_buf = tmc_etr_get_sysfs_buffer(csdev);
@@ -1212,12 +1202,24 @@ static int tmc_enable_etr_sink_sysfs(struct coresight_device *csdev)
 		return PTR_ERR(sysfs_buf);
 
 	spin_lock_irqsave(&drvdata->spinlock, flags);
+
+	/*
+	 * In sysFS mode we can have multiple writers per sink.  Since this
+	 * sink is already enabled no memory is needed and the HW need not be
+	 * touched, even if the buffer size has changed.
+	 */
+	if (drvdata->mode == CS_MODE_SYSFS) {
+		atomic_inc(&csdev->refcnt);
+		goto out;
+	}
+
 	ret = tmc_etr_enable_hw(drvdata, sysfs_buf);
 	if (!ret) {
 		drvdata->mode = CS_MODE_SYSFS;
 		atomic_inc(&csdev->refcnt);
 	}
 
+out:
 	spin_unlock_irqrestore(&drvdata->spinlock, flags);
 
 	if (!ret)
-- 
2.41.0


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply related	[flat|nested] 3+ messages in thread
* Re: [PATCH] coresight: Fix run time warnings while reusing ETR buffer
  2023-08-23  4:29 [PATCH] coresight: Fix run time warnings while reusing ETR buffer Linu Cherian
@ 2023-09-15 13:24 ` James Clark
  2023-09-18 10:34 ` Suzuki K Poulose
  1 sibling, 0 replies; 3+ messages in thread
From: James Clark @ 2023-09-15 13:24 UTC (permalink / raw)
  To: Linu Cherian, suzuki.poulose, mike.leach, leo.yan
  Cc: linux-arm-kernel, coresight, sgoutham



On 23/08/2023 05:29, Linu Cherian wrote:
> Fix the below warning by avoding calls to tmc_etr_enable_hw,
> if we are reusing the ETR buffer for multiple sources in sysfs mode.
> 

The change makes sense to me. Seems like I didn't separate out the
allocation and hw_enable parts correctly when I made the change you linked.

It would be good to get some kselftests for this though as it seems like
a pretty simple scenario, and I couldn't find anything coresight related
in the selftests folder. But that's not really related to this commit so:

Reviewed-by: James Clark <james.clark@arm.com>


> echo 1 > /sys/bus/coresight/devices/tmc_etr0/enable_sink
> echo 1 > /sys/bus/coresight/devices/ete1/enable_source
> echo 1 > /sys/bus/coresight/devices/ete2/enable_source
> [  166.918290] ------------[ cut here ]------------
> [  166.922905] WARNING: CPU: 4 PID: 2288 at
> drivers/hwtracing/coresight/coresight-tmc-etr.c:1037
> tmc_etr_enable_hw+0xb0/0xc8
> [  166.933862] Modules linked in:
> [  166.936911] CPU: 4 PID: 2288 Comm: bash Not tainted 6.5.0-rc7 #132
> [  166.943084] Hardware name: Marvell CN106XX board (DT)
> [  166.948127] pstate: 834000c9 (Nzcv daIF +PAN -UAO +TCO +DIT -SSBS
> BTYPE=--)
> [  166.955083] pc : tmc_etr_enable_hw+0xb0/0xc8
> [  166.959345] lr : tmc_enable_etr_sink+0x134/0x210
> snip..
>   167.038545] Call trace:
> [  167.040982]  tmc_etr_enable_hw+0xb0/0xc8
> [  167.044897]  tmc_enable_etr_sink+0x134/0x210
> [  167.049160]  coresight_enable_path+0x160/0x278
> [  167.053596]  coresight_enable+0xd4/0x298
> [  167.057510]  enable_source_store+0x54/0xa0
> [  167.061598]  dev_attr_store+0x20/0x40
> [  167.065254]  sysfs_kf_write+0x4c/0x68
> [  167.068909]  kernfs_fop_write_iter+0x128/0x200
> [  167.073345]  vfs_write+0x1ac/0x2f8
> [  167.076739]  ksys_write+0x74/0x110
> [  167.080132]  __arm64_sys_write+0x24/0x38
> [  167.084045]  invoke_syscall.constprop.0+0x58/0xf8
> [  167.088744]  do_el0_svc+0x60/0x160
> [  167.092137]  el0_svc+0x40/0x170
> [  167.095273]  el0t_64_sync_handler+0x100/0x130
> [  167.099621]  el0t_64_sync+0x190/0x198
> [  167.103277] ---[ end trace 0000000000000000 ]---
> -bash: echo: write error: Device or resource busy
> 
> Fixes: 296b01fd106e ("coresight: Refactor out buffer allocation function for ETR")
> Signed-off-by: Linu Cherian <lcherian@marvell.com>
> ---
>  .../hwtracing/coresight/coresight-tmc-etr.c   | 24 ++++++++++---------
>  1 file changed, 13 insertions(+), 11 deletions(-)
> 
> diff --git a/drivers/hwtracing/coresight/coresight-tmc-etr.c b/drivers/hwtracing/coresight/coresight-tmc-etr.c
> index 766325de0e29..6a79c4dc09f2 100644
> --- a/drivers/hwtracing/coresight/coresight-tmc-etr.c
> +++ b/drivers/hwtracing/coresight/coresight-tmc-etr.c
> @@ -1172,16 +1172,6 @@ static struct etr_buf *tmc_etr_get_sysfs_buffer(struct coresight_device *csdev)
>  		goto out;
>  	}
>  
> -	/*
> -	 * In sysFS mode we can have multiple writers per sink.  Since this
> -	 * sink is already enabled no memory is needed and the HW need not be
> -	 * touched, even if the buffer size has changed.
> -	 */
> -	if (drvdata->mode == CS_MODE_SYSFS) {
> -		atomic_inc(&csdev->refcnt);
> -		goto out;
> -	}
> -
>  	/*
>  	 * If we don't have a buffer or it doesn't match the requested size,
>  	 * use the buffer allocated above. Otherwise reuse the existing buffer.
> @@ -1203,7 +1193,7 @@ static struct etr_buf *tmc_etr_get_sysfs_buffer(struct coresight_device *csdev)
>  
>  static int tmc_enable_etr_sink_sysfs(struct coresight_device *csdev)
>  {
> -	int ret;
> +	int ret = 0;
>  	unsigned long flags;
>  	struct tmc_drvdata *drvdata = dev_get_drvdata(csdev->dev.parent);
>  	struct etr_buf *sysfs_buf = tmc_etr_get_sysfs_buffer(csdev);
> @@ -1212,12 +1202,24 @@ static int tmc_enable_etr_sink_sysfs(struct coresight_device *csdev)
>  		return PTR_ERR(sysfs_buf);
>  
>  	spin_lock_irqsave(&drvdata->spinlock, flags);
> +
> +	/*
> +	 * In sysFS mode we can have multiple writers per sink.  Since this
> +	 * sink is already enabled no memory is needed and the HW need not be
> +	 * touched, even if the buffer size has changed.
> +	 */
> +	if (drvdata->mode == CS_MODE_SYSFS) {
> +		atomic_inc(&csdev->refcnt);
> +		goto out;
> +	}
> +
>  	ret = tmc_etr_enable_hw(drvdata, sysfs_buf);
>  	if (!ret) {
>  		drvdata->mode = CS_MODE_SYSFS;
>  		atomic_inc(&csdev->refcnt);
>  	}
>  
> +out:
>  	spin_unlock_irqrestore(&drvdata->spinlock, flags);
>  
>  	if (!ret)

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 3+ messages in thread
* Re: [PATCH] coresight: Fix run time warnings while reusing ETR buffer
  2023-08-23  4:29 [PATCH] coresight: Fix run time warnings while reusing ETR buffer Linu Cherian
  2023-09-15 13:24 ` James Clark
@ 2023-09-18 10:34 ` Suzuki K Poulose
  1 sibling, 0 replies; 3+ messages in thread
From: Suzuki K Poulose @ 2023-09-18 10:34 UTC (permalink / raw)
  To: leo.yan, mike.leach, james.clark, Linu Cherian
  Cc: Suzuki K Poulose, linux-arm-kernel, coresight, sgoutham

On Wed, 23 Aug 2023 09:59:48 +0530, Linu Cherian wrote:
> Fix the below warning by avoding calls to tmc_etr_enable_hw,
> if we are reusing the ETR buffer for multiple sources in sysfs mode.
> 
> echo 1 > /sys/bus/coresight/devices/tmc_etr0/enable_sink
> echo 1 > /sys/bus/coresight/devices/ete1/enable_source
> echo 1 > /sys/bus/coresight/devices/ete2/enable_source
> [  166.918290] ------------[ cut here ]------------
> [  166.922905] WARNING: CPU: 4 PID: 2288 at
> drivers/hwtracing/coresight/coresight-tmc-etr.c:1037
> tmc_etr_enable_hw+0xb0/0xc8
> [  166.933862] Modules linked in:
> [  166.936911] CPU: 4 PID: 2288 Comm: bash Not tainted 6.5.0-rc7 #132
> [  166.943084] Hardware name: Marvell CN106XX board (DT)
> [  166.948127] pstate: 834000c9 (Nzcv daIF +PAN -UAO +TCO +DIT -SSBS
> BTYPE=--)
> [  166.955083] pc : tmc_etr_enable_hw+0xb0/0xc8
> [  166.959345] lr : tmc_enable_etr_sink+0x134/0x210
> snip..
>   167.038545] Call trace:
> [  167.040982]  tmc_etr_enable_hw+0xb0/0xc8
> [  167.044897]  tmc_enable_etr_sink+0x134/0x210
> [  167.049160]  coresight_enable_path+0x160/0x278
> [  167.053596]  coresight_enable+0xd4/0x298
> [  167.057510]  enable_source_store+0x54/0xa0
> [  167.061598]  dev_attr_store+0x20/0x40
> [  167.065254]  sysfs_kf_write+0x4c/0x68
> [  167.068909]  kernfs_fop_write_iter+0x128/0x200
> [  167.073345]  vfs_write+0x1ac/0x2f8
> [  167.076739]  ksys_write+0x74/0x110
> [  167.080132]  __arm64_sys_write+0x24/0x38
> [  167.084045]  invoke_syscall.constprop.0+0x58/0xf8
> [  167.088744]  do_el0_svc+0x60/0x160
> [  167.092137]  el0_svc+0x40/0x170
> [  167.095273]  el0t_64_sync_handler+0x100/0x130
> [  167.099621]  el0t_64_sync+0x190/0x198
> [  167.103277] ---[ end trace 0000000000000000 ]---
> -bash: echo: write error: Device or resource busy
> 
> [...]

Applied to fixes, thanks!

[1/1] coresight: Fix run time warnings while reusing ETR buffer
      https://git.kernel.org/coresight/c/bd2767ec3df2

Best regards,
-- 
Suzuki K Poulose <suzuki.poulose@arm.com>

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 3+ messages in thread
end of thread, other threads:[~2023-09-18 10:35 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-08-23  4:29 [PATCH] coresight: Fix run time warnings while reusing ETR buffer Linu Cherian
2023-09-15 13:24 ` James Clark
2023-09-18 10:34 ` 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;
as well as URLs for NNTP newsgroup(s).