The kernel start address is already cached in the machine struct once it
is initialised, so storing it in the cs_etm struct is unnecessary.
It also depends on kernel maps being available to be initialised.
Therefore cs_etm__setup_queues() isn't an appropriate place to call it
because it could be called before processing starts. It would be better
to initialise it at the point when it is needed, then we can be sure
that all the necessary maps are available. Also by calling
machine__kernel_start() multiple times it can be initialised at some
point, even if it failed to initialise previously due to missing maps.
In a later commit cs_etm__setup_queues() will be moved which is the
motivation for this change.
Signed-off-by: James Clark <[email protected]>
---
tools/perf/util/cs-etm.c | 6 +-----
1 file changed, 1 insertion(+), 5 deletions(-)
diff --git a/tools/perf/util/cs-etm.c b/tools/perf/util/cs-etm.c
index bc1f64873c8f..4c69ef391f60 100644
--- a/tools/perf/util/cs-etm.c
+++ b/tools/perf/util/cs-etm.c
@@ -62,7 +62,6 @@ struct cs_etm_auxtrace {
u64 instructions_sample_period;
u64 instructions_id;
u64 **metadata;
- u64 kernel_start;
unsigned int pmu_type;
};
@@ -691,7 +690,7 @@ static u8 cs_etm__cpu_mode(struct cs_etm_queue *etmq, u64 address)
machine = etmq->etm->machine;
- if (address >= etmq->etm->kernel_start) {
+ if (address >= machine__kernel_start(machine)) {
if (machine__is_host(machine))
return PERF_RECORD_MISC_KERNEL;
else
@@ -901,9 +900,6 @@ static int cs_etm__setup_queues(struct cs_etm_auxtrace *etm)
unsigned int i;
int ret;
- if (!etm->kernel_start)
- etm->kernel_start = machine__kernel_start(etm->machine);
-
for (i = 0; i < etm->queues.nr_queues; i++) {
ret = cs_etm__setup_queue(etm, &etm->queues.queue_array[i], i);
if (ret)
--
2.28.0
On Tue, Jul 13, 2021 at 04:40:03PM +0100, James Clark wrote:
> The kernel start address is already cached in the machine struct once it
> is initialised, so storing it in the cs_etm struct is unnecessary.
>
> It also depends on kernel maps being available to be initialised.
> Therefore cs_etm__setup_queues() isn't an appropriate place to call it
> because it could be called before processing starts. It would be better
> to initialise it at the point when it is needed, then we can be sure
> that all the necessary maps are available. Also by calling
> machine__kernel_start() multiple times it can be initialised at some
> point, even if it failed to initialise previously due to missing maps.
>
> In a later commit cs_etm__setup_queues() will be moved which is the
> motivation for this change.
>
> Signed-off-by: James Clark <[email protected]>
Reviewed-by: Mathieu Poirier <[email protected]>
> ---
> tools/perf/util/cs-etm.c | 6 +-----
> 1 file changed, 1 insertion(+), 5 deletions(-)
>
> diff --git a/tools/perf/util/cs-etm.c b/tools/perf/util/cs-etm.c
> index bc1f64873c8f..4c69ef391f60 100644
> --- a/tools/perf/util/cs-etm.c
> +++ b/tools/perf/util/cs-etm.c
> @@ -62,7 +62,6 @@ struct cs_etm_auxtrace {
> u64 instructions_sample_period;
> u64 instructions_id;
> u64 **metadata;
> - u64 kernel_start;
> unsigned int pmu_type;
> };
>
> @@ -691,7 +690,7 @@ static u8 cs_etm__cpu_mode(struct cs_etm_queue *etmq, u64 address)
>
> machine = etmq->etm->machine;
>
> - if (address >= etmq->etm->kernel_start) {
> + if (address >= machine__kernel_start(machine)) {
> if (machine__is_host(machine))
> return PERF_RECORD_MISC_KERNEL;
> else
> @@ -901,9 +900,6 @@ static int cs_etm__setup_queues(struct cs_etm_auxtrace *etm)
> unsigned int i;
> int ret;
>
> - if (!etm->kernel_start)
> - etm->kernel_start = machine__kernel_start(etm->machine);
> -
> for (i = 0; i < etm->queues.nr_queues; i++) {
> ret = cs_etm__setup_queue(etm, &etm->queues.queue_array[i], i);
> if (ret)
> --
> 2.28.0
>