| Message ID | 20230823042948.12879-1-lcherian@marvell.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | coresight: Fix run time warnings while reusing ETR buffer | expand |
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)
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,
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)
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(-)