Re: [PATCH] kprobes: Warn if the kprobe is reregistered

[Ananth N Mavinakayanahalli]

On 2/3/21 8:29 PM, Masami Hiramatsu wrote:

Warn if the kprobe is reregistered, since there must be
a software bug (actively used resource must not be re-registered)
and caller must be fixed.

Signed-off-by: Masami Hiramatsu 


Acked-by: Ananth N Mavinakayanahalli 

[tip:perf/urgent] MAINTAINERS: Add Naveen N. Rao as kprobes co-maintainer

[tip-bot for Ananth N Mavinakayanahalli]

Commit-ID:  4799f6856fdd38c8078a190eca3288029287cf66
Gitweb: https://git.kernel.org/tip/4799f6856fdd38c8078a190eca3288029287cf66
Author: Ananth N Mavinakayanahalli 
AuthorDate: Tue, 17 Jul 2018 11:32:37 +0530
Committer:  Ingo Molnar 
CommitDate: Tue, 24 Jul 2018 17:01:28 +0200

MAINTAINERS: Add Naveen N. Rao as kprobes co-maintainer

Naveen has been contributing consistently reviewing and hardening
kprobes for some time now. I have not been able to do the same due
to other commitments.

Signed-off-by: Ananth N Mavinakayanahalli 
Cc: Naveen N. Rao 
Cc: Linus Torvalds 
Cc: Peter Zijlstra 
Cc: Thomas Gleixner 
Cc: Anil S Keshavamurthy 
Cc: "David S. Miller" 
Cc: Masami Hiramatsu 
Cc: Arnaldo Carvalho de Melo 
Cc: Namhyung Kim 
Cc: Jiri Olsa 
Cc: a...@linux-foundation.org
Cc: mhira...@kernel.org
Link: 
http://lkml.kernel.org/r/153180735790.1914.15547706781664285286.stgit@thinktux
Signed-off-by: Ingo Molnar 
---
 MAINTAINERS | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/MAINTAINERS b/MAINTAINERS
index 0fe4228f78cb..42a884c1b0f7 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -7984,7 +7984,7 @@ F:lib/test_kmod.c
 F: tools/testing/selftests/kmod/
 
 KPROBES
-M:     Ananth N Mavinakayanahalli 
+M: Naveen N. Rao 
 M: Anil S Keshavamurthy 
 M: "David S. Miller" 
 M: Masami Hiramatsu 

[tip:perf/urgent] MAINTAINERS: Add Naveen N. Rao as kprobes co-maintainer

[tip-bot for Ananth N Mavinakayanahalli]

Commit-ID:  4799f6856fdd38c8078a190eca3288029287cf66
Gitweb: https://git.kernel.org/tip/4799f6856fdd38c8078a190eca3288029287cf66
Author: Ananth N Mavinakayanahalli 
AuthorDate: Tue, 17 Jul 2018 11:32:37 +0530
Committer:  Ingo Molnar 
CommitDate: Tue, 24 Jul 2018 17:01:28 +0200

MAINTAINERS: Add Naveen N. Rao as kprobes co-maintainer

Naveen has been contributing consistently reviewing and hardening
kprobes for some time now. I have not been able to do the same due
to other commitments.

Signed-off-by: Ananth N Mavinakayanahalli 
Cc: Naveen N. Rao 
Cc: Linus Torvalds 
Cc: Peter Zijlstra 
Cc: Thomas Gleixner 
Cc: Anil S Keshavamurthy 
Cc: "David S. Miller" 
Cc: Masami Hiramatsu 
Cc: Arnaldo Carvalho de Melo 
Cc: Namhyung Kim 
Cc: Jiri Olsa 
Cc: a...@linux-foundation.org
Cc: mhira...@kernel.org
Link: 
http://lkml.kernel.org/r/153180735790.1914.15547706781664285286.stgit@thinktux
Signed-off-by: Ingo Molnar 
---
 MAINTAINERS | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/MAINTAINERS b/MAINTAINERS
index 0fe4228f78cb..42a884c1b0f7 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -7984,7 +7984,7 @@ F:lib/test_kmod.c
 F: tools/testing/selftests/kmod/
 
 KPROBES
-M:     Ananth N Mavinakayanahalli 
+M: Naveen N. Rao 
 M: Anil S Keshavamurthy 
 M: "David S. Miller" 
 M: Masami Hiramatsu 

[tip:perf/urgent] MAINTAINERS: Add Naveen N. Rao as kprobes co-maintainer

[tip-bot for Ananth N Mavinakayanahalli]

Commit-ID:  1b51a2fe84d87ae3df11b169cfb38db16df0c9af
Gitweb: https://git.kernel.org/tip/1b51a2fe84d87ae3df11b169cfb38db16df0c9af
Author: Ananth N Mavinakayanahalli 
AuthorDate: Tue, 17 Jul 2018 11:32:37 +0530
Committer:  Ingo Molnar 
CommitDate: Tue, 17 Jul 2018 09:23:23 +0200

MAINTAINERS: Add Naveen N. Rao as kprobes co-maintainer

Naveen has been contributing consistently reviewing and hardening
kprobes for some time now. I have not been able to do the same due
to other commitments.

Signed-off-by: Ananth N Mavinakayanahalli 
Cc: Naveen N. Rao 
Cc: Linus Torvalds 
Cc: Peter Zijlstra 
Cc: Thomas Gleixner 
Cc: Anil S Keshavamurthy 
Cc: "David S. Miller" 
Cc: Masami Hiramatsu 
Cc: Arnaldo Carvalho de Melo 
Cc: Namhyung Kim 
Cc: Jiri Olsa 
Cc: a...@linux-foundation.org
Cc: mhira...@kernel.org
Link: 
http://lkml.kernel.org/r/153180735790.1914.15547706781664285286.stgit@thinktux
Signed-off-by: Ingo Molnar 
---
 MAINTAINERS | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/MAINTAINERS b/MAINTAINERS
index 192d7f73fd01..7aec3c9709ba 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -7985,7 +7985,7 @@ F:lib/test_kmod.c
 F: tools/testing/selftests/kmod/
 
 KPROBES
-M:     Ananth N Mavinakayanahalli 
+M: Naveen N. Rao 
 M: Anil S Keshavamurthy 
 M: "David S. Miller" 
 M: Masami Hiramatsu 

[tip:perf/urgent] MAINTAINERS: Add Naveen N. Rao as kprobes co-maintainer

[tip-bot for Ananth N Mavinakayanahalli]

Commit-ID:  1b51a2fe84d87ae3df11b169cfb38db16df0c9af
Gitweb: https://git.kernel.org/tip/1b51a2fe84d87ae3df11b169cfb38db16df0c9af
Author: Ananth N Mavinakayanahalli 
AuthorDate: Tue, 17 Jul 2018 11:32:37 +0530
Committer:  Ingo Molnar 
CommitDate: Tue, 17 Jul 2018 09:23:23 +0200

MAINTAINERS: Add Naveen N. Rao as kprobes co-maintainer

Naveen has been contributing consistently reviewing and hardening
kprobes for some time now. I have not been able to do the same due
to other commitments.

Signed-off-by: Ananth N Mavinakayanahalli 
Cc: Naveen N. Rao 
Cc: Linus Torvalds 
Cc: Peter Zijlstra 
Cc: Thomas Gleixner 
Cc: Anil S Keshavamurthy 
Cc: "David S. Miller" 
Cc: Masami Hiramatsu 
Cc: Arnaldo Carvalho de Melo 
Cc: Namhyung Kim 
Cc: Jiri Olsa 
Cc: a...@linux-foundation.org
Cc: mhira...@kernel.org
Link: 
http://lkml.kernel.org/r/153180735790.1914.15547706781664285286.stgit@thinktux
Signed-off-by: Ingo Molnar 
---
 MAINTAINERS | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/MAINTAINERS b/MAINTAINERS
index 192d7f73fd01..7aec3c9709ba 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -7985,7 +7985,7 @@ F:lib/test_kmod.c
 F: tools/testing/selftests/kmod/
 
 KPROBES
-M:     Ananth N Mavinakayanahalli 
+M: Naveen N. Rao 
 M: Anil S Keshavamurthy 
 M: "David S. Miller" 
 M: Masami Hiramatsu 

[PATCH] kprobes: Kprobes maintainer change

[Ananth N Mavinakayanahalli]

Naveen has been contributing consistently reviewing and hardening
kprobes for some time now. I have not been able to do the same due
to other commitments.

Signed-off-by: Ananth N Mavinakayanahalli 
---
 MAINTAINERS |2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/MAINTAINERS b/MAINTAINERS
index 192d7f73fd01..7aec3c9709ba 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -7985,7 +7985,7 @@ F:lib/test_kmod.c
 F: tools/testing/selftests/kmod/
 
 KPROBES
-M: Ananth N Mavinakayanahalli 
+M: Naveen N. Rao 
 M: Anil S Keshavamurthy 
 M: "David S. Miller" 
 M: Masami Hiramatsu 

[PATCH] kprobes: Kprobes maintainer change

[Ananth N Mavinakayanahalli]

Naveen has been contributing consistently reviewing and hardening
kprobes for some time now. I have not been able to do the same due
to other commitments.

Signed-off-by: Ananth N Mavinakayanahalli 
---
 MAINTAINERS |2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/MAINTAINERS b/MAINTAINERS
index 192d7f73fd01..7aec3c9709ba 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -7985,7 +7985,7 @@ F:lib/test_kmod.c
 F: tools/testing/selftests/kmod/
 
 KPROBES
-M: Ananth N Mavinakayanahalli 
+M: Naveen N. Rao 
 M: Anil S Keshavamurthy 
 M: "David S. Miller" 
 M: Masami Hiramatsu 

Re: [RFC][PATCH] Lock down kprobes

[Ananth N Mavinakayanahalli]

On Thu, Nov 09, 2017 at 09:54:50PM +, David Howells wrote:
> Ananth N Mavinakayanahalli <ana...@linux.vnet.ibm.com> wrote:
> 
> > Yes, this patch will prevent any kprobe registration.
> 
> Can I put that down as a Reviewed-by?

Reviewed-by: Ananth N Mavinakayanahalli <ana...@linux.vnet.ibm.com>

Re: [RFC][PATCH] Lock down kprobes

[Ananth N Mavinakayanahalli]

On Thu, Nov 09, 2017 at 09:54:50PM +, David Howells wrote:
> Ananth N Mavinakayanahalli  wrote:
> 
> > Yes, this patch will prevent any kprobe registration.
> 
> Can I put that down as a Reviewed-by?

Reviewed-by: Ananth N Mavinakayanahalli 

Re: [RFC][PATCH] Lock down kprobes

[Ananth N Mavinakayanahalli]

On Thu, Nov 09, 2017 at 04:52:05PM +, David Howells wrote:
> Hi,
> 
> I need to lock down kprobes under secure boot conditions as part of the patch
> series that can be found here:
> 
>   
> https://git.kernel.org/pub/scm/linux/kernel/git/dhowells/linux-fs.git/log/?h=efi-lock-down
> 
> Can you tell me that if the attached patch is sufficient to the cause?

Yes, this patch will prevent any kprobe registration.

Ananth

> 
> Thanks,
> David
> ---
> commit ffb3484d6e0f1d625f8e84a6a19c139a28a52499
> Author: David Howells 
> Date:   Wed Nov 8 16:14:12 2017 +
> 
> Lock down kprobes
> 
> Disallow the creation of kprobes when the kernel is locked down by
> preventing their registration.  This prevents kprobes from being used to
> access kernel memory, either to make modifications or to steal crypto 
> data.
> 
> Reported-by: Alexei Starovoitov 
> Signed-off-by: David Howells 
> 
> diff --git a/kernel/kprobes.c b/kernel/kprobes.c
> index a1606a4224e1..f06023b0936c 100644
> --- a/kernel/kprobes.c
> +++ b/kernel/kprobes.c
> @@ -1530,6 +1530,9 @@ int register_kprobe(struct kprobe *p)
>   struct module *probed_mod;
>   kprobe_opcode_t *addr;
>  
> + if (kernel_is_locked_down("Use of kprobes"))
> + return -EPERM;
> +
>   /* Adjust probe address from symbol */
>   addr = kprobe_addr(p);
>   if (IS_ERR(addr))

Re: [RFC][PATCH] Lock down kprobes

[Ananth N Mavinakayanahalli]

On Thu, Nov 09, 2017 at 04:52:05PM +, David Howells wrote:
> Hi,
> 
> I need to lock down kprobes under secure boot conditions as part of the patch
> series that can be found here:
> 
>   
> https://git.kernel.org/pub/scm/linux/kernel/git/dhowells/linux-fs.git/log/?h=efi-lock-down
> 
> Can you tell me that if the attached patch is sufficient to the cause?

Yes, this patch will prevent any kprobe registration.

Ananth

> 
> Thanks,
> David
> ---
> commit ffb3484d6e0f1d625f8e84a6a19c139a28a52499
> Author: David Howells 
> Date:   Wed Nov 8 16:14:12 2017 +
> 
> Lock down kprobes
> 
> Disallow the creation of kprobes when the kernel is locked down by
> preventing their registration.  This prevents kprobes from being used to
> access kernel memory, either to make modifications or to steal crypto 
> data.
> 
> Reported-by: Alexei Starovoitov 
> Signed-off-by: David Howells 
> 
> diff --git a/kernel/kprobes.c b/kernel/kprobes.c
> index a1606a4224e1..f06023b0936c 100644
> --- a/kernel/kprobes.c
> +++ b/kernel/kprobes.c
> @@ -1530,6 +1530,9 @@ int register_kprobe(struct kprobe *p)
>   struct module *probed_mod;
>   kprobe_opcode_t *addr;
>  
> + if (kernel_is_locked_down("Use of kprobes"))
> + return -EPERM;
> +
>   /* Adjust probe address from symbol */
>   addr = kprobe_addr(p);
>   if (IS_ERR(addr))

Re: [RFC][PATCH] Lock down kprobes

[Ananth N Mavinakayanahalli]

On Wed, Nov 08, 2017 at 04:21:33PM +, David Howells wrote:
> Hi,
> 
> I need to lock down kprobes under secure boot conditions as part of the patch
> series that can be found here:
> 
>   
> https://git.kernel.org/pub/scm/linux/kernel/git/dhowells/linux-fs.git/log/?h=efi-lock-down
> 
> Can you tell me that if the attached patch is sufficient to the cause?

This will not prevent the raw kprobe events from working. If your
intention is to prevent *any* kprobe registration, the best place to do
that is in register_kprobe() in kernel/probes.c

Ananth

Re: [RFC][PATCH] Lock down kprobes

[Ananth N Mavinakayanahalli]

On Wed, Nov 08, 2017 at 04:21:33PM +, David Howells wrote:
> Hi,
> 
> I need to lock down kprobes under secure boot conditions as part of the patch
> series that can be found here:
> 
>   
> https://git.kernel.org/pub/scm/linux/kernel/git/dhowells/linux-fs.git/log/?h=efi-lock-down
> 
> Can you tell me that if the attached patch is sufficient to the cause?

This will not prevent the raw kprobe events from working. If your
intention is to prevent *any* kprobe registration, the best place to do
that is in register_kprobe() in kernel/probes.c

Ananth

Re: [PATCH v2] ppc64/kprobe: Fix oops when kprobed on 'stdu' instruction

[Ananth N Mavinakayanahalli]

On Tue, Apr 11, 2017 at 10:38:13AM +0530, Ravi Bangoria wrote:
> If we set a kprobe on a 'stdu' instruction on powerpc64, we see a kernel 
> OOPS:
> 
>   [ 1275.165932] Bad kernel stack pointer cd93c840 at c0009868
>   [ 1275.166378] Oops: Bad kernel stack pointer, sig: 6 [#1]
>   ...
>   GPR00: c01fcd93cb30 cd93c840 c15c5e00 cd93c840
>   ...
>   [ 1275.178305] NIP [c0009868] resume_kernel+0x2c/0x58
>   [ 1275.178594] LR [c0006208] program_check_common+0x108/0x180
> 
> Basically, on 64 bit system, when user probes on 'stdu' instruction,
> kernel does not emulate actual store in emulate_step itself because it
> may corrupt exception frame. So kernel does actual store operation in
> exception return code i.e. resume_kernel().
> 
> resume_kernel() loads the saved stack pointer from memory using lwz,
> effectively loading a corrupt (32bit) address, causing the kernel crash.
> 
> Fix this by loading the 64bit value instead.
> 
> Fixes: be96f63375a1 ("powerpc: Split out instruction analysis part of 
> emulate_step()") 
> Signed-off-by: Ravi Bangoria <ravi.bango...@linux.vnet.ibm.com>
> Reviewed-by: Naveen N. Rao <naveen.n@linux.vnet.ibm.com> 

Reviewed-by: Ananth N Mavinakayanahalli <ana...@linux.vnet.ibm.com>

Re: [PATCH v2] ppc64/kprobe: Fix oops when kprobed on 'stdu' instruction

[Ananth N Mavinakayanahalli]

On Tue, Apr 11, 2017 at 10:38:13AM +0530, Ravi Bangoria wrote:
> If we set a kprobe on a 'stdu' instruction on powerpc64, we see a kernel 
> OOPS:
> 
>   [ 1275.165932] Bad kernel stack pointer cd93c840 at c0009868
>   [ 1275.166378] Oops: Bad kernel stack pointer, sig: 6 [#1]
>   ...
>   GPR00: c01fcd93cb30 cd93c840 c15c5e00 cd93c840
>   ...
>   [ 1275.178305] NIP [c0009868] resume_kernel+0x2c/0x58
>   [ 1275.178594] LR [c0006208] program_check_common+0x108/0x180
> 
> Basically, on 64 bit system, when user probes on 'stdu' instruction,
> kernel does not emulate actual store in emulate_step itself because it
> may corrupt exception frame. So kernel does actual store operation in
> exception return code i.e. resume_kernel().
> 
> resume_kernel() loads the saved stack pointer from memory using lwz,
> effectively loading a corrupt (32bit) address, causing the kernel crash.
> 
> Fix this by loading the 64bit value instead.
> 
> Fixes: be96f63375a1 ("powerpc: Split out instruction analysis part of 
> emulate_step()") 
> Signed-off-by: Ravi Bangoria 
> Reviewed-by: Naveen N. Rao  

Reviewed-by: Ananth N Mavinakayanahalli 

Re: [PATCH v2 2/5] powerpc: kretprobes: override default function entry offset

[Ananth N Mavinakayanahalli]

On Wed, Feb 22, 2017 at 07:23:38PM +0530, Naveen N. Rao wrote:
> With ABIv2, we offset 8 bytes into a function to get at the local entry
> point.
> 

Looks good.

> Signed-off-by: Naveen N. Rao <naveen.n@linux.vnet.ibm.com>

Acked-by: Ananth N Mavinakayanahalli <ana...@linux.vnet.ibm.com>

Re: [PATCH v2 2/5] powerpc: kretprobes: override default function entry offset

[Ananth N Mavinakayanahalli]

On Wed, Feb 22, 2017 at 07:23:38PM +0530, Naveen N. Rao wrote:
> With ABIv2, we offset 8 bytes into a function to get at the local entry
> point.
> 

Looks good.

> Signed-off-by: Naveen N. Rao 

Acked-by: Ananth N Mavinakayanahalli 

Re: [PATCH 1/3] powerpc: kprobes: add support for KPROBES_ON_FTRACE

[Ananth N Mavinakayanahalli]

On Wed, Feb 15, 2017 at 12:28:34AM +0530, Naveen N. Rao wrote:
> Allow kprobes to be placed on ftrace _mcount() call sites. This
> optimization avoids the use of a trap, by riding on ftrace
> infrastructure.
> 
> This depends on HAVE_DYNAMIC_FTRACE_WITH_REGS which depends on
> MPROFILE_KERNEL, which is only currently enabled on powerpc64le with
> newer toolchains.
> 
> Based on the x86 code by Masami.
> 
> Signed-off-by: Naveen N. Rao 
> ---
>  arch/powerpc/Kconfig |   1 +
>  arch/powerpc/include/asm/kprobes.h   |  10 
>  arch/powerpc/kernel/Makefile |   3 ++
>  arch/powerpc/kernel/kprobes-ftrace.c | 100 
> +++
>  arch/powerpc/kernel/kprobes.c|   4 +-
>  arch/powerpc/kernel/optprobes.c  |   3 ++
>  6 files changed, 120 insertions(+), 1 deletion(-)
>  create mode 100644 arch/powerpc/kernel/kprobes-ftrace.c

You'll also need to update
Documentation/features/debug/kprobes-on-ftrace/arch-support.txt

> +/* Ftrace callback handler for kprobes */
> +void kprobe_ftrace_handler(unsigned long nip, unsigned long parent_nip,
> +struct ftrace_ops *ops, struct pt_regs *regs)
> +{
> + struct kprobe *p;
> + struct kprobe_ctlblk *kcb;
> + unsigned long flags;
> +
> + /* Disable irq for emulating a breakpoint and avoiding preempt */
> + local_irq_save(flags);
> + hard_irq_disable();
> +
> + p = get_kprobe((kprobe_opcode_t *)nip);
> + if (unlikely(!p) || kprobe_disabled(p))
> + goto end;
> +
> + kcb = get_kprobe_ctlblk();
> + if (kprobe_running()) {
> + kprobes_inc_nmissed_count(p);
> + } else {
> + unsigned long orig_nip = regs->nip;
> + /* Kprobe handler expects regs->nip = nip + 1 as breakpoint hit 
> */

Can you clarify this? On powerpc, the regs->nip at the time of
breakpoint hit points to the probed instruction, not the one after.

Ananth

Re: [PATCH 1/3] powerpc: kprobes: add support for KPROBES_ON_FTRACE

[Ananth N Mavinakayanahalli]

On Wed, Feb 15, 2017 at 12:28:34AM +0530, Naveen N. Rao wrote:
> Allow kprobes to be placed on ftrace _mcount() call sites. This
> optimization avoids the use of a trap, by riding on ftrace
> infrastructure.
> 
> This depends on HAVE_DYNAMIC_FTRACE_WITH_REGS which depends on
> MPROFILE_KERNEL, which is only currently enabled on powerpc64le with
> newer toolchains.
> 
> Based on the x86 code by Masami.
> 
> Signed-off-by: Naveen N. Rao 
> ---
>  arch/powerpc/Kconfig |   1 +
>  arch/powerpc/include/asm/kprobes.h   |  10 
>  arch/powerpc/kernel/Makefile |   3 ++
>  arch/powerpc/kernel/kprobes-ftrace.c | 100 
> +++
>  arch/powerpc/kernel/kprobes.c|   4 +-
>  arch/powerpc/kernel/optprobes.c  |   3 ++
>  6 files changed, 120 insertions(+), 1 deletion(-)
>  create mode 100644 arch/powerpc/kernel/kprobes-ftrace.c

You'll also need to update
Documentation/features/debug/kprobes-on-ftrace/arch-support.txt

> +/* Ftrace callback handler for kprobes */
> +void kprobe_ftrace_handler(unsigned long nip, unsigned long parent_nip,
> +struct ftrace_ops *ops, struct pt_regs *regs)
> +{
> + struct kprobe *p;
> + struct kprobe_ctlblk *kcb;
> + unsigned long flags;
> +
> + /* Disable irq for emulating a breakpoint and avoiding preempt */
> + local_irq_save(flags);
> + hard_irq_disable();
> +
> + p = get_kprobe((kprobe_opcode_t *)nip);
> + if (unlikely(!p) || kprobe_disabled(p))
> + goto end;
> +
> + kcb = get_kprobe_ctlblk();
> + if (kprobe_running()) {
> + kprobes_inc_nmissed_count(p);
> + } else {
> + unsigned long orig_nip = regs->nip;
> + /* Kprobe handler expects regs->nip = nip + 1 as breakpoint hit 
> */

Can you clarify this? On powerpc, the regs->nip at the time of
breakpoint hit points to the probed instruction, not the one after.

Ananth

Re: [PATCH 3/3] powerpc: kprobes: emulate instructions on kprobe handler re-entry

[Ananth N Mavinakayanahalli]

On Tue, Feb 14, 2017 at 02:08:03PM +0530, Naveen N. Rao wrote:
> On kprobe handler re-entry, try to emulate the instruction rather than
> single stepping always.
> 
> As a related change, remove the duplicate saving of msr as that is
> already done in set_current_kprobe()
> 
> Signed-off-by: Naveen N. Rao <naveen.n@linux.vnet.ibm.com>

Acked-by: Ananth N Mavinakayanahalli <ana...@linux.vnet.ibm.com>

Re: [PATCH 3/3] powerpc: kprobes: emulate instructions on kprobe handler re-entry

[Ananth N Mavinakayanahalli]

On Tue, Feb 14, 2017 at 02:08:03PM +0530, Naveen N. Rao wrote:
> On kprobe handler re-entry, try to emulate the instruction rather than
> single stepping always.
> 
> As a related change, remove the duplicate saving of msr as that is
> already done in set_current_kprobe()
> 
> Signed-off-by: Naveen N. Rao 

Acked-by: Ananth N Mavinakayanahalli 

Re: [PATCH 2/3] powerpc: kprobes: factor out code to emulate instruction into a helper

[Ananth N Mavinakayanahalli]

On Tue, Feb 14, 2017 at 02:08:02PM +0530, Naveen N. Rao wrote:
> This helper will be used in a subsequent patch to emulate instructions
> on re-entering the kprobe handler. No functional change.
> 
> Signed-off-by: Naveen N. Rao <naveen.n@linux.vnet.ibm.com>

Acked-by: Ananth N Mavinakayanahalli <ana...@linux.vnet.ibm.com>

Re: [PATCH 2/3] powerpc: kprobes: factor out code to emulate instruction into a helper

[Ananth N Mavinakayanahalli]

On Tue, Feb 14, 2017 at 02:08:02PM +0530, Naveen N. Rao wrote:
> This helper will be used in a subsequent patch to emulate instructions
> on re-entering the kprobe handler. No functional change.
> 
> Signed-off-by: Naveen N. Rao 

Acked-by: Ananth N Mavinakayanahalli 

Re: [PATCH 1/3] powerpc: kprobes: fix handling of function offsets on ABIv2

[Ananth N Mavinakayanahalli]

On Tue, Feb 14, 2017 at 02:08:01PM +0530, Naveen N. Rao wrote:
> commit 239aeba76409 ("perf powerpc: Fix kprobe and kretprobe handling
> with kallsyms on ppc64le") changed how we use the offset field in struct
> kprobe on ABIv2. perf now offsets from the GEP (Global entry point) if an
> offset is specified and otherwise chooses the LEP (Local entry point).
> 
> Fix the same in kernel for kprobe API users. We do this by extending
> kprobe_lookup_name() to accept an additional parameter to indicate the
> offset specified with the kprobe registration. If offset is 0, we return
> the local function entry and return the global entry point otherwise.
> 
> With:
>   # cd /sys/kernel/debug/tracing/
>   # echo "p _do_fork" >> kprobe_events
>   # echo "p _do_fork+0x10" >> kprobe_events
> 
> before this patch:
>   # cat ../kprobes/list
>   c00d0748  k  _do_fork+0x8[DISABLED]
>   c00d0758  k  _do_fork+0x18[DISABLED]
>   c00412b0  k  kretprobe_trampoline+0x0[OPTIMIZED]
> 
> and after:
>   # cat ../kprobes/list
>   c00d04c8  k  _do_fork+0x8[DISABLED]
>   c00d04d0  k  _do_fork+0x10[DISABLED]
>   c00412b0  k  kretprobe_trampoline+0x0    [OPTIMIZED]
> 
> Signed-off-by: Naveen N. Rao <naveen.n@linux.vnet.ibm.com>

Acked-by: Ananth N Mavinakayanahalli <ana...@linux.vnet.ibm.com>

Re: [PATCH 1/3] powerpc: kprobes: fix handling of function offsets on ABIv2

[Ananth N Mavinakayanahalli]

On Tue, Feb 14, 2017 at 02:08:01PM +0530, Naveen N. Rao wrote:
> commit 239aeba76409 ("perf powerpc: Fix kprobe and kretprobe handling
> with kallsyms on ppc64le") changed how we use the offset field in struct
> kprobe on ABIv2. perf now offsets from the GEP (Global entry point) if an
> offset is specified and otherwise chooses the LEP (Local entry point).
> 
> Fix the same in kernel for kprobe API users. We do this by extending
> kprobe_lookup_name() to accept an additional parameter to indicate the
> offset specified with the kprobe registration. If offset is 0, we return
> the local function entry and return the global entry point otherwise.
> 
> With:
>   # cd /sys/kernel/debug/tracing/
>   # echo "p _do_fork" >> kprobe_events
>   # echo "p _do_fork+0x10" >> kprobe_events
> 
> before this patch:
>   # cat ../kprobes/list
>   c00d0748  k  _do_fork+0x8[DISABLED]
>   c00d0758  k  _do_fork+0x18[DISABLED]
>   c00412b0  k  kretprobe_trampoline+0x0[OPTIMIZED]
> 
> and after:
>   # cat ../kprobes/list
>   c00d04c8  k  _do_fork+0x8[DISABLED]
>   c00d04d0  k  _do_fork+0x10[DISABLED]
>   c00412b0  k  kretprobe_trampoline+0x0[OPTIMIZED]
> 
> Signed-off-by: Naveen N. Rao 

Acked-by: Ananth N Mavinakayanahalli 

Re: [PATCH] kretprobes: reject registration if a symbol offset is specified

[Ananth N Mavinakayanahalli]

On Tue, Feb 14, 2017 at 02:01:18PM +0530, Naveen N. Rao wrote:
> Users shouldn't be able to specify an offset with kretprobes, as we always
> want to probe at function entry. Otherwise, we won't be able to capture
> the proper return address resulting in the kretprobe never firing.
> 
> With samples/kprobes/kretprobe_example.c including an offset:
>   my_kretprobe.kp.offset = 40;
> 
> Before this patch, the probe gets planted but never fires.
> 
> After this patch:
>   $ sudo insmod samples/kprobes/kretprobe_example.ko
>   [sudo] password for naveen: 
>   insmod: ERROR: could not insert module 
> samples/kprobes/kretprobe_example.ko: Operation not permitted
> 
> And dmesg:
>   [48253.757629] register_kretprobe failed, returned -22
> 
> Signed-off-by: Naveen N. Rao <naveen.n....@linux.vnet.ibm.com>

Acked-by: Ananth N Mavinakayanahalli <ana...@linux.vnet.ibm.com>

Re: [PATCH] kretprobes: reject registration if a symbol offset is specified

[Ananth N Mavinakayanahalli]

On Tue, Feb 14, 2017 at 02:01:18PM +0530, Naveen N. Rao wrote:
> Users shouldn't be able to specify an offset with kretprobes, as we always
> want to probe at function entry. Otherwise, we won't be able to capture
> the proper return address resulting in the kretprobe never firing.
> 
> With samples/kprobes/kretprobe_example.c including an offset:
>   my_kretprobe.kp.offset = 40;
> 
> Before this patch, the probe gets planted but never fires.
> 
> After this patch:
>   $ sudo insmod samples/kprobes/kretprobe_example.ko
>   [sudo] password for naveen: 
>   insmod: ERROR: could not insert module 
> samples/kprobes/kretprobe_example.ko: Operation not permitted
> 
> And dmesg:
>   [48253.757629] register_kretprobe failed, returned -22
> 
> Signed-off-by: Naveen N. Rao 

Acked-by: Ananth N Mavinakayanahalli 

Re: [PATCH 1/2] perf: add container identifier entry in perf sample data

[Ananth N Mavinakayanahalli]

On Thu, Aug 25, 2016 at 10:50:18PM +0530, Hari Bathini wrote:
> 
> 
> On Thursday 25 August 2016 06:31 PM, Peter Zijlstra wrote:
> >On Thu, Aug 25, 2016 at 05:27:54PM +0530, Hari Bathini wrote:
> >
> >>diff --git a/include/uapi/linux/perf_event.h 
> >>b/include/uapi/linux/perf_event.h
> >>index c66a485..fb4f902 100644
> >>--- a/include/uapi/linux/perf_event.h
> >>+++ b/include/uapi/linux/perf_event.h
> >>@@ -139,8 +139,9 @@ enum perf_event_sample_format {
> >>PERF_SAMPLE_IDENTIFIER  = 1U << 16,
> >>PERF_SAMPLE_TRANSACTION = 1U << 17,
> >>PERF_SAMPLE_REGS_INTR   = 1U << 18,
> >>+   PERF_SAMPLE_CID = 1U << 19,
> >>-   PERF_SAMPLE_MAX = 1U << 19, /* non-ABI */
> >>+   PERF_SAMPLE_MAX = 1U << 20, /* non-ABI */
> >>  };
> >This forgets to update the comment that goes with PERF_RECORD_SAMPLE.
> >This patch would also need an update to the manpage:
> >
> >   
> > http://git.kernel.org/cgit/docs/man-pages/man-pages.git/tree/man2/perf_event_open.2
> >
> >   http://www.man7.org/linux/man-pages/man2/perf_event_open.2.html
> >
> >>+   if (sample_type & PERF_SAMPLE_CID) {
> >>+   int size = sizeof(u64);
> >>+
> >>+   /*
> >>+* Container identifier for a given task.
> >>+* Using cgroup namespace inode number for this.
> >>+*/
> >>+   data->cid_entry.cid = current->nsproxy->cgroup_ns->ns.inum;
> >>+   data->cid_entry.reserved = 0;
> >>+   header->size += size;
> >>+   }
> >>  }
> >Does this compile with CONFIG_CGROUP=n ?
> >
> 
> My bad. Will update..
> Actually, on second thought, how about using inode number of some
> other namespace that any container would have (mount, probably?)..

I am not sure about every implementation of 'containers' but I would
think that the cgroup namespace is the right choice here. Mount
namespaces can be potentially shared between containers AFAIK.

Ananth

Re: [PATCH 1/2] perf: add container identifier entry in perf sample data

[Ananth N Mavinakayanahalli]

On Thu, Aug 25, 2016 at 10:50:18PM +0530, Hari Bathini wrote:
> 
> 
> On Thursday 25 August 2016 06:31 PM, Peter Zijlstra wrote:
> >On Thu, Aug 25, 2016 at 05:27:54PM +0530, Hari Bathini wrote:
> >
> >>diff --git a/include/uapi/linux/perf_event.h 
> >>b/include/uapi/linux/perf_event.h
> >>index c66a485..fb4f902 100644
> >>--- a/include/uapi/linux/perf_event.h
> >>+++ b/include/uapi/linux/perf_event.h
> >>@@ -139,8 +139,9 @@ enum perf_event_sample_format {
> >>PERF_SAMPLE_IDENTIFIER  = 1U << 16,
> >>PERF_SAMPLE_TRANSACTION = 1U << 17,
> >>PERF_SAMPLE_REGS_INTR   = 1U << 18,
> >>+   PERF_SAMPLE_CID = 1U << 19,
> >>-   PERF_SAMPLE_MAX = 1U << 19, /* non-ABI */
> >>+   PERF_SAMPLE_MAX = 1U << 20, /* non-ABI */
> >>  };
> >This forgets to update the comment that goes with PERF_RECORD_SAMPLE.
> >This patch would also need an update to the manpage:
> >
> >   
> > http://git.kernel.org/cgit/docs/man-pages/man-pages.git/tree/man2/perf_event_open.2
> >
> >   http://www.man7.org/linux/man-pages/man2/perf_event_open.2.html
> >
> >>+   if (sample_type & PERF_SAMPLE_CID) {
> >>+   int size = sizeof(u64);
> >>+
> >>+   /*
> >>+* Container identifier for a given task.
> >>+* Using cgroup namespace inode number for this.
> >>+*/
> >>+   data->cid_entry.cid = current->nsproxy->cgroup_ns->ns.inum;
> >>+   data->cid_entry.reserved = 0;
> >>+   header->size += size;
> >>+   }
> >>  }
> >Does this compile with CONFIG_CGROUP=n ?
> >
> 
> My bad. Will update..
> Actually, on second thought, how about using inode number of some
> other namespace that any container would have (mount, probably?)..

I am not sure about every implementation of 'containers' but I would
think that the cgroup namespace is the right choice here. Mount
namespaces can be potentially shared between containers AFAIK.

Ananth

Re: [PATCH tip/master] [BUGFIX] kprobes/x86: Fix to clear TF bit in fault-on-single-stepping

[Ananth N Mavinakayanahalli]

On Sat, Jun 11, 2016 at 11:06:53PM +0900, Masami Hiramatsu wrote:
> Fix kprobe_fault_handler to clear TF (trap flag) bit of
> flags register in the case of fault fixup on single-stepping.
> 
> If we put a kprobe on the instruction which can cause a
> page fault (e.g. actual mov instructions in copy_user_*),
> that fault happens on a single-stepping buffer. In this
> case, kprobes resets running instance so that the CPU can
> retry execution on the original ip address.
> However, current code forgets reset TF bit. Since this
> fault happens with TF bit set for enabling single-stepping,
> when it retries, it causes a debug exception and kprobes
> can not handle it because it already reset itself.
> 
> On the most of x86-64 platform, it can be easily reproduced
> by using kprobe tracer. E.g.
> 
>   # cd /sys/kernel/debug/tracing
>   # echo p copy_user_enhanced_fast_string+5 > kprobe_events
>   # echo 1 > events/kprobes/enable
> 
> And you'll see a kernel panic on do_debug(), since the debug
> trap is not handled by kprobes.
> 
> To fix this problem, we just need to clear the TF bit when
> resetting running kprobe.
> 
> Signed-off-by: Masami Hiramatsu <mhira...@kernel.org>

Good catch!

Reviewed-by: Ananth N Mavinakayanahalli <ana...@linux.vnet.ibm.com>

Re: [PATCH tip/master] [BUGFIX] kprobes/x86: Fix to clear TF bit in fault-on-single-stepping

[Ananth N Mavinakayanahalli]

On Sat, Jun 11, 2016 at 11:06:53PM +0900, Masami Hiramatsu wrote:
> Fix kprobe_fault_handler to clear TF (trap flag) bit of
> flags register in the case of fault fixup on single-stepping.
> 
> If we put a kprobe on the instruction which can cause a
> page fault (e.g. actual mov instructions in copy_user_*),
> that fault happens on a single-stepping buffer. In this
> case, kprobes resets running instance so that the CPU can
> retry execution on the original ip address.
> However, current code forgets reset TF bit. Since this
> fault happens with TF bit set for enabling single-stepping,
> when it retries, it causes a debug exception and kprobes
> can not handle it because it already reset itself.
> 
> On the most of x86-64 platform, it can be easily reproduced
> by using kprobe tracer. E.g.
> 
>   # cd /sys/kernel/debug/tracing
>   # echo p copy_user_enhanced_fast_string+5 > kprobe_events
>   # echo 1 > events/kprobes/enable
> 
> And you'll see a kernel panic on do_debug(), since the debug
> trap is not handled by kprobes.
> 
> To fix this problem, we just need to clear the TF bit when
> resetting running kprobe.
> 
> Signed-off-by: Masami Hiramatsu 

Good catch!

Reviewed-by: Ananth N Mavinakayanahalli 

[MAINTAINERS] Update email address

[Ananth N Mavinakayanahalli]

The current ID is going away soon... update email address

Signed-off-by: Ananth N Mavinakayanahalli <ana...@linux.vnet.ibm.com>

diff --git a/MAINTAINERS b/MAINTAINERS
index 1d5b4be..dc23998 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -6400,7 +6400,7 @@ F:mm/kmemleak.c
 F: mm/kmemleak-test.c
 
 KPROBES
-M: Ananth N Mavinakayanahalli <ana...@in.ibm.com>
+M:     Ananth N Mavinakayanahalli <ana...@linux.vnet.ibm.com>
 M: Anil S Keshavamurthy <anil.s.keshavamur...@intel.com>
 M: "David S. Miller" <da...@davemloft.net>
 M: Masami Hiramatsu <mhira...@kernel.org>

[MAINTAINERS] Update email address

[Ananth N Mavinakayanahalli]

The current ID is going away soon... update email address

Signed-off-by: Ananth N Mavinakayanahalli 

diff --git a/MAINTAINERS b/MAINTAINERS
index 1d5b4be..dc23998 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -6400,7 +6400,7 @@ F:mm/kmemleak.c
 F: mm/kmemleak-test.c
 
 KPROBES
-M: Ananth N Mavinakayanahalli 
+M: Ananth N Mavinakayanahalli 
 M: Anil S Keshavamurthy 
 M: "David S. Miller" 
 M: Masami Hiramatsu 

Re: [PATCH 1/2] perf/powerpc: Fix kprobe and kretprobe handling with kallsyms

[Ananth N Mavinakayanahalli]

On Wed, Apr 06, 2016 at 06:02:57PM +0530, Naveen N. Rao wrote:

> + if (!pev->uprobes && map->dso->symtab_type == DSO_BINARY_TYPE__KALLSYMS)
>   tev->point.offset += PPC64LE_LEP_OFFSET;

uprobes check against kallsysms? Am I missing something here?

Ananth

Re: [PATCH 1/2] perf/powerpc: Fix kprobe and kretprobe handling with kallsyms

[Ananth N Mavinakayanahalli]

On Wed, Apr 06, 2016 at 06:02:57PM +0530, Naveen N. Rao wrote:

> + if (!pev->uprobes && map->dso->symtab_type == DSO_BINARY_TYPE__KALLSYMS)
>   tev->point.offset += PPC64LE_LEP_OFFSET;

uprobes check against kallsysms? Am I missing something here?

Ananth

Re: [PATCH 2/2] tools/perf: Fix kallsyms perf test on ppc64le

[Ananth N Mavinakayanahalli]

On Wed, Apr 06, 2016 at 06:02:58PM +0530, Naveen N. Rao wrote:
> ppc64le functions have a Global Entry Point (GEP) and a Local Entry
> Point (LEP). While placing a probe, we always prefer the LEP since it
> catches function calls through both the GEP and the LEP. In order to do
> this, we fixup the function entry points during elf symbol table lookup
> to point to the LEPs. This works, but breaks 'perf test kallsyms' since
> the symbols loaded from the symbol table (pointing to the LEP) do not
> match the symbols in kallsyms.
> 
> To fix this, we do not adjust all the symbols during symbol table load,
> but only adjust the probe trace point.
> 
> Cc: Mark Wielaard <m...@redhat.com>
> Cc: Thiago Jung Bauermann <bauer...@linux.vnet.ibm.com>
> Cc: Ananth N Mavinakayanahalli <ana...@in.ibm.com>
> Cc: Arnaldo Carvalho de Melo <a...@redhat.com>
> Cc: Masami Hiramatsu <masami.hiramatsu...@hitachi.com>
> Reported-by: Michael Ellerman <m...@ellerman.id.au>
> Signed-off-by: Naveen N. Rao <naveen.n@linux.vnet.ibm.com>

Acked-by: Ananth N Mavinakayanahalli <ana...@in.ibm.com>

Re: [PATCH 2/2] tools/perf: Fix kallsyms perf test on ppc64le

[Ananth N Mavinakayanahalli]

On Wed, Apr 06, 2016 at 06:02:58PM +0530, Naveen N. Rao wrote:
> ppc64le functions have a Global Entry Point (GEP) and a Local Entry
> Point (LEP). While placing a probe, we always prefer the LEP since it
> catches function calls through both the GEP and the LEP. In order to do
> this, we fixup the function entry points during elf symbol table lookup
> to point to the LEPs. This works, but breaks 'perf test kallsyms' since
> the symbols loaded from the symbol table (pointing to the LEP) do not
> match the symbols in kallsyms.
> 
> To fix this, we do not adjust all the symbols during symbol table load,
> but only adjust the probe trace point.
> 
> Cc: Mark Wielaard 
> Cc: Thiago Jung Bauermann 
> Cc: Ananth N Mavinakayanahalli 
> Cc: Arnaldo Carvalho de Melo 
> Cc: Masami Hiramatsu 
> Reported-by: Michael Ellerman 
> Signed-off-by: Naveen N. Rao 

Acked-by: Ananth N Mavinakayanahalli 

[PATCH V3 2/2] kprobes: Mark OPTPROBES na for powerpc

[Ananth N Mavinakayanahalli]

Kprobes uses a breakpoint instruction to trap into execution flow
and the probed instruction is single-stepped from an alternate location.

On some architectures like x86, under certain conditions, the OPTPROBES
feature enables replacing the probed instruction with a jump instead,
resulting in a significant perfomance boost (both the breakpoint and
single-step exception is bypassed for each kprobe).

Powerpc has an in-kernel instruction emulator. Kprobes on powerpc uses
this emulator already and bypasses the single-step exception, with a
lot less complexity. There is a potential gain to be had with a direct
jump instead of a breakpoint, but the caveats need to be traded off
with the complexity it brings in.

For now, mark OPTPROBES na for powerpc.

Signed-off-by: Ananth N Mavinakayanahalli 
---
 .../features/debug/optprobes/arch-support.txt  |2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/Documentation/features/debug/optprobes/arch-support.txt 
b/Documentation/features/debug/optprobes/arch-support.txt
index b8999d8..73662f9 100644
--- a/Documentation/features/debug/optprobes/arch-support.txt
+++ b/Documentation/features/debug/optprobes/arch-support.txt
@@ -27,7 +27,7 @@
 |   nios2: | TODO |
 |openrisc: | TODO |
 |  parisc: | TODO |
-| powerpc: | TODO |
+| powerpc: |  na  |
 |s390: | TODO |
 |   score: | TODO |
 |  sh: | TODO |

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

[PATCH V3 1/2] Documentation/features: Add na key to arch-support.txt

[Ananth N Mavinakayanahalli]

To be used for features we will not support on a particular architecture.
The git log that adds this needs to provide the justification 'why?'

Signed-off-by: Ananth N Mavinakayanahalli 
---
 Documentation/features/arch-support.txt |1 +
 1 file changed, 1 insertion(+)

diff --git a/Documentation/features/arch-support.txt 
b/Documentation/features/arch-support.txt
index d22a109..c0bc92b 100644
--- a/Documentation/features/arch-support.txt
+++ b/Documentation/features/arch-support.txt
@@ -8,4 +8,5 @@ The meaning of entries in the tables is:
 | ok |  # feature supported by the architecture
 |TODO|  # feature not yet supported by the architecture
 | .. |  # feature cannot be supported by the hardware
+| na |  # feature is not needed by the architecture
 

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

[PATCH V3 2/2] kprobes: Mark OPTPROBES na for powerpc

[Ananth N Mavinakayanahalli]

Kprobes uses a breakpoint instruction to trap into execution flow
and the probed instruction is single-stepped from an alternate location.

On some architectures like x86, under certain conditions, the OPTPROBES
feature enables replacing the probed instruction with a jump instead,
resulting in a significant perfomance boost (both the breakpoint and
single-step exception is bypassed for each kprobe).

Powerpc has an in-kernel instruction emulator. Kprobes on powerpc uses
this emulator already and bypasses the single-step exception, with a
lot less complexity. There is a potential gain to be had with a direct
jump instead of a breakpoint, but the caveats need to be traded off
with the complexity it brings in.

For now, mark OPTPROBES na for powerpc.

Signed-off-by: Ananth N Mavinakayanahalli ana...@in.ibm.com
---
 .../features/debug/optprobes/arch-support.txt  |2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/Documentation/features/debug/optprobes/arch-support.txt 
b/Documentation/features/debug/optprobes/arch-support.txt
index b8999d8..73662f9 100644
--- a/Documentation/features/debug/optprobes/arch-support.txt
+++ b/Documentation/features/debug/optprobes/arch-support.txt
@@ -27,7 +27,7 @@
 |   nios2: | TODO |
 |openrisc: | TODO |
 |  parisc: | TODO |
-| powerpc: | TODO |
+| powerpc: |  na  |
 |s390: | TODO |
 |   score: | TODO |
 |  sh: | TODO |

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

[PATCH V3 1/2] Documentation/features: Add na key to arch-support.txt

[Ananth N Mavinakayanahalli]

To be used for features we will not support on a particular architecture.
The git log that adds this needs to provide the justification 'why?'

Signed-off-by: Ananth N Mavinakayanahalli ana...@in.ibm.com
---
 Documentation/features/arch-support.txt |1 +
 1 file changed, 1 insertion(+)

diff --git a/Documentation/features/arch-support.txt 
b/Documentation/features/arch-support.txt
index d22a109..c0bc92b 100644
--- a/Documentation/features/arch-support.txt
+++ b/Documentation/features/arch-support.txt
@@ -8,4 +8,5 @@ The meaning of entries in the tables is:
 | ok |  # feature supported by the architecture
 |TODO|  # feature not yet supported by the architecture
 | .. |  # feature cannot be supported by the hardware
+| na |  # feature is not needed by the architecture
 

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

[PATCH V2 2/2] kprobes: Mark OPTPROBES na for powerpc

[Ananth N Mavinakayanahalli]

Kprobes uses a breakpoint instruction to trap into execution flow
and the probed instruction is single-stepped from an alternate location.

On some architectures like x86, under certain conditions, the OPTPROBES
feature enables replacing the probed instruction with a jump instead,
resulting in a significant perfomance boost (one single-step exception
is bypassed for each kprobe).

Powerpc has an in-kernel instruction emulator. Kprobes on powerpc uses
this emulator already and bypasses the single-step exception, with a
lot less complexity.

Hence, mark OPTPROBES na for powerpc.

Signed-off-by: Ananth N Mavinakayanahalli 
---
 .../features/debug/optprobes/arch-support.txt  |2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/Documentation/features/debug/optprobes/arch-support.txt 
b/Documentation/features/debug/optprobes/arch-support.txt
index b8999d8..73662f9 100644
--- a/Documentation/features/debug/optprobes/arch-support.txt
+++ b/Documentation/features/debug/optprobes/arch-support.txt
@@ -27,7 +27,7 @@
 |   nios2: | TODO |
 |openrisc: | TODO |
 |  parisc: | TODO |
-| powerpc: | TODO |
+| powerpc: |  na  |
 |s390: | TODO |
 |   score: | TODO |
 |  sh: | TODO |

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

[PATCH V2 1/2] Documentation/features: Add na key to arch-support.txt

[Ananth N Mavinakayanahalli]

To be used for features we will not support on a particular architecture.
The git log that adds this needs to provide the justification 'why?'

Signed-off-by: Ananth N Mavinakayanahalli 
---
 Documentation/features/arch-support.txt |1 +
 1 file changed, 1 insertion(+)

diff --git a/Documentation/features/arch-support.txt 
b/Documentation/features/arch-support.txt
index d22a109..f2b272e 100644
--- a/Documentation/features/arch-support.txt
+++ b/Documentation/features/arch-support.txt
@@ -8,4 +8,5 @@ The meaning of entries in the tables is:
 | ok |  # feature supported by the architecture
 |TODO|  # feature not yet supported by the architecture
 | .. |  # feature cannot be supported by the hardware
+| na |  # feature will not be supported by the architecture
 

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

[PATCH V2 1/2] Documentation/features: Add na key to arch-support.txt

[Ananth N Mavinakayanahalli]

To be used for features we will not support on a particular architecture.
The git log that adds this needs to provide the justification 'why?'

Signed-off-by: Ananth N Mavinakayanahalli ana...@in.ibm.com
---
 Documentation/features/arch-support.txt |1 +
 1 file changed, 1 insertion(+)

diff --git a/Documentation/features/arch-support.txt 
b/Documentation/features/arch-support.txt
index d22a109..f2b272e 100644
--- a/Documentation/features/arch-support.txt
+++ b/Documentation/features/arch-support.txt
@@ -8,4 +8,5 @@ The meaning of entries in the tables is:
 | ok |  # feature supported by the architecture
 |TODO|  # feature not yet supported by the architecture
 | .. |  # feature cannot be supported by the hardware
+| na |  # feature will not be supported by the architecture
 

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

[PATCH V2 2/2] kprobes: Mark OPTPROBES na for powerpc

[Ananth N Mavinakayanahalli]

Kprobes uses a breakpoint instruction to trap into execution flow
and the probed instruction is single-stepped from an alternate location.

On some architectures like x86, under certain conditions, the OPTPROBES
feature enables replacing the probed instruction with a jump instead,
resulting in a significant perfomance boost (one single-step exception
is bypassed for each kprobe).

Powerpc has an in-kernel instruction emulator. Kprobes on powerpc uses
this emulator already and bypasses the single-step exception, with a
lot less complexity.

Hence, mark OPTPROBES na for powerpc.

Signed-off-by: Ananth N Mavinakayanahalli ana...@in.ibm.com
---
 .../features/debug/optprobes/arch-support.txt  |2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/Documentation/features/debug/optprobes/arch-support.txt 
b/Documentation/features/debug/optprobes/arch-support.txt
index b8999d8..73662f9 100644
--- a/Documentation/features/debug/optprobes/arch-support.txt
+++ b/Documentation/features/debug/optprobes/arch-support.txt
@@ -27,7 +27,7 @@
 |   nios2: | TODO |
 |openrisc: | TODO |
 |  parisc: | TODO |
-| powerpc: | TODO |
+| powerpc: |  na  |
 |s390: | TODO |
 |   score: | TODO |
 |  sh: | TODO |

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

Re: Re: [PATCH v6 0/6] arm64: Add kernel probes (kprobes) support

[Ananth N Mavinakayanahalli]

On Thu, May 14, 2015 at 09:01:03AM +0900, Masami Hiramatsu wrote:
> On 2015/05/14 0:41, William Cohen wrote:
> > On 05/13/2015 05:22 AM, Masami Hiramatsu wrote:
> >> On 2015/05/12 21:48, William Cohen wrote:
> > 
> >>> Hi Dave,
> >>>
> >>> In some of the previous diagnostic output it looked like things would go 
> >>> wrong
> >>> in the entry.S when the D bit was cleared and the debug interrupts were 
> >>> unmasksed.  I wonder if some of the issue might be due to the starting 
> >>> the 
> >>> kprobe for the trampoline, but leaving things in an odd state when another
> >>> set of krpobe/kretporbes are hit when the trampoline is running.
> >>
> >> Hmm, does this mean we have a trouble if a user kprobe handler calls the
> >> function which is probed by other kprobe? Or, is this just a problem
> >> only for kretprobes?
> > 
> > Hi Masami,
> > 
> > I wrote an example based off of sample/kprobes/kprobes_sample.c to force 
> > the reentry issue for kprobes (the attached kprobe_rentry_example.c). That
> > seemed to run fine.  I think the reason that the trampoline handler got 
> > into trouble is because of the reset_current_kprobe() before the possible
> > call to kfree, but I haven't verified it.
> 
> I still doubt about kfree() reentrant call, since kretprobe handler only
> calls recycle_rp_inst() which doesn't free the instance but just push it back
> to the unused instance list.
> 
> > It seems like that should be at the end of trampoline handler just before
> > the return.  Other architectures have similar trampoline handlers,
> > so I am surprised that the other architectures haven't encountered this
> > issue with kretprobes.  Maybe this is due to specific of arm64 exception
> > handling.
> 
> Ah, indeed the reset_current_kprobe() here seems not good since some
> interruption or some other reason, another kprobes can be hit before
> returning.
> 
> If kprobes can handle reentered probes correctly, I think your patch
> (directly branch from trampoline) looks good to fix this problem.
> Actually, arm32 and x86 already has same method.
> 
> It seems that powerpc will have similar issue, does someone checked
> that? Ananth?

Yes, there is a window where this can happen on powerpc. I haven't
however been able to trigger it thus far -- will try with a different
sample and test.

Thanks for the heads-up.

Ananth

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

Re: Re: [PATCH v6 0/6] arm64: Add kernel probes (kprobes) support

[Ananth N Mavinakayanahalli]

On Thu, May 14, 2015 at 09:01:03AM +0900, Masami Hiramatsu wrote:
 On 2015/05/14 0:41, William Cohen wrote:
  On 05/13/2015 05:22 AM, Masami Hiramatsu wrote:
  On 2015/05/12 21:48, William Cohen wrote:
  
  Hi Dave,
 
  In some of the previous diagnostic output it looked like things would go 
  wrong
  in the entry.S when the D bit was cleared and the debug interrupts were 
  unmasksed.  I wonder if some of the issue might be due to the starting 
  the 
  kprobe for the trampoline, but leaving things in an odd state when another
  set of krpobe/kretporbes are hit when the trampoline is running.
 
  Hmm, does this mean we have a trouble if a user kprobe handler calls the
  function which is probed by other kprobe? Or, is this just a problem
  only for kretprobes?
  
  Hi Masami,
  
  I wrote an example based off of sample/kprobes/kprobes_sample.c to force 
  the reentry issue for kprobes (the attached kprobe_rentry_example.c). That
  seemed to run fine.  I think the reason that the trampoline handler got 
  into trouble is because of the reset_current_kprobe() before the possible
  call to kfree, but I haven't verified it.
 
 I still doubt about kfree() reentrant call, since kretprobe handler only
 calls recycle_rp_inst() which doesn't free the instance but just push it back
 to the unused instance list.
 
  It seems like that should be at the end of trampoline handler just before
  the return.  Other architectures have similar trampoline handlers,
  so I am surprised that the other architectures haven't encountered this
  issue with kretprobes.  Maybe this is due to specific of arm64 exception
  handling.
 
 Ah, indeed the reset_current_kprobe() here seems not good since some
 interruption or some other reason, another kprobes can be hit before
 returning.
 
 If kprobes can handle reentered probes correctly, I think your patch
 (directly branch from trampoline) looks good to fix this problem.
 Actually, arm32 and x86 already has same method.
 
 It seems that powerpc will have similar issue, does someone checked
 that? Ananth?

Yes, there is a window where this can happen on powerpc. I haven't
however been able to trigger it thus far -- will try with a different
sample and test.

Thanks for the heads-up.

Ananth

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

[tip:perf/core] perf probe ppc64le: Fix ppc64 ABIv2 symbol decoding

[tip-bot for Ananth N Mavinakayanahalli]

Commit-ID:  c50fc0a43e33a6c3257c5cbb954cd747d7b9a680
Gitweb: http://git.kernel.org/tip/c50fc0a43e33a6c3257c5cbb954cd747d7b9a680
Author: Ananth N Mavinakayanahalli 
AuthorDate: Tue, 28 Apr 2015 17:35:38 +0530
Committer:  Arnaldo Carvalho de Melo 
CommitDate: Mon, 4 May 2015 12:43:45 -0300

perf probe ppc64le: Fix ppc64 ABIv2 symbol decoding

ppc64 ELF ABIv2 has a Global Entry Point (GEP) and a Local Entry Point
(LEP). For purposes of probing, we need the LEP - the offset to which is
encoded in st_other.

Signed-off-by: Ananth N Mavinakayanahalli 
Reviewed-by: Srikar Dronamraju 
Cc: Masami Hiramatsu 
Cc: Michael Ellerman 
Cc: Sukadev Bhattiprolu 
Cc: linuxppc-...@lists.ozlabs.org
Link: 
http://lkml.kernel.org/r/ab9cc5e2b9de4cbaaf50f6ef2346a6a81100bad1.1430217967.git.naveen.n@linux.vnet.ibm.com
Signed-off-by: Naveen N. Rao 
Signed-off-by: Arnaldo Carvalho de Melo 
---
 tools/perf/arch/powerpc/util/sym-handling.c | 7 +++
 tools/perf/util/symbol-elf.c| 4 
 tools/perf/util/symbol.h| 1 +
 3 files changed, 12 insertions(+)

diff --git a/tools/perf/arch/powerpc/util/sym-handling.c 
b/tools/perf/arch/powerpc/util/sym-handling.c
index 2de2cc4..012a0f8 100644
--- a/tools/perf/arch/powerpc/util/sym-handling.c
+++ b/tools/perf/arch/powerpc/util/sym-handling.c
@@ -17,6 +17,13 @@ bool elf__needs_adjust_symbols(GElf_Ehdr ehdr)
   ehdr.e_type == ET_REL ||
   ehdr.e_type == ET_DYN;
 }
+
+#if defined(_CALL_ELF) && _CALL_ELF == 2
+void arch__elf_sym_adjust(GElf_Sym *sym)
+{
+   sym->st_value += PPC64_LOCAL_ENTRY_OFFSET(sym->st_other);
+}
+#endif
 #endif
 
 #if !defined(_CALL_ELF) || _CALL_ELF != 2
diff --git a/tools/perf/util/symbol-elf.c b/tools/perf/util/symbol-elf.c
index 54347ba..d99b442 100644
--- a/tools/perf/util/symbol-elf.c
+++ b/tools/perf/util/symbol-elf.c
@@ -775,6 +775,8 @@ static bool want_demangle(bool is_kernel_sym)
return is_kernel_sym ? symbol_conf.demangle_kernel : 
symbol_conf.demangle;
 }
 
+void __weak arch__elf_sym_adjust(GElf_Sym *sym __maybe_unused) { }
+
 int dso__load_sym(struct dso *dso, struct map *map,
  struct symsrc *syms_ss, struct symsrc *runtime_ss,
  symbol_filter_t filter, int kmodule)
@@ -939,6 +941,8 @@ int dso__load_sym(struct dso *dso, struct map *map,
(sym.st_value & 1))
--sym.st_value;
 
+   arch__elf_sym_adjust();
+
if (dso->kernel || kmodule) {
char dso_name[PATH_MAX];
 
diff --git a/tools/perf/util/symbol.h b/tools/perf/util/symbol.h
index bd50ba0..9096529 100644
--- a/tools/perf/util/symbol.h
+++ b/tools/perf/util/symbol.h
@@ -305,6 +305,7 @@ int setup_intlist(struct intlist **list, const char 
*list_str,
 
 #ifdef HAVE_LIBELF_SUPPORT
 bool elf__needs_adjust_symbols(GElf_Ehdr ehdr);
+void arch__elf_sym_adjust(GElf_Sym *sym);
 #endif
 
 #define SYMBOL_A 0
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[tip:perf/core] perf probe ppc64le: Fix ppc64 ABIv2 symbol decoding

[tip-bot for Ananth N Mavinakayanahalli]

Commit-ID:  c50fc0a43e33a6c3257c5cbb954cd747d7b9a680
Gitweb: http://git.kernel.org/tip/c50fc0a43e33a6c3257c5cbb954cd747d7b9a680
Author: Ananth N Mavinakayanahalli ana...@in.ibm.com
AuthorDate: Tue, 28 Apr 2015 17:35:38 +0530
Committer:  Arnaldo Carvalho de Melo a...@redhat.com
CommitDate: Mon, 4 May 2015 12:43:45 -0300

perf probe ppc64le: Fix ppc64 ABIv2 symbol decoding

ppc64 ELF ABIv2 has a Global Entry Point (GEP) and a Local Entry Point
(LEP). For purposes of probing, we need the LEP - the offset to which is
encoded in st_other.

Signed-off-by: Ananth N Mavinakayanahalli ana...@in.ibm.com
Reviewed-by: Srikar Dronamraju sri...@linux.vnet.ibm.com
Cc: Masami Hiramatsu masami.hiramatsu...@hitachi.com
Cc: Michael Ellerman m...@ellerman.id.au
Cc: Sukadev Bhattiprolu suka...@linux.vnet.ibm.com
Cc: linuxppc-...@lists.ozlabs.org
Link: 
http://lkml.kernel.org/r/ab9cc5e2b9de4cbaaf50f6ef2346a6a81100bad1.1430217967.git.naveen.n@linux.vnet.ibm.com
Signed-off-by: Naveen N. Rao naveen.n@linux.vnet.ibm.com
Signed-off-by: Arnaldo Carvalho de Melo a...@redhat.com
---
 tools/perf/arch/powerpc/util/sym-handling.c | 7 +++
 tools/perf/util/symbol-elf.c| 4 
 tools/perf/util/symbol.h| 1 +
 3 files changed, 12 insertions(+)

diff --git a/tools/perf/arch/powerpc/util/sym-handling.c 
b/tools/perf/arch/powerpc/util/sym-handling.c
index 2de2cc4..012a0f8 100644
--- a/tools/perf/arch/powerpc/util/sym-handling.c
+++ b/tools/perf/arch/powerpc/util/sym-handling.c
@@ -17,6 +17,13 @@ bool elf__needs_adjust_symbols(GElf_Ehdr ehdr)
   ehdr.e_type == ET_REL ||
   ehdr.e_type == ET_DYN;
 }
+
+#if defined(_CALL_ELF)  _CALL_ELF == 2
+void arch__elf_sym_adjust(GElf_Sym *sym)
+{
+   sym-st_value += PPC64_LOCAL_ENTRY_OFFSET(sym-st_other);
+}
+#endif
 #endif
 
 #if !defined(_CALL_ELF) || _CALL_ELF != 2
diff --git a/tools/perf/util/symbol-elf.c b/tools/perf/util/symbol-elf.c
index 54347ba..d99b442 100644
--- a/tools/perf/util/symbol-elf.c
+++ b/tools/perf/util/symbol-elf.c
@@ -775,6 +775,8 @@ static bool want_demangle(bool is_kernel_sym)
return is_kernel_sym ? symbol_conf.demangle_kernel : 
symbol_conf.demangle;
 }
 
+void __weak arch__elf_sym_adjust(GElf_Sym *sym __maybe_unused) { }
+
 int dso__load_sym(struct dso *dso, struct map *map,
  struct symsrc *syms_ss, struct symsrc *runtime_ss,
  symbol_filter_t filter, int kmodule)
@@ -939,6 +941,8 @@ int dso__load_sym(struct dso *dso, struct map *map,
(sym.st_value  1))
--sym.st_value;
 
+   arch__elf_sym_adjust(sym);
+
if (dso-kernel || kmodule) {
char dso_name[PATH_MAX];
 
diff --git a/tools/perf/util/symbol.h b/tools/perf/util/symbol.h
index bd50ba0..9096529 100644
--- a/tools/perf/util/symbol.h
+++ b/tools/perf/util/symbol.h
@@ -305,6 +305,7 @@ int setup_intlist(struct intlist **list, const char 
*list_str,
 
 #ifdef HAVE_LIBELF_SUPPORT
 bool elf__needs_adjust_symbols(GElf_Ehdr ehdr);
+void arch__elf_sym_adjust(GElf_Sym *sym);
 #endif
 
 #define SYMBOL_A 0
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCHv2 2/8] perf probe: Improve detection of file/function name in the probe pattern

[Ananth N Mavinakayanahalli]

On Thu, Mar 12, 2015 at 05:24:14PM -0300, Arnaldo Carvalho de Melo wrote:
> Em Mon, Dec 15, 2014 at 08:20:32PM +0530, Naveen N. Rao escreveu:
> > Currently, perf probe considers patterns including a '.' to be a file.
> > However, this causes problems on powerpc ABIv1 where all functions have
> > a leading '.':
> > 
> >   $ perf probe -F | grep schedule_timeout_interruptible
> >   .schedule_timeout_interruptible
> >   $ perf probe .schedule_timeout_interruptible
> >   Semantic error :File always requires line number or lazy pattern.
> > Error: Command Parse Error.
> > 
> > Fix this by checking the probe pattern in more detail.
> 
> Masami, can I have your Acked-by or Reviewed-by?

Arnaldo,

FWIW, I have reviewed this code...

Reviewed-by: Ananth N Mavinakayanahalli 

> 
> 
> - Arnaldo
> 
> > Signed-off-by: Naveen N. Rao 
> > ---
> >  tools/perf/util/probe-event.c | 23 ---
> >  1 file changed, 20 insertions(+), 3 deletions(-)
> > 
> > diff --git a/tools/perf/util/probe-event.c b/tools/perf/util/probe-event.c
> > index 28eb141..9943ff3 100644
> > --- a/tools/perf/util/probe-event.c
> > +++ b/tools/perf/util/probe-event.c
> > @@ -999,6 +999,24 @@ static int parse_perf_probe_point(char *arg, struct 
> > perf_probe_event *pev)
> > arg = tmp;
> > }
> >  
> > +   /*
> > +* Check arg is function or file name and copy it.
> > +*
> > +* We consider arg to be a file spec if and only if it satisfies
> > +* all of the below criteria::
> > +* - it does not include any of "+@%",
> > +* - it includes one of ":;", and
> > +* - it has a period '.' in the name.
> > +*
> > +* Otherwise, we consider arg to be a function specification.
> > +*/
> > +   c = 0;
> > +   if (!strpbrk(arg, "+@%") && (ptr = strpbrk(arg, ";:")) != NULL) {
> > +   /* This is a file spec if it includes a '.' before ; or : */
> > +   if (memchr(arg, '.', ptr-arg))
> > +   c = 1;
> > +   }
> > +
> > ptr = strpbrk(arg, ";:+@%");
> > if (ptr) {
> > nc = *ptr;
> > @@ -1009,10 +1027,9 @@ static int parse_perf_probe_point(char *arg, struct 
> > perf_probe_event *pev)
> > if (tmp == NULL)
> > return -ENOMEM;
> >  
> > -   /* Check arg is function or file and copy it */
> > -   if (strchr(tmp, '.'))   /* File */
> > +   if (c == 1)
> > pp->file = tmp;
> > -   else/* Function */
> > +   else
> > pp->function = tmp;
> >  
> > /* Parse other options */
> > -- 
> > 2.1.3

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

Re: [PATCHv2 2/8] perf probe: Improve detection of file/function name in the probe pattern

[Ananth N Mavinakayanahalli]

On Thu, Mar 12, 2015 at 05:24:14PM -0300, Arnaldo Carvalho de Melo wrote:
 Em Mon, Dec 15, 2014 at 08:20:32PM +0530, Naveen N. Rao escreveu:
  Currently, perf probe considers patterns including a '.' to be a file.
  However, this causes problems on powerpc ABIv1 where all functions have
  a leading '.':
  
$ perf probe -F | grep schedule_timeout_interruptible
.schedule_timeout_interruptible
$ perf probe .schedule_timeout_interruptible
Semantic error :File always requires line number or lazy pattern.
  Error: Command Parse Error.
  
  Fix this by checking the probe pattern in more detail.
 
 Masami, can I have your Acked-by or Reviewed-by?

Arnaldo,

FWIW, I have reviewed this code...

Reviewed-by: Ananth N Mavinakayanahalli ana...@in.ibm.com

 
 
 - Arnaldo
 
  Signed-off-by: Naveen N. Rao naveen.n@linux.vnet.ibm.com
  ---
   tools/perf/util/probe-event.c | 23 ---
   1 file changed, 20 insertions(+), 3 deletions(-)
  
  diff --git a/tools/perf/util/probe-event.c b/tools/perf/util/probe-event.c
  index 28eb141..9943ff3 100644
  --- a/tools/perf/util/probe-event.c
  +++ b/tools/perf/util/probe-event.c
  @@ -999,6 +999,24 @@ static int parse_perf_probe_point(char *arg, struct 
  perf_probe_event *pev)
  arg = tmp;
  }
   
  +   /*
  +* Check arg is function or file name and copy it.
  +*
  +* We consider arg to be a file spec if and only if it satisfies
  +* all of the below criteria::
  +* - it does not include any of +@%,
  +* - it includes one of :;, and
  +* - it has a period '.' in the name.
  +*
  +* Otherwise, we consider arg to be a function specification.
  +*/
  +   c = 0;
  +   if (!strpbrk(arg, +@%)  (ptr = strpbrk(arg, ;:)) != NULL) {
  +   /* This is a file spec if it includes a '.' before ; or : */
  +   if (memchr(arg, '.', ptr-arg))
  +   c = 1;
  +   }
  +
  ptr = strpbrk(arg, ;:+@%);
  if (ptr) {
  nc = *ptr;
  @@ -1009,10 +1027,9 @@ static int parse_perf_probe_point(char *arg, struct 
  perf_probe_event *pev)
  if (tmp == NULL)
  return -ENOMEM;
   
  -   /* Check arg is function or file and copy it */
  -   if (strchr(tmp, '.'))   /* File */
  +   if (c == 1)
  pp-file = tmp;
  -   else/* Function */
  +   else
  pp-function = tmp;
   
  /* Parse other options */
  -- 
  2.1.3

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

Re: [RFC PATCH 0/8] Fix perf probe issues on powerpc

[Ananth N Mavinakayanahalli]

On Tue, Dec 09, 2014 at 11:03:58PM +0530, Naveen N. Rao wrote:
> This patchset fixes various issues with perf probe on powerpc
> across ABIv1 and ABIv2:
> - in the presence of DWARF debug-info,
> - in the absence of DWARF, but with the symbol table, and
> - in the absence of debug-info, but with kallsyms.
> 
> Applies cleanly on v3.18 and on -tip with minor changes to patch 6.
> Tested on ppc64 BE and LE.
> 
> - Naveen
> 
> Naveen N. Rao (8):
>   kprobes: Fix kallsyms lookup across powerpc ABIv1 and ABIv2
>   perf probe powerpc: Fix symbol fixup issues due to ELF type
>   perf probe: Improve detection of file/function name in the probe
> pattern
>   perf probe powerpc: Handle powerpc dot symbols
>   perf probe powerpc: Allow matching against dot symbols
>   perf tools powerpc: Fix PPC64 ELF ABIv2 symbol decoding
>   perf probe powerpc: Use DWARF info only if necessary
>   perf probe powerpc: Fixup function entry if using kallsyms lookup
> 
>  arch/powerpc/include/asm/code-patching.h  | 26 
>  arch/powerpc/include/asm/kprobes.h| 58 
> ++-
>  tools/perf/arch/powerpc/Makefile  |  1 +
>  tools/perf/arch/powerpc/util/elf-sym-decode.c | 27 +
>  tools/perf/config/Makefile|  1 +
>  tools/perf/util/elf_sym.h | 13 ++
>  tools/perf/util/probe-event.c | 57 --
>  tools/perf/util/symbol-elf.c  | 11 -
>  tools/perf/util/symbol.c  |  6 +++
>  9 files changed, 170 insertions(+), 30 deletions(-)
>  create mode 100644 tools/perf/arch/powerpc/util/elf-sym-decode.c
>  create mode 100644 tools/perf/util/elf_sym.h

For the full patchset...

Reviewed-by: Ananth N Mavinakayanahalli 

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

Re: [RFC PATCH 8/8] perf probe powerpc: Fixup function entry if using kallsyms lookup

[Ananth N Mavinakayanahalli]

On Tue, Dec 09, 2014 at 06:14:00PM -0300, Arnaldo Carvalho de Melo wrote:
> Em Tue, Dec 09, 2014 at 11:04:06PM +0530, Naveen N. Rao escreveu:
> > On powerpc ABIv2, if no debug-info is found and we use kallsyms,
> > we need to fixup the function entry to point to the local entry point.
> > Use offset of 8 since current toolchains always generate 2
> > instructions (8 bytes).
> 
> Hi Michael and Ananth, may I have your Acked-by or Reviewed-by for these
> patches?
> 
> The ones, like this, that are affects only ppc I'm can assume was tested
> and applying it won't break other arch users, but having a/rev-by from
> ppc developers should speed up this process.

Hi Arnaldo,

Yes, I have reviewed the patches. So, for all patches...

Reviewed-by: Ananth N Mavinakayanahalli 

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

Re: [RFC PATCH 8/8] perf probe powerpc: Fixup function entry if using kallsyms lookup

[Ananth N Mavinakayanahalli]

On Tue, Dec 09, 2014 at 06:14:00PM -0300, Arnaldo Carvalho de Melo wrote:
 Em Tue, Dec 09, 2014 at 11:04:06PM +0530, Naveen N. Rao escreveu:
  On powerpc ABIv2, if no debug-info is found and we use kallsyms,
  we need to fixup the function entry to point to the local entry point.
  Use offset of 8 since current toolchains always generate 2
  instructions (8 bytes).
 
 Hi Michael and Ananth, may I have your Acked-by or Reviewed-by for these
 patches?
 
 The ones, like this, that are affects only ppc I'm can assume was tested
 and applying it won't break other arch users, but having a/rev-by from
 ppc developers should speed up this process.

Hi Arnaldo,

Yes, I have reviewed the patches. So, for all patches...

Reviewed-by: Ananth N Mavinakayanahalli ana...@in.ibm.com

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

Re: [RFC PATCH 0/8] Fix perf probe issues on powerpc

[Ananth N Mavinakayanahalli]

On Tue, Dec 09, 2014 at 11:03:58PM +0530, Naveen N. Rao wrote:
 This patchset fixes various issues with perf probe on powerpc
 across ABIv1 and ABIv2:
 - in the presence of DWARF debug-info,
 - in the absence of DWARF, but with the symbol table, and
 - in the absence of debug-info, but with kallsyms.
 
 Applies cleanly on v3.18 and on -tip with minor changes to patch 6.
 Tested on ppc64 BE and LE.
 
 - Naveen
 
 Naveen N. Rao (8):
   kprobes: Fix kallsyms lookup across powerpc ABIv1 and ABIv2
   perf probe powerpc: Fix symbol fixup issues due to ELF type
   perf probe: Improve detection of file/function name in the probe
 pattern
   perf probe powerpc: Handle powerpc dot symbols
   perf probe powerpc: Allow matching against dot symbols
   perf tools powerpc: Fix PPC64 ELF ABIv2 symbol decoding
   perf probe powerpc: Use DWARF info only if necessary
   perf probe powerpc: Fixup function entry if using kallsyms lookup
 
  arch/powerpc/include/asm/code-patching.h  | 26 
  arch/powerpc/include/asm/kprobes.h| 58 
 ++-
  tools/perf/arch/powerpc/Makefile  |  1 +
  tools/perf/arch/powerpc/util/elf-sym-decode.c | 27 +
  tools/perf/config/Makefile|  1 +
  tools/perf/util/elf_sym.h | 13 ++
  tools/perf/util/probe-event.c | 57 --
  tools/perf/util/symbol-elf.c  | 11 -
  tools/perf/util/symbol.c  |  6 +++
  9 files changed, 170 insertions(+), 30 deletions(-)
  create mode 100644 tools/perf/arch/powerpc/util/elf-sym-decode.c
  create mode 100644 tools/perf/util/elf_sym.h

For the full patchset...

Reviewed-by: Ananth N Mavinakayanahalli ana...@in.ibm.com

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

Re: [PATCH] kprobes: add kprobe_is_function_probed()

[Ananth N Mavinakayanahalli]

On Tue, Oct 21, 2014 at 05:48:30PM +0200, Jiri Kosina wrote:

>  kernel/kprobes.c| 28 
>  2 files changed, 33 insertions(+)
> 
> diff --git a/include/linux/kprobes.h b/include/linux/kprobes.h
> index f7296e5..f760555 100644
> --- a/include/linux/kprobes.h
> +++ b/include/linux/kprobes.h
> @@ -384,6 +384,7 @@ int disable_kprobe(struct kprobe *kp);
>  int enable_kprobe(struct kprobe *kp);
> 
>  void dump_kprobe(struct kprobe *kp);
> +bool kprobe_is_function_probed(const char *name);

Better name? function_is_kprobed() or such?

...

> +bool kprobe_is_function_probed(const char *name)
> +{
> + struct hlist_head *head;
> + struct kprobe *p, *kp;

kp not used

> + const char *sym = NULL;
> + unsigned long offset = 0;
> + unsigned int i;
> + char *modname, namebuf[KSYM_NAME_LEN];
> +
> + preempt_disable();
> + for (i = 0; i < KPROBE_TABLE_SIZE; i++) {
> + head = _table[i];
> + hlist_for_each_entry_rcu(p, head, hlist) {
> + sym = kallsyms_lookup((unsigned long)p->addr,
> + NULL, , ,
> + namebuf);
> + if (!strcmp(sym, name))

Don't you want a strncmp instead?

Ananth

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

Re: [PATCH] kprobes: add kprobe_is_function_probed()

[Ananth N Mavinakayanahalli]

On Tue, Oct 21, 2014 at 05:48:30PM +0200, Jiri Kosina wrote:

  kernel/kprobes.c| 28 
  2 files changed, 33 insertions(+)
 
 diff --git a/include/linux/kprobes.h b/include/linux/kprobes.h
 index f7296e5..f760555 100644
 --- a/include/linux/kprobes.h
 +++ b/include/linux/kprobes.h
 @@ -384,6 +384,7 @@ int disable_kprobe(struct kprobe *kp);
  int enable_kprobe(struct kprobe *kp);
 
  void dump_kprobe(struct kprobe *kp);
 +bool kprobe_is_function_probed(const char *name);

Better name? function_is_kprobed() or such?

...

 +bool kprobe_is_function_probed(const char *name)
 +{
 + struct hlist_head *head;
 + struct kprobe *p, *kp;

kp not used

 + const char *sym = NULL;
 + unsigned long offset = 0;
 + unsigned int i;
 + char *modname, namebuf[KSYM_NAME_LEN];
 +
 + preempt_disable();
 + for (i = 0; i  KPROBE_TABLE_SIZE; i++) {
 + head = kprobe_table[i];
 + hlist_for_each_entry_rcu(p, head, hlist) {
 + sym = kallsyms_lookup((unsigned long)p-addr,
 + NULL, offset, modname,
 + namebuf);
 + if (!strcmp(sym, name))

Don't you want a strncmp instead?

Ananth

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

Re: [PATCH -tip ] [BUGFIX] kprobes: Skip kretprobe hit in NMI context to avoid deadlock

[Ananth N Mavinakayanahalli]

On Fri, Aug 01, 2014 at 08:42:54AM +, Masami Hiramatsu wrote:
> Skip kretprobe hit in NMI context, because if an NMI happens
> inside the critical section protected by kretprobe_table.lock
> and another(or same) kretprobe hit, pre_kretprobe_handler
> tries to lock kretprobe_table.lock again.
> Normal interrupts have no problem because they are disabled
> with the lock.
> 
> Signed-off-by: Masami Hiramatsu 

Acked-by: Ananth N Mavinakayanahalli 

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

Re: [PATCH -tip ] [BUGFIX] kprobes: Skip kretprobe hit in NMI context to avoid deadlock

[Ananth N Mavinakayanahalli]

On Fri, Aug 01, 2014 at 08:42:54AM +, Masami Hiramatsu wrote:
 Skip kretprobe hit in NMI context, because if an NMI happens
 inside the critical section protected by kretprobe_table.lock
 and another(or same) kretprobe hit, pre_kretprobe_handler
 tries to lock kretprobe_table.lock again.
 Normal interrupts have no problem because they are disabled
 with the lock.
 
 Signed-off-by: Masami Hiramatsu masami.hiramatsu...@hitachi.com

Acked-by: Ananth N Mavinakayanahalli ana...@in.ibm.com

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

Re: [PATCH 0/1] (Was: cleanup DO_ERROR*() to prepare for uprobes fixes)

[Ananth N Mavinakayanahalli]

On Mon, May 12, 2014 at 07:08:22PM +0200, Oleg Nesterov wrote:
> On 05/08, Oleg Nesterov wrote:
> >
> > But let me send the initial changes first for review. If they pass the 
> > review
> > and if nobody objects, I'd like to route them along with the pending uprobes
> > fixes.
> 
> OK, nobody cares ;)
> 
> So I am going to ask Ingo to pull these changes plus the following minor fix.
> 
> To remind, I think that traps.c needs more cleanups: do_general_protection()
> and math_error() should be converted into DO_ERROR(), DIE_GPF should die, and
> the users of show_unhandled_signals() should be unified/cleanuped.
> 
> Ananth, David, this is x86 specific, but at first glance powerpc/arm might
> want to use uprobe_get_trap_addr() too.

Thanks for the heads-up Oleg.

Ananth

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

Re: [PATCH 0/1] (Was: cleanup DO_ERROR*() to prepare for uprobes fixes)

[Ananth N Mavinakayanahalli]

On Mon, May 12, 2014 at 07:08:22PM +0200, Oleg Nesterov wrote:
 On 05/08, Oleg Nesterov wrote:
 
  But let me send the initial changes first for review. If they pass the 
  review
  and if nobody objects, I'd like to route them along with the pending uprobes
  fixes.
 
 OK, nobody cares ;)
 
 So I am going to ask Ingo to pull these changes plus the following minor fix.
 
 To remind, I think that traps.c needs more cleanups: do_general_protection()
 and math_error() should be converted into DO_ERROR(), DIE_GPF should die, and
 the users of show_unhandled_signals() should be unified/cleanuped.
 
 Ananth, David, this is x86 specific, but at first glance powerpc/arm might
 want to use uprobe_get_trap_addr() too.

Thanks for the heads-up Oleg.

Ananth

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

Re: [RFT PATCH -next ] [BUGFIX] kprobes: Fix "Failed to find blacklist" error on ia64 and ppc64

[Ananth N Mavinakayanahalli]

On Thu, May 08, 2014 at 02:40:00PM +0900, Masami Hiramatsu wrote:
> (2014/05/08 13:47), Ananth N Mavinakayanahalli wrote:
> > On Wed, May 07, 2014 at 08:55:51PM +0900, Masami Hiramatsu wrote:
> > 
> > ...
> > 
> >> +#if defined(CONFIG_PPC64) && (!defined(_CALL_ELF) || _CALL_ELF == 1)
> >> +/*
> >> + * On PPC64 ABIv1 the function pointer actually points to the
> >> + * function's descriptor. The first entry in the descriptor is the
> >> + * address of the function text.
> >> + */
> >> +#define constant_function_entry(fn)   (((func_descr_t *)(fn))->entry)
> >> +#else
> >> +#define constant_function_entry(fn)   ((unsigned long)(fn))
> >> +#endif
> >> +
> >>  #endif /* __ASSEMBLY__ */
> > 
> > Hi Masami,
> > 
> > You could just use ppc_function_entry() instead.
> 
> No, I think ppc_function_entry() has two problems (on the latest -next kernel)
> 
> At first, that is an inlined functions which is not applied in build time.
> Since the NOKPROBE_SYMBOL() is used outside of any functions as like as
> EXPORT_SYMBOL(), we can only use preprocessed macros.
> Next, on PPC64 ABI*v2*, ppc_function_entry() returns local function entry,
> which seems global function entry + 2 insns. I'm not sure about implementation
> of the kallsyms on PPC64 ABIv2, but I guess we need global function entry
> for kallsyms.

ABIv2 does away with function descriptors and Anton fixed up that
routine to handle the change (the +2 is an artefact of that).

> BTW, could you test this patch on the latest -next tree on PPC64 if possible?

I'll test it, but it may take a bit.

Ananth

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

Re: [RFT PATCH -next ] [BUGFIX] kprobes: Fix Failed to find blacklist error on ia64 and ppc64

[Ananth N Mavinakayanahalli]

On Thu, May 08, 2014 at 02:40:00PM +0900, Masami Hiramatsu wrote:
 (2014/05/08 13:47), Ananth N Mavinakayanahalli wrote:
  On Wed, May 07, 2014 at 08:55:51PM +0900, Masami Hiramatsu wrote:
  
  ...
  
  +#if defined(CONFIG_PPC64)  (!defined(_CALL_ELF) || _CALL_ELF == 1)
  +/*
  + * On PPC64 ABIv1 the function pointer actually points to the
  + * function's descriptor. The first entry in the descriptor is the
  + * address of the function text.
  + */
  +#define constant_function_entry(fn)   (((func_descr_t *)(fn))-entry)
  +#else
  +#define constant_function_entry(fn)   ((unsigned long)(fn))
  +#endif
  +
   #endif /* __ASSEMBLY__ */
  
  Hi Masami,
  
  You could just use ppc_function_entry() instead.
 
 No, I think ppc_function_entry() has two problems (on the latest -next kernel)
 
 At first, that is an inlined functions which is not applied in build time.
 Since the NOKPROBE_SYMBOL() is used outside of any functions as like as
 EXPORT_SYMBOL(), we can only use preprocessed macros.
 Next, on PPC64 ABI*v2*, ppc_function_entry() returns local function entry,
 which seems global function entry + 2 insns. I'm not sure about implementation
 of the kallsyms on PPC64 ABIv2, but I guess we need global function entry
 for kallsyms.

ABIv2 does away with function descriptors and Anton fixed up that
routine to handle the change (the +2 is an artefact of that).

 BTW, could you test this patch on the latest -next tree on PPC64 if possible?

I'll test it, but it may take a bit.

Ananth

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

Re: [RFT PATCH -next ] [BUGFIX] kprobes: Fix "Failed to find blacklist" error on ia64 and ppc64

[Ananth N Mavinakayanahalli]

On Wed, May 07, 2014 at 08:55:51PM +0900, Masami Hiramatsu wrote:

...

> +#if defined(CONFIG_PPC64) && (!defined(_CALL_ELF) || _CALL_ELF == 1)
> +/*
> + * On PPC64 ABIv1 the function pointer actually points to the
> + * function's descriptor. The first entry in the descriptor is the
> + * address of the function text.
> + */
> +#define constant_function_entry(fn)  (((func_descr_t *)(fn))->entry)
> +#else
> +#define constant_function_entry(fn)  ((unsigned long)(fn))
> +#endif
> +
>  #endif /* __ASSEMBLY__ */

Hi Masami,

You could just use ppc_function_entry() instead.

Ananth

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

Re: [RFT PATCH -next ] [BUGFIX] kprobes: Fix Failed to find blacklist error on ia64 and ppc64

[Ananth N Mavinakayanahalli]

On Wed, May 07, 2014 at 08:55:51PM +0900, Masami Hiramatsu wrote:

...

 +#if defined(CONFIG_PPC64)  (!defined(_CALL_ELF) || _CALL_ELF == 1)
 +/*
 + * On PPC64 ABIv1 the function pointer actually points to the
 + * function's descriptor. The first entry in the descriptor is the
 + * address of the function text.
 + */
 +#define constant_function_entry(fn)  (((func_descr_t *)(fn))-entry)
 +#else
 +#define constant_function_entry(fn)  ((unsigned long)(fn))
 +#endif
 +
  #endif /* __ASSEMBLY__ */

Hi Masami,

You could just use ppc_function_entry() instead.

Ananth

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

Re: [PATCH 0/2] uprobes: typeof(arch_uprobe->insn) cleanups

[Ananth N Mavinakayanahalli]

On Sat, Nov 09, 2013 at 06:53:50PM +0100, Oleg Nesterov wrote:
> Hello.
> 
> Ananth, could you please explicitly ack or nack 2/2 ? It is
> really simple, but obviously I can't test it. And even if it
> is correct it should be merged only if you like it, this is
> the minor cleanup.

The changes look good and I have acked it.

Thanks Oleg!

Ananth

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

Re: [PATCH 2/2] uprobes/powerpc: Kill arch_uprobe->ainsn

[Ananth N Mavinakayanahalli]

On Sat, Nov 09, 2013 at 06:54:09PM +0100, Oleg Nesterov wrote:
> powerpc has both arch_uprobe->insn and arch_uprobe->ainsn to
> make the generic code happy. This is no longer needed after
> the previous change, powerpc can just use "u32 insn".
> 
> Signed-off-by: Oleg Nesterov 
> ---
>  arch/powerpc/include/asm/uprobes.h |5 ++---
>  arch/powerpc/kernel/uprobes.c  |2 +-
>  2 files changed, 3 insertions(+), 4 deletions(-)
> 
> diff --git a/arch/powerpc/include/asm/uprobes.h 
> b/arch/powerpc/include/asm/uprobes.h
> index 75c6ecd..7422a99 100644
> --- a/arch/powerpc/include/asm/uprobes.h
> +++ b/arch/powerpc/include/asm/uprobes.h
> @@ -36,9 +36,8 @@ typedef ppc_opcode_t uprobe_opcode_t;
> 
>  struct arch_uprobe {
>   union {
> - u8  insn[MAX_UINSN_BYTES];
> - u8  ixol[MAX_UINSN_BYTES];
> - u32 ainsn;
> + u32 insn;
> + u32 ixol;
>   };
>  };
> 
> diff --git a/arch/powerpc/kernel/uprobes.c b/arch/powerpc/kernel/uprobes.c
> index 59f419b..003b209 100644
> --- a/arch/powerpc/kernel/uprobes.c
> +++ b/arch/powerpc/kernel/uprobes.c
> @@ -186,7 +186,7 @@ bool arch_uprobe_skip_sstep(struct arch_uprobe *auprobe, 
> struct pt_regs *regs)
>* emulate_step() returns 1 if the insn was successfully emulated.
>* For all other cases, we need to single-step in hardware.
>*/
> - ret = emulate_step(regs, auprobe->ainsn);
> + ret = emulate_step(regs, auprobe->insn);
>   if (ret > 0)
>   return true;

Acked-by: Ananth N Mavinakayanahalli 

Thanks Oleg.

Ananth

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

Re: [PATCH 2/2] uprobes/powerpc: Kill arch_uprobe-ainsn

[Ananth N Mavinakayanahalli]

On Sat, Nov 09, 2013 at 06:54:09PM +0100, Oleg Nesterov wrote:
 powerpc has both arch_uprobe-insn and arch_uprobe-ainsn to
 make the generic code happy. This is no longer needed after
 the previous change, powerpc can just use u32 insn.
 
 Signed-off-by: Oleg Nesterov o...@redhat.com
 ---
  arch/powerpc/include/asm/uprobes.h |5 ++---
  arch/powerpc/kernel/uprobes.c  |2 +-
  2 files changed, 3 insertions(+), 4 deletions(-)
 
 diff --git a/arch/powerpc/include/asm/uprobes.h 
 b/arch/powerpc/include/asm/uprobes.h
 index 75c6ecd..7422a99 100644
 --- a/arch/powerpc/include/asm/uprobes.h
 +++ b/arch/powerpc/include/asm/uprobes.h
 @@ -36,9 +36,8 @@ typedef ppc_opcode_t uprobe_opcode_t;
 
  struct arch_uprobe {
   union {
 - u8  insn[MAX_UINSN_BYTES];
 - u8  ixol[MAX_UINSN_BYTES];
 - u32 ainsn;
 + u32 insn;
 + u32 ixol;
   };
  };
 
 diff --git a/arch/powerpc/kernel/uprobes.c b/arch/powerpc/kernel/uprobes.c
 index 59f419b..003b209 100644
 --- a/arch/powerpc/kernel/uprobes.c
 +++ b/arch/powerpc/kernel/uprobes.c
 @@ -186,7 +186,7 @@ bool arch_uprobe_skip_sstep(struct arch_uprobe *auprobe, 
 struct pt_regs *regs)
* emulate_step() returns 1 if the insn was successfully emulated.
* For all other cases, we need to single-step in hardware.
*/
 - ret = emulate_step(regs, auprobe-ainsn);
 + ret = emulate_step(regs, auprobe-insn);
   if (ret  0)
   return true;

Acked-by: Ananth N Mavinakayanahalli ana...@in.ibm.com

Thanks Oleg.

Ananth

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

Re: [PATCH 0/2] uprobes: typeof(arch_uprobe-insn) cleanups

[Ananth N Mavinakayanahalli]

On Sat, Nov 09, 2013 at 06:53:50PM +0100, Oleg Nesterov wrote:
 Hello.
 
 Ananth, could you please explicitly ack or nack 2/2 ? It is
 really simple, but obviously I can't test it. And even if it
 is correct it should be merged only if you like it, this is
 the minor cleanup.

The changes look good and I have acked it.

Thanks Oleg!

Ananth

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

Re: [PATCH 3/3] kprobes: Use KSYM_NAME_LEN to size identifier buffers

[Ananth N Mavinakayanahalli]

On Sat, Aug 10, 2013 at 05:55:33PM -0700, Andi Kleen wrote:
> From: Joe Mario 
> 
> Use KSYM_NAME_LEN to size identifier buffers, so that it can
> be easier increased.
> 
> Cc: ana...@in.ibm.com
> Signed-off-by: Joe Mario 
> Signed-off-by: Andi Kleen 

Acked-by: Ananth N Mavinakayanahalli 

> ---
>  kernel/kprobes.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/kernel/kprobes.c b/kernel/kprobes.c
> index 6e33498..e174daf 100644
> --- a/kernel/kprobes.c
> +++ b/kernel/kprobes.c
> @@ -2083,7 +2083,7 @@ static int __init init_kprobes(void)
>  {
>   int i, err = 0;
>   unsigned long offset = 0, size = 0;
> - char *modname, namebuf[128];
> + char *modname, namebuf[KSYM_NAME_LEN];
>   const char *symbol_name;
>   void *addr;
>   struct kprobe_blackpoint *kb;
> @@ -2209,7 +2209,7 @@ static int __kprobes show_kprobe_addr(struct seq_file 
> *pi, void *v)
>   const char *sym = NULL;
>   unsigned int i = *(loff_t *) v;
>   unsigned long offset = 0;
> - char *modname, namebuf[128];
> + char *modname, namebuf[KSYM_NAME_LEN];
> 
>   head = _table[i];
>   preempt_disable();
> -- 
> 1.8.3.1

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

Re: [PATCH 3/3] kprobes: Use KSYM_NAME_LEN to size identifier buffers

[Ananth N Mavinakayanahalli]

On Sat, Aug 10, 2013 at 05:55:33PM -0700, Andi Kleen wrote:
 From: Joe Mario jma...@redhat.com
 
 Use KSYM_NAME_LEN to size identifier buffers, so that it can
 be easier increased.
 
 Cc: ana...@in.ibm.com
 Signed-off-by: Joe Mario jma...@redhat.com
 Signed-off-by: Andi Kleen a...@linux.intel.com

Acked-by: Ananth N Mavinakayanahalli ana...@in.ibm.com

 ---
  kernel/kprobes.c | 4 ++--
  1 file changed, 2 insertions(+), 2 deletions(-)
 
 diff --git a/kernel/kprobes.c b/kernel/kprobes.c
 index 6e33498..e174daf 100644
 --- a/kernel/kprobes.c
 +++ b/kernel/kprobes.c
 @@ -2083,7 +2083,7 @@ static int __init init_kprobes(void)
  {
   int i, err = 0;
   unsigned long offset = 0, size = 0;
 - char *modname, namebuf[128];
 + char *modname, namebuf[KSYM_NAME_LEN];
   const char *symbol_name;
   void *addr;
   struct kprobe_blackpoint *kb;
 @@ -2209,7 +2209,7 @@ static int __kprobes show_kprobe_addr(struct seq_file 
 *pi, void *v)
   const char *sym = NULL;
   unsigned int i = *(loff_t *) v;
   unsigned long offset = 0;
 - char *modname, namebuf[128];
 + char *modname, namebuf[KSYM_NAME_LEN];
 
   head = kprobe_table[i];
   preempt_disable();
 -- 
 1.8.3.1

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

Re: [PATCH -tip ] [BUGFIX] kprobes: Fix a double lock bug of kprobe_mutex

[Ananth N Mavinakayanahalli]

On Thu, Apr 18, 2013 at 06:33:18PM +0900, Masami Hiramatsu wrote:
> Fix a double locking bug caused when debug.kprobe-optimization=0.
> While the proc_kprobes_optimization_handler locks kprobe_mutex,
> wait_for_kprobe_optimizer locks it again and that causes a double lock.
> To fix the bug, this introduces different mutex for protecting
> sysctl parameter and locks it in proc_kprobes_optimization_handler.
> Of course, since we need to lock kprobe_mutex when touching kprobes
> resources, that is done in *optimize_all_kprobes().
> 
> This bug was from ad72b3bea744b4db01c89af0f86f3e8920d354df
> 
> Signed-off-by: Masami Hiramatsu 
> Cc: Ingo Molnar 
> Cc: Tejun Heo 
> Cc: Ananth N Mavinakayanahalli 
> Cc: "David S. Miller" 

Acked-by: Ananth N Mavinakayanahalli 

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

Re: [PATCH -tip ] [BUGFIX] kprobes: Fix a double lock bug of kprobe_mutex

[Ananth N Mavinakayanahalli]

On Thu, Apr 18, 2013 at 06:33:18PM +0900, Masami Hiramatsu wrote:
 Fix a double locking bug caused when debug.kprobe-optimization=0.
 While the proc_kprobes_optimization_handler locks kprobe_mutex,
 wait_for_kprobe_optimizer locks it again and that causes a double lock.
 To fix the bug, this introduces different mutex for protecting
 sysctl parameter and locks it in proc_kprobes_optimization_handler.
 Of course, since we need to lock kprobe_mutex when touching kprobes
 resources, that is done in *optimize_all_kprobes().
 
 This bug was from ad72b3bea744b4db01c89af0f86f3e8920d354df
 
 Signed-off-by: Masami Hiramatsu masami.hiramatsu...@hitachi.com
 Cc: Ingo Molnar mi...@redhat.com
 Cc: Tejun Heo t...@kernel.org
 Cc: Ananth N Mavinakayanahalli ana...@in.ibm.com
 Cc: David S. Miller da...@davemloft.net

Acked-by: Ananth N Mavinakayanahalli ana...@in.ibm.com

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

Re: [PATCH v1 0/9] uretprobes: Return uprobes implementation

[Ananth N Mavinakayanahalli]

On Wed, Apr 03, 2013 at 07:45:52PM +0200, Oleg Nesterov wrote:
> On 04/03, Anton Arapov wrote:

...

> Looks fine to me. I am going to add this to
> git://git.kernel.org/pub/scm/linux/kernel/git/oleg/misc uprobes/core
> 
> Ananth. "4/9 uretprobes/ppc" looks "obviously correct", but could you
> please review and ack/nack ?

Oleg,
4/9 is fine; I have acked it.

Ananth

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

Re: [PATCH v1 4/9] uretprobes/ppc: Hijack return address

[Ananth N Mavinakayanahalli]

On Wed, Apr 03, 2013 at 06:00:34PM +0200, Anton Arapov wrote:
> Hijack the return address and replace it with a trampoline address.
> PowerPC implementation.
> 
> Signed-off-by: Anton Arapov 

Acked-by: Ananth N Mavinakayanahalli 

> ---
>  arch/powerpc/include/asm/uprobes.h |  1 +
>  arch/powerpc/kernel/uprobes.c  | 13 +
>  2 files changed, 14 insertions(+)
> 
> diff --git a/arch/powerpc/include/asm/uprobes.h 
> b/arch/powerpc/include/asm/uprobes.h
> index b532060..2301602 100644
> --- a/arch/powerpc/include/asm/uprobes.h
> +++ b/arch/powerpc/include/asm/uprobes.h
> @@ -51,4 +51,5 @@ extern int  arch_uprobe_post_xol(struct arch_uprobe *aup, 
> struct pt_regs *regs);
>  extern bool arch_uprobe_xol_was_trapped(struct task_struct *tsk);
>  extern int  arch_uprobe_exception_notify(struct notifier_block *self, 
> unsigned long val, void *data);
>  extern void arch_uprobe_abort_xol(struct arch_uprobe *aup, struct pt_regs 
> *regs);
> +extern unsigned long arch_uretprobe_hijack_return_addr(unsigned long 
> trampoline_vaddr, struct pt_regs *regs);
>  #endif   /* _ASM_UPROBES_H */
> diff --git a/arch/powerpc/kernel/uprobes.c b/arch/powerpc/kernel/uprobes.c
> index bc77834..567b975 100644
> --- a/arch/powerpc/kernel/uprobes.c
> +++ b/arch/powerpc/kernel/uprobes.c
> @@ -188,3 +188,16 @@ bool arch_uprobe_skip_sstep(struct arch_uprobe *auprobe, 
> struct pt_regs *regs)
> 
>   return false;
>  }
> +
> +unsigned long
> +arch_uretprobe_hijack_return_addr(unsigned long trampoline_vaddr, struct 
> pt_regs *regs)
> +{
> + unsigned long orig_ret_vaddr;
> +
> + orig_ret_vaddr = regs->link;
> +
> + /* Replace the return addr with trampoline addr */
> + regs->link = trampoline_vaddr;
> +
> + return orig_ret_vaddr;
> +}
> -- 
> 1.8.1.4

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

Re: [PATCH v1 4/9] uretprobes/ppc: Hijack return address

[Ananth N Mavinakayanahalli]

On Wed, Apr 03, 2013 at 06:00:34PM +0200, Anton Arapov wrote:
 Hijack the return address and replace it with a trampoline address.
 PowerPC implementation.
 
 Signed-off-by: Anton Arapov an...@redhat.com

Acked-by: Ananth N Mavinakayanahalli ana...@in.ibm.com

 ---
  arch/powerpc/include/asm/uprobes.h |  1 +
  arch/powerpc/kernel/uprobes.c  | 13 +
  2 files changed, 14 insertions(+)
 
 diff --git a/arch/powerpc/include/asm/uprobes.h 
 b/arch/powerpc/include/asm/uprobes.h
 index b532060..2301602 100644
 --- a/arch/powerpc/include/asm/uprobes.h
 +++ b/arch/powerpc/include/asm/uprobes.h
 @@ -51,4 +51,5 @@ extern int  arch_uprobe_post_xol(struct arch_uprobe *aup, 
 struct pt_regs *regs);
  extern bool arch_uprobe_xol_was_trapped(struct task_struct *tsk);
  extern int  arch_uprobe_exception_notify(struct notifier_block *self, 
 unsigned long val, void *data);
  extern void arch_uprobe_abort_xol(struct arch_uprobe *aup, struct pt_regs 
 *regs);
 +extern unsigned long arch_uretprobe_hijack_return_addr(unsigned long 
 trampoline_vaddr, struct pt_regs *regs);
  #endif   /* _ASM_UPROBES_H */
 diff --git a/arch/powerpc/kernel/uprobes.c b/arch/powerpc/kernel/uprobes.c
 index bc77834..567b975 100644
 --- a/arch/powerpc/kernel/uprobes.c
 +++ b/arch/powerpc/kernel/uprobes.c
 @@ -188,3 +188,16 @@ bool arch_uprobe_skip_sstep(struct arch_uprobe *auprobe, 
 struct pt_regs *regs)
 
   return false;
  }
 +
 +unsigned long
 +arch_uretprobe_hijack_return_addr(unsigned long trampoline_vaddr, struct 
 pt_regs *regs)
 +{
 + unsigned long orig_ret_vaddr;
 +
 + orig_ret_vaddr = regs-link;
 +
 + /* Replace the return addr with trampoline addr */
 + regs-link = trampoline_vaddr;
 +
 + return orig_ret_vaddr;
 +}
 -- 
 1.8.1.4

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

Re: [PATCH v1 0/9] uretprobes: Return uprobes implementation

[Ananth N Mavinakayanahalli]

On Wed, Apr 03, 2013 at 07:45:52PM +0200, Oleg Nesterov wrote:
 On 04/03, Anton Arapov wrote:

...

 Looks fine to me. I am going to add this to
 git://git.kernel.org/pub/scm/linux/kernel/git/oleg/misc uprobes/core
 
 Ananth. 4/9 uretprobes/ppc looks obviously correct, but could you
 please review and ack/nack ?

Oleg,
4/9 is fine; I have acked it.

Ananth

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

[PATCH v2 4/4] uprobes/powerpc: remove additional trap instruction check

[Ananth N Mavinakayanahalli]

From: Ananth N Mavinakayanahalli 

prepare_uprobe() already checks if the underlying unstruction
(on file) is a trap variant. We don't need to check this again.

Signed-off-by: Ananth N Mavinakayanahalli 
---
 arch/powerpc/kernel/uprobes.c |6 --
 1 file changed, 6 deletions(-)

Index: linux-3.9-rc3/arch/powerpc/kernel/uprobes.c
===
--- linux-3.9-rc3.orig/arch/powerpc/kernel/uprobes.c
+++ linux-3.9-rc3/arch/powerpc/kernel/uprobes.c
@@ -53,12 +53,6 @@ int arch_uprobe_analyze_insn(struct arch
if (addr & 0x03)
return -EINVAL;
 
-   /*
-* We currently don't support a uprobe on an already
-* existing breakpoint instruction underneath
-*/
-   if (is_trap(auprobe->ainsn))
-   return -ENOTSUPP;
return 0;
 }
 

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

[PATCH v2 3/4] uprobes/powerpc: teach uprobes to ignore gdb breakpoints

[Ananth N Mavinakayanahalli]

From: Ananth N Mavinakayanahalli 

Powerpc has many trap variants that could be used by entities like gdb.
Currently, running gdb on a program being traced by uprobes causes an
endless loop since uprobes doesn't understand that the trap was inserted
by some other entity and a SIGTRAP needs to be delivered.

Teach uprobes to ignore breakpoints that do not belong to it.

Signed-off-by: Ananth N Mavinakayanahalli 
---
 arch/powerpc/kernel/uprobes.c |   10 ++
 1 file changed, 10 insertions(+)

Index: linux-3.9-rc3/arch/powerpc/kernel/uprobes.c
===
--- linux-3.9-rc3.orig/arch/powerpc/kernel/uprobes.c
+++ linux-3.9-rc3/arch/powerpc/kernel/uprobes.c
@@ -31,6 +31,16 @@
 #define UPROBE_TRAP_NR UINT_MAX
 
 /**
+ * is_trap_insn - check if the instruction is a trap variant
+ * @insn: instruction to be checked.
+ * Returns true if @insn is a trap variant.
+ */
+bool is_trap_insn(uprobe_opcode_t *insn)
+{
+   return (is_trap(*insn));
+}
+
+/**
  * arch_uprobe_analyze_insn
  * @mm: the probed address space.
  * @arch_uprobe: the probepoint information.

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

[PATCH v2 2/4] uprobes: refuse uprobe on trap variants

[Ananth N Mavinakayanahalli]

From: Ananth N Mavinakayanahalli 

Refuse to place a uprobe if a trap variant already exists in the
file copy at the address.

Signed-off-by: Ananth N Mavinakayanahalli 
---
 kernel/events/uprobes.c |2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Index: linux-3.9-rc3/kernel/events/uprobes.c
===
--- linux-3.9-rc3.orig/kernel/events/uprobes.c
+++ linux-3.9-rc3/kernel/events/uprobes.c
@@ -573,7 +573,7 @@ static int prepare_uprobe(struct uprobe
goto out;
 
ret = -ENOTSUPP;
-   if (is_swbp_insn((uprobe_opcode_t *)uprobe->arch.insn))
+   if (is_trap_insn((uprobe_opcode_t *)uprobe->arch.insn))
goto out;
 
ret = arch_uprobe_analyze_insn(>arch, mm, vaddr);

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

[PATCH v2 1/4] uprobes: add trap variant helper

[Ananth N Mavinakayanahalli]

From: Ananth N Mavinakayanahalli 

Some architectures like powerpc have multiple variants of the trap
instruction. Introduce an additional helper is_trap_insn() for run-time
handling of non-uprobe traps on such architectures.

While there, change is_swbp_at_addr() to is_trap_at_addr() for reading
clarity.

With this change, the uprobe registration path will supercede any trap
instruction inserted at the requested location, while taking care of
delivering the SIGTRAP for cases where the trap notification came in
for an address without a uprobe. See [1] for a more detailed explanation.

[1] https://lists.ozlabs.org/pipermail/linuxppc-dev/2013-March/104771.html

This change was suggested by Oleg Nesterov.

Signed-off-by: Ananth N Mavinakayanahalli 
---
 include/linux/uprobes.h |1 +
 kernel/events/uprobes.c |   32 
 2 files changed, 29 insertions(+), 4 deletions(-)

Index: linux-3.9-rc3/include/linux/uprobes.h
===
--- linux-3.9-rc3.orig/include/linux/uprobes.h
+++ linux-3.9-rc3/include/linux/uprobes.h
@@ -100,6 +100,7 @@ struct uprobes_state {
 extern int __weak set_swbp(struct arch_uprobe *aup, struct mm_struct *mm, 
unsigned long vaddr);
 extern int __weak set_orig_insn(struct arch_uprobe *aup, struct mm_struct *mm, 
unsigned long vaddr);
 extern bool __weak is_swbp_insn(uprobe_opcode_t *insn);
+extern bool __weak is_trap_insn(uprobe_opcode_t *insn);
 extern int uprobe_register(struct inode *inode, loff_t offset, struct 
uprobe_consumer *uc);
 extern int uprobe_apply(struct inode *inode, loff_t offset, struct 
uprobe_consumer *uc, bool);
 extern void uprobe_unregister(struct inode *inode, loff_t offset, struct 
uprobe_consumer *uc);
Index: linux-3.9-rc3/kernel/events/uprobes.c
===
--- linux-3.9-rc3.orig/kernel/events/uprobes.c
+++ linux-3.9-rc3/kernel/events/uprobes.c
@@ -173,6 +173,20 @@ bool __weak is_swbp_insn(uprobe_opcode_t
return *insn == UPROBE_SWBP_INSN;
 }
 
+/**
+ * is_trap_insn - check if instruction is breakpoint instruction.
+ * @insn: instruction to be checked.
+ * Default implementation of is_trap_insn
+ * Returns true if @insn is a breakpoint instruction.
+ *
+ * This function is needed for the case where an architecture has multiple
+ * trap instructions (like powerpc).
+ */
+bool __weak is_trap_insn(uprobe_opcode_t *insn)
+{
+   return is_swbp_insn(insn);
+}
+
 static void copy_opcode(struct page *page, unsigned long vaddr, 
uprobe_opcode_t *opcode)
 {
void *kaddr = kmap_atomic(page);
@@ -185,6 +199,15 @@ static int verify_opcode(struct page *pa
uprobe_opcode_t old_opcode;
bool is_swbp;
 
+   /*
+* Note: We only check if the old_opcode is UPROBE_SWBP_INSN here.
+* We do not check if it is any other 'trap variant' which could
+* be conditional trap instruction such as the one powerpc supports.
+*
+* The logic is that we do not care if the underlying instruction
+* is a trap variant; uprobes always wins over any other (gdb)
+* breakpoint.
+*/
copy_opcode(page, vaddr, _opcode);
is_swbp = is_swbp_insn(_opcode);
 
@@ -204,7 +227,7 @@ static int verify_opcode(struct page *pa
  * Expect the breakpoint instruction to be the smallest size instruction for
  * the architecture. If an arch has variable length instruction and the
  * breakpoint instruction is not of the smallest length instruction
- * supported by that architecture then we need to modify is_swbp_at_addr and
+ * supported by that architecture then we need to modify is_trap_at_addr and
  * write_opcode accordingly. This would never be a problem for archs that
  * have fixed length instructions.
  */
@@ -1431,7 +1454,7 @@ static void mmf_recalc_uprobes(struct mm
clear_bit(MMF_HAS_UPROBES, >flags);
 }
 
-static int is_swbp_at_addr(struct mm_struct *mm, unsigned long vaddr)
+static int is_trap_at_addr(struct mm_struct *mm, unsigned long vaddr)
 {
struct page *page;
uprobe_opcode_t opcode;
@@ -1452,7 +1475,8 @@ static int is_swbp_at_addr(struct mm_str
copy_opcode(page, vaddr, );
put_page(page);
  out:
-   return is_swbp_insn();
+   /* This needs to return true for any variant of the trap insn */
+   return is_trap_insn();
 }
 
 static struct uprobe *find_active_uprobe(unsigned long bp_vaddr, int *is_swbp)
@@ -1472,7 +1496,7 @@ static struct uprobe *find_active_uprobe
}
 
if (!uprobe)
-   *is_swbp = is_swbp_at_addr(mm, bp_vaddr);
+   *is_swbp = is_trap_at_addr(mm, bp_vaddr);
} else {
*is_swbp = -EFAULT;
}

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.htm

Re: [PATCH 1/3] uprobes: add trap variant helper

[Ananth N Mavinakayanahalli]

On Fri, Mar 22, 2013 at 03:54:06PM +0100, Oleg Nesterov wrote:
> On 03/22, Ananth N Mavinakayanahalli wrote:
> >
> > +/**
> > + * is_trap_insn - check if instruction is breakpoint instruction.
> > + * @insn: instruction to be checked.
> > + * Default implementation of is_trap_insn
> > + * Returns true if @insn is a breakpoint instruction.
> > + *
> > + * This function is needed for the case where an architecture has multiple
> > + * trap instructions (like powerpc).
> > + */
> > +bool __weak is_trap_insn(uprobe_opcode_t *insn)
> > +{
> > +   return is_swbp_insn(insn);
> > +}
> 
> OK, thanks, the whole series looks fine, just one note...

OK.

> My patch also changed prepare_uprobe() to use is_trap_insn(), and I think
> this is right. Exactly because of 3/3 which removes is_trap() from
> arch_uprobe_analyze_insn().
> 
> If the original insn is_trap(), we do not want to singlestep it and get
> another trap after we hit handle_swbp().

OK, I'll send a v2 with that change.

Ananth

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

[PATCH 3/3] uprobes/powerpc: ignore trap variants during register

[Ananth N Mavinakayanahalli]

From: Ananth N Mavinakayanahalli 

The current implementation of uprobes assumes that uprobes always wins
even when a register request is at a location with a conditional
breakpoint by some other entity. Refer to [1] for more details.

Remove the breakpoint instruction check during registration on powerpc,
so that uprobes behavior on powerpc matches that of x86.

[1] https://lists.ozlabs.org/pipermail/linuxppc-dev/2013-March/104771.html

Signed-off-by: Ananth N Mavinakayanahalli 
---
 arch/powerpc/kernel/uprobes.c |6 --
 1 file changed, 6 deletions(-)

Index: linux-3.9-rc3/arch/powerpc/kernel/uprobes.c
===
--- linux-3.9-rc3.orig/arch/powerpc/kernel/uprobes.c
+++ linux-3.9-rc3/arch/powerpc/kernel/uprobes.c
@@ -53,12 +53,6 @@ int arch_uprobe_analyze_insn(struct arch
if (addr & 0x03)
return -EINVAL;
 
-   /*
-* We currently don't support a uprobe on an already
-* existing breakpoint instruction underneath
-*/
-   if (is_trap(auprobe->ainsn))
-   return -ENOTSUPP;
return 0;
 }
 

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

[PATCH 2/3] uprobes/powerpc: teach uprobes to ignore gdb breakpoints

[Ananth N Mavinakayanahalli]

From: Ananth N Mavinakayanahalli 

Powerpc has many trap variants that could be used by entities like gdb.
Currently, running gdb on a program being traced by uprobes causes an
endless loop since uprobes doesn't understand that the trap was inserted
by some other entity and a SIGTRAP needs to be delivered.

Teach uprobes to ignore breakpoints that do not belong to it.

Signed-off-by: Ananth N Mavinakayanahalli 
---
 arch/powerpc/kernel/uprobes.c |   10 ++
 1 file changed, 10 insertions(+)

Index: linux-3.9-rc3/arch/powerpc/kernel/uprobes.c
===
--- linux-3.9-rc3.orig/arch/powerpc/kernel/uprobes.c
+++ linux-3.9-rc3/arch/powerpc/kernel/uprobes.c
@@ -31,6 +31,16 @@
 #define UPROBE_TRAP_NR UINT_MAX
 
 /**
+ * is_trap_insn - check if the instruction is a trap variant
+ * @insn: instruction to be checked.
+ * Returns true if @insn is a trap variant.
+ */
+bool is_trap_insn(uprobe_opcode_t *insn)
+{
+   return (is_trap(*insn));
+}
+
+/**
  * arch_uprobe_analyze_insn
  * @mm: the probed address space.
  * @arch_uprobe: the probepoint information.

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

[PATCH 1/3] uprobes: add trap variant helper

[Ananth N Mavinakayanahalli]

From: Ananth N Mavinakayanahalli 

Some architectures like powerpc have multiple variants of the trap
instruction. Introduce an additional helper is_trap_insn() for run-time
handling of non-uprobe traps on such architectures.

While there, change is_swbp_at_addr() to is_trap_at_addr() for reading
clarity.

With this change, the uprobe registration path will supercede any trap
instruction inserted at the requested location, while taking care of
delivering the SIGTRAP for cases where the trap notification came in
for an address without a uprobe. See [1] for a more detailed explanation.

[1] https://lists.ozlabs.org/pipermail/linuxppc-dev/2013-March/104771.html

This change was suggested by Oleg Nesterov.

Signed-off-by: Ananth N Mavinakayanahalli 
---
 include/linux/uprobes.h |1 +
 kernel/events/uprobes.c |   32 
 2 files changed, 29 insertions(+), 4 deletions(-)

Index: linux-3.9-rc3/include/linux/uprobes.h
===
--- linux-3.9-rc3.orig/include/linux/uprobes.h
+++ linux-3.9-rc3/include/linux/uprobes.h
@@ -100,6 +100,7 @@ struct uprobes_state {
 extern int __weak set_swbp(struct arch_uprobe *aup, struct mm_struct *mm, 
unsigned long vaddr);
 extern int __weak set_orig_insn(struct arch_uprobe *aup, struct mm_struct *mm, 
unsigned long vaddr);
 extern bool __weak is_swbp_insn(uprobe_opcode_t *insn);
+extern bool __weak is_trap_insn(uprobe_opcode_t *insn);
 extern int uprobe_register(struct inode *inode, loff_t offset, struct 
uprobe_consumer *uc);
 extern int uprobe_apply(struct inode *inode, loff_t offset, struct 
uprobe_consumer *uc, bool);
 extern void uprobe_unregister(struct inode *inode, loff_t offset, struct 
uprobe_consumer *uc);
Index: linux-3.9-rc3/kernel/events/uprobes.c
===
--- linux-3.9-rc3.orig/kernel/events/uprobes.c
+++ linux-3.9-rc3/kernel/events/uprobes.c
@@ -173,6 +173,20 @@ bool __weak is_swbp_insn(uprobe_opcode_t
return *insn == UPROBE_SWBP_INSN;
 }
 
+/**
+ * is_trap_insn - check if instruction is breakpoint instruction.
+ * @insn: instruction to be checked.
+ * Default implementation of is_trap_insn
+ * Returns true if @insn is a breakpoint instruction.
+ *
+ * This function is needed for the case where an architecture has multiple
+ * trap instructions (like powerpc).
+ */
+bool __weak is_trap_insn(uprobe_opcode_t *insn)
+{
+   return is_swbp_insn(insn);
+}
+
 static void copy_opcode(struct page *page, unsigned long vaddr, 
uprobe_opcode_t *opcode)
 {
void *kaddr = kmap_atomic(page);
@@ -185,6 +199,15 @@ static int verify_opcode(struct page *pa
uprobe_opcode_t old_opcode;
bool is_swbp;
 
+   /*
+* Note: We only check if the old_opcode is UPROBE_SWBP_INSN here.
+* We do not check if it is any other 'trap variant' which could
+* be conditional trap instruction such as the one powerpc supports.
+*
+* The logic is that we do not care if the underlying instruction
+* is a trap variant; uprobes always wins over any other (gdb)
+* breakpoint.
+*/
copy_opcode(page, vaddr, _opcode);
is_swbp = is_swbp_insn(_opcode);
 
@@ -204,7 +227,7 @@ static int verify_opcode(struct page *pa
  * Expect the breakpoint instruction to be the smallest size instruction for
  * the architecture. If an arch has variable length instruction and the
  * breakpoint instruction is not of the smallest length instruction
- * supported by that architecture then we need to modify is_swbp_at_addr and
+ * supported by that architecture then we need to modify is_trap_at_addr and
  * write_opcode accordingly. This would never be a problem for archs that
  * have fixed length instructions.
  */
@@ -1431,7 +1454,7 @@ static void mmf_recalc_uprobes(struct mm
clear_bit(MMF_HAS_UPROBES, >flags);
 }
 
-static int is_swbp_at_addr(struct mm_struct *mm, unsigned long vaddr)
+static int is_trap_at_addr(struct mm_struct *mm, unsigned long vaddr)
 {
struct page *page;
uprobe_opcode_t opcode;
@@ -1452,7 +1475,8 @@ static int is_swbp_at_addr(struct mm_str
copy_opcode(page, vaddr, );
put_page(page);
  out:
-   return is_swbp_insn();
+   /* This needs to return true for any variant of the trap insn */
+   return is_trap_insn();
 }
 
 static struct uprobe *find_active_uprobe(unsigned long bp_vaddr, int *is_swbp)
@@ -1472,7 +1496,7 @@ static struct uprobe *find_active_uprobe
}
 
if (!uprobe)
-   *is_swbp = is_swbp_at_addr(mm, bp_vaddr);
+   *is_swbp = is_trap_at_addr(mm, bp_vaddr);
} else {
*is_swbp = -EFAULT;
}

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.htm

[PATCH 1/3] uprobes: add trap variant helper

[Ananth N Mavinakayanahalli]

From: Ananth N Mavinakayanahalli ana...@in.ibm.com

Some architectures like powerpc have multiple variants of the trap
instruction. Introduce an additional helper is_trap_insn() for run-time
handling of non-uprobe traps on such architectures.

While there, change is_swbp_at_addr() to is_trap_at_addr() for reading
clarity.

With this change, the uprobe registration path will supercede any trap
instruction inserted at the requested location, while taking care of
delivering the SIGTRAP for cases where the trap notification came in
for an address without a uprobe. See [1] for a more detailed explanation.

[1] https://lists.ozlabs.org/pipermail/linuxppc-dev/2013-March/104771.html

This change was suggested by Oleg Nesterov.

Signed-off-by: Ananth N Mavinakayanahalli ana...@in.ibm.com
---
 include/linux/uprobes.h |1 +
 kernel/events/uprobes.c |   32 
 2 files changed, 29 insertions(+), 4 deletions(-)

Index: linux-3.9-rc3/include/linux/uprobes.h
===
--- linux-3.9-rc3.orig/include/linux/uprobes.h
+++ linux-3.9-rc3/include/linux/uprobes.h
@@ -100,6 +100,7 @@ struct uprobes_state {
 extern int __weak set_swbp(struct arch_uprobe *aup, struct mm_struct *mm, 
unsigned long vaddr);
 extern int __weak set_orig_insn(struct arch_uprobe *aup, struct mm_struct *mm, 
unsigned long vaddr);
 extern bool __weak is_swbp_insn(uprobe_opcode_t *insn);
+extern bool __weak is_trap_insn(uprobe_opcode_t *insn);
 extern int uprobe_register(struct inode *inode, loff_t offset, struct 
uprobe_consumer *uc);
 extern int uprobe_apply(struct inode *inode, loff_t offset, struct 
uprobe_consumer *uc, bool);
 extern void uprobe_unregister(struct inode *inode, loff_t offset, struct 
uprobe_consumer *uc);
Index: linux-3.9-rc3/kernel/events/uprobes.c
===
--- linux-3.9-rc3.orig/kernel/events/uprobes.c
+++ linux-3.9-rc3/kernel/events/uprobes.c
@@ -173,6 +173,20 @@ bool __weak is_swbp_insn(uprobe_opcode_t
return *insn == UPROBE_SWBP_INSN;
 }
 
+/**
+ * is_trap_insn - check if instruction is breakpoint instruction.
+ * @insn: instruction to be checked.
+ * Default implementation of is_trap_insn
+ * Returns true if @insn is a breakpoint instruction.
+ *
+ * This function is needed for the case where an architecture has multiple
+ * trap instructions (like powerpc).
+ */
+bool __weak is_trap_insn(uprobe_opcode_t *insn)
+{
+   return is_swbp_insn(insn);
+}
+
 static void copy_opcode(struct page *page, unsigned long vaddr, 
uprobe_opcode_t *opcode)
 {
void *kaddr = kmap_atomic(page);
@@ -185,6 +199,15 @@ static int verify_opcode(struct page *pa
uprobe_opcode_t old_opcode;
bool is_swbp;
 
+   /*
+* Note: We only check if the old_opcode is UPROBE_SWBP_INSN here.
+* We do not check if it is any other 'trap variant' which could
+* be conditional trap instruction such as the one powerpc supports.
+*
+* The logic is that we do not care if the underlying instruction
+* is a trap variant; uprobes always wins over any other (gdb)
+* breakpoint.
+*/
copy_opcode(page, vaddr, old_opcode);
is_swbp = is_swbp_insn(old_opcode);
 
@@ -204,7 +227,7 @@ static int verify_opcode(struct page *pa
  * Expect the breakpoint instruction to be the smallest size instruction for
  * the architecture. If an arch has variable length instruction and the
  * breakpoint instruction is not of the smallest length instruction
- * supported by that architecture then we need to modify is_swbp_at_addr and
+ * supported by that architecture then we need to modify is_trap_at_addr and
  * write_opcode accordingly. This would never be a problem for archs that
  * have fixed length instructions.
  */
@@ -1431,7 +1454,7 @@ static void mmf_recalc_uprobes(struct mm
clear_bit(MMF_HAS_UPROBES, mm-flags);
 }
 
-static int is_swbp_at_addr(struct mm_struct *mm, unsigned long vaddr)
+static int is_trap_at_addr(struct mm_struct *mm, unsigned long vaddr)
 {
struct page *page;
uprobe_opcode_t opcode;
@@ -1452,7 +1475,8 @@ static int is_swbp_at_addr(struct mm_str
copy_opcode(page, vaddr, opcode);
put_page(page);
  out:
-   return is_swbp_insn(opcode);
+   /* This needs to return true for any variant of the trap insn */
+   return is_trap_insn(opcode);
 }
 
 static struct uprobe *find_active_uprobe(unsigned long bp_vaddr, int *is_swbp)
@@ -1472,7 +1496,7 @@ static struct uprobe *find_active_uprobe
}
 
if (!uprobe)
-   *is_swbp = is_swbp_at_addr(mm, bp_vaddr);
+   *is_swbp = is_trap_at_addr(mm, bp_vaddr);
} else {
*is_swbp = -EFAULT;
}

--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info

[PATCH 2/3] uprobes/powerpc: teach uprobes to ignore gdb breakpoints

[Ananth N Mavinakayanahalli]

From: Ananth N Mavinakayanahalli ana...@in.ibm.com

Powerpc has many trap variants that could be used by entities like gdb.
Currently, running gdb on a program being traced by uprobes causes an
endless loop since uprobes doesn't understand that the trap was inserted
by some other entity and a SIGTRAP needs to be delivered.

Teach uprobes to ignore breakpoints that do not belong to it.

Signed-off-by: Ananth N Mavinakayanahalli ana...@in.ibm.com
---
 arch/powerpc/kernel/uprobes.c |   10 ++
 1 file changed, 10 insertions(+)

Index: linux-3.9-rc3/arch/powerpc/kernel/uprobes.c
===
--- linux-3.9-rc3.orig/arch/powerpc/kernel/uprobes.c
+++ linux-3.9-rc3/arch/powerpc/kernel/uprobes.c
@@ -31,6 +31,16 @@
 #define UPROBE_TRAP_NR UINT_MAX
 
 /**
+ * is_trap_insn - check if the instruction is a trap variant
+ * @insn: instruction to be checked.
+ * Returns true if @insn is a trap variant.
+ */
+bool is_trap_insn(uprobe_opcode_t *insn)
+{
+   return (is_trap(*insn));
+}
+
+/**
  * arch_uprobe_analyze_insn
  * @mm: the probed address space.
  * @arch_uprobe: the probepoint information.

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

[PATCH 3/3] uprobes/powerpc: ignore trap variants during register

[Ananth N Mavinakayanahalli]

From: Ananth N Mavinakayanahalli ana...@in.ibm.com

The current implementation of uprobes assumes that uprobes always wins
even when a register request is at a location with a conditional
breakpoint by some other entity. Refer to [1] for more details.

Remove the breakpoint instruction check during registration on powerpc,
so that uprobes behavior on powerpc matches that of x86.

[1] https://lists.ozlabs.org/pipermail/linuxppc-dev/2013-March/104771.html

Signed-off-by: Ananth N Mavinakayanahalli ana...@in.ibm.com
---
 arch/powerpc/kernel/uprobes.c |6 --
 1 file changed, 6 deletions(-)

Index: linux-3.9-rc3/arch/powerpc/kernel/uprobes.c
===
--- linux-3.9-rc3.orig/arch/powerpc/kernel/uprobes.c
+++ linux-3.9-rc3/arch/powerpc/kernel/uprobes.c
@@ -53,12 +53,6 @@ int arch_uprobe_analyze_insn(struct arch
if (addr  0x03)
return -EINVAL;
 
-   /*
-* We currently don't support a uprobe on an already
-* existing breakpoint instruction underneath
-*/
-   if (is_trap(auprobe-ainsn))
-   return -ENOTSUPP;
return 0;
 }
 

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

Re: [PATCH 1/3] uprobes: add trap variant helper

[Ananth N Mavinakayanahalli]

On Fri, Mar 22, 2013 at 03:54:06PM +0100, Oleg Nesterov wrote:
 On 03/22, Ananth N Mavinakayanahalli wrote:
 
  +/**
  + * is_trap_insn - check if instruction is breakpoint instruction.
  + * @insn: instruction to be checked.
  + * Default implementation of is_trap_insn
  + * Returns true if @insn is a breakpoint instruction.
  + *
  + * This function is needed for the case where an architecture has multiple
  + * trap instructions (like powerpc).
  + */
  +bool __weak is_trap_insn(uprobe_opcode_t *insn)
  +{
  +   return is_swbp_insn(insn);
  +}
 
 OK, thanks, the whole series looks fine, just one note...

OK.

 My patch also changed prepare_uprobe() to use is_trap_insn(), and I think
 this is right. Exactly because of 3/3 which removes is_trap() from
 arch_uprobe_analyze_insn().
 
 If the original insn is_trap(), we do not want to singlestep it and get
 another trap after we hit handle_swbp().

OK, I'll send a v2 with that change.

Ananth

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

[PATCH v2 1/4] uprobes: add trap variant helper

[Ananth N Mavinakayanahalli]

From: Ananth N Mavinakayanahalli ana...@in.ibm.com

Some architectures like powerpc have multiple variants of the trap
instruction. Introduce an additional helper is_trap_insn() for run-time
handling of non-uprobe traps on such architectures.

While there, change is_swbp_at_addr() to is_trap_at_addr() for reading
clarity.

With this change, the uprobe registration path will supercede any trap
instruction inserted at the requested location, while taking care of
delivering the SIGTRAP for cases where the trap notification came in
for an address without a uprobe. See [1] for a more detailed explanation.

[1] https://lists.ozlabs.org/pipermail/linuxppc-dev/2013-March/104771.html

This change was suggested by Oleg Nesterov.

Signed-off-by: Ananth N Mavinakayanahalli ana...@in.ibm.com
---
 include/linux/uprobes.h |1 +
 kernel/events/uprobes.c |   32 
 2 files changed, 29 insertions(+), 4 deletions(-)

Index: linux-3.9-rc3/include/linux/uprobes.h
===
--- linux-3.9-rc3.orig/include/linux/uprobes.h
+++ linux-3.9-rc3/include/linux/uprobes.h
@@ -100,6 +100,7 @@ struct uprobes_state {
 extern int __weak set_swbp(struct arch_uprobe *aup, struct mm_struct *mm, 
unsigned long vaddr);
 extern int __weak set_orig_insn(struct arch_uprobe *aup, struct mm_struct *mm, 
unsigned long vaddr);
 extern bool __weak is_swbp_insn(uprobe_opcode_t *insn);
+extern bool __weak is_trap_insn(uprobe_opcode_t *insn);
 extern int uprobe_register(struct inode *inode, loff_t offset, struct 
uprobe_consumer *uc);
 extern int uprobe_apply(struct inode *inode, loff_t offset, struct 
uprobe_consumer *uc, bool);
 extern void uprobe_unregister(struct inode *inode, loff_t offset, struct 
uprobe_consumer *uc);
Index: linux-3.9-rc3/kernel/events/uprobes.c
===
--- linux-3.9-rc3.orig/kernel/events/uprobes.c
+++ linux-3.9-rc3/kernel/events/uprobes.c
@@ -173,6 +173,20 @@ bool __weak is_swbp_insn(uprobe_opcode_t
return *insn == UPROBE_SWBP_INSN;
 }
 
+/**
+ * is_trap_insn - check if instruction is breakpoint instruction.
+ * @insn: instruction to be checked.
+ * Default implementation of is_trap_insn
+ * Returns true if @insn is a breakpoint instruction.
+ *
+ * This function is needed for the case where an architecture has multiple
+ * trap instructions (like powerpc).
+ */
+bool __weak is_trap_insn(uprobe_opcode_t *insn)
+{
+   return is_swbp_insn(insn);
+}
+
 static void copy_opcode(struct page *page, unsigned long vaddr, 
uprobe_opcode_t *opcode)
 {
void *kaddr = kmap_atomic(page);
@@ -185,6 +199,15 @@ static int verify_opcode(struct page *pa
uprobe_opcode_t old_opcode;
bool is_swbp;
 
+   /*
+* Note: We only check if the old_opcode is UPROBE_SWBP_INSN here.
+* We do not check if it is any other 'trap variant' which could
+* be conditional trap instruction such as the one powerpc supports.
+*
+* The logic is that we do not care if the underlying instruction
+* is a trap variant; uprobes always wins over any other (gdb)
+* breakpoint.
+*/
copy_opcode(page, vaddr, old_opcode);
is_swbp = is_swbp_insn(old_opcode);
 
@@ -204,7 +227,7 @@ static int verify_opcode(struct page *pa
  * Expect the breakpoint instruction to be the smallest size instruction for
  * the architecture. If an arch has variable length instruction and the
  * breakpoint instruction is not of the smallest length instruction
- * supported by that architecture then we need to modify is_swbp_at_addr and
+ * supported by that architecture then we need to modify is_trap_at_addr and
  * write_opcode accordingly. This would never be a problem for archs that
  * have fixed length instructions.
  */
@@ -1431,7 +1454,7 @@ static void mmf_recalc_uprobes(struct mm
clear_bit(MMF_HAS_UPROBES, mm-flags);
 }
 
-static int is_swbp_at_addr(struct mm_struct *mm, unsigned long vaddr)
+static int is_trap_at_addr(struct mm_struct *mm, unsigned long vaddr)
 {
struct page *page;
uprobe_opcode_t opcode;
@@ -1452,7 +1475,8 @@ static int is_swbp_at_addr(struct mm_str
copy_opcode(page, vaddr, opcode);
put_page(page);
  out:
-   return is_swbp_insn(opcode);
+   /* This needs to return true for any variant of the trap insn */
+   return is_trap_insn(opcode);
 }
 
 static struct uprobe *find_active_uprobe(unsigned long bp_vaddr, int *is_swbp)
@@ -1472,7 +1496,7 @@ static struct uprobe *find_active_uprobe
}
 
if (!uprobe)
-   *is_swbp = is_swbp_at_addr(mm, bp_vaddr);
+   *is_swbp = is_trap_at_addr(mm, bp_vaddr);
} else {
*is_swbp = -EFAULT;
}

--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info

[PATCH v2 2/4] uprobes: refuse uprobe on trap variants

[Ananth N Mavinakayanahalli]

From: Ananth N Mavinakayanahalli ana...@in.ibm.com

Refuse to place a uprobe if a trap variant already exists in the
file copy at the address.

Signed-off-by: Ananth N Mavinakayanahalli ana...@in.ibm.com
---
 kernel/events/uprobes.c |2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Index: linux-3.9-rc3/kernel/events/uprobes.c
===
--- linux-3.9-rc3.orig/kernel/events/uprobes.c
+++ linux-3.9-rc3/kernel/events/uprobes.c
@@ -573,7 +573,7 @@ static int prepare_uprobe(struct uprobe
goto out;
 
ret = -ENOTSUPP;
-   if (is_swbp_insn((uprobe_opcode_t *)uprobe-arch.insn))
+   if (is_trap_insn((uprobe_opcode_t *)uprobe-arch.insn))
goto out;
 
ret = arch_uprobe_analyze_insn(uprobe-arch, mm, vaddr);

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

[PATCH v2 3/4] uprobes/powerpc: teach uprobes to ignore gdb breakpoints

[Ananth N Mavinakayanahalli]

From: Ananth N Mavinakayanahalli ana...@in.ibm.com

Powerpc has many trap variants that could be used by entities like gdb.
Currently, running gdb on a program being traced by uprobes causes an
endless loop since uprobes doesn't understand that the trap was inserted
by some other entity and a SIGTRAP needs to be delivered.

Teach uprobes to ignore breakpoints that do not belong to it.

Signed-off-by: Ananth N Mavinakayanahalli ana...@in.ibm.com
---
 arch/powerpc/kernel/uprobes.c |   10 ++
 1 file changed, 10 insertions(+)

Index: linux-3.9-rc3/arch/powerpc/kernel/uprobes.c
===
--- linux-3.9-rc3.orig/arch/powerpc/kernel/uprobes.c
+++ linux-3.9-rc3/arch/powerpc/kernel/uprobes.c
@@ -31,6 +31,16 @@
 #define UPROBE_TRAP_NR UINT_MAX
 
 /**
+ * is_trap_insn - check if the instruction is a trap variant
+ * @insn: instruction to be checked.
+ * Returns true if @insn is a trap variant.
+ */
+bool is_trap_insn(uprobe_opcode_t *insn)
+{
+   return (is_trap(*insn));
+}
+
+/**
  * arch_uprobe_analyze_insn
  * @mm: the probed address space.
  * @arch_uprobe: the probepoint information.

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

[PATCH v2 4/4] uprobes/powerpc: remove additional trap instruction check

[Ananth N Mavinakayanahalli]

From: Ananth N Mavinakayanahalli ana...@in.ibm.com

prepare_uprobe() already checks if the underlying unstruction
(on file) is a trap variant. We don't need to check this again.

Signed-off-by: Ananth N Mavinakayanahalli ana...@in.ibm.com
---
 arch/powerpc/kernel/uprobes.c |6 --
 1 file changed, 6 deletions(-)

Index: linux-3.9-rc3/arch/powerpc/kernel/uprobes.c
===
--- linux-3.9-rc3.orig/arch/powerpc/kernel/uprobes.c
+++ linux-3.9-rc3/arch/powerpc/kernel/uprobes.c
@@ -53,12 +53,6 @@ int arch_uprobe_analyze_insn(struct arch
if (addr  0x03)
return -EINVAL;
 
-   /*
-* We currently don't support a uprobe on an already
-* existing breakpoint instruction underneath
-*/
-   if (is_trap(auprobe-ainsn))
-   return -ENOTSUPP;
return 0;
 }
 

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

[tip:perf/urgent] perf probe: Fix segfault

[tip-bot for Ananth N Mavinakayanahalli]

Commit-ID:  79146a69c8bc3f28e51c5267633abc6babf47a31
Gitweb: http://git.kernel.org/tip/79146a69c8bc3f28e51c5267633abc6babf47a31
Author: Ananth N Mavinakayanahalli 
AuthorDate: Tue, 12 Mar 2013 14:32:17 +0530
Committer:  Arnaldo Carvalho de Melo 
CommitDate: Wed, 13 Mar 2013 17:00:33 -0300

perf probe: Fix segfault

Fix segfault in perf probe due to a bug introduced by commit d8639f068
(perf tools: Stop using 'self' in strlist).

Signed-off-by: Ananth N Mavinakayanahalli 
Acked-by: Srikar Dronamraju 
Cc: Srikar Dronamraju 
Link: http://lkml.kernel.org/r/20130312090217.gc4...@in.ibm.com
Signed-off-by: Arnaldo Carvalho de Melo 
---
 tools/perf/util/strlist.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/perf/util/strlist.c b/tools/perf/util/strlist.c
index 55433aa..eabdce0 100644
--- a/tools/perf/util/strlist.c
+++ b/tools/perf/util/strlist.c
@@ -143,7 +143,7 @@ struct strlist *strlist__new(bool dupstr, const char *list)
slist->rblist.node_delete = strlist__node_delete;
 
slist->dupstr= dupstr;
-   if (slist && strlist__parse_list(slist, list) != 0)
+   if (list && strlist__parse_list(slist, list) != 0)
goto out_error;
}
 
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[tip:perf/urgent] perf probe: Fix segfault

[tip-bot for Ananth N Mavinakayanahalli]

Commit-ID:  79146a69c8bc3f28e51c5267633abc6babf47a31
Gitweb: http://git.kernel.org/tip/79146a69c8bc3f28e51c5267633abc6babf47a31
Author: Ananth N Mavinakayanahalli ana...@in.ibm.com
AuthorDate: Tue, 12 Mar 2013 14:32:17 +0530
Committer:  Arnaldo Carvalho de Melo a...@redhat.com
CommitDate: Wed, 13 Mar 2013 17:00:33 -0300

perf probe: Fix segfault

Fix segfault in perf probe due to a bug introduced by commit d8639f068
(perf tools: Stop using 'self' in strlist).

Signed-off-by: Ananth N Mavinakayanahalli ana...@in.ibm.com
Acked-by: Srikar Dronamraju sri...@linux.vnet.ibm.com
Cc: Srikar Dronamraju sri...@linux.vnet.ibm.com
Link: http://lkml.kernel.org/r/20130312090217.gc4...@in.ibm.com
Signed-off-by: Arnaldo Carvalho de Melo a...@redhat.com
---
 tools/perf/util/strlist.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/perf/util/strlist.c b/tools/perf/util/strlist.c
index 55433aa..eabdce0 100644
--- a/tools/perf/util/strlist.c
+++ b/tools/perf/util/strlist.c
@@ -143,7 +143,7 @@ struct strlist *strlist__new(bool dupstr, const char *list)
slist-rblist.node_delete = strlist__node_delete;
 
slist-dupstr= dupstr;
-   if (slist  strlist__parse_list(slist, list) != 0)
+   if (list  strlist__parse_list(slist, list) != 0)
goto out_error;
}
 
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH -tip v2 2/2] [BUGFIX] kprobes/x86: Check Interrupt Flag modifier when registering probe

[Ananth N Mavinakayanahalli]

On Thu, Mar 14, 2013 at 08:52:43PM +0900, Masami Hiramatsu wrote:
> Currently kprobes check whether the copied instruction modifies
> IF (interrupt flag) on each probe hit. This means not only
> introducing overhead but also involving inat_get_opcode_attribute
> into kprobes hot path, and it can cause an infinit recursive
> call (and kernel panic in the end).
> 
> Actually, since the copied instruction itself never be modified
> on the buffer, it is needless to analyze the instruction every
> probe hit.
> 
> To fix this issue, we checks it only once when registering probe
> and store the result on ainsn->if_modifier.
> 
> Signed-off-by: Masami Hiramatsu 
> Reported-by: Timo Juhani Lindfors 
> Cc: "David S. Miller" 
> Cc: Ananth N Mavinakayanahalli 
> Cc: Thomas Gleixner 
> Cc: Ingo Molnar 
> Cc: "H. Peter Anvin" 
> Cc: Steven Rostedt 
> Cc: Linus Torvalds 

Acked-by: Ananth N Mavinakayanahalli 

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

Re: [PATCH -tip v2 1/2] [BUGFIX] kprobes: make hash_64() as always inlined

[Ananth N Mavinakayanahalli]

On Thu, Mar 14, 2013 at 08:52:30PM +0900, Masami Hiramatsu wrote:
> Because hash_64() is called from the get_kprobe() inside
> int3 handler, kernel causes int3 recursion and crashes if
> kprobes user puts a probe on it.
> 
> Usually hash_64() is inlined into caller function, but in
> some cases, it has instances by gcc's interprocedural
> constant propagation.
> 
> This patch uses __always_inline instead of inline to
> prevent gcc from doing such things.
> 
> Changes in v2:
>  - Use __always_inline instead of using __kprobes
> 
> Signed-off-by: Masami Hiramatsu 
> Reported-by: Timo Juhani Lindfors 
> Cc: "David S. Miller" 
> Cc: Nadia Yvette Chambers 
> Cc: Pavel Emelyanov 
> Cc: Jiri Kosina 
> Cc: Ananth N Mavinakayanahalli 
> Cc: Ingo Molnar 
> Cc: Linus Torvalds 

Acked-by: Ananth N Mavinakayanahalli 

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

Re: [PATCH -tip v2 1/2] [BUGFIX] kprobes: make hash_64() as always inlined

[Ananth N Mavinakayanahalli]

On Thu, Mar 14, 2013 at 08:52:30PM +0900, Masami Hiramatsu wrote:
 Because hash_64() is called from the get_kprobe() inside
 int3 handler, kernel causes int3 recursion and crashes if
 kprobes user puts a probe on it.
 
 Usually hash_64() is inlined into caller function, but in
 some cases, it has instances by gcc's interprocedural
 constant propagation.
 
 This patch uses __always_inline instead of inline to
 prevent gcc from doing such things.
 
 Changes in v2:
  - Use __always_inline instead of using __kprobes
 
 Signed-off-by: Masami Hiramatsu masami.hiramatsu...@hitachi.com
 Reported-by: Timo Juhani Lindfors timo.lindf...@iki.fi
 Cc: David S. Miller da...@davemloft.net
 Cc: Nadia Yvette Chambers n...@holomorphy.com
 Cc: Pavel Emelyanov xe...@parallels.com
 Cc: Jiri Kosina jkos...@suse.cz
 Cc: Ananth N Mavinakayanahalli ana...@in.ibm.com
 Cc: Ingo Molnar mi...@kernel.org
 Cc: Linus Torvalds torva...@linux-foundation.org

Acked-by: Ananth N Mavinakayanahalli ana...@in.ibm.com

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

Re: [PATCH -tip v2 2/2] [BUGFIX] kprobes/x86: Check Interrupt Flag modifier when registering probe

[Ananth N Mavinakayanahalli]

On Thu, Mar 14, 2013 at 08:52:43PM +0900, Masami Hiramatsu wrote:
 Currently kprobes check whether the copied instruction modifies
 IF (interrupt flag) on each probe hit. This means not only
 introducing overhead but also involving inat_get_opcode_attribute
 into kprobes hot path, and it can cause an infinit recursive
 call (and kernel panic in the end).
 
 Actually, since the copied instruction itself never be modified
 on the buffer, it is needless to analyze the instruction every
 probe hit.
 
 To fix this issue, we checks it only once when registering probe
 and store the result on ainsn-if_modifier.
 
 Signed-off-by: Masami Hiramatsu masami.hiramatsu...@hitachi.com
 Reported-by: Timo Juhani Lindfors timo.lindf...@iki.fi
 Cc: David S. Miller da...@davemloft.net
 Cc: Ananth N Mavinakayanahalli ana...@in.ibm.com
 Cc: Thomas Gleixner t...@linutronix.de
 Cc: Ingo Molnar mi...@redhat.com
 Cc: H. Peter Anvin h...@zytor.com
 Cc: Steven Rostedt rost...@goodmis.org
 Cc: Linus Torvalds torva...@linux-foundation.org

Acked-by: Ananth N Mavinakayanahalli ana...@in.ibm.com

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

[PATCH] Fix segfault in perf probe

[Ananth N Mavinakayanahalli]

From: Ananth N Mavinakayanahalli 

Fix segfault in perf probe due to a bug introduced by commit d8639f068
(perf tools: Stop using 'self' in strlist).


Signed-off-by: Ananth N Mavinakayanahalli 

---
Index: linus/tools/perf/util/strlist.c
===
--- linus.orig/tools/perf/util/strlist.c
+++ linus/tools/perf/util/strlist.c
@@ -143,7 +143,7 @@ struct strlist *strlist__new(bool dupstr
slist->rblist.node_delete = strlist__node_delete;
 
slist->dupstr= dupstr;
-   if (slist && strlist__parse_list(slist, list) != 0)
+   if (list && strlist__parse_list(slist, list) != 0)
goto out_error;
}
 

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