[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <8080ae06-7014-4afe-8620-ffaca6e3c597@arm.com>
Date: Tue, 7 May 2024 11:52:15 +0530
From: Anshuman Khandual <anshuman.khandual@....com>
To: James Clark <james.clark@....com>, linux-perf-users@...r.kernel.org,
gankulkarni@...amperecomputing.com, scclevenger@...amperecomputing.com,
coresight@...ts.linaro.org, suzuki.poulose@....com, mike.leach@...aro.org
Cc: Alexander Shishkin <alexander.shishkin@...ux.intel.com>,
Maxime Coquelin <mcoquelin.stm32@...il.com>,
Alexandre Torgue <alexandre.torgue@...s.st.com>,
Peter Zijlstra <peterz@...radead.org>, Ingo Molnar <mingo@...hat.com>,
Arnaldo Carvalho de Melo <acme@...nel.org>,
Namhyung Kim <namhyung@...nel.org>, Mark Rutland <mark.rutland@....com>,
Jiri Olsa <jolsa@...nel.org>, Ian Rogers <irogers@...gle.com>,
Adrian Hunter <adrian.hunter@...el.com>, John Garry
<john.g.garry@...cle.com>, Will Deacon <will@...nel.org>,
Leo Yan <leo.yan@...ux.dev>, linux-arm-kernel@...ts.infradead.org,
linux-kernel@...r.kernel.org, linux-stm32@...md-mailman.stormreply.com
Subject: Re: [PATCH 12/17] coresight: Make CPU id map a property of a trace ID
map
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@....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;
> };
>
> /**
Powered by blists - more mailing lists