mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: Jiri Olsa <jolsa@redhat.com>
To: Don Zickus <dzickus@redhat.com>
Cc: acme@ghostprotocols.net, peterz@infradead.org,
	LKML <linux-kernel@vger.kernel.org>,
	namhyung@gmail.com, eranian@google.com,
	Andi Kleen <andi@firstfloor.org>
Subject: Re: [PATCH 2/6] Revert "perf: Disable PERF_RECORD_MMAP2 support"
Date: Mon, 19 May 2014 13:45:39 +0200	[thread overview]
Message-ID: <20140519114539.GE5027@krava.brq.redhat.com> (raw)
In-Reply-To: <20140516132630.GM50500@redhat.com>

On Fri, May 16, 2014 at 09:26:30AM -0400, Don Zickus wrote:
> On Fri, May 16, 2014 at 01:25:14PM +0200, Jiri Olsa wrote:
> > On Tue, May 13, 2014 at 12:48:13PM -0400, Don Zickus wrote:
> > 
> > SNIP
> > 
> > > -		/*
> > > - 		 * Anon maps don't have the execname.
> > > - 		 */
> > > -		if (n < 4)
> > > +		n = sscanf(bf, "%"PRIx64"-%"PRIx64" %s %"PRIx64" %x:%x %u %s\n",
> > > +		       &event->mmap2.start, &event->mmap2.len, prot,
> > > +		       &event->mmap2.pgoff, &event->mmap2.maj,
> > > +		       &event->mmap2.min,
> > > +		       &ino, execname);
> > > +
> > > +		event->mmap2.ino = (u64)ino;
> > > +
> > > +		if (n < 7)
> > >  			continue;
> > 
> > any reason for changing this from 'if (n != 8)' ?
> 
> Yes, read 9d4ecc8893832337daf241236841db966fa53489. :-)
> 
> This wasn't a clean revert because of the change for (n != 5) -> (n < 4).
> So I maintainted the spirit of the upstream code with this revert.  Let me
> know if that is wrong.

ook, I found another issue.. perf test 25 is failing, because
it's using perf_event__synthesize_mmap_events to store map events
plus machine__process_mmap_event to feed them into machine object

it needs to use machine__process_mmap2_event now, because
perf_event__synthesize_mmap_events uses MMAP2 events now
please check attached patch

I think it's better to have this hunk in this patch, so we dont
break tests.. also please run them ebfore posting patch

please update the changelog to have some info about this.. plus
the Revert reason info and plus the 'if (n != 8)' thing ;-)

thanks,
jirka

---
diff --git a/tools/perf/tests/dwarf-unwind.c b/tools/perf/tests/dwarf-unwind.c
index 108f0cd..96adb73 100644
--- a/tools/perf/tests/dwarf-unwind.c
+++ b/tools/perf/tests/dwarf-unwind.c
@@ -15,7 +15,7 @@ static int mmap_handler(struct perf_tool *tool __maybe_unused,
 			struct perf_sample *sample __maybe_unused,
 			struct machine *machine)
 {
-	return machine__process_mmap_event(machine, event, NULL);
+	return machine__process_mmap2_event(machine, event, NULL);
 }
 
 static int init_live_machine(struct machine *machine)

  reply	other threads:[~2014-05-19 11:46 UTC|newest]

Thread overview: 30+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-05-13 16:48 [PATCH 0/6 V2] perf: Enable mmap2 and add dcacheline sorting Don Zickus
2014-05-13 16:48 ` [PATCH 1/6] events, perf: Pass protection and flags bits through mmap2 interface Don Zickus
2014-05-16 12:22   ` Peter Zijlstra
2014-05-16 13:33     ` Don Zickus
2014-05-16 15:45       ` Peter Zijlstra
2014-05-16 16:20         ` Don Zickus
2014-05-13 16:48 ` [PATCH 2/6] Revert "perf: Disable PERF_RECORD_MMAP2 support" Don Zickus
2014-05-16 11:25   ` Jiri Olsa
2014-05-16 13:26     ` Don Zickus
2014-05-19 11:45       ` Jiri Olsa [this message]
2014-05-19 11:21   ` Jiri Olsa
2014-05-13 16:48 ` [PATCH 3/6] perf: Update mmap2 interface with protection and flag bits Don Zickus
2014-05-13 16:48 ` [PATCH 4/6] perf report: Add mem-mode documentation to report command Don Zickus
2014-05-13 16:48 ` [PATCH 5/6] perf: Add cpumode to struct hist_entry Don Zickus
2014-05-13 16:48 ` [PATCH 6/6] perf: Add dcacheline sort Don Zickus
2014-05-16 11:47   ` Jiri Olsa
2014-05-16 13:30     ` Don Zickus
2014-05-16 14:05       ` Jiri Olsa
2014-05-16 14:30         ` Don Zickus
2014-05-19 13:34           ` Jiri Olsa
2014-05-19 14:18             ` Don Zickus
2014-05-16 14:09   ` Stephane Eranian
2014-05-16 15:18     ` Don Zickus
2014-05-16 15:59     ` Peter Zijlstra
2014-05-16 16:02       ` Stephane Eranian
2014-05-16 16:24         ` Don Zickus
2014-05-16 16:27           ` Stephane Eranian
2014-05-19 11:25           ` Jiri Olsa
2014-05-19 13:20             ` Don Zickus
2014-05-13 16:52 ` [PATCH 0/6 V2] perf: Enable mmap2 and add dcacheline sorting Don Zickus

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=20140519114539.GE5027@krava.brq.redhat.com \
    --to=jolsa@redhat.com \
    --cc=acme@ghostprotocols.net \
    --cc=andi@firstfloor.org \
    --cc=dzickus@redhat.com \
    --cc=eranian@google.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=namhyung@gmail.com \
    --cc=peterz@infradead.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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®