All of lore.kernel.org
 help / color / mirror / Atom feed
From: Peter Zijlstra <peterz@infradead.org>
To: Jiri Olsa <jolsa@redhat.com>
Cc: Jiri Olsa <jolsa@kernel.org>,
	Arnaldo Carvalho de Melo <acme@kernel.org>,
	lkml <linux-kernel@vger.kernel.org>,
	Ingo Molnar <mingo@kernel.org>,
	Namhyung Kim <namhyung@kernel.org>,
	Alexander Shishkin <alexander.shishkin@linux.intel.com>,
	Andi Kleen <andi@firstfloor.org>
Subject: Re: [PATCH] perf: Prevent recursion in ring buffer
Date: Thu, 13 Sep 2018 11:37:54 +0200	[thread overview]
Message-ID: <20180913093754.GV24124@hirez.programming.kicks-ass.net> (raw)
In-Reply-To: <20180913074607.GB15173@krava>

On Thu, Sep 13, 2018 at 09:46:07AM +0200, Jiri Olsa wrote:
> On Thu, Sep 13, 2018 at 09:07:40AM +0200, Peter Zijlstra wrote:
> > On Wed, Sep 12, 2018 at 09:33:17PM +0200, Jiri Olsa wrote:
> > > Some of the scheduling tracepoints allow the perf_tp_event
> > > code to write to ring buffer under different cpu than the
> > > code is running on.
> > 
> > ARGH.. that is indeed borken.

> I was first thinking to just leave it on the current cpu,
> but not sure current users would be ok with that ;-)

> ---
> diff --git a/kernel/events/core.c b/kernel/events/core.c
> index abaed4f8bb7f..9b534a2ecf17 100644
> --- a/kernel/events/core.c
> +++ b/kernel/events/core.c
> @@ -8308,6 +8308,8 @@ void perf_tp_event(u16 event_type, u64 count, void *record, int entry_size,
>  				continue;
>  			if (event->attr.config != entry->type)
>  				continue;
> +			if (event->cpu != smp_processor_id())
> +				continue;
>  			if (perf_tp_event_match(event, &data, regs))
>  				perf_swevent_event(event, count, &data, regs);
>  		}

That might indeed be the best we can do.

So the whole TP muck would be responsible for placing only matching
events on the hlist, which is where our normal CPU filter is I think.

The above then does the same for @task. Which without this would also be
getting nr_cpus copies of the event I think.

It does mean not getting any events if the @task only has a per-task
buffer, but there's nothing to be done about that. And I'm not even sure
we can create a useful warning for that :/

  reply	other threads:[~2018-09-13  9:38 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-09-12 19:33 [PATCH] perf: Prevent recursion in ring buffer Jiri Olsa
2018-09-13  7:07 ` Peter Zijlstra
2018-09-13  7:41   ` Jiri Olsa
2018-09-13  7:46   ` Jiri Olsa
2018-09-13  9:37     ` Peter Zijlstra [this message]
2018-09-23 16:13       ` [PATCHv2] perf: Prevent concurent ring buffer access Jiri Olsa
2018-10-02 10:01         ` [tip:perf/core] perf/ring_buffer: " tip-bot for Jiri Olsa
2018-09-13  7:40 ` [PATCH] perf: Prevent recursion in ring buffer Peter Zijlstra
2018-09-13  7:53   ` Jiri Olsa

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=20180913093754.GV24124@hirez.programming.kicks-ass.net \
    --to=peterz@infradead.org \
    --cc=acme@kernel.org \
    --cc=alexander.shishkin@linux.intel.com \
    --cc=andi@firstfloor.org \
    --cc=jolsa@kernel.org \
    --cc=jolsa@redhat.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mingo@kernel.org \
    --cc=namhyung@kernel.org \
    /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
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.