Messages in this thread | | | Date | Tue, 2 Apr 2019 11:47:12 +0100 | From | Will Deacon <> | Subject | Re: BUG in "perf: Suppress AUX/OVERWRITE records"? |
| |
On Wed, Mar 27, 2019 at 11:17:43AM +0200, Alexander Shishkin wrote: > Ben Gainey <Ben.Gainey@arm.com> writes: > > >> It was an unintentional side effect that it also > >> happened to coincide with context switches in the overwrite mode. > > > > I'm not using overwrite mode, I'm opening the mmap with PROT_WRITE > > (i.e. in truncate mode). > > Now I get it. Does the below fix the problem for you? > > From bf52320cce0e74a2c0d987db7bd571f7687b4f4f Mon Sep 17 00:00:00 2001 > From: Alexander Shishkin <alexander.shishkin@linux.intel.com> > Date: Wed, 27 Mar 2019 11:05:53 +0200 > Subject: [PATCH] perf: Fix AUX record suppression > > Commit 1627314fb54a33e ("perf: Suppress AUX/OVERWRITE records") has an > unintended side-effect of also suppressing all AUX records with no flags > and non-zero size, so all the regular records in the full trace mode. > This breaks some use cases for people. > > Fix this by restoring "regular" AUX records to their former glory. > > Signed-off-by: Alexander Shishkin <alexander.shishkin@linux.intel.com> > Fixes: 1627314fb54a33e ("perf: Suppress AUX/OVERWRITE records") > Reported-by: Ben Gainey <Ben.Gainey@arm.com> > CC: stable@vger.kernel.org # v4.20+ > --- > kernel/events/ring_buffer.c | 33 +++++++++++++++------------------ > 1 file changed, 15 insertions(+), 18 deletions(-) > > diff --git a/kernel/events/ring_buffer.c b/kernel/events/ring_buffer.c > index 678ccec60d8f..626256dc26c1 100644 > --- a/kernel/events/ring_buffer.c > +++ b/kernel/events/ring_buffer.c > @@ -455,24 +455,21 @@ void perf_aux_output_end(struct perf_output_handle *handle, unsigned long size) > rb->aux_head += size; > } > > - if (size || handle->aux_flags) { > - /* > - * Only send RECORD_AUX if we have something useful to communicate > - * > - * Note: the OVERWRITE records by themselves are not considered > - * useful, as they don't communicate any *new* information, > - * aside from the short-lived offset, that becomes history at > - * the next event sched-in and therefore isn't useful. > - * The userspace that needs to copy out AUX data in overwrite > - * mode should know to use user_page::aux_head for the actual > - * offset. So, from now on we don't output AUX records that > - * have *only* OVERWRITE flag set. > - */ > - > - if (handle->aux_flags & ~(u64)PERF_AUX_FLAG_OVERWRITE) > - perf_event_aux_event(handle->event, aux_head, size, > - handle->aux_flags); > - } > + /* > + * Only send RECORD_AUX if we have something useful to communicate > + * > + * Note: the OVERWRITE records by themselves are not considered > + * useful, as they don't communicate any *new* information, > + * aside from the short-lived offset, that becomes history at > + * the next event sched-in and therefore isn't useful. > + * The userspace that needs to copy out AUX data in overwrite > + * mode should know to use user_page::aux_head for the actual > + * offset. So, from now on we don't output AUX records that > + * have *only* OVERWRITE flag set. > + */ > + if (size || (handle->aux_flags & ~(u64)PERF_AUX_FLAG_OVERWRITE)) > + perf_event_aux_event(handle->event, aux_head, size, > + handle->aux_flags);
Acked-by: Will Deacon <will.deacon@arm.com>
Assuming this will make it into 5.1 as a fix?
Will
| |