[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <ea6e3e0f-dea4-39dd-cc29-14001091a4ec@arm.com>
Date: Wed, 8 May 2019 17:43:53 +0100
From: Suzuki K Poulose <suzuki.poulose@....com>
To: mathieu.poirier@...aro.org
Cc: linux-arm-kernel@...ts.infradead.org, linux-kernel@...r.kernel.org,
coresight@...ts.linaro.org
Subject: Re: [PATCH 1/2] coresight: Do not call smp_processor_id() from
preemptible
On 08/05/2019 17:39, Mathieu Poirier wrote:
> Hi Suzuki,
>
> On Fri, 3 May 2019 at 10:04, Suzuki K Poulose <suzuki.poulose@....com> wrote:
>>
>> Instead of using smp_processor_id() to figure out the node,
>> use the numa_node_id() for the current CPU node to avoid
>> splats like :
>>
>> BUG: using smp_processor_id() in preemptible [00000000] code: perf/1743
>> caller is alloc_etr_buf.isra.6+0x80/0xa0
>> CPU: 1 PID: 1743 Comm: perf Not tainted 5.1.0-rc6-147786-g116841e #344
>> Hardware name: ARM LTD ARM Juno Development Platform/ARM Juno Development Platform, BIOS EDK II Feb 1 2019
>> Call trace:
>> dump_backtrace+0x0/0x150
>> show_stack+0x14/0x20
>> dump_stack+0x9c/0xc4
>> debug_smp_processor_id+0x10c/0x110
>> alloc_etr_buf.isra.6+0x80/0xa0
>> tmc_alloc_etr_buffer+0x12c/0x1f0
>> etm_setup_aux+0x1c4/0x230
>> rb_alloc_aux+0x1b8/0x2b8
>> perf_mmap+0x35c/0x478
>> mmap_region+0x34c/0x4f0
>> do_mmap+0x2d8/0x418
>> vm_mmap_pgoff+0xd0/0xf8
>> ksys_mmap_pgoff+0x88/0xf8
>> __arm64_sys_mmap+0x28/0x38
>> el0_svc_handler+0xd8/0x138
>> el0_svc+0x8/0xc
>>
>
> That is very interesting...
>
>> Fixes: 855ab61c16bf70b646 ("coresight: tmc-etr: Refactor function tmc_etr_setup_perf_buf()")
>> Cc: Mathieu Poirier <mathieu.poirier@...aro.org>
>> Signed-off-by: Suzuki K Poulose <suzuki.poulose@....com>
>> ---
>> drivers/hwtracing/coresight/coresight-tmc-etr.c | 6 ++----
>> 1 file changed, 2 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/hwtracing/coresight/coresight-tmc-etr.c b/drivers/hwtracing/coresight/coresight-tmc-etr.c
>> index 793639f..74578bd 100644
>> --- a/drivers/hwtracing/coresight/coresight-tmc-etr.c
>> +++ b/drivers/hwtracing/coresight/coresight-tmc-etr.c
>> @@ -1323,13 +1323,11 @@ static struct etr_perf_buffer *
>> tmc_etr_setup_perf_buf(struct tmc_drvdata *drvdata, struct perf_event *event,
>> int nr_pages, void **pages, bool snapshot)
>> {
>> - int node, cpu = event->cpu;
>> + int node;
>> struct etr_buf *etr_buf;
>> struct etr_perf_buffer *etr_perf;
>>
>> - if (cpu == -1)
>> - cpu = smp_processor_id();
>> - node = cpu_to_node(cpu);
>> + node = (event->cpu == -1)? numa_node_id() : cpu_to_node(event->cpu);
>
> Seems to me using numa_node_id() simply circumvent function
> debug_smp_processor_id() and using get_cpu() and put_cpu() would be
> more appropriate. But I'll trust the NUMA people have thought about
> this long before me. Would you mind sending another revision that fix
> the same code for ETB and ETF?
Sure, will send it soon.
Suzuki
Powered by blists - more mailing lists