| Message ID | 20240429152207.479221-14-james.clark@arm.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | coresight: Use per-sink trace ID maps for Perf sessions | expand |
On 4/29/24 20:51, James Clark wrote: > The global CPU ID mappings won't work for per-sink ID maps so move it to > the ID map struct. coresight_trace_id_release_all_pending() is hard > coded to operate on the default map, but once Perf sessions use their > own maps the pending release mechanism will be deleted. So it doesn't > need to be extended to accept a trace ID map argument at this point. > > Signed-off-by: James Clark <james.clark@arm.com> > --- > .../hwtracing/coresight/coresight-etm-perf.c | 3 +- > .../coresight/coresight-etm3x-core.c | 3 +- > .../coresight/coresight-etm4x-core.c | 3 +- > .../hwtracing/coresight/coresight-trace-id.c | 28 ++++++++----------- > .../hwtracing/coresight/coresight-trace-id.h | 2 +- > include/linux/coresight.h | 1 + > 6 files changed, 20 insertions(+), 20 deletions(-) > > diff --git a/drivers/hwtracing/coresight/coresight-etm-perf.c b/drivers/hwtracing/coresight/coresight-etm-perf.c > index 4afb9d29f355..25f1f87c90d1 100644 > --- a/drivers/hwtracing/coresight/coresight-etm-perf.c > +++ b/drivers/hwtracing/coresight/coresight-etm-perf.c > @@ -508,7 +508,8 @@ static void etm_event_start(struct perf_event *event, int flags) > hw_id = FIELD_PREP(CS_AUX_HW_ID_VERSION_MASK, > CS_AUX_HW_ID_CURR_VERSION); > hw_id |= FIELD_PREP(CS_AUX_HW_ID_TRACE_ID_MASK, > - coresight_trace_id_read_cpu_id(cpu)); > + coresight_trace_id_read_cpu_id(cpu, > + coresight_trace_id_map_default())); > perf_report_aux_output_id(event, hw_id); > } > > diff --git a/drivers/hwtracing/coresight/coresight-etm3x-core.c b/drivers/hwtracing/coresight/coresight-etm3x-core.c > index 4149e7675ceb..b21f5ad94e63 100644 > --- a/drivers/hwtracing/coresight/coresight-etm3x-core.c > +++ b/drivers/hwtracing/coresight/coresight-etm3x-core.c > @@ -501,7 +501,8 @@ static int etm_enable_perf(struct coresight_device *csdev, > * with perf locks - we know the ID cannot change until perf shuts down > * the session > */ > - trace_id = coresight_trace_id_read_cpu_id(drvdata->cpu); > + trace_id = coresight_trace_id_read_cpu_id(drvdata->cpu, > + coresight_trace_id_map_default()); > if (!IS_VALID_CS_TRACE_ID(trace_id)) { > dev_err(&drvdata->csdev->dev, "Failed to set trace ID for %s on CPU%d\n", > dev_name(&drvdata->csdev->dev), drvdata->cpu); > diff --git a/drivers/hwtracing/coresight/coresight-etm4x-core.c b/drivers/hwtracing/coresight/coresight-etm4x-core.c > index f32c8cd7742d..d16d6efb26fa 100644 > --- a/drivers/hwtracing/coresight/coresight-etm4x-core.c > +++ b/drivers/hwtracing/coresight/coresight-etm4x-core.c > @@ -776,7 +776,8 @@ static int etm4_enable_perf(struct coresight_device *csdev, > * with perf locks - we know the ID cannot change until perf shuts down > * the session > */ > - trace_id = coresight_trace_id_read_cpu_id(drvdata->cpu); > + trace_id = coresight_trace_id_read_cpu_id(drvdata->cpu, > + coresight_trace_id_map_default()); > if (!IS_VALID_CS_TRACE_ID(trace_id)) { > dev_err(&drvdata->csdev->dev, "Failed to set trace ID for %s on CPU%d\n", > dev_name(&drvdata->csdev->dev), drvdata->cpu); > diff --git a/drivers/hwtracing/coresight/coresight-trace-id.c b/drivers/hwtracing/coresight/coresight-trace-id.c > index 45ddd50d09a6..b393603dd713 100644 > --- a/drivers/hwtracing/coresight/coresight-trace-id.c > +++ b/drivers/hwtracing/coresight/coresight-trace-id.c > @@ -13,10 +13,12 @@ > #include "coresight-trace-id.h" > > /* Default trace ID map. Used in sysfs mode and for system sources */ > -static struct coresight_trace_id_map id_map_default; > +static DEFINE_PER_CPU(atomic_t, id_map_default_cpu_ids) = ATOMIC_INIT(0); > +static struct coresight_trace_id_map id_map_default = { > + .cpu_map = &id_map_default_cpu_ids > +}; > > -/* maintain a record of the mapping of IDs and pending releases per cpu */ > -static DEFINE_PER_CPU(atomic_t, cpu_id) = ATOMIC_INIT(0); > +/* maintain a record of the pending releases per cpu */ > static cpumask_t cpu_id_release_pending; > > /* perf session active counter */ > @@ -46,12 +48,6 @@ static void coresight_trace_id_dump_table(struct coresight_trace_id_map *id_map, > #define PERF_SESSION(n) > #endif > > -/* unlocked read of current trace ID value for given CPU */ > -static int _coresight_trace_id_read_cpu_id(int cpu) > -{ > - return atomic_read(&per_cpu(cpu_id, cpu)); > -} Just wondering where this per cpu cpu_id ^^ is being dropped off as well OR is it still getting used ? > - > /* look for next available odd ID, return 0 if none found */ > static int coresight_trace_id_find_odd_id(struct coresight_trace_id_map *id_map) > { > @@ -145,7 +141,7 @@ static void coresight_trace_id_release_all_pending(void) > clear_bit(bit, id_map->pend_rel_ids); > } > for_each_cpu(cpu, &cpu_id_release_pending) { > - atomic_set(&per_cpu(cpu_id, cpu), 0); > + atomic_set(per_cpu_ptr(id_map_default.cpu_map, cpu), 0); > cpumask_clear_cpu(cpu, &cpu_id_release_pending); > } > spin_unlock_irqrestore(&id_map_lock, flags); > @@ -160,7 +156,7 @@ int coresight_trace_id_get_cpu_id(int cpu, struct coresight_trace_id_map *id_map > spin_lock_irqsave(&id_map_lock, flags); > > /* check for existing allocation for this CPU */ > - id = _coresight_trace_id_read_cpu_id(cpu); > + id = coresight_trace_id_read_cpu_id(cpu, id_map); > if (id) > goto get_cpu_id_clr_pend; > > @@ -181,7 +177,7 @@ int coresight_trace_id_get_cpu_id(int cpu, struct coresight_trace_id_map *id_map > goto get_cpu_id_out_unlock; > > /* allocate the new id to the cpu */ > - atomic_set(&per_cpu(cpu_id, cpu), id); > + atomic_set(per_cpu_ptr(id_map->cpu_map, cpu), id); > > get_cpu_id_clr_pend: > /* we are (re)using this ID - so ensure it is not marked for release */ > @@ -203,7 +199,7 @@ void coresight_trace_id_put_cpu_id(int cpu, struct coresight_trace_id_map *id_ma > int id; > > /* check for existing allocation for this CPU */ > - id = _coresight_trace_id_read_cpu_id(cpu); > + id = coresight_trace_id_read_cpu_id(cpu, id_map); > if (!id) > return; > > @@ -216,7 +212,7 @@ void coresight_trace_id_put_cpu_id(int cpu, struct coresight_trace_id_map *id_ma > } else { > /* otherwise clear id */ > coresight_trace_id_free(id, id_map); > - atomic_set(&per_cpu(cpu_id, cpu), 0); > + atomic_set(per_cpu_ptr(id_map->cpu_map, cpu), 0); > } > > spin_unlock_irqrestore(&id_map_lock, flags); > @@ -258,9 +254,9 @@ struct coresight_trace_id_map *coresight_trace_id_map_default(void) > } > EXPORT_SYMBOL_GPL(coresight_trace_id_map_default); > > -int coresight_trace_id_read_cpu_id(int cpu) > +int coresight_trace_id_read_cpu_id(int cpu, struct coresight_trace_id_map *id_map) > { > - return _coresight_trace_id_read_cpu_id(cpu); > + return atomic_read(per_cpu_ptr(id_map->cpu_map, cpu)); > } > EXPORT_SYMBOL_GPL(coresight_trace_id_read_cpu_id); > > diff --git a/drivers/hwtracing/coresight/coresight-trace-id.h b/drivers/hwtracing/coresight/coresight-trace-id.h > index 54b9d8ed903b..ed2bc4b3ad2a 100644 > --- a/drivers/hwtracing/coresight/coresight-trace-id.h > +++ b/drivers/hwtracing/coresight/coresight-trace-id.h > @@ -93,7 +93,7 @@ void coresight_trace_id_put_cpu_id(int cpu, struct coresight_trace_id_map *id_ma > * > * return: current value, will be 0 if unallocated. > */ > -int coresight_trace_id_read_cpu_id(int cpu); > +int coresight_trace_id_read_cpu_id(int cpu, struct coresight_trace_id_map *id_map); > > /** > * Allocate a CoreSight trace ID for a system component. > diff --git a/include/linux/coresight.h b/include/linux/coresight.h > index c16c61a8411d..7d62b88bfb5c 100644 > --- a/include/linux/coresight.h > +++ b/include/linux/coresight.h > @@ -234,6 +234,7 @@ struct coresight_sysfs_link { > struct coresight_trace_id_map { > DECLARE_BITMAP(used_ids, CORESIGHT_TRACE_IDS_MAX); > DECLARE_BITMAP(pend_rel_ids, CORESIGHT_TRACE_IDS_MAX); > + atomic_t __percpu *cpu_map; > }; > > /**
On 07/05/2024 07:22, Anshuman Khandual wrote: > > > On 4/29/24 20:51, James Clark wrote: >> The global CPU ID mappings won't work for per-sink ID maps so move it to >> the ID map struct. coresight_trace_id_release_all_pending() is hard >> coded to operate on the default map, but once Perf sessions use their >> own maps the pending release mechanism will be deleted. So it doesn't >> need to be extended to accept a trace ID map argument at this point. >> >> Signed-off-by: James Clark <james.clark@arm.com> >> --- >> .../hwtracing/coresight/coresight-etm-perf.c | 3 +- >> .../coresight/coresight-etm3x-core.c | 3 +- >> .../coresight/coresight-etm4x-core.c | 3 +- >> .../hwtracing/coresight/coresight-trace-id.c | 28 ++++++++----------- >> .../hwtracing/coresight/coresight-trace-id.h | 2 +- >> include/linux/coresight.h | 1 + >> 6 files changed, 20 insertions(+), 20 deletions(-) >> >> diff --git a/drivers/hwtracing/coresight/coresight-etm-perf.c b/drivers/hwtracing/coresight/coresight-etm-perf.c >> index 4afb9d29f355..25f1f87c90d1 100644 >> --- a/drivers/hwtracing/coresight/coresight-etm-perf.c >> +++ b/drivers/hwtracing/coresight/coresight-etm-perf.c >> @@ -508,7 +508,8 @@ static void etm_event_start(struct perf_event *event, int flags) >> hw_id = FIELD_PREP(CS_AUX_HW_ID_VERSION_MASK, >> CS_AUX_HW_ID_CURR_VERSION); >> hw_id |= FIELD_PREP(CS_AUX_HW_ID_TRACE_ID_MASK, >> - coresight_trace_id_read_cpu_id(cpu)); >> + coresight_trace_id_read_cpu_id(cpu, >> + coresight_trace_id_map_default())); >> perf_report_aux_output_id(event, hw_id); >> } >> >> diff --git a/drivers/hwtracing/coresight/coresight-etm3x-core.c b/drivers/hwtracing/coresight/coresight-etm3x-core.c >> index 4149e7675ceb..b21f5ad94e63 100644 >> --- a/drivers/hwtracing/coresight/coresight-etm3x-core.c >> +++ b/drivers/hwtracing/coresight/coresight-etm3x-core.c >> @@ -501,7 +501,8 @@ static int etm_enable_perf(struct coresight_device *csdev, >> * with perf locks - we know the ID cannot change until perf shuts down >> * the session >> */ >> - trace_id = coresight_trace_id_read_cpu_id(drvdata->cpu); >> + trace_id = coresight_trace_id_read_cpu_id(drvdata->cpu, >> + coresight_trace_id_map_default()); >> if (!IS_VALID_CS_TRACE_ID(trace_id)) { >> dev_err(&drvdata->csdev->dev, "Failed to set trace ID for %s on CPU%d\n", >> dev_name(&drvdata->csdev->dev), drvdata->cpu); >> diff --git a/drivers/hwtracing/coresight/coresight-etm4x-core.c b/drivers/hwtracing/coresight/coresight-etm4x-core.c >> index f32c8cd7742d..d16d6efb26fa 100644 >> --- a/drivers/hwtracing/coresight/coresight-etm4x-core.c >> +++ b/drivers/hwtracing/coresight/coresight-etm4x-core.c >> @@ -776,7 +776,8 @@ static int etm4_enable_perf(struct coresight_device *csdev, >> * with perf locks - we know the ID cannot change until perf shuts down >> * the session >> */ >> - trace_id = coresight_trace_id_read_cpu_id(drvdata->cpu); >> + trace_id = coresight_trace_id_read_cpu_id(drvdata->cpu, >> + coresight_trace_id_map_default()); >> if (!IS_VALID_CS_TRACE_ID(trace_id)) { >> dev_err(&drvdata->csdev->dev, "Failed to set trace ID for %s on CPU%d\n", >> dev_name(&drvdata->csdev->dev), drvdata->cpu); >> diff --git a/drivers/hwtracing/coresight/coresight-trace-id.c b/drivers/hwtracing/coresight/coresight-trace-id.c >> index 45ddd50d09a6..b393603dd713 100644 >> --- a/drivers/hwtracing/coresight/coresight-trace-id.c >> +++ b/drivers/hwtracing/coresight/coresight-trace-id.c >> @@ -13,10 +13,12 @@ >> #include "coresight-trace-id.h" >> >> /* Default trace ID map. Used in sysfs mode and for system sources */ >> -static struct coresight_trace_id_map id_map_default; >> +static DEFINE_PER_CPU(atomic_t, id_map_default_cpu_ids) = ATOMIC_INIT(0); >> +static struct coresight_trace_id_map id_map_default = { >> + .cpu_map = &id_map_default_cpu_ids >> +}; >> >> -/* maintain a record of the mapping of IDs and pending releases per cpu */ >> -static DEFINE_PER_CPU(atomic_t, cpu_id) = ATOMIC_INIT(0); >> +/* maintain a record of the pending releases per cpu */ >> static cpumask_t cpu_id_release_pending; >> >> /* perf session active counter */ >> @@ -46,12 +48,6 @@ static void coresight_trace_id_dump_table(struct coresight_trace_id_map *id_map, >> #define PERF_SESSION(n) >> #endif >> >> -/* unlocked read of current trace ID value for given CPU */ >> -static int _coresight_trace_id_read_cpu_id(int cpu) >> -{ >> - return atomic_read(&per_cpu(cpu_id, cpu)); >> -} > > Just wondering where this per cpu cpu_id ^^ is being dropped off as well > OR is it still getting used ? > No it's still needed. It's not dropped in this set. I just moved the implementation into coresight_trace_id_read_cpu_id() rather than add a new argument to _coresight_trace_id_read_cpu_id(). Although I might change that because of Mike's comment about keeping both the map and non map versions of the functions to reduce some of the churn changing the callers.
diff --git a/drivers/hwtracing/coresight/coresight-etm-perf.c b/drivers/hwtracing/coresight/coresight-etm-perf.c index 4afb9d29f355..25f1f87c90d1 100644 --- a/drivers/hwtracing/coresight/coresight-etm-perf.c +++ b/drivers/hwtracing/coresight/coresight-etm-perf.c @@ -508,7 +508,8 @@ static void etm_event_start(struct perf_event *event, int flags) hw_id = FIELD_PREP(CS_AUX_HW_ID_VERSION_MASK, CS_AUX_HW_ID_CURR_VERSION); hw_id |= FIELD_PREP(CS_AUX_HW_ID_TRACE_ID_MASK, - coresight_trace_id_read_cpu_id(cpu)); + coresight_trace_id_read_cpu_id(cpu, + coresight_trace_id_map_default())); perf_report_aux_output_id(event, hw_id); } diff --git a/drivers/hwtracing/coresight/coresight-etm3x-core.c b/drivers/hwtracing/coresight/coresight-etm3x-core.c index 4149e7675ceb..b21f5ad94e63 100644 --- a/drivers/hwtracing/coresight/coresight-etm3x-core.c +++ b/drivers/hwtracing/coresight/coresight-etm3x-core.c @@ -501,7 +501,8 @@ static int etm_enable_perf(struct coresight_device *csdev, * with perf locks - we know the ID cannot change until perf shuts down * the session */ - trace_id = coresight_trace_id_read_cpu_id(drvdata->cpu); + trace_id = coresight_trace_id_read_cpu_id(drvdata->cpu, + coresight_trace_id_map_default()); if (!IS_VALID_CS_TRACE_ID(trace_id)) { dev_err(&drvdata->csdev->dev, "Failed to set trace ID for %s on CPU%d\n", dev_name(&drvdata->csdev->dev), drvdata->cpu); diff --git a/drivers/hwtracing/coresight/coresight-etm4x-core.c b/drivers/hwtracing/coresight/coresight-etm4x-core.c index f32c8cd7742d..d16d6efb26fa 100644 --- a/drivers/hwtracing/coresight/coresight-etm4x-core.c +++ b/drivers/hwtracing/coresight/coresight-etm4x-core.c @@ -776,7 +776,8 @@ static int etm4_enable_perf(struct coresight_device *csdev, * with perf locks - we know the ID cannot change until perf shuts down * the session */ - trace_id = coresight_trace_id_read_cpu_id(drvdata->cpu); + trace_id = coresight_trace_id_read_cpu_id(drvdata->cpu, + coresight_trace_id_map_default()); if (!IS_VALID_CS_TRACE_ID(trace_id)) { dev_err(&drvdata->csdev->dev, "Failed to set trace ID for %s on CPU%d\n", dev_name(&drvdata->csdev->dev), drvdata->cpu); diff --git a/drivers/hwtracing/coresight/coresight-trace-id.c b/drivers/hwtracing/coresight/coresight-trace-id.c index 45ddd50d09a6..b393603dd713 100644 --- a/drivers/hwtracing/coresight/coresight-trace-id.c +++ b/drivers/hwtracing/coresight/coresight-trace-id.c @@ -13,10 +13,12 @@ #include "coresight-trace-id.h" /* Default trace ID map. Used in sysfs mode and for system sources */ -static struct coresight_trace_id_map id_map_default; +static DEFINE_PER_CPU(atomic_t, id_map_default_cpu_ids) = ATOMIC_INIT(0); +static struct coresight_trace_id_map id_map_default = { + .cpu_map = &id_map_default_cpu_ids +}; -/* maintain a record of the mapping of IDs and pending releases per cpu */ -static DEFINE_PER_CPU(atomic_t, cpu_id) = ATOMIC_INIT(0); +/* maintain a record of the pending releases per cpu */ static cpumask_t cpu_id_release_pending; /* perf session active counter */ @@ -46,12 +48,6 @@ static void coresight_trace_id_dump_table(struct coresight_trace_id_map *id_map, #define PERF_SESSION(n) #endif -/* unlocked read of current trace ID value for given CPU */ -static int _coresight_trace_id_read_cpu_id(int cpu) -{ - return atomic_read(&per_cpu(cpu_id, cpu)); -} - /* look for next available odd ID, return 0 if none found */ static int coresight_trace_id_find_odd_id(struct coresight_trace_id_map *id_map) { @@ -145,7 +141,7 @@ static void coresight_trace_id_release_all_pending(void) clear_bit(bit, id_map->pend_rel_ids); } for_each_cpu(cpu, &cpu_id_release_pending) { - atomic_set(&per_cpu(cpu_id, cpu), 0); + atomic_set(per_cpu_ptr(id_map_default.cpu_map, cpu), 0); cpumask_clear_cpu(cpu, &cpu_id_release_pending); } spin_unlock_irqrestore(&id_map_lock, flags); @@ -160,7 +156,7 @@ int coresight_trace_id_get_cpu_id(int cpu, struct coresight_trace_id_map *id_map spin_lock_irqsave(&id_map_lock, flags); /* check for existing allocation for this CPU */ - id = _coresight_trace_id_read_cpu_id(cpu); + id = coresight_trace_id_read_cpu_id(cpu, id_map); if (id) goto get_cpu_id_clr_pend; @@ -181,7 +177,7 @@ int coresight_trace_id_get_cpu_id(int cpu, struct coresight_trace_id_map *id_map goto get_cpu_id_out_unlock; /* allocate the new id to the cpu */ - atomic_set(&per_cpu(cpu_id, cpu), id); + atomic_set(per_cpu_ptr(id_map->cpu_map, cpu), id); get_cpu_id_clr_pend: /* we are (re)using this ID - so ensure it is not marked for release */ @@ -203,7 +199,7 @@ void coresight_trace_id_put_cpu_id(int cpu, struct coresight_trace_id_map *id_ma int id; /* check for existing allocation for this CPU */ - id = _coresight_trace_id_read_cpu_id(cpu); + id = coresight_trace_id_read_cpu_id(cpu, id_map); if (!id) return; @@ -216,7 +212,7 @@ void coresight_trace_id_put_cpu_id(int cpu, struct coresight_trace_id_map *id_ma } else { /* otherwise clear id */ coresight_trace_id_free(id, id_map); - atomic_set(&per_cpu(cpu_id, cpu), 0); + atomic_set(per_cpu_ptr(id_map->cpu_map, cpu), 0); } spin_unlock_irqrestore(&id_map_lock, flags); @@ -258,9 +254,9 @@ struct coresight_trace_id_map *coresight_trace_id_map_default(void) } EXPORT_SYMBOL_GPL(coresight_trace_id_map_default); -int coresight_trace_id_read_cpu_id(int cpu) +int coresight_trace_id_read_cpu_id(int cpu, struct coresight_trace_id_map *id_map) { - return _coresight_trace_id_read_cpu_id(cpu); + return atomic_read(per_cpu_ptr(id_map->cpu_map, cpu)); } EXPORT_SYMBOL_GPL(coresight_trace_id_read_cpu_id); diff --git a/drivers/hwtracing/coresight/coresight-trace-id.h b/drivers/hwtracing/coresight/coresight-trace-id.h index 54b9d8ed903b..ed2bc4b3ad2a 100644 --- a/drivers/hwtracing/coresight/coresight-trace-id.h +++ b/drivers/hwtracing/coresight/coresight-trace-id.h @@ -93,7 +93,7 @@ void coresight_trace_id_put_cpu_id(int cpu, struct coresight_trace_id_map *id_ma * * return: current value, will be 0 if unallocated. */ -int coresight_trace_id_read_cpu_id(int cpu); +int coresight_trace_id_read_cpu_id(int cpu, struct coresight_trace_id_map *id_map); /** * Allocate a CoreSight trace ID for a system component. diff --git a/include/linux/coresight.h b/include/linux/coresight.h index c16c61a8411d..7d62b88bfb5c 100644 --- a/include/linux/coresight.h +++ b/include/linux/coresight.h @@ -234,6 +234,7 @@ struct coresight_sysfs_link { struct coresight_trace_id_map { DECLARE_BITMAP(used_ids, CORESIGHT_TRACE_IDS_MAX); DECLARE_BITMAP(pend_rel_ids, CORESIGHT_TRACE_IDS_MAX); + atomic_t __percpu *cpu_map; }; /**
The global CPU ID mappings won't work for per-sink ID maps so move it to the ID map struct. coresight_trace_id_release_all_pending() is hard coded to operate on the default map, but once Perf sessions use their own maps the pending release mechanism will be deleted. So it doesn't need to be extended to accept a trace ID map argument at this point. Signed-off-by: James Clark <james.clark@arm.com> --- .../hwtracing/coresight/coresight-etm-perf.c | 3 +- .../coresight/coresight-etm3x-core.c | 3 +- .../coresight/coresight-etm4x-core.c | 3 +- .../hwtracing/coresight/coresight-trace-id.c | 28 ++++++++----------- .../hwtracing/coresight/coresight-trace-id.h | 2 +- include/linux/coresight.h | 1 + 6 files changed, 20 insertions(+), 20 deletions(-)