Re: perf report does not resolve symbols on s390x

Re: perf report does not resolve symbols on s390x

[Arnaldo Carvalho de Melo]

Em Tue, Jul 11, 2017 at 04:38:28PM -0300, Arnaldo Carvalho de Melo escreveu:
> Em Tue, Jul 11, 2017 at 04:03:04PM -0300, Arnaldo Carvalho de Melo escreveu:
> > Em Fri, Jul 07, 2017 at 02:17:25PM +0200, Thomas-Mich Richter escreveu:
> > > On 07/06/2017 02:35 PM, Thomas-Mich Richter wrote:
> > > It determines the kernel starts at address 1<<63 and loads the kernel address mapping.
> > > On s390x
> > > - The kernel starts at 0x0 (value of map->start) and thus all checks in function 
> > >   thread__find_addr_map() fail and no symbol is found for the specified addresses
> > >   because the kernel starts at 0x8000000000000000. Which is wrong the kernel start at 0x0.

> > Hi Thomas, really nice debugging session!

> > I'm trying the one-liner below, Adrian, can you please check this and
> > provide an ack? I think that that comment about the address that it will
> > default when map__load() fails needs rewriting in light of Thomas
> > comments about other arches (see further below)?

> > I did a quick check of machine->kernel_start usage in Intel PT and since
> > on x86 that assumption about partitioning the address space holds, no
> > problem should be introduced by the one-liner fix, right?
 
> Argh, this is also broken:
 
> static inline bool machine__kernel_ip(struct machine *machine, u64 ip)
> {
>         u64 kernel_start = machine__kernel_start(machine);
> 
>         return ip >= kernel_start;
> }
> 
> We can't judge if a address is in the kernel like that :-\

So, this is used by:

[acme@jouet linux]$ find tools/ -name "*.[ch]" | xargs grep -w machine__kernel_ip
tools/perf/builtin-script.c:	kernel = machine__kernel_ip(machine, start);
tools/perf/builtin-script.c:	if (kernel != machine__kernel_ip(machine, end)) {

That is just for "brstackinsn", would that make sense for Sparc, S/390?

tools/perf/util/intel-bts.c:	if (machine__kernel_ip(machine, ip))
tools/perf/util/intel-bts.c:		if (!machine__kernel_ip(btsq->bts->machine, branch->from) &&
tools/perf/util/intel-bts.c:		    machine__kernel_ip(btsq->bts->machine, branch->to) &&

Intel specific stuff, so should be ok.

tools/perf/util/event.c:		    machine__kernel_ip(machine, al->addr)) {

For this last one, that affects all arches, I think we can just remove
this check and look at the kernel when not finding it anywhere else?

diff --git a/tools/perf/util/event.c b/tools/perf/util/event.c
index dc5c3bb69d73..8e435baaae6a 100644
--- a/tools/perf/util/event.c
+++ b/tools/perf/util/event.c
@@ -1432,8 +1432,7 @@ void thread__find_addr_map(struct thread *thread, u8 cpumode,
 		 * in the whole kernel symbol list.
 		 */
 		if (cpumode == PERF_RECORD_MISC_USER && machine &&
-		    mg != &machine->kmaps &&
-		    machine__kernel_ip(machine, al->addr)) {
+		    mg != &machine->kmaps) {
 			mg = &machine->kmaps;
 			load_map = true;
 			goto try_again;

> > > This raises 2 questions:
> > > 1. s390 has a 64 bit address space for user and kernel. The processor status word (PSW)
> > >    determines which address space to use. That requires the PSW in the sample. Not sure
> > >    this is the case?
> > > 2. How does this work on sparc and other architectures with the same addressing scheme?
> > >
> > > Thanks.
> > > --
> > > Thomas Richter, Dept 3303, IBM LTC Boeblingen Germany

Re: perf report does not resolve symbols on s390x

[Thomas-Mich Richter]

On 07/11/2017 09:48 PM, Arnaldo Carvalho de Melo wrote:
> Em Tue, Jul 11, 2017 at 04:38:28PM -0300, Arnaldo Carvalho de Melo escreveu:
>> Em Tue, Jul 11, 2017 at 04:03:04PM -0300, Arnaldo Carvalho de Melo escreveu:
>>> Em Fri, Jul 07, 2017 at 02:17:25PM +0200, Thomas-Mich Richter escreveu:
.....

> 
>> Argh, this is also broken:
> 
>> static inline bool machine__kernel_ip(struct machine *machine, u64 ip)
>> {
>>         u64 kernel_start = machine__kernel_start(machine);
>>
>>         return ip >= kernel_start;
>> }
>>
>> We can't judge if a address is in the kernel like that :-\
> 
> So, this is used by:
> 
> [acme@jouet linux]$ find tools/ -name "*.[ch]" | xargs grep -w machine__kernel_ip
> tools/perf/builtin-script.c:	kernel = machine__kernel_ip(machine, start);
> tools/perf/builtin-script.c:	if (kernel != machine__kernel_ip(machine, end)) {
> 
> That is just for "brstackinsn", would that make sense for Sparc, S/390?

No we don't have that on s/390
> 
> tools/perf/util/intel-bts.c:	if (machine__kernel_ip(machine, ip))
> tools/perf/util/intel-bts.c:		if (!machine__kernel_ip(btsq->bts->machine, branch->from) &&
> tools/perf/util/intel-bts.c:		    machine__kernel_ip(btsq->bts->machine, branch->to) &&
> 
> Intel specific stuff, so should be ok.
> 
> tools/perf/util/event.c:		    machine__kernel_ip(machine, al->addr)) {
> 
> For this last one, that affects all arches, I think we can just remove
> this check and look at the kernel when not finding it anywhere else?
> 
> diff --git a/tools/perf/util/event.c b/tools/perf/util/event.c
> index dc5c3bb69d73..8e435baaae6a 100644
> --- a/tools/perf/util/event.c
> +++ b/tools/perf/util/event.c
> @@ -1432,8 +1432,7 @@ void thread__find_addr_map(struct thread *thread, u8 cpumode,
>  		 * in the whole kernel symbol list.
>  		 */
>  		if (cpumode == PERF_RECORD_MISC_USER && machine &&
> -		    mg != &machine->kmaps &&
> -		    machine__kernel_ip(machine, al->addr)) {
> +		    mg != &machine->kmaps) {
>  			mg = &machine->kmaps;
>  			load_map = true;
>  			goto try_again;
> 
>>>> This raises 2 questions:
>>>> 1. s390 has a 64 bit address space for user and kernel. The processor status word (PSW)
>>>>    determines which address space to use. That requires the PSW in the sample. Not sure
>>>>    this is the case?
>>>> 2. How does this work on sparc and other architectures with the same addressing scheme?
>>>>
>>>> Thanks.
>>>> --
>>>> Thomas Richter, Dept 3303, IBM LTC Boeblingen Germany
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-perf-users" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 


-- 
Thomas Richter, Dept 3303, IBM LTC Boeblingen Germany
--
Vorsitzende des Aufsichtsrats: Martina Koederitz 
Geschäftsführung: Dirk Wittkopp
Sitz der Gesellschaft: Böblingen / Registergericht: Amtsgericht Stuttgart, HRB 243294

Re: perf report does not resolve symbols on s390x

[Michael Ellerman]

Arnaldo Carvalho de Melo <acme@kernel.org> writes:

> Em Tue, Jul 11, 2017 at 04:38:28PM -0300, Arnaldo Carvalho de Melo escreveu:
>> Em Tue, Jul 11, 2017 at 04:03:04PM -0300, Arnaldo Carvalho de Melo escreveu:
>> > Em Fri, Jul 07, 2017 at 02:17:25PM +0200, Thomas-Mich Richter escreveu:
>> > > On 07/06/2017 02:35 PM, Thomas-Mich Richter wrote:
>> > > It determines the kernel starts at address 1<<63 and loads the kernel address mapping.
>> > > On s390x
>> > > - The kernel starts at 0x0 (value of map->start) and thus all checks in function 
>> > >   thread__find_addr_map() fail and no symbol is found for the specified addresses
>> > >   because the kernel starts at 0x8000000000000000. Which is wrong the kernel start at 0x0.
>
>> > Hi Thomas, really nice debugging session!
>
>> > I'm trying the one-liner below, Adrian, can you please check this and
>> > provide an ack? I think that that comment about the address that it will
>> > default when map__load() fails needs rewriting in light of Thomas
>> > comments about other arches (see further below)?
>
>> > I did a quick check of machine->kernel_start usage in Intel PT and since
>> > on x86 that assumption about partitioning the address space holds, no
>> > problem should be introduced by the one-liner fix, right?
>  
>> Argh, this is also broken:
>  
>> static inline bool machine__kernel_ip(struct machine *machine, u64 ip)
>> {
>>         u64 kernel_start = machine__kernel_start(machine);
>> 
>>         return ip >= kernel_start;
>> }
>> 
>> We can't judge if a address is in the kernel like that :-\
>
> So, this is used by:
>
> [acme@jouet linux]$ find tools/ -name "*.[ch]" | xargs grep -w machine__kernel_ip
> tools/perf/builtin-script.c:	kernel = machine__kernel_ip(machine, start);
> tools/perf/builtin-script.c:	if (kernel != machine__kernel_ip(machine, end)) {
>
> That is just for "brstackinsn", would that make sense for Sparc, S/390?
>
> tools/perf/util/intel-bts.c:	if (machine__kernel_ip(machine, ip))
> tools/perf/util/intel-bts.c:		if (!machine__kernel_ip(btsq->bts->machine, branch->from) &&
> tools/perf/util/intel-bts.c:		    machine__kernel_ip(btsq->bts->machine, branch->to) &&
>
> Intel specific stuff, so should be ok.
>
> tools/perf/util/event.c:		    machine__kernel_ip(machine, al->addr)) {
>
> For this last one, that affects all arches, I think we can just remove
> this check and look at the kernel when not finding it anywhere else?
>
> diff --git a/tools/perf/util/event.c b/tools/perf/util/event.c
> index dc5c3bb69d73..8e435baaae6a 100644
> --- a/tools/perf/util/event.c
> +++ b/tools/perf/util/event.c
> @@ -1432,8 +1432,7 @@ void thread__find_addr_map(struct thread *thread, u8 cpumode,
>  		 * in the whole kernel symbol list.
>  		 */
>  		if (cpumode == PERF_RECORD_MISC_USER && machine &&
> -		    mg != &machine->kmaps &&
> -		    machine__kernel_ip(machine, al->addr)) {
> +		    mg != &machine->kmaps) {
>  			mg = &machine->kmaps;
>  			load_map = true;
>  			goto try_again;

Am I reading this right? We have a sample that claims to be in
userspace, but was not found in any symbol map, so we try looking for it
in the kernel map.

And the change is that previously we checked if the address was >= (1 << 63),
whereas after we don't bother.

Seems harmless™.

cheers

Re: perf report does not resolve symbols on s390x

[Arnaldo Carvalho de Melo]

Em Wed, Jul 12, 2017 at 08:40:57PM +1000, Michael Ellerman escreveu:
> Arnaldo Carvalho de Melo <acme@kernel.org> writes:
> 
> > Em Tue, Jul 11, 2017 at 04:38:28PM -0300, Arnaldo Carvalho de Melo escreveu:
> >> Em Tue, Jul 11, 2017 at 04:03:04PM -0300, Arnaldo Carvalho de Melo escreveu:
> >> > Em Fri, Jul 07, 2017 at 02:17:25PM +0200, Thomas-Mich Richter escreveu:
> >> > > On 07/06/2017 02:35 PM, Thomas-Mich Richter wrote:
> >> > > It determines the kernel starts at address 1<<63 and loads the kernel address mapping.
> >> > > On s390x
> >> > > - The kernel starts at 0x0 (value of map->start) and thus all checks in function 
> >> > >   thread__find_addr_map() fail and no symbol is found for the specified addresses
> >> > >   because the kernel starts at 0x8000000000000000. Which is wrong the kernel start at 0x0.
> >
> >> > Hi Thomas, really nice debugging session!
> >
> >> > I'm trying the one-liner below, Adrian, can you please check this and
> >> > provide an ack? I think that that comment about the address that it will
> >> > default when map__load() fails needs rewriting in light of Thomas
> >> > comments about other arches (see further below)?
> >
> >> > I did a quick check of machine->kernel_start usage in Intel PT and since
> >> > on x86 that assumption about partitioning the address space holds, no
> >> > problem should be introduced by the one-liner fix, right?
> >  
> >> Argh, this is also broken:
> >  
> >> static inline bool machine__kernel_ip(struct machine *machine, u64 ip)
> >> {
> >>         u64 kernel_start = machine__kernel_start(machine);
> >> 
> >>         return ip >= kernel_start;
> >> }
> >> 
> >> We can't judge if a address is in the kernel like that :-\
> >
> > So, this is used by:
> >
> > [acme@jouet linux]$ find tools/ -name "*.[ch]" | xargs grep -w machine__kernel_ip
> > tools/perf/builtin-script.c:	kernel = machine__kernel_ip(machine, start);
> > tools/perf/builtin-script.c:	if (kernel != machine__kernel_ip(machine, end)) {
> >
> > That is just for "brstackinsn", would that make sense for Sparc, S/390?
> >
> > tools/perf/util/intel-bts.c:	if (machine__kernel_ip(machine, ip))
> > tools/perf/util/intel-bts.c:		if (!machine__kernel_ip(btsq->bts->machine, branch->from) &&
> > tools/perf/util/intel-bts.c:		    machine__kernel_ip(btsq->bts->machine, branch->to) &&
> >
> > Intel specific stuff, so should be ok.
> >
> > tools/perf/util/event.c:		    machine__kernel_ip(machine, al->addr)) {
> >
> > For this last one, that affects all arches, I think we can just remove
> > this check and look at the kernel when not finding it anywhere else?
> >
> > diff --git a/tools/perf/util/event.c b/tools/perf/util/event.c
> > index dc5c3bb69d73..8e435baaae6a 100644
> > --- a/tools/perf/util/event.c
> > +++ b/tools/perf/util/event.c
> > @@ -1432,8 +1432,7 @@ void thread__find_addr_map(struct thread *thread, u8 cpumode,
> >  		 * in the whole kernel symbol list.
> >  		 */
> >  		if (cpumode == PERF_RECORD_MISC_USER && machine &&
> > -		    mg != &machine->kmaps &&
> > -		    machine__kernel_ip(machine, al->addr)) {
> > +		    mg != &machine->kmaps) {
> >  			mg = &machine->kmaps;
> >  			load_map = true;
> >  			goto try_again;
> 
> Am I reading this right? We have a sample that claims to be in
> userspace, but was not found in any symbol map, so we try looking for it
> in the kernel map.
> 
> And the change is that previously we checked if the address was >= (1 << 63),
> whereas after we don't bother.
> 
> Seems harmless™.

Thanks, will take that as an Acked-by:, ok?

 
> cheers

Re: perf report does not resolve symbols on s390x

[Michael Ellerman]

Arnaldo Carvalho de Melo <acme@kernel.org> writes:

> Em Wed, Jul 12, 2017 at 08:40:57PM +1000, Michael Ellerman escreveu:
>> Arnaldo Carvalho de Melo <acme@kernel.org> writes:
>> 
>> > Em Tue, Jul 11, 2017 at 04:38:28PM -0300, Arnaldo Carvalho de Melo escreveu:
>> >> Em Tue, Jul 11, 2017 at 04:03:04PM -0300, Arnaldo Carvalho de Melo escreveu:
>> >> > Em Fri, Jul 07, 2017 at 02:17:25PM +0200, Thomas-Mich Richter escreveu:
>> >> > > On 07/06/2017 02:35 PM, Thomas-Mich Richter wrote:
>> >> > > It determines the kernel starts at address 1<<63 and loads the kernel address mapping.
>> >> > > On s390x
>> >> > > - The kernel starts at 0x0 (value of map->start) and thus all checks in function 
>> >> > >   thread__find_addr_map() fail and no symbol is found for the specified addresses
>> >> > >   because the kernel starts at 0x8000000000000000. Which is wrong the kernel start at 0x0.
>> >
>> >> > Hi Thomas, really nice debugging session!
>> >
>> >> > I'm trying the one-liner below, Adrian, can you please check this and
>> >> > provide an ack? I think that that comment about the address that it will
>> >> > default when map__load() fails needs rewriting in light of Thomas
>> >> > comments about other arches (see further below)?
>> >
>> >> > I did a quick check of machine->kernel_start usage in Intel PT and since
>> >> > on x86 that assumption about partitioning the address space holds, no
>> >> > problem should be introduced by the one-liner fix, right?
>> >  
>> >> Argh, this is also broken:
>> >  
>> >> static inline bool machine__kernel_ip(struct machine *machine, u64 ip)
>> >> {
>> >>         u64 kernel_start = machine__kernel_start(machine);
>> >> 
>> >>         return ip >= kernel_start;
>> >> }
>> >> 
>> >> We can't judge if a address is in the kernel like that :-\
>> >
>> > So, this is used by:
>> >
>> > [acme@jouet linux]$ find tools/ -name "*.[ch]" | xargs grep -w machine__kernel_ip
>> > tools/perf/builtin-script.c:	kernel = machine__kernel_ip(machine, start);
>> > tools/perf/builtin-script.c:	if (kernel != machine__kernel_ip(machine, end)) {
>> >
>> > That is just for "brstackinsn", would that make sense for Sparc, S/390?
>> >
>> > tools/perf/util/intel-bts.c:	if (machine__kernel_ip(machine, ip))
>> > tools/perf/util/intel-bts.c:		if (!machine__kernel_ip(btsq->bts->machine, branch->from) &&
>> > tools/perf/util/intel-bts.c:		    machine__kernel_ip(btsq->bts->machine, branch->to) &&
>> >
>> > Intel specific stuff, so should be ok.
>> >
>> > tools/perf/util/event.c:		    machine__kernel_ip(machine, al->addr)) {
>> >
>> > For this last one, that affects all arches, I think we can just remove
>> > this check and look at the kernel when not finding it anywhere else?
>> >
>> > diff --git a/tools/perf/util/event.c b/tools/perf/util/event.c
>> > index dc5c3bb69d73..8e435baaae6a 100644
>> > --- a/tools/perf/util/event.c
>> > +++ b/tools/perf/util/event.c
>> > @@ -1432,8 +1432,7 @@ void thread__find_addr_map(struct thread *thread, u8 cpumode,
>> >  		 * in the whole kernel symbol list.
>> >  		 */
>> >  		if (cpumode == PERF_RECORD_MISC_USER && machine &&
>> > -		    mg != &machine->kmaps &&
>> > -		    machine__kernel_ip(machine, al->addr)) {
>> > +		    mg != &machine->kmaps) {
>> >  			mg = &machine->kmaps;
>> >  			load_map = true;
>> >  			goto try_again;
>> 
>> Am I reading this right? We have a sample that claims to be in
>> userspace, but was not found in any symbol map, so we try looking for it
>> in the kernel map.
>> 
>> And the change is that previously we checked if the address was >= (1 << 63),
>> whereas after we don't bother.
>> 
>> Seems harmless™.
>
> Thanks, will take that as an Acked-by:, ok?

Seems-harmless-but-will-probably-break-something-obscure-by: ... :)

Sure.

Acked-by: Michael Ellerman <mpe@ellerman.id.au> (powerpc)

cheers