From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754431AbdDKJk5 (ORCPT ); Tue, 11 Apr 2017 05:40:57 -0400 Received: from mx1.redhat.com ([209.132.183.28]:35980 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752908AbdDKJkz (ORCPT ); Tue, 11 Apr 2017 05:40:55 -0400 DMARC-Filter: OpenDMARC Filter v1.3.2 mx1.redhat.com 61372804F4 Authentication-Results: ext-mx03.extmail.prod.ext.phx2.redhat.com; dmarc=none (p=none dis=none) header.from=redhat.com Authentication-Results: ext-mx03.extmail.prod.ext.phx2.redhat.com; spf=pass smtp.mailfrom=jolsa@redhat.com DKIM-Filter: OpenDKIM Filter v2.11.0 mx1.redhat.com 61372804F4 Date: Tue, 11 Apr 2017 11:40:50 +0200 From: Jiri Olsa To: David Carrillo-Cisneros Cc: linux-kernel@vger.kernel.org, Peter Zijlstra , Ingo Molnar , Arnaldo Carvalho de Melo , Alexander Shishkin , Andi Kleen , Simon Que , Wang Nan , Jiri Olsa , He Kuang , Masami Hiramatsu , Stephane Eranian , Paul Turner Subject: Re: [PATCH 2/7] perf inject: copy events when reordering events in pipe mode Message-ID: <20170411094050.GA21238@krava> References: <20170410201432.24807-1-davidcc@google.com> <20170410201432.24807-3-davidcc@google.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20170410201432.24807-3-davidcc@google.com> User-Agent: Mutt/1.8.0 (2017-02-23) X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.27]); Tue, 11 Apr 2017 09:40:54 +0000 (UTC) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Mon, Apr 10, 2017 at 01:14:27PM -0700, David Carrillo-Cisneros wrote: > __perf_session__process_pipe_events reuses the same memory buffer to > process all events in the pipe. > > When reordering is needed (e.g. -b option), events are not immediately > flushed, but kept around until reordering is possible, causing > memory corruption. > > The problem is usually observed by a "Unknown sample error" output. It > can easily be reproduced by: > > perf record -o - noploop | perf inject -b > output > > Signed-off-by: David Carrillo-Cisneros > --- > tools/perf/util/ordered-events.c | 3 ++- > tools/perf/util/session.c | 1 + > 2 files changed, 3 insertions(+), 1 deletion(-) > > diff --git a/tools/perf/util/ordered-events.c b/tools/perf/util/ordered-events.c > index fe84df1875aa..e70e935b1841 100644 > --- a/tools/perf/util/ordered-events.c > +++ b/tools/perf/util/ordered-events.c > @@ -79,7 +79,7 @@ static union perf_event *dup_event(struct ordered_events *oe, > > static void free_dup_event(struct ordered_events *oe, union perf_event *event) > { > - if (oe->copy_on_queue) { > + if (event && oe->copy_on_queue) { > oe->cur_alloc_size -= event->header.size; > free(event); > } > @@ -150,6 +150,7 @@ void ordered_events__delete(struct ordered_events *oe, struct ordered_event *eve > list_move(&event->list, &oe->cache); > oe->nr_events--; > free_dup_event(oe, event->event); > + event->event = NULL; based on the changelog I understand the need for the change below, but I dont get why do we need this cleanu and check above thanks, jirka > } > > int ordered_events__queue(struct ordered_events *oe, union perf_event *event, > diff --git a/tools/perf/util/session.c b/tools/perf/util/session.c > index 24259bc2c598..a25302bc55a8 100644 > --- a/tools/perf/util/session.c > +++ b/tools/perf/util/session.c > @@ -1656,6 +1656,7 @@ static int __perf_session__process_pipe_events(struct perf_session *session) > buf = malloc(cur_size); > if (!buf) > return -errno; > + ordered_events__set_copy_on_queue(oe, true); > more: > event = buf; > err = readn(fd, event, sizeof(struct perf_event_header)); > -- > 2.12.2.715.g7642488e1d-goog >