Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1398082 > unrolled thread

[PATCH 0/2] perf, pt: Fix massive data losses

Back to article view | Back to linux.kernel

Contents

  [PATCH 0/2] perf, pt: Fix massive data losses Alexander Shishkin <alexander.shishkin@linux.intel.com> - 2016-05-10 15:20 +0200
    [PATCH 1/2] perf/x86/intel/pt: Generate PMI in the STOP region as well Alexander Shishkin <alexander.shishkin@linux.intel.com> - 2016-05-10 15:20 +0200
      [tip:perf/core] perf/x86/intel/pt: Generate PMI in the STOP region  as well tip-bot for Alexander Shishkin <tipbot@zytor.com> - 2016-05-12 12:40 +0200
        Re: [tip:perf/core] perf/x86/intel/pt: Generate PMI in the STOP region as well Alexander Shishkin <alexander.shishkin@linux.intel.com> - 2016-05-12 14:30 +0200
          Re: [tip:perf/core] perf/x86/intel/pt: Generate PMI in the STOP  region as well Ingo Molnar <mingo@kernel.org> - 2016-05-12 14:50 +0200
            Re: [tip:perf/core] perf/x86/intel/pt: Generate PMI in the STOP region as well Alexander Shishkin <alexander.shishkin@linux.intel.com> - 2016-05-12 15:10 +0200
      [tip:perf/urgent] perf/x86/intel/pt: Generate PMI in the STOP  region as well tip-bot for Alexander Shishkin <tipbot@zytor.com> - 2016-05-12 15:00 +0200
        Re: [tip:perf/urgent] perf/x86/intel/pt: Generate PMI in the STOP region as well Alexander Shishkin <alexander.shishkin@linux.intel.com> - 2016-05-17 09:40 +0200
          Re: [tip:perf/urgent] perf/x86/intel/pt: Generate PMI in the STOP  region as well Ingo Molnar <mingo@kernel.org> - 2016-05-17 10:50 +0200
            Re: [tip:perf/urgent] perf/x86/intel/pt: Generate PMI in the STOP  region as well Greg KH <gregkh@linuxfoundation.org> - 2016-05-21 07:30 +0200
    [PATCH 2/2] perf: Disable the event on a truncated AUX record Alexander Shishkin <alexander.shishkin@linux.intel.com> - 2016-05-10 15:20 +0200
      Re: [PATCH 2/2] perf: Disable the event on a truncated AUX record Peter Zijlstra <peterz@infradead.org> - 2016-05-11 11:20 +0200
        Re: [PATCH 2/2] perf: Disable the event on a truncated AUX record Alexander Shishkin <alexander.shishkin@linux.intel.com> - 2016-05-11 11:50 +0200
          Re: [PATCH 2/2] perf: Disable the event on a truncated AUX record Peter Zijlstra <peterz@infradead.org> - 2016-05-11 12:00 +0200
            Re: [PATCH 2/2] perf: Disable the event on a truncated AUX record Alexander Shishkin <alexander.shishkin@linux.intel.com> - 2016-05-11 12:10 +0200
      [tip:perf/core] perf/core: Disable the event on a truncated AUX  record tip-bot for Alexander Shishkin <tipbot@zytor.com> - 2016-05-12 12:40 +0200
      [tip:perf/urgent] perf/core: Disable the event on a truncated AUX  record tip-bot for Alexander Shishkin <tipbot@zytor.com> - 2016-05-12 15:00 +0200

#1398082 — [PATCH 0/2] perf, pt: Fix massive data losses

Hi Peter,

Markus reported that PT is losing a lot of its data in cpu-wide mode.
There are two problems in play here, both are amplified by the fact
that cpu-wide events don't normally get re-scheduled, so if we don't
disable them when their buffers are full, they get forever stuck.
Hence two patches. I propose to put these through perf/urgent and also
stable trees so that people with older kernels still get to collect
humongous amounts of PT traces in cpu-wide mode.

Alexander Shishkin (2):
  perf/x86/intel/pt: Generate PMI in the STOP region as well
  perf: Disable the event on a truncated AUX record

 arch/x86/events/intel/pt.c  |  2 ++
 kernel/events/ring_buffer.c | 10 +++++++++-
 2 files changed, 11 insertions(+), 1 deletion(-)

-- 
2.8.1

[toc] | [next] | [standalone]

#1398083 — [PATCH 1/2] perf/x86/intel/pt: Generate PMI in the STOP region as well

Currently, the PT driver always sets the PMI bit one region (page) before
the STOP region so that we can wake up the consumer before we run out of
room in the buffer and have to disable the event. However, we also need
an interrupt in the last output region, so that we actually get to disable
the event (if no more room from new data is available at that point),
otherwise hardware just quietly refuses to start, but the event is
scheduled in and we end up losing trace data till the event gets removed.

For a cpu-wide event it is even worse since there may not be any
re-scheduling at all and no chance for the ring buffer code to notice
that its buffer is filled up and the event needs to be disabled (so that
the consumer can re-enable it when it finishes reading the data out). In
other words, all the trace data will be lost after the buffer gets filled
up.

This patch makes PT also generate a PMI when the last output region is
full.

Reported-by: Markus Metzger <markus.t.metzger@intel.com>
Signed-off-by: Alexander Shishkin <alexander.shishkin@linux.intel.com>
---
 arch/x86/events/intel/pt.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/arch/x86/events/intel/pt.c b/arch/x86/events/intel/pt.c
index 5b648d4a94..a157cf67a9 100644
--- a/arch/x86/events/intel/pt.c
+++ b/arch/x86/events/intel/pt.c
@@ -861,6 +861,7 @@ static int pt_buffer_reset_markers(struct pt_buffer *buf,
 
 	/* clear STOP and INT from current entry */
 	buf->topa_index[buf->stop_pos]->stop = 0;
+	buf->topa_index[buf->stop_pos]->intr = 0;
 	buf->topa_index[buf->intr_pos]->intr = 0;
 
 	/* how many pages till the STOP marker */
@@ -885,6 +886,7 @@ static int pt_buffer_reset_markers(struct pt_buffer *buf,
 	buf->intr_pos = idx;
 
 	buf->topa_index[buf->stop_pos]->stop = 1;
+	buf->topa_index[buf->stop_pos]->intr = 1;
 	buf->topa_index[buf->intr_pos]->intr = 1;
 
 	return 0;
-- 
2.8.1

[toc] | [prev] | [next] | [standalone]

#1399903 — [tip:perf/core] perf/x86/intel/pt: Generate PMI in the STOP region as well

Commit-ID:  5fbe4788b55540a6c4fe2c47e05482ac356eaf74
Gitweb:     http://git.kernel.org/tip/5fbe4788b55540a6c4fe2c47e05482ac356eaf74
Author:     Alexander Shishkin <alexander.shishkin@linux.intel.com>
AuthorDate: Tue, 10 May 2016 16:18:32 +0300
Committer:  Ingo Molnar <mingo@kernel.org>
CommitDate: Thu, 12 May 2016 10:14:55 +0200

perf/x86/intel/pt: Generate PMI in the STOP region as well

Currently, the PT driver always sets the PMI bit one region (page) before
the STOP region so that we can wake up the consumer before we run out of
room in the buffer and have to disable the event. However, we also need
an interrupt in the last output region, so that we actually get to disable
the event (if no more room from new data is available at that point),
otherwise hardware just quietly refuses to start, but the event is
scheduled in and we end up losing trace data till the event gets removed.

For a cpu-wide event it is even worse since there may not be any
re-scheduling at all and no chance for the ring buffer code to notice
that its buffer is filled up and the event needs to be disabled (so that
the consumer can re-enable it when it finishes reading the data out). In
other words, all the trace data will be lost after the buffer gets filled
up.

This patch makes PT also generate a PMI when the last output region is
full.

Reported-by: Markus Metzger <markus.t.metzger@intel.com>
Signed-off-by: Alexander Shishkin <alexander.shishkin@linux.intel.com>
Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
Cc: Arnaldo Carvalho de Melo <acme@infradead.org>
Cc: Arnaldo Carvalho de Melo <acme@redhat.com>
Cc: Borislav Petkov <bp@alien8.de>
Cc: Jiri Olsa <jolsa@redhat.com>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Stephane Eranian <eranian@google.com>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Vince Weaver <vincent.weaver@maine.edu>
Cc: vince@deater.net
Link: http://lkml.kernel.org/r/1462886313-13660-2-git-send-email-alexander.shishkin@linux.intel.com
Signed-off-by: Ingo Molnar <mingo@kernel.org>
---
 arch/x86/events/intel/pt.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/arch/x86/events/intel/pt.c b/arch/x86/events/intel/pt.c
index 54fa238..04bb5fb 100644
--- a/arch/x86/events/intel/pt.c
+++ b/arch/x86/events/intel/pt.c
@@ -835,6 +835,7 @@ static int pt_buffer_reset_markers(struct pt_buffer *buf,
 
 	/* clear STOP and INT from current entry */
 	buf->topa_index[buf->stop_pos]->stop = 0;
+	buf->topa_index[buf->stop_pos]->intr = 0;
 	buf->topa_index[buf->intr_pos]->intr = 0;
 
 	/* how many pages till the STOP marker */
@@ -859,6 +860,7 @@ static int pt_buffer_reset_markers(struct pt_buffer *buf,
 	buf->intr_pos = idx;
 
 	buf->topa_index[buf->stop_pos]->stop = 1;
+	buf->topa_index[buf->stop_pos]->intr = 1;
 	buf->topa_index[buf->intr_pos]->intr = 1;
 
 	return 0;

[toc] | [prev] | [next] | [standalone]

#1400014 — Re: [tip:perf/core] perf/x86/intel/pt: Generate PMI in the STOP region as well

tip-bot for Alexander Shishkin <tipbot@zytor.com> writes:

> Commit-ID:  5fbe4788b55540a6c4fe2c47e05482ac356eaf74
> Gitweb:     http://git.kernel.org/tip/5fbe4788b55540a6c4fe2c47e05482ac356eaf74
> Author:     Alexander Shishkin <alexander.shishkin@linux.intel.com>
> AuthorDate: Tue, 10 May 2016 16:18:32 +0300
> Committer:  Ingo Molnar <mingo@kernel.org>
> CommitDate: Thu, 12 May 2016 10:14:55 +0200
>
> perf/x86/intel/pt: Generate PMI in the STOP region as well

I was hoping to get this (and the other "truncated" thing patch) in via
perf/urgent, them being bugs that cause people to lose trace data.

Regards,
--
Alex

[toc] | [prev] | [next] | [standalone]

#1400039 — Re: [tip:perf/core] perf/x86/intel/pt: Generate PMI in the STOP region as well

* Alexander Shishkin <alexander.shishkin@linux.intel.com> wrote:

> tip-bot for Alexander Shishkin <tipbot@zytor.com> writes:
> 
> > Commit-ID:  5fbe4788b55540a6c4fe2c47e05482ac356eaf74
> > Gitweb:     http://git.kernel.org/tip/5fbe4788b55540a6c4fe2c47e05482ac356eaf74
> > Author:     Alexander Shishkin <alexander.shishkin@linux.intel.com>
> > AuthorDate: Tue, 10 May 2016 16:18:32 +0300
> > Committer:  Ingo Molnar <mingo@kernel.org>
> > CommitDate: Thu, 12 May 2016 10:14:55 +0200
> >
> > perf/x86/intel/pt: Generate PMI in the STOP region as well
> 
> I was hoping to get this (and the other "truncated" thing patch) in via
> perf/urgent, them being bugs that cause people to lose trace data.

Ok, agreed - I cherry-picked them over into perf/urgent as well.

Thanks,

	Ingo

[toc] | [prev] | [next] | [standalone]

#1400057 — Re: [tip:perf/core] perf/x86/intel/pt: Generate PMI in the STOP region as well

Ingo Molnar <mingo@kernel.org> writes:

> * Alexander Shishkin <alexander.shishkin@linux.intel.com> wrote:
>
>> tip-bot for Alexander Shishkin <tipbot@zytor.com> writes:
>> 
>> > Commit-ID:  5fbe4788b55540a6c4fe2c47e05482ac356eaf74
>> > Gitweb:     http://git.kernel.org/tip/5fbe4788b55540a6c4fe2c47e05482ac356eaf74
>> > Author:     Alexander Shishkin <alexander.shishkin@linux.intel.com>
>> > AuthorDate: Tue, 10 May 2016 16:18:32 +0300
>> > Committer:  Ingo Molnar <mingo@kernel.org>
>> > CommitDate: Thu, 12 May 2016 10:14:55 +0200
>> >
>> > perf/x86/intel/pt: Generate PMI in the STOP region as well
>> 
>> I was hoping to get this (and the other "truncated" thing patch) in via
>> perf/urgent, them being bugs that cause people to lose trace data.
>
> Ok, agreed - I cherry-picked them over into perf/urgent as well.

Thanks!

Regards,
--
Alex

[toc] | [prev] | [next] | [standalone]

#1400041 — [tip:perf/urgent] perf/x86/intel/pt: Generate PMI in the STOP region as well

Commit-ID:  ab92b232ae05c382c3df0e3d6a5c6d16b639ac8c
Gitweb:     http://git.kernel.org/tip/ab92b232ae05c382c3df0e3d6a5c6d16b639ac8c
Author:     Alexander Shishkin <alexander.shishkin@linux.intel.com>
AuthorDate: Tue, 10 May 2016 16:18:32 +0300
Committer:  Ingo Molnar <mingo@kernel.org>
CommitDate: Thu, 12 May 2016 14:45:59 +0200

perf/x86/intel/pt: Generate PMI in the STOP region as well

Currently, the PT driver always sets the PMI bit one region (page) before
the STOP region so that we can wake up the consumer before we run out of
room in the buffer and have to disable the event. However, we also need
an interrupt in the last output region, so that we actually get to disable
the event (if no more room from new data is available at that point),
otherwise hardware just quietly refuses to start, but the event is
scheduled in and we end up losing trace data till the event gets removed.

For a cpu-wide event it is even worse since there may not be any
re-scheduling at all and no chance for the ring buffer code to notice
that its buffer is filled up and the event needs to be disabled (so that
the consumer can re-enable it when it finishes reading the data out). In
other words, all the trace data will be lost after the buffer gets filled
up.

This patch makes PT also generate a PMI when the last output region is
full.

Reported-by: Markus Metzger <markus.t.metzger@intel.com>
Signed-off-by: Alexander Shishkin <alexander.shishkin@linux.intel.com>
Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
Cc: <stable@vger.kernel.org>
Cc: Arnaldo Carvalho de Melo <acme@infradead.org>
Cc: Arnaldo Carvalho de Melo <acme@redhat.com>
Cc: Borislav Petkov <bp@alien8.de>
Cc: Jiri Olsa <jolsa@redhat.com>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Stephane Eranian <eranian@google.com>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Vince Weaver <vincent.weaver@maine.edu>
Cc: vince@deater.net
Link: http://lkml.kernel.org/r/1462886313-13660-2-git-send-email-alexander.shishkin@linux.intel.com
Signed-off-by: Ingo Molnar <mingo@kernel.org>
---
 arch/x86/events/intel/pt.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/arch/x86/events/intel/pt.c b/arch/x86/events/intel/pt.c
index 09a77db..7377814 100644
--- a/arch/x86/events/intel/pt.c
+++ b/arch/x86/events/intel/pt.c
@@ -709,6 +709,7 @@ static int pt_buffer_reset_markers(struct pt_buffer *buf,
 
 	/* clear STOP and INT from current entry */
 	buf->topa_index[buf->stop_pos]->stop = 0;
+	buf->topa_index[buf->stop_pos]->intr = 0;
 	buf->topa_index[buf->intr_pos]->intr = 0;
 
 	/* how many pages till the STOP marker */
@@ -733,6 +734,7 @@ static int pt_buffer_reset_markers(struct pt_buffer *buf,
 	buf->intr_pos = idx;
 
 	buf->topa_index[buf->stop_pos]->stop = 1;
+	buf->topa_index[buf->stop_pos]->intr = 1;
 	buf->topa_index[buf->intr_pos]->intr = 1;
 
 	return 0;

[toc] | [prev] | [next] | [standalone]

#1402161 — Re: [tip:perf/urgent] perf/x86/intel/pt: Generate PMI in the STOP region as well

tip-bot for Alexander Shishkin <tipbot@zytor.com> writes:

> Commit-ID:  ab92b232ae05c382c3df0e3d6a5c6d16b639ac8c
> Gitweb:     http://git.kernel.org/tip/ab92b232ae05c382c3df0e3d6a5c6d16b639ac8c
> Author:     Alexander Shishkin <alexander.shishkin@linux.intel.com>
> AuthorDate: Tue, 10 May 2016 16:18:32 +0300
> Committer:  Ingo Molnar <mingo@kernel.org>
> CommitDate: Thu, 12 May 2016 14:45:59 +0200
>
> perf/x86/intel/pt: Generate PMI in the STOP region as well
>
> Currently, the PT driver always sets the PMI bit one region (page) before
> the STOP region so that we can wake up the consumer before we run out of
> room in the buffer and have to disable the event. However, we also need
> an interrupt in the last output region, so that we actually get to disable
> the event (if no more room from new data is available at that point),
> otherwise hardware just quietly refuses to start, but the event is
> scheduled in and we end up losing trace data till the event gets removed.
>
> For a cpu-wide event it is even worse since there may not be any
> re-scheduling at all and no chance for the ring buffer code to notice
> that its buffer is filled up and the event needs to be disabled (so that
> the consumer can re-enable it when it finishes reading the data out). In
> other words, all the trace data will be lost after the buffer gets filled
> up.
>
> This patch makes PT also generate a PMI when the last output region is
> full.
>
> Reported-by: Markus Metzger <markus.t.metzger@intel.com>
> Signed-off-by: Alexander Shishkin <alexander.shishkin@linux.intel.com>
> Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
> Cc: <stable@vger.kernel.org>

Can we also have this one queued up for stable 4.4 and 4.5?

Thanks,
--
Alex

[toc] | [prev] | [next] | [standalone]

#1402202 — Re: [tip:perf/urgent] perf/x86/intel/pt: Generate PMI in the STOP region as well

* Alexander Shishkin <alexander.shishkin@linux.intel.com> wrote:

> tip-bot for Alexander Shishkin <tipbot@zytor.com> writes:
> 
> > Commit-ID:  ab92b232ae05c382c3df0e3d6a5c6d16b639ac8c
> > Gitweb:     http://git.kernel.org/tip/ab92b232ae05c382c3df0e3d6a5c6d16b639ac8c
> > Author:     Alexander Shishkin <alexander.shishkin@linux.intel.com>
> > AuthorDate: Tue, 10 May 2016 16:18:32 +0300
> > Committer:  Ingo Molnar <mingo@kernel.org>
> > CommitDate: Thu, 12 May 2016 14:45:59 +0200
> >
> > perf/x86/intel/pt: Generate PMI in the STOP region as well
> >
> > Currently, the PT driver always sets the PMI bit one region (page) before
> > the STOP region so that we can wake up the consumer before we run out of
> > room in the buffer and have to disable the event. However, we also need
> > an interrupt in the last output region, so that we actually get to disable
> > the event (if no more room from new data is available at that point),
> > otherwise hardware just quietly refuses to start, but the event is
> > scheduled in and we end up losing trace data till the event gets removed.
> >
> > For a cpu-wide event it is even worse since there may not be any
> > re-scheduling at all and no chance for the ring buffer code to notice
> > that its buffer is filled up and the event needs to be disabled (so that
> > the consumer can re-enable it when it finishes reading the data out). In
> > other words, all the trace data will be lost after the buffer gets filled
> > up.
> >
> > This patch makes PT also generate a PMI when the last output region is
> > full.
> >
> > Reported-by: Markus Metzger <markus.t.metzger@intel.com>
> > Signed-off-by: Alexander Shishkin <alexander.shishkin@linux.intel.com>
> > Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
> > Cc: <stable@vger.kernel.org>
> 
> Can we also have this one queued up for stable 4.4 and 4.5?

Agreed.

Acked-by: Ingo Molnar <mingo@kernel.org>

Thanks,

	Ingo

[toc] | [prev] | [next] | [standalone]

#1404812 — Re: [tip:perf/urgent] perf/x86/intel/pt: Generate PMI in the STOP region as well

On Tue, May 17, 2016 at 10:40:34AM +0200, Ingo Molnar wrote:
> 
> * Alexander Shishkin <alexander.shishkin@linux.intel.com> wrote:
> 
> > tip-bot for Alexander Shishkin <tipbot@zytor.com> writes:
> > 
> > > Commit-ID:  ab92b232ae05c382c3df0e3d6a5c6d16b639ac8c
> > > Gitweb:     http://git.kernel.org/tip/ab92b232ae05c382c3df0e3d6a5c6d16b639ac8c
> > > Author:     Alexander Shishkin <alexander.shishkin@linux.intel.com>
> > > AuthorDate: Tue, 10 May 2016 16:18:32 +0300
> > > Committer:  Ingo Molnar <mingo@kernel.org>
> > > CommitDate: Thu, 12 May 2016 14:45:59 +0200
> > >
> > > perf/x86/intel/pt: Generate PMI in the STOP region as well
> > >
> > > Currently, the PT driver always sets the PMI bit one region (page) before
> > > the STOP region so that we can wake up the consumer before we run out of
> > > room in the buffer and have to disable the event. However, we also need
> > > an interrupt in the last output region, so that we actually get to disable
> > > the event (if no more room from new data is available at that point),
> > > otherwise hardware just quietly refuses to start, but the event is
> > > scheduled in and we end up losing trace data till the event gets removed.
> > >
> > > For a cpu-wide event it is even worse since there may not be any
> > > re-scheduling at all and no chance for the ring buffer code to notice
> > > that its buffer is filled up and the event needs to be disabled (so that
> > > the consumer can re-enable it when it finishes reading the data out). In
> > > other words, all the trace data will be lost after the buffer gets filled
> > > up.
> > >
> > > This patch makes PT also generate a PMI when the last output region is
> > > full.
> > >
> > > Reported-by: Markus Metzger <markus.t.metzger@intel.com>
> > > Signed-off-by: Alexander Shishkin <alexander.shishkin@linux.intel.com>
> > > Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
> > > Cc: <stable@vger.kernel.org>
> > 
> > Can we also have this one queued up for stable 4.4 and 4.5?
> 
> Agreed.
> 
> Acked-by: Ingo Molnar <mingo@kernel.org>

The filename moved around, now fixed and queued up, thanks.

greg k-h

[toc] | [prev] | [next] | [standalone]

#1398085 — [PATCH 2/2] perf: Disable the event on a truncated AUX record

When the PMU driver reports a truncated AUX record, it effectively means
that there is no more usable room in the event's AUX buffer (even though
there may still be some room, so that perf_aux_output_begin() doesn't take
action). At this point the consumer still has to be woken up and the event
has to be disabled, otherwise the event will just keep spinning between
perf_aux_output_begin() and perf_aux_output_end() until its context gets
unscheduled.

Again, for cpu-wide events this means never, so once in this condition,
they will be forever losing data.

Fix this by disabling the event and waking up the consumer in case of a
truncated AUX record.

Reported-by: Markus Metzger <markus.t.metzger@intel.com>
Signed-off-by: Alexander Shishkin <alexander.shishkin@linux.intel.com>
---
 kernel/events/ring_buffer.c | 10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/kernel/events/ring_buffer.c b/kernel/events/ring_buffer.c
index 0f7f011710..5dd23b2d03 100644
--- a/kernel/events/ring_buffer.c
+++ b/kernel/events/ring_buffer.c
@@ -405,6 +405,7 @@ void perf_aux_output_end(struct perf_output_handle *handle, unsigned long size,
 			 bool truncated)
 {
 	struct ring_buffer *rb = handle->rb;
+	bool wakeup = truncated;
 	unsigned long aux_head;
 	u64 flags = 0;
 
@@ -433,9 +434,16 @@ void perf_aux_output_end(struct perf_output_handle *handle, unsigned long size,
 	aux_head = rb->user_page->aux_head = local_read(&rb->aux_head);
 
 	if (aux_head - local_read(&rb->aux_wakeup) >= rb->aux_watermark) {
-		perf_output_wakeup(handle);
+		wakeup = true;
 		local_add(rb->aux_watermark, &rb->aux_wakeup);
 	}
+
+	if (wakeup) {
+		if (truncated)
+			handle->event->pending_disable = 1;
+		perf_output_wakeup(handle);
+	}
+
 	handle->event = NULL;
 
 	local_set(&rb->aux_nest, 0);
-- 
2.8.1

[toc] | [prev] | [next] | [standalone]

#1398820 — Re: [PATCH 2/2] perf: Disable the event on a truncated AUX record

On Tue, May 10, 2016 at 04:18:33PM +0300, Alexander Shishkin wrote:
> When the PMU driver reports a truncated AUX record, it effectively means
> that there is no more usable room in the event's AUX buffer (even though
> there may still be some room, so that perf_aux_output_begin() doesn't take
> action). At this point the consumer still has to be woken up and the event
> has to be disabled, otherwise the event will just keep spinning between
> perf_aux_output_begin() and perf_aux_output_end() until its context gets
> unscheduled.
> 
> Again, for cpu-wide events this means never, so once in this condition,
> they will be forever losing data.
> 
> Fix this by disabling the event and waking up the consumer in case of a
> truncated AUX record.

> +	if (wakeup) {
> +		if (truncated)
> +			handle->event->pending_disable = 1;
> +		perf_output_wakeup(handle);
> +	}

Does the userspace tool know how to deal with this and re-enable it?

[toc] | [prev] | [next] | [standalone]

#1398872 — Re: [PATCH 2/2] perf: Disable the event on a truncated AUX record

Peter Zijlstra <peterz@infradead.org> writes:

> On Tue, May 10, 2016 at 04:18:33PM +0300, Alexander Shishkin wrote:
>> When the PMU driver reports a truncated AUX record, it effectively means
>> that there is no more usable room in the event's AUX buffer (even though
>> there may still be some room, so that perf_aux_output_begin() doesn't take
>> action). At this point the consumer still has to be woken up and the event
>> has to be disabled, otherwise the event will just keep spinning between
>> perf_aux_output_begin() and perf_aux_output_end() until its context gets
>> unscheduled.
>> 
>> Again, for cpu-wide events this means never, so once in this condition,
>> they will be forever losing data.
>> 
>> Fix this by disabling the event and waking up the consumer in case of a
>> truncated AUX record.
>
>> +	if (wakeup) {
>> +		if (truncated)
>> +			handle->event->pending_disable = 1;
>> +		perf_output_wakeup(handle);
>> +	}
>
> Does the userspace tool know how to deal with this and re-enable it?

Yes, it knows to ioctl(EVENT_ENABLE). Also, we're already doing the
disabling for the no-room-in-buffer condition in
perf_aux_output_begin().

Regards,
--
Alex

[toc] | [prev] | [next] | [standalone]

#1398883 — Re: [PATCH 2/2] perf: Disable the event on a truncated AUX record

On Wed, May 11, 2016 at 12:41:27PM +0300, Alexander Shishkin wrote:
> Peter Zijlstra <peterz@infradead.org> writes:
> >
> > Does the userspace tool know how to deal with this and re-enable it?
> 
> Yes, it knows to ioctl(EVENT_ENABLE). Also, we're already doing the
> disabling for the no-room-in-buffer condition in
> perf_aux_output_begin().

Awesome; gots them queued.

[toc] | [prev] | [next] | [standalone]

#1398888 — Re: [PATCH 2/2] perf: Disable the event on a truncated AUX record

Peter Zijlstra <peterz@infradead.org> writes:

> On Wed, May 11, 2016 at 12:41:27PM +0300, Alexander Shishkin wrote:
>> Peter Zijlstra <peterz@infradead.org> writes:
>> >
>> > Does the userspace tool know how to deal with this and re-enable it?
>> 
>> Yes, it knows to ioctl(EVENT_ENABLE). Also, we're already doing the
>> disabling for the no-room-in-buffer condition in
>> perf_aux_output_begin().
>
> Awesome; gots them queued.

Much thanks!

Regards,
--
Alex

[toc] | [prev] | [next] | [standalone]

#1399896 — [tip:perf/core] perf/core: Disable the event on a truncated AUX record

Commit-ID:  3f56e687a138481894a1088d5aa7d41951bdb020
Gitweb:     http://git.kernel.org/tip/3f56e687a138481894a1088d5aa7d41951bdb020
Author:     Alexander Shishkin <alexander.shishkin@linux.intel.com>
AuthorDate: Tue, 10 May 2016 16:18:33 +0300
Committer:  Ingo Molnar <mingo@kernel.org>
CommitDate: Thu, 12 May 2016 10:14:55 +0200

perf/core: Disable the event on a truncated AUX record

When the PMU driver reports a truncated AUX record, it effectively means
that there is no more usable room in the event's AUX buffer (even though
there may still be some room, so that perf_aux_output_begin() doesn't take
action). At this point the consumer still has to be woken up and the event
has to be disabled, otherwise the event will just keep spinning between
perf_aux_output_begin() and perf_aux_output_end() until its context gets
unscheduled.

Again, for cpu-wide events this means never, so once in this condition,
they will be forever losing data.

Fix this by disabling the event and waking up the consumer in case of a
truncated AUX record.

Reported-by: Markus Metzger <markus.t.metzger@intel.com>
Signed-off-by: Alexander Shishkin <alexander.shishkin@linux.intel.com>
Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
Cc: Arnaldo Carvalho de Melo <acme@infradead.org>
Cc: Arnaldo Carvalho de Melo <acme@redhat.com>
Cc: Borislav Petkov <bp@alien8.de>
Cc: Jiri Olsa <jolsa@redhat.com>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Stephane Eranian <eranian@google.com>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Vince Weaver <vincent.weaver@maine.edu>
Cc: vince@deater.net
Link: http://lkml.kernel.org/r/1462886313-13660-3-git-send-email-alexander.shishkin@linux.intel.com
Signed-off-by: Ingo Molnar <mingo@kernel.org>
---
 kernel/events/ring_buffer.c | 10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/kernel/events/ring_buffer.c b/kernel/events/ring_buffer.c
index c49bab4..ae9b90d 100644
--- a/kernel/events/ring_buffer.c
+++ b/kernel/events/ring_buffer.c
@@ -405,6 +405,7 @@ void perf_aux_output_end(struct perf_output_handle *handle, unsigned long size,
 			 bool truncated)
 {
 	struct ring_buffer *rb = handle->rb;
+	bool wakeup = truncated;
 	unsigned long aux_head;
 	u64 flags = 0;
 
@@ -433,9 +434,16 @@ void perf_aux_output_end(struct perf_output_handle *handle, unsigned long size,
 	aux_head = rb->user_page->aux_head = local_read(&rb->aux_head);
 
 	if (aux_head - local_read(&rb->aux_wakeup) >= rb->aux_watermark) {
-		perf_output_wakeup(handle);
+		wakeup = true;
 		local_add(rb->aux_watermark, &rb->aux_wakeup);
 	}
+
+	if (wakeup) {
+		if (truncated)
+			handle->event->pending_disable = 1;
+		perf_output_wakeup(handle);
+	}
+
 	handle->event = NULL;
 
 	local_set(&rb->aux_nest, 0);

[toc] | [prev] | [next] | [standalone]

#1400052 — [tip:perf/urgent] perf/core: Disable the event on a truncated AUX record

Commit-ID:  9f448cd3cbcec8995935e60b27802ae56aac8cc0
Gitweb:     http://git.kernel.org/tip/9f448cd3cbcec8995935e60b27802ae56aac8cc0
Author:     Alexander Shishkin <alexander.shishkin@linux.intel.com>
AuthorDate: Tue, 10 May 2016 16:18:33 +0300
Committer:  Ingo Molnar <mingo@kernel.org>
CommitDate: Thu, 12 May 2016 14:46:11 +0200

perf/core: Disable the event on a truncated AUX record

When the PMU driver reports a truncated AUX record, it effectively means
that there is no more usable room in the event's AUX buffer (even though
there may still be some room, so that perf_aux_output_begin() doesn't take
action). At this point the consumer still has to be woken up and the event
has to be disabled, otherwise the event will just keep spinning between
perf_aux_output_begin() and perf_aux_output_end() until its context gets
unscheduled.

Again, for cpu-wide events this means never, so once in this condition,
they will be forever losing data.

Fix this by disabling the event and waking up the consumer in case of a
truncated AUX record.

Reported-by: Markus Metzger <markus.t.metzger@intel.com>
Signed-off-by: Alexander Shishkin <alexander.shishkin@linux.intel.com>
Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
Cc: <stable@vger.kernel.org>
Cc: Arnaldo Carvalho de Melo <acme@infradead.org>
Cc: Arnaldo Carvalho de Melo <acme@redhat.com>
Cc: Borislav Petkov <bp@alien8.de>
Cc: Jiri Olsa <jolsa@redhat.com>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Stephane Eranian <eranian@google.com>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Vince Weaver <vincent.weaver@maine.edu>
Cc: vince@deater.net
Link: http://lkml.kernel.org/r/1462886313-13660-3-git-send-email-alexander.shishkin@linux.intel.com
Signed-off-by: Ingo Molnar <mingo@kernel.org>
---
 kernel/events/ring_buffer.c | 10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/kernel/events/ring_buffer.c b/kernel/events/ring_buffer.c
index c61f0cb..7611d0f 100644
--- a/kernel/events/ring_buffer.c
+++ b/kernel/events/ring_buffer.c
@@ -347,6 +347,7 @@ void perf_aux_output_end(struct perf_output_handle *handle, unsigned long size,
 			 bool truncated)
 {
 	struct ring_buffer *rb = handle->rb;
+	bool wakeup = truncated;
 	unsigned long aux_head;
 	u64 flags = 0;
 
@@ -375,9 +376,16 @@ void perf_aux_output_end(struct perf_output_handle *handle, unsigned long size,
 	aux_head = rb->user_page->aux_head = local_read(&rb->aux_head);
 
 	if (aux_head - local_read(&rb->aux_wakeup) >= rb->aux_watermark) {
-		perf_output_wakeup(handle);
+		wakeup = true;
 		local_add(rb->aux_watermark, &rb->aux_wakeup);
 	}
+
+	if (wakeup) {
+		if (truncated)
+			handle->event->pending_disable = 1;
+		perf_output_wakeup(handle);
+	}
+
 	handle->event = NULL;
 
 	local_set(&rb->aux_nest, 0);

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web