On Thu, Jan 24, 2013 at 04:10:35PM +0100, Stephane Eranian wrote:
SNIP
>
> +static void ip__resolve_data(struct machine *self, struct thread *thread,
> + u8 m,
> + struct addr_map_symbol *ams,
> + u64 addr)
> +{
> + struct addr_location al;
> +
> + memset(&al, 0, sizeof(al));
> +
> + thread__find_addr_location(thread, self, m, MAP__VARIABLE, addr, &al,
> + NULL);
> + ams->addr = addr;
> + ams->al_addr = al.addr;
> + ams->sym = al.sym;
> + ams->map = al.map;
> +}
> +
> +struct mem_info *machine__resolve_mem(struct machine *self,
> + struct thread *thr,
> + struct perf_sample *sample,
> + u8 cpumode)
> +{
> + struct mem_info *mi;
> +
> + mi = calloc(1, sizeof(struct mem_info));
> + if (!mi)
> + return NULL;
> +
> + ip__resolve_ams(self, thr, &mi->iaddr, sample->ip);
> + ip__resolve_data(self, thr, cpumode, &mi->daddr, sample->addr);
question, should this be the other way around? like:
ip__resolve_ams(machine, thr, &mi->daddr, sample->addr);
ip__resolve_data(machine, thr, cpumode, &mi->iaddr, sample->ip);
we have correct cpumode for sample->ip, but I think it's the
PEBS->dla (sample->addr) where we need to guess.. right?
that makes me think that we could probably use ip__resolve_data
for both.. hummm.. but we could access data cross the user/kernel
boundary, so cpumode would be different for ip and accessed data
thanks,
jirka
On Wed, 2013-03-27 at 15:14 +0100, Jiri Olsa wrote:
> we have correct cpumode for sample->ip, but I think it's the
> PEBS->dla (sample->addr) where we need to guess.. right?
kernel mode very much fakes the cpumode/segment stuff for PEBS. PEBS
assumes you're running in a linear/flat mode.
On Wed, Mar 27, 2013 at 03:14:25PM +0100, Jiri Olsa wrote:
> On Thu, Jan 24, 2013 at 04:10:35PM +0100, Stephane Eranian wrote:
>
> SNIP
>
> >
> > +static void ip__resolve_data(struct machine *self, struct thread *thread,
> > + u8 m,
> > + struct addr_map_symbol *ams,
> > + u64 addr)
> > +{
> > + struct addr_location al;
> > +
> > + memset(&al, 0, sizeof(al));
> > +
> > + thread__find_addr_location(thread, self, m, MAP__VARIABLE, addr, &al,
> > + NULL);
> > + ams->addr = addr;
> > + ams->al_addr = al.addr;
> > + ams->sym = al.sym;
> > + ams->map = al.map;
> > +}
> > +
> > +struct mem_info *machine__resolve_mem(struct machine *self,
> > + struct thread *thr,
> > + struct perf_sample *sample,
> > + u8 cpumode)
> > +{
> > + struct mem_info *mi;
> > +
> > + mi = calloc(1, sizeof(struct mem_info));
> > + if (!mi)
> > + return NULL;
> > +
> > + ip__resolve_ams(self, thr, &mi->iaddr, sample->ip);
> > + ip__resolve_data(self, thr, cpumode, &mi->daddr, sample->addr);
>
> question, should this be the other way around? like:
>
> ip__resolve_ams(machine, thr, &mi->daddr, sample->addr);
> ip__resolve_data(machine, thr, cpumode, &mi->iaddr, sample->ip);
ugh, I missed the MAP__VARIABLE/MAP__FUNCTION difference there, thanks Arnaldo! ;-)
still, no need to guess the cpumode for ip and guess it for data?
jirka
On Wed, Mar 27, 2013 at 03:20:14PM +0100, Peter Zijlstra wrote:
> On Wed, 2013-03-27 at 15:14 +0100, Jiri Olsa wrote:
> > we have correct cpumode for sample->ip, but I think it's the
> > PEBS->dla (sample->addr) where we need to guess.. right?
>
> kernel mode very much fakes the cpumode/segment stuff for PEBS. PEBS
> assumes you're running in a linear/flat mode.
>
say we hit the sample when kernel accesses the user data, we will endup
with IP in kernel space and DATA ptr in user space.. in theory ;-)
and that would need the cpumode guessing for DATA ptr, because
cpumode value is deduced from cs register
jirka
On Wed, Mar 27, 2013 at 3:34 PM, Jiri Olsa <[email protected]> wrote:
>
> On Wed, Mar 27, 2013 at 03:20:14PM +0100, Peter Zijlstra wrote:
> > On Wed, 2013-03-27 at 15:14 +0100, Jiri Olsa wrote:
> > > we have correct cpumode for sample->ip, but I think it's the
> > > PEBS->dla (sample->addr) where we need to guess.. right?
> >
> > kernel mode very much fakes the cpumode/segment stuff for PEBS. PEBS
> > assumes you're running in a linear/flat mode.
> >
>
> say we hit the sample when kernel accesses the user data, we will endup
> with IP in kernel space and DATA ptr in user space.. in theory ;-)
>
Yes, this is possible. So I think we could probably leaverage ip__resolve_ams()
and pass the extra parameter for MAP_VARIABLE vs. MAP_FUNCTION.
>
> and that would need the cpumode guessing for DATA ptr, because
> cpumode value is deduced from cs register
>
> jirka
Em Wed, Mar 27, 2013 at 03:48:15PM +0100, Stephane Eranian escreveu:
> On Wed, Mar 27, 2013 at 3:34 PM, Jiri Olsa <[email protected]> wrote:
> > On Wed, Mar 27, 2013 at 03:20:14PM +0100, Peter Zijlstra wrote:
> > > On Wed, 2013-03-27 at 15:14 +0100, Jiri Olsa wrote:
> > > > we have correct cpumode for sample->ip, but I think it's the
> > > > PEBS->dla (sample->addr) where we need to guess.. right?
> > > kernel mode very much fakes the cpumode/segment stuff for PEBS. PEBS
> > > assumes you're running in a linear/flat mode.
> > say we hit the sample when kernel accesses the user data, we will endup
> > with IP in kernel space and DATA ptr in user space.. in theory ;-)
> Yes, this is possible. So I think we could probably leaverage ip__resolve_ams()
> and pass the extra parameter for MAP_VARIABLE vs. MAP_FUNCTION.
BTW, I fixed up the patches (kernel and user parts) and have them in
perf/mem at:
git://git.kernel.org/pub/scm/linux/kernel/git/acme/linux
Hope to push it to Ingo today/tomorrow, after some more testing, thanks
Jiri for reviewing it.
Stephane, if you could give it a try again to see that the fixups I did
(documented in the commit logs, just before my Signed-off-by) are ok,
that would be good.
- Arnaldo
>
> >
> > and that would need the cpumode guessing for DATA ptr, because
> > cpumode value is deduced from cs register
> >
> > jirka
Arnaldo,
On Wed, Mar 27, 2013 at 5:56 PM, Arnaldo Carvalho de Melo
<[email protected]> wrote:
> Em Wed, Mar 27, 2013 at 03:48:15PM +0100, Stephane Eranian escreveu:
>> On Wed, Mar 27, 2013 at 3:34 PM, Jiri Olsa <[email protected]> wrote:
>> > On Wed, Mar 27, 2013 at 03:20:14PM +0100, Peter Zijlstra wrote:
>> > > On Wed, 2013-03-27 at 15:14 +0100, Jiri Olsa wrote:
>> > > > we have correct cpumode for sample->ip, but I think it's the
>> > > > PEBS->dla (sample->addr) where we need to guess.. right?
>
>> > > kernel mode very much fakes the cpumode/segment stuff for PEBS. PEBS
>> > > assumes you're running in a linear/flat mode.
>
>> > say we hit the sample when kernel accesses the user data, we will endup
>> > with IP in kernel space and DATA ptr in user space.. in theory ;-)
>
>> Yes, this is possible. So I think we could probably leaverage ip__resolve_ams()
>> and pass the extra parameter for MAP_VARIABLE vs. MAP_FUNCTION.
>
> BTW, I fixed up the patches (kernel and user parts) and have them in
> perf/mem at:
>
> git://git.kernel.org/pub/scm/linux/kernel/git/acme/linux
>
> Hope to push it to Ingo today/tomorrow, after some more testing, thanks
> Jiri for reviewing it.
>
> Stephane, if you could give it a try again to see that the fixups I did
> (documented in the commit logs, just before my Signed-off-by) are ok,
> that would be good.
>
I tried on a few examples on both NHM (only loads, no TLB) and SNB
and got the right answers for my tests, including data symbol resolution.
What we discussed with Jiri yesterday can be added later on.
Thanks for the integration work. Looks good to me.
> - Arnaldo
>
>>
>> >
>> > and that would need the cpumode guessing for DATA ptr, because
>> > cpumode value is deduced from cs register
>> >
>> > jirka
Em Thu, Mar 28, 2013 at 03:24:30PM +0100, Stephane Eranian escreveu:
> On Wed, Mar 27, 2013 at 5:56 PM, Arnaldo Carvalho de Melo
> > Stephane, if you could give it a try again to see that the fixups I did
> > (documented in the commit logs, just before my Signed-off-by) are ok,
> > that would be good.
> I tried on a few examples on both NHM (only loads, no TLB) and SNB
> and got the right answers for my tests, including data symbol resolution.
> What we discussed with Jiri yesterday can be added later on.
> Thanks for the integration work. Looks good to me.
Humm, I just tried it with a simple:
perf mem -t load rec
And got an OOPS, trying again, and this machine was suspended, perhaps
perf/core doesn't have that PEBS fix, will check.
- Arnaldo
On Thu, Mar 28, 2013 at 4:00 PM, Arnaldo Carvalho de Melo
<[email protected]> wrote:
> Em Thu, Mar 28, 2013 at 03:24:30PM +0100, Stephane Eranian escreveu:
>> On Wed, Mar 27, 2013 at 5:56 PM, Arnaldo Carvalho de Melo
>> > Stephane, if you could give it a try again to see that the fixups I did
>> > (documented in the commit logs, just before my Signed-off-by) are ok,
>> > that would be good.
>
>> I tried on a few examples on both NHM (only loads, no TLB) and SNB
>> and got the right answers for my tests, including data symbol resolution.
>
>> What we discussed with Jiri yesterday can be added later on.
>
>> Thanks for the integration work. Looks good to me.
>
> Humm, I just tried it with a simple:
>
> perf mem -t load rec
>
That cmdline should not product anything. It is missing a command to run?
> And got an OOPS, trying again, and this machine was suspended, perhaps
> perf/core doesn't have that PEBS fix, will check.
>
What HW is this running on?
I was running with your kernel.
Em Thu, Mar 28, 2013 at 12:00:18PM -0300, Arnaldo Carvalho de Melo escreveu:
> Em Thu, Mar 28, 2013 at 03:24:30PM +0100, Stephane Eranian escreveu:
> > On Wed, Mar 27, 2013 at 5:56 PM, Arnaldo Carvalho de Melo
> > > Stephane, if you could give it a try again to see that the fixups I did
> > > (documented in the commit logs, just before my Signed-off-by) are ok,
> > > that would be good.
>
> > I tried on a few examples on both NHM (only loads, no TLB) and SNB
> > and got the right answers for my tests, including data symbol resolution.
>
> > What we discussed with Jiri yesterday can be added later on.
>
> > Thanks for the integration work. Looks good to me.
>
> Humm, I just tried it with a simple:
>
> perf mem -t load rec
>
> And got an OOPS, trying again, and this machine was suspended, perhaps
> perf/core doesn't have that PEBS fix, will check.
Yeah, after a fresh reboot it doesn't OOPses, the fix:
commit 1d9d8639c063caf6efc2447f5f26aa637f844ff6
Author: Stephane Eranian <[email protected]>
Date: Fri Mar 15 14:26:07 2013 +0100
perf,x86: fix kernel crash with PEBS/BTS after suspend/resume
--------
Isn't in perf/core, cool, before your test results I thougt I had messed
up something :-)
- Arnaldo
On Thu, Mar 28, 2013 at 4:12 PM, Arnaldo Carvalho de Melo
<[email protected]> wrote:
> Em Thu, Mar 28, 2013 at 12:00:18PM -0300, Arnaldo Carvalho de Melo escreveu:
>> Em Thu, Mar 28, 2013 at 03:24:30PM +0100, Stephane Eranian escreveu:
>> > On Wed, Mar 27, 2013 at 5:56 PM, Arnaldo Carvalho de Melo
>> > > Stephane, if you could give it a try again to see that the fixups I did
>> > > (documented in the commit logs, just before my Signed-off-by) are ok,
>> > > that would be good.
>>
>> > I tried on a few examples on both NHM (only loads, no TLB) and SNB
>> > and got the right answers for my tests, including data symbol resolution.
>>
>> > What we discussed with Jiri yesterday can be added later on.
>>
>> > Thanks for the integration work. Looks good to me.
>>
>> Humm, I just tried it with a simple:
>>
>> perf mem -t load rec
>>
>> And got an OOPS, trying again, and this machine was suspended, perhaps
>> perf/core doesn't have that PEBS fix, will check.
>
> Yeah, after a fresh reboot it doesn't OOPses, the fix:
>
> commit 1d9d8639c063caf6efc2447f5f26aa637f844ff6
> Author: Stephane Eranian <[email protected]>
> Date: Fri Mar 15 14:26:07 2013 +0100
>
> perf,x86: fix kernel crash with PEBS/BTS after suspend/resume
>
Okay, so you too fell into that trap! I mean laptop guys, trying to save
battery, yet running perf, c'mon... ;->
> --------
>
> Isn't in perf/core, cool, before your test results I thougt I had messed
> up something :-)
>
yeah, closed your laptop lid.....
Should work better with the suspend/resume fix now, hopefully.
Of course, on SNB systems, you also need that firmware patch
to enable PEBS, IIRC.