[PATCH] coresight: Fix run time warnings while reusing ETR buffer
James Clark
james.clark at arm.com
Fri Sep 15 06:24:24 PDT 2023
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 at 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 at 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)
More information about the linux-arm-kernel
mailing list