| Message ID | 20241216115006.415861-1-yeoreum.yun@arm.com (mailing list archive) |
|---|---|
| Headers | show |
| Series | coresight: change some driver' spinlock type to raw_spinlock_t | expand |
Hi, On Mon, 16 Dec 2024 at 11:50, Yeoreum Yun <yeoreum.yun@arm.com> wrote: > > In some coresight drivers, drvdata->spinlock can be held during __schedule() > by perf_event_task_sched_out()/in(). > > Since drvdata->spinlock type is spinlock_t and > perf_event_task_sched_out()/in() is called after acquiring rq_lock, > which is raw_spinlock_t (an unsleepable lock), > this poses an issue in PREEMPT_RT kernel where spinlock_t is sleepable. > > To address this,change type drvdata->spinlock in some coresight drivers, > which can be called by perf_event_task_sched_out()/in(), > from spinlock_t to raw_spinlock_t. > > Reviewed-by: James Clark <james.clark@linaro.org> > > v2 to v3: > - Fix build error > > v1 to v2: > - seperate patchsets to change locktype and apply gurad API. > > Yeoreum Yun (9): > coresight: change coresight_device lock type to raw_spinlock_t > coresight-etm4x: change etmv4_drvdata spinlock type to raw_spinlock_t > coresight: change coresight_trace_id_map's lock type to > raw_spinlock_t > coresight-cti: change cti_drvdata spinlock's type to raw_spinlock_t > coresight-etb10: change etb_drvdata spinlock's type to raw_spinlock_t > coresight-funnel: change funnel_drvdata spinlock's type to > raw_spinlock_t > coresight-replicator: change replicator_drvdata spinlock's type to > raw_spinlock_t > coresight-tmc: change tmc_drvdata spinlock's type to raw_spinlock_t > coresight/ultrasoc: change cti_drvdata spinlock's type to > raw_spinlock_t > > .../hwtracing/coresight/coresight-config.c | 8 +- > .../hwtracing/coresight/coresight-config.h | 2 +- > drivers/hwtracing/coresight/coresight-core.c | 2 +- > .../hwtracing/coresight/coresight-cti-core.c | 44 +-- > .../hwtracing/coresight/coresight-cti-sysfs.c | 76 +++--- > drivers/hwtracing/coresight/coresight-cti.h | 2 +- > drivers/hwtracing/coresight/coresight-etb10.c | 26 +- > .../coresight/coresight-etm4x-core.c | 18 +- > .../coresight/coresight-etm4x-sysfs.c | 250 +++++++++--------- > drivers/hwtracing/coresight/coresight-etm4x.h | 2 +- > .../hwtracing/coresight/coresight-funnel.c | 12 +- > .../coresight/coresight-replicator.c | 12 +- > .../hwtracing/coresight/coresight-syscfg.c | 26 +- > .../hwtracing/coresight/coresight-tmc-core.c | 6 +- > .../hwtracing/coresight/coresight-tmc-etf.c | 48 ++-- > .../hwtracing/coresight/coresight-tmc-etr.c | 40 +-- > drivers/hwtracing/coresight/coresight-tmc.h | 2 +- > .../hwtracing/coresight/coresight-trace-id.c | 22 +- > drivers/hwtracing/coresight/ultrasoc-smb.c | 12 +- > drivers/hwtracing/coresight/ultrasoc-smb.h | 2 +- > include/linux/coresight.h | 4 +- > 21 files changed, 308 insertions(+), 308 deletions(-) > > -- > LEVI:{C3F47F37-75D8-414A-A8BA-3980EC8A46D7} > Notably missing is the same changes for the etm3x driver. The ETMv3.x and PTM1.x are supported by this driver, and these trace source variants are also supported in perf in the cs_etm.c code. STM is also missing, though this is not directly enabled via perf - but could perhaps run concurrently as it can be a target output for ftrace. Regards Mike
Hi Mike, > Hi, > > On Mon, 16 Dec 2024 at 11:50, Yeoreum Yun <yeoreum.yun@arm.com> wrote: > > > > In some coresight drivers, drvdata->spinlock can be held during __schedule() > > by perf_event_task_sched_out()/in(). > > > > Since drvdata->spinlock type is spinlock_t and > > perf_event_task_sched_out()/in() is called after acquiring rq_lock, > > which is raw_spinlock_t (an unsleepable lock), > > this poses an issue in PREEMPT_RT kernel where spinlock_t is sleepable. > > > > To address this,change type drvdata->spinlock in some coresight drivers, > > which can be called by perf_event_task_sched_out()/in(), > > from spinlock_t to raw_spinlock_t. > > > > Reviewed-by: James Clark <james.clark@linaro.org> > > > > v2 to v3: > > - Fix build error > > > > v1 to v2: > > - seperate patchsets to change locktype and apply gurad API. > > > > Yeoreum Yun (9): > > coresight: change coresight_device lock type to raw_spinlock_t > > coresight-etm4x: change etmv4_drvdata spinlock type to raw_spinlock_t > > coresight: change coresight_trace_id_map's lock type to > > raw_spinlock_t > > coresight-cti: change cti_drvdata spinlock's type to raw_spinlock_t > > coresight-etb10: change etb_drvdata spinlock's type to raw_spinlock_t > > coresight-funnel: change funnel_drvdata spinlock's type to > > raw_spinlock_t > > coresight-replicator: change replicator_drvdata spinlock's type to > > raw_spinlock_t > > coresight-tmc: change tmc_drvdata spinlock's type to raw_spinlock_t > > coresight/ultrasoc: change cti_drvdata spinlock's type to > > raw_spinlock_t > > > > .../hwtracing/coresight/coresight-config.c | 8 +- > > .../hwtracing/coresight/coresight-config.h | 2 +- > > drivers/hwtracing/coresight/coresight-core.c | 2 +- > > .../hwtracing/coresight/coresight-cti-core.c | 44 +-- > > .../hwtracing/coresight/coresight-cti-sysfs.c | 76 +++--- > > drivers/hwtracing/coresight/coresight-cti.h | 2 +- > > drivers/hwtracing/coresight/coresight-etb10.c | 26 +- > > .../coresight/coresight-etm4x-core.c | 18 +- > > .../coresight/coresight-etm4x-sysfs.c | 250 +++++++++--------- > > drivers/hwtracing/coresight/coresight-etm4x.h | 2 +- > > .../hwtracing/coresight/coresight-funnel.c | 12 +- > > .../coresight/coresight-replicator.c | 12 +- > > .../hwtracing/coresight/coresight-syscfg.c | 26 +- > > .../hwtracing/coresight/coresight-tmc-core.c | 6 +- > > .../hwtracing/coresight/coresight-tmc-etf.c | 48 ++-- > > .../hwtracing/coresight/coresight-tmc-etr.c | 40 +-- > > drivers/hwtracing/coresight/coresight-tmc.h | 2 +- > > .../hwtracing/coresight/coresight-trace-id.c | 22 +- > > drivers/hwtracing/coresight/ultrasoc-smb.c | 12 +- > > drivers/hwtracing/coresight/ultrasoc-smb.h | 2 +- > > include/linux/coresight.h | 4 +- > > 21 files changed, 308 insertions(+), 308 deletions(-) > > > > -- > > LEVI:{C3F47F37-75D8-414A-A8BA-3980EC8A46D7} > > > > Notably missing is the same changes for the etm3x driver. The ETMv3.x > and PTM1.x are supported by this driver, and these trace source > variants are also supported in perf in the cs_etm.c code. > > STM is also missing, though this is not directly enabled via perf - > but could perhaps run concurrently as it can be a target output for > ftrace. > > Regards Thanks for review. okay I'll back with new patch set including etm3 / stm. Thanks! > -- > Mike Leach > Principal Engineer, ARM Ltd. > Manchester Design Centre. UK
Hi Mike. > Notably missing is the same changes for the etm3x driver. The ETMv3.x > and PTM1.x are supported by this driver, and these trace source > variants are also supported in perf in the cs_etm.c code. But I wonder etmv3 needs to change. Because its spinlock is used only via sysfs enable/disable path. So, I think it doesn't need to change the lock type. > STM is also missing, though this is not directly enabled via perf - > but could perhaps run concurrently as it can be a target output for > ftrace. Actually, I couldn't find out the path where the STM's lock could be grabbed under other raw_spin_lock (including csdev) If you don't mind would you let me the code path please? Thanks > -- > Mike Leach > Principal Engineer, ARM Ltd. > Manchester Design Centre. UK
On 20/12/2024 10:38, Yeoreum Yun wrote: > Hi Mike. > >> Notably missing is the same changes for the etm3x driver. The ETMv3.x >> and PTM1.x are supported by this driver, and these trace source >> variants are also supported in perf in the cs_etm.c code. > > But I wonder etmv3 needs to change. Because its spinlock is used only > via sysfs enable/disable path. > So, I think it doesn't need to change the lock type. ETM3 can be used in perf mode, similar to the ETM4x. So, you need to fix it as well. > >> STM is also missing, though this is not directly enabled via perf - >> but could perhaps run concurrently as it can be a target output for >> ftrace. > > Actually, I couldn't find out the path where > the STM's lock could be grabbed under other raw_spin_lock (including csdev) > If you don't mind would you let me the code path please? STM can't be used in perf mode, and as such you may skip it. Suzuki > > Thanks >> -- >> Mike Leach >> Principal Engineer, ARM Ltd. >> Manchester Design Centre. UK
Hi Suzuki, > On 20/12/2024 10:38, Yeoreum Yun wrote: > > Hi Mike. > > > > > Notably missing is the same changes for the etm3x driver. The ETMv3.x > > > and PTM1.x are supported by this driver, and these trace source > > > variants are also supported in perf in the cs_etm.c code. > > > > But I wonder etmv3 needs to change. Because its spinlock is used only > > via sysfs enable/disable path. > > So, I think it doesn't need to change the lock type. > > ETM3 can be used in perf mode, similar to the ETM4x. > > So, you need to fix it as well. Yes. But etmv3's etmdata->spinlock doesn't used in perf path its usage is only in sysfs interface path. That's why I think it could skip too. Thanks.
On 20/12/2024 11:39, Yeoreum Yun wrote: > Hi Suzuki, >> On 20/12/2024 10:38, Yeoreum Yun wrote: >>> Hi Mike. >>> >>>> Notably missing is the same changes for the etm3x driver. The ETMv3.x >>>> and PTM1.x are supported by this driver, and these trace source >>>> variants are also supported in perf in the cs_etm.c code. >>> >>> But I wonder etmv3 needs to change. Because its spinlock is used only >>> via sysfs enable/disable path. >>> So, I think it doesn't need to change the lock type. >> >> ETM3 can be used in perf mode, similar to the ETM4x. >> >> So, you need to fix it as well. > > Yes. But etmv3's etmdata->spinlock doesn't used in perf path > its usage is only in sysfs interface path. > That's why I think it could skip too. Ok, which I think is a problem, since the sysfs mode could overwrite the "config" while perf is preparing the config from the event parsing. And we would need it there. So, for the time being, we can accept this series, pending other review comments and address this issue separately Suzuki > > Thanks.
Hi Suzuki, > > > > > > > > > Notably missing is the same changes for the etm3x driver. The ETMv3.x > > > > > and PTM1.x are supported by this driver, and these trace source > > > > > variants are also supported in perf in the cs_etm.c code. > > > > > > > > But I wonder etmv3 needs to change. Because its spinlock is used only > > > > via sysfs enable/disable path. > > > > So, I think it doesn't need to change the lock type. > > > > > > ETM3 can be used in perf mode, similar to the ETM4x. > > > > > > So, you need to fix it as well. > > > > Yes. But etmv3's etmdata->spinlock doesn't used in perf path > > its usage is only in sysfs interface path. > > That's why I think it could skip too. > > Ok, which I think is a problem, since the sysfs mode could overwrite the > "config" while perf is preparing the config from the event parsing. > And we would need it there. So, for the time being, we can accept this > series, pending other review comments and address this issue separately Agree. also it has the same problem in etm4. I have a patchset for etm3/etm4 for this. but it's based on this series. So, after this patch is mereged. I'll send related patch. Thanks.
Hi Suzuki. > On 20/12/2024 11:39, Yeoreum Yun wrote: > > Hi Suzuki, > > > On 20/12/2024 10:38, Yeoreum Yun wrote: > > > > Hi Mike. > > > > > > > > > Notably missing is the same changes for the etm3x driver. The ETMv3.x > > > > > and PTM1.x are supported by this driver, and these trace source > > > > > variants are also supported in perf in the cs_etm.c code. > > > > > > > > But I wonder etmv3 needs to change. Because its spinlock is used only > > > > via sysfs enable/disable path. > > > > So, I think it doesn't need to change the lock type. > > > > > > ETM3 can be used in perf mode, similar to the ETM4x. > > > > > > So, you need to fix it as well. > > > > Yes. But etmv3's etmdata->spinlock doesn't used in perf path > > its usage is only in sysfs interface path. > > That's why I think it could skip too. > > Ok, which I think is a problem, since the sysfs mode could overwrite the > "config" while perf is preparing the config from the event parsing. > And we would need it there. So, for the time being, we can accept this > series, pending other review comments and address this issue separately > Here is the patchset fixing above one: - https://lore.kernel.org/linux-arm-kernel/20241221165934.1161856-1-yeoreum.yun@arm.com/
In some coresight drivers, drvdata->spinlock can be held during __schedule() by perf_event_task_sched_out()/in(). Since drvdata->spinlock type is spinlock_t and perf_event_task_sched_out()/in() is called after acquiring rq_lock, which is raw_spinlock_t (an unsleepable lock), this poses an issue in PREEMPT_RT kernel where spinlock_t is sleepable. To address this,change type drvdata->spinlock in some coresight drivers, which can be called by perf_event_task_sched_out()/in(), from spinlock_t to raw_spinlock_t. Reviewed-by: James Clark <james.clark@linaro.org> v2 to v3: - Fix build error v1 to v2: - seperate patchsets to change locktype and apply gurad API. Yeoreum Yun (9): coresight: change coresight_device lock type to raw_spinlock_t coresight-etm4x: change etmv4_drvdata spinlock type to raw_spinlock_t coresight: change coresight_trace_id_map's lock type to raw_spinlock_t coresight-cti: change cti_drvdata spinlock's type to raw_spinlock_t coresight-etb10: change etb_drvdata spinlock's type to raw_spinlock_t coresight-funnel: change funnel_drvdata spinlock's type to raw_spinlock_t coresight-replicator: change replicator_drvdata spinlock's type to raw_spinlock_t coresight-tmc: change tmc_drvdata spinlock's type to raw_spinlock_t coresight/ultrasoc: change cti_drvdata spinlock's type to raw_spinlock_t .../hwtracing/coresight/coresight-config.c | 8 +- .../hwtracing/coresight/coresight-config.h | 2 +- drivers/hwtracing/coresight/coresight-core.c | 2 +- .../hwtracing/coresight/coresight-cti-core.c | 44 +-- .../hwtracing/coresight/coresight-cti-sysfs.c | 76 +++--- drivers/hwtracing/coresight/coresight-cti.h | 2 +- drivers/hwtracing/coresight/coresight-etb10.c | 26 +- .../coresight/coresight-etm4x-core.c | 18 +- .../coresight/coresight-etm4x-sysfs.c | 250 +++++++++--------- drivers/hwtracing/coresight/coresight-etm4x.h | 2 +- .../hwtracing/coresight/coresight-funnel.c | 12 +- .../coresight/coresight-replicator.c | 12 +- .../hwtracing/coresight/coresight-syscfg.c | 26 +- .../hwtracing/coresight/coresight-tmc-core.c | 6 +- .../hwtracing/coresight/coresight-tmc-etf.c | 48 ++-- .../hwtracing/coresight/coresight-tmc-etr.c | 40 +-- drivers/hwtracing/coresight/coresight-tmc.h | 2 +- .../hwtracing/coresight/coresight-trace-id.c | 22 +- drivers/hwtracing/coresight/ultrasoc-smb.c | 12 +- drivers/hwtracing/coresight/ultrasoc-smb.h | 2 +- include/linux/coresight.h | 4 +- 21 files changed, 308 insertions(+), 308 deletions(-) -- LEVI:{C3F47F37-75D8-414A-A8BA-3980EC8A46D7}