LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
From: Mathieu Poirier <mathieu.poirier@linaro.org>
To: Suzuki K Poulose <suzuki.poulose@arm.com>
Cc: linux-arm-kernel <linux-arm-kernel@lists.infradead.org>,
	Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
	Coresight ML <coresight@lists.linaro.org>
Subject: Re: [PATCH 1/2] coresight: Do not call smp_processor_id() from preemptible
Date: Wed, 8 May 2019 10:39:30 -0600	[thread overview]
Message-ID: <CANLsYkyxDY8g9zyhTyTqALgF5dmVX1F7DA_93ECpnvAaACYX8g@mail.gmail.com> (raw)
In-Reply-To: <1556899459-27785-1-git-send-email-suzuki.poulose@arm.com>

Hi Suzuki,

On Fri, 3 May 2019 at 10:04, Suzuki K Poulose <suzuki.poulose@arm.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@linaro.org>
> Signed-off-by: Suzuki K Poulose <suzuki.poulose@arm.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?

Thanks,
Mathieu

>
>         etr_perf = kzalloc_node(sizeof(*etr_perf), GFP_KERNEL, node);
>         if (!etr_perf)
> --
> 2.7.4
>

  parent reply	other threads:[~2019-05-08 16:39 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-05-03 16:04 Suzuki K Poulose
2019-05-03 16:04 ` [PATCH 2/2] coresight: alloc_perf_buf: Do not call smp_processor_id " Suzuki K Poulose
2019-05-08 16:39 ` Mathieu Poirier [this message]
2019-05-08 16:43   ` [PATCH 1/2] coresight: Do not call smp_processor_id() " Suzuki K Poulose

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=CANLsYkyxDY8g9zyhTyTqALgF5dmVX1F7DA_93ECpnvAaACYX8g@mail.gmail.com \
    --to=mathieu.poirier@linaro.org \
    --cc=coresight@lists.linaro.org \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=suzuki.poulose@arm.com \
    --subject='Re: [PATCH 1/2] coresight: Do not call smp_processor_id() from preemptible' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).