Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]


Groups > linux.kernel > #1182556 > unrolled thread

[PATCH] ARC: make sure instruction_pointer() returns unsigned value

Started byAlexey Brodkin <Alexey.Brodkin@synopsys.com>
First post2015-07-13 09:30 +0200
Last post2015-07-13 10:10 +0200
Articles 2 — 2 participants

Back to article view | Back to linux.kernel


Contents

  [PATCH] ARC: make sure instruction_pointer() returns unsigned value Alexey Brodkin <Alexey.Brodkin@synopsys.com> - 2015-07-13 09:30 +0200
    Re: [PATCH] ARC: make sure instruction_pointer() returns unsigned  value Vineet Gupta <Vineet.Gupta1@synopsys.com> - 2015-07-13 10:10 +0200

#1182556 — [PATCH] ARC: make sure instruction_pointer() returns unsigned value

FromAlexey Brodkin <Alexey.Brodkin@synopsys.com>
Date2015-07-13 09:30 +0200
Subject[PATCH] ARC: make sure instruction_pointer() returns unsigned value
Message-ID<pLGlz-2ap-3@gated-at.bofh.it>
Currently instruction_pointer() returns pt_regs->ret and so return value
is of type "long", which implicitly stands for "signed long".

While that's perfectly fine when dealing with 32-bit values if return
value of instruction_pointer() gets assigned to 64-bit variable sign
extension may happen.

And at least in one real use-case it happens already.
In perf_prepare_sample() return value of perf_instruction_pointer()
(which is an alias to instruction_pointer() in case of ARC) is assigned
to (struct perf_sample_data)->ip (which type is "u64").

And what we see if instuction pointer points to user-space application
that in case of ARC lays below 0x8000_0000 "ip" gets set properly with
leading 32 zeros. But if instruction pointer points to kernel address
space that starts from 0x8000_0000 then "ip" is set with 32 leadig
"f"-s. I.e. id instruction_pointer() returns 0x8100_0000, "ip" will be
assigned with 0xffff_ffff__8100_0000. Which is obviously wrong.

In particular that issuse broke output of perf, because perf was unable
to associate addresses like 0xffff_ffff__8100_0000 with anything from
/proc/kallsyms.

That's what we used to see:
 ----------->8----------
  6.27%  ls       [unknown]                [k] 0xffffffff8046c5cc
  2.96%  ls       libuClibc-0.9.34-git.so  [.] memcpy
  2.25%  ls       libuClibc-0.9.34-git.so  [.] memset
  1.66%  ls       [unknown]                [k] 0xffffffff80666536
  1.54%  ls       libuClibc-0.9.34-git.so  [.] 0x000224d6
  1.18%  ls       libuClibc-0.9.34-git.so  [.] 0x00022472
 ----------->8----------

With that change perf output looks much better now:
 ----------->8----------
  8.21%  ls       [kernel.kallsyms]        [k] memset
  3.52%  ls       libuClibc-0.9.34-git.so  [.] memcpy
  2.11%  ls       libuClibc-0.9.34-git.so  [.] malloc
  1.88%  ls       libuClibc-0.9.34-git.so  [.] memset
  1.64%  ls       [kernel.kallsyms]        [k] _raw_spin_unlock_irqrestore
  1.41%  ls       [kernel.kallsyms]        [k] __d_lookup_rcu
 ----------->8----------

Signed-off-by: Alexey Brodkin <abrodkin@synopsys.com>
Cc: arc-linux-dev@synopsys.com
Cc: stable@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
---
 arch/arc/include/asm/ptrace.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/arc/include/asm/ptrace.h b/arch/arc/include/asm/ptrace.h
index 9175597..91694ec 100644
--- a/arch/arc/include/asm/ptrace.h
+++ b/arch/arc/include/asm/ptrace.h
@@ -106,7 +106,7 @@ struct callee_regs {
 	long r25, r24, r23, r22, r21, r20, r19, r18, r17, r16, r15, r14, r13;
 };
 
-#define instruction_pointer(regs)	((regs)->ret)
+#define instruction_pointer(regs)	(unsigned long)((regs)->ret)
 #define profile_pc(regs)		instruction_pointer(regs)
 
 /* return 1 if user mode or 0 if kernel mode */
-- 
2.4.3

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [next] | [standalone]


#1182582 — Re: [PATCH] ARC: make sure instruction_pointer() returns unsigned value

FromVineet Gupta <Vineet.Gupta1@synopsys.com>
Date2015-07-13 10:10 +0200
SubjectRe: [PATCH] ARC: make sure instruction_pointer() returns unsigned value
Message-ID<pLGYj-2CQ-25@gated-at.bofh.it>
In reply to#1182556
On Monday 13 July 2015 12:55 PM, Alexey Brodkin wrote:
> Currently instruction_pointer() returns pt_regs->ret and so return value
> is of type "long", which implicitly stands for "signed long".
>
> While that's perfectly fine when dealing with 32-bit values if return
> value of instruction_pointer() gets assigned to 64-bit variable sign
> extension may happen.
>
> And at least in one real use-case it happens already.
> In perf_prepare_sample() return value of perf_instruction_pointer()
> (which is an alias to instruction_pointer() in case of ARC) is assigned
> to (struct perf_sample_data)->ip (which type is "u64").
>
> And what we see if instuction pointer points to user-space application
> that in case of ARC lays below 0x8000_0000 "ip" gets set properly with
> leading 32 zeros. But if instruction pointer points to kernel address
> space that starts from 0x8000_0000 then "ip" is set with 32 leadig
> "f"-s. I.e. id instruction_pointer() returns 0x8100_0000, "ip" will be
> assigned with 0xffff_ffff__8100_0000. Which is obviously wrong.
>
> In particular that issuse broke output of perf, because perf was unable
> to associate addresses like 0xffff_ffff__8100_0000 with anything from
> /proc/kallsyms.
>
> That's what we used to see:
>  ----------->8----------
>   6.27%  ls       [unknown]                [k] 0xffffffff8046c5cc
>   2.96%  ls       libuClibc-0.9.34-git.so  [.] memcpy
>   2.25%  ls       libuClibc-0.9.34-git.so  [.] memset
>   1.66%  ls       [unknown]                [k] 0xffffffff80666536
>   1.54%  ls       libuClibc-0.9.34-git.so  [.] 0x000224d6
>   1.18%  ls       libuClibc-0.9.34-git.so  [.] 0x00022472
>  ----------->8----------
>
> With that change perf output looks much better now:
>  ----------->8----------
>   8.21%  ls       [kernel.kallsyms]        [k] memset
>   3.52%  ls       libuClibc-0.9.34-git.so  [.] memcpy
>   2.11%  ls       libuClibc-0.9.34-git.so  [.] malloc
>   1.88%  ls       libuClibc-0.9.34-git.so  [.] memset
>   1.64%  ls       [kernel.kallsyms]        [k] _raw_spin_unlock_irqrestore
>   1.41%  ls       [kernel.kallsyms]        [k] __d_lookup_rcu
>  ----------->8----------
>
> Signed-off-by: Alexey Brodkin <abrodkin@synopsys.com>

Thx Alexey - this solves a long standing mystery with some weird perf profiles.

Applied !

Thx,
-Vineet
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [standalone]


Back to top | Article view | linux.kernel


csiph-web