Hi,
On 9/15/26 12:34, Yeoreum Yun wrote:
The current ETM3x configuration via sysfs can lead to the following inconsistencies:
- If a configuration is modified via sysfs while a perf session is active, the running configuration may differ between before a sched-out and after a subsequent sched-in.
To resolve these issues, separate the configuration into:
- active_config: the configuration applied to the current session
- config: the configuration set via sysfs
Same comment as for the etm4 config naming
Acked.
Additionally:
Since active_config and related fields are accessed only by the local CPU in etm_enable/disable_sysfs_smp_call() (similar to perf enable/disable), remove the lock/unlock from the sysfs enable/disable path and starting/dying_cpu path except when to access config fields only.
Some of sysfs interface read etm register directly. To reduce lock scope while the etm_enable_hw()/etm_disable_hw(), handle it via IPI so that registers could be read from on proper CPU.
Fixes: 1925a470ce69 ("coresight: etm3x: splitting struct etm_drvdata") Signed-off-by: Yeoreum Yun yeoreum.yun@arm.com
drivers/hwtracing/coresight/coresight-etm.h | 4 +- drivers/hwtracing/coresight/coresight-etm3x-core.c | 68 ++++++++++-------- .../hwtracing/coresight/coresight-etm3x-sysfs.c | 80 +++++++++++++++------- 3 files changed, 100 insertions(+), 52 deletions(-)
diff --git a/drivers/hwtracing/coresight/coresight-etm.h b/drivers/hwtracing/coresight/coresight-etm.h index 1d753cca29439..f3796162168d4 100644 --- a/drivers/hwtracing/coresight/coresight-etm.h +++ b/drivers/hwtracing/coresight/coresight-etm.h @@ -226,7 +226,8 @@ struct etm_config {
- @etmccr: value of register ETMCCR.
- @etmccer: value of register ETMCCER.
- @traceid: value of the current ID for this component.
- @config: structure holding configuration parameters.
- @active_config: structure holding current running configuration.
*/ struct etm_drvdata { struct csdev_access csa;
- @config: structure holding sysfs mode configuration.
@@ -248,6 +249,7 @@ struct etm_drvdata { u32 etmccr; u32 etmccer; u32 traceid;
- struct etm_config active_config; struct etm_config config; };
diff --git a/drivers/hwtracing/coresight/coresight-etm3x-core.c b/drivers/hwtracing/coresight/coresight-etm3x-core.c index 862ad0786699c..fd76a57e5f861 100644 --- a/drivers/hwtracing/coresight/coresight-etm3x-core.c +++ b/drivers/hwtracing/coresight/coresight-etm3x-core.c @@ -308,7 +308,7 @@ void etm_config_trace_mode(struct etm_config *config) static int etm_parse_event_config(struct etm_drvdata *drvdata, struct perf_event *event) {
- struct etm_config *config = &drvdata->config;
- struct etm_config *config = &drvdata->active_config; struct perf_event_attr *attr = &event->attr; u8 ts_level;
@@ -367,7 +367,7 @@ static int etm_enable_hw(struct etm_drvdata *drvdata) { int i, rc; u32 etmcr;
- struct etm_config *config = &drvdata->config;
- struct etm_config *config = &drvdata->active_config; struct coresight_device *csdev = drvdata->csdev; CS_UNLOCK(drvdata->csa.base);
@@ -442,32 +442,30 @@ static int etm_enable_hw(struct etm_drvdata *drvdata) struct etm_enable_arg { struct etm_drvdata *drvdata; struct coresight_path *path;
- struct etm_config config; int rc; }; static void etm_enable_sysfs_smp_call(void *info) { struct etm_enable_arg *arg = info;
- struct etm_drvdata *drvdata; struct coresight_device *csdev; if (WARN_ON(!arg)) return;
- csdev = arg->drvdata->csdev;
- if (!coresight_take_mode(csdev, CS_MODE_SYSFS)) {
/* Someone is already using the tracer */arg->rc = -EBUSY;return;- }
- drvdata = arg->drvdata;
- csdev = drvdata->csdev;
- arg->rc = etm_enable_hw(arg->drvdata);
- drvdata->active_config = arg->config;
- drvdata->traceid = arg->path->trace_id;
- /* The tracer didn't start */
- if (arg->rc) {
coresight_set_mode(csdev, CS_MODE_DISABLED);
- arg->rc = etm_enable_hw(arg->drvdata);
- if (arg->rc) return;
- }
- drvdata->sticky_enable = true; csdev->path = arg->path; }
@@ -512,9 +510,10 @@ static int etm_enable_sysfs(struct coresight_device *csdev, struct coresight_pat struct etm_enable_arg arg = { }; int ret;
- spin_lock(&drvdata->spinlock);
- drvdata->traceid = path->trace_id;
- if (!coresight_take_mode(csdev, CS_MODE_SYSFS)) {
/* Someone is already using the tracer */return -EBUSY;- } /*
- Configure the ETM only if the CPU is online. If it isn't online
@@ -523,23 +522,27 @@ static int etm_enable_sysfs(struct coresight_device *csdev, struct coresight_pat if (cpu_online(drvdata->cpu)) { arg.drvdata = drvdata; arg.path = path;
scoped_guard(spinlock, &drvdata->spinlock) {arg.config = drvdata->config;}Again I think the arg.config is unnecessary
Agree. I'll change as etm4's comment!
Thanks!
[...]