All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] x86, kprobes: Hide skip_singlestep forward declaration properly
@ 2012-09-19 13:46 Borislav Petkov
  2012-09-20  9:01 ` Masami Hiramatsu
  0 siblings, 1 reply; 10+ messages in thread
From: Borislav Petkov @ 2012-09-19 13:46 UTC (permalink / raw)
  To: LKML; +Cc: X86-ML, Borislav Petkov, Masami Hiramatsu, Steven Rostedt

From: Borislav Petkov <borislav.petkov@amd.com>

I get

arch/x86/kernel/kprobes.c:544:23: warning: ‘skip_singlestep’ declared ‘static’ but never defined [-Wunused-function]

on tip/auto-latest.

Hide the forward declaration in the KPROBES_CAN_USE_FTRACE as it
is done in its callsites.

Cc: Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>
Cc: Steven Rostedt <rostedt@goodmis.org>
Signed-off-by: Borislav Petkov <borislav.petkov@amd.com>
---
 arch/x86/kernel/kprobes.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/arch/x86/kernel/kprobes.c b/arch/x86/kernel/kprobes.c
index b7c2a85d1926..ac4188368fdf 100644
--- a/arch/x86/kernel/kprobes.c
+++ b/arch/x86/kernel/kprobes.c
@@ -541,8 +541,11 @@ reenter_kprobe(struct kprobe *p, struct pt_regs *regs, struct kprobe_ctlblk *kcb
 	return 1;
 }
 
+#ifdef KPROBES_CAN_USE_FTRACE
 static void __kprobes skip_singlestep(struct kprobe *p, struct pt_regs *regs,
 				      struct kprobe_ctlblk *kcb);
+#endif
+
 /*
  * Interrupts are disabled on entry as trap3 is an interrupt gate and they
  * remain disabled throughout this function.
-- 
1.7.11.rc1


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* Re: [PATCH] x86, kprobes: Hide skip_singlestep forward declaration properly
  2012-09-19 13:46 [PATCH] x86, kprobes: Hide skip_singlestep forward declaration properly Borislav Petkov
@ 2012-09-20  9:01 ` Masami Hiramatsu
  2012-09-20  9:25   ` Ingo Molnar
  0 siblings, 1 reply; 10+ messages in thread
From: Masami Hiramatsu @ 2012-09-20  9:01 UTC (permalink / raw)
  To: Borislav Petkov, Ingo Molnar
  Cc: LKML, X86-ML, Borislav Petkov, Steven Rostedt

(2012/09/19 22:46), Borislav Petkov wrote:
> From: Borislav Petkov <borislav.petkov@amd.com>
> 
> I get
> 
> arch/x86/kernel/kprobes.c:544:23: warning: ‘skip_singlestep’ declared ‘static’ but never defined [-Wunused-function]
> 
> on tip/auto-latest.
> 
> Hide the forward declaration in the KPROBES_CAN_USE_FTRACE as it
> is done in its callsites.

Thank you for reporting and fixing :)

Acked-by: Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>

Ingo, please pull this.

Thank you,

> 
> Cc: Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>
> Cc: Steven Rostedt <rostedt@goodmis.org>
> Signed-off-by: Borislav Petkov <borislav.petkov@amd.com>
> ---
>  arch/x86/kernel/kprobes.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/arch/x86/kernel/kprobes.c b/arch/x86/kernel/kprobes.c
> index b7c2a85d1926..ac4188368fdf 100644
> --- a/arch/x86/kernel/kprobes.c
> +++ b/arch/x86/kernel/kprobes.c
> @@ -541,8 +541,11 @@ reenter_kprobe(struct kprobe *p, struct pt_regs *regs, struct kprobe_ctlblk *kcb
>  	return 1;
>  }
>  
> +#ifdef KPROBES_CAN_USE_FTRACE
>  static void __kprobes skip_singlestep(struct kprobe *p, struct pt_regs *regs,
>  				      struct kprobe_ctlblk *kcb);
> +#endif
> +
>  /*
>   * Interrupts are disabled on entry as trap3 is an interrupt gate and they
>   * remain disabled throughout this function.
> 


-- 
Masami HIRAMATSU
Software Platform Research Dept. Linux Technology Center
Hitachi, Ltd., Yokohama Research Laboratory
E-mail: masami.hiramatsu.pt@hitachi.com



^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] x86, kprobes: Hide skip_singlestep forward declaration properly
  2012-09-20  9:01 ` Masami Hiramatsu
@ 2012-09-20  9:25   ` Ingo Molnar
  2012-09-20  9:36     ` Masami Hiramatsu
  0 siblings, 1 reply; 10+ messages in thread
From: Ingo Molnar @ 2012-09-20  9:25 UTC (permalink / raw)
  To: Masami Hiramatsu
  Cc: Borislav Petkov, Ingo Molnar, LKML, X86-ML, Borislav Petkov,
	Steven Rostedt


* Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com> wrote:

> (2012/09/19 22:46), Borislav Petkov wrote:
> > From: Borislav Petkov <borislav.petkov@amd.com>
> > 
> > I get
> > 
> > arch/x86/kernel/kprobes.c:544:23: warning: ‘skip_singlestep’ declared ‘static’ but never defined [-Wunused-function]
> > 
> > on tip/auto-latest.
> > 
> > Hide the forward declaration in the KPROBES_CAN_USE_FTRACE as it
> > is done in its callsites.
> 
> Thank you for reporting and fixing :)

Why is a forward declation needed, why isn't the function in the 
proper spot in the file to begin with?

Thanks,

	Ingo

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] x86, kprobes: Hide skip_singlestep forward declaration properly
  2012-09-20  9:25   ` Ingo Molnar
@ 2012-09-20  9:36     ` Masami Hiramatsu
  2012-09-20  9:40       ` Ingo Molnar
  0 siblings, 1 reply; 10+ messages in thread
From: Masami Hiramatsu @ 2012-09-20  9:36 UTC (permalink / raw)
  To: Ingo Molnar
  Cc: Borislav Petkov, Ingo Molnar, LKML, X86-ML, Borislav Petkov,
	Steven Rostedt

(2012/09/20 18:25), Ingo Molnar wrote:
> 
> * Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com> wrote:
> 
>> (2012/09/19 22:46), Borislav Petkov wrote:
>>> From: Borislav Petkov <borislav.petkov@amd.com>
>>>
>>> I get
>>>
>>> arch/x86/kernel/kprobes.c:544:23: warning: ‘skip_singlestep’ declared ‘static’ but never defined [-Wunused-function]
>>>
>>> on tip/auto-latest.
>>>
>>> Hide the forward declaration in the KPROBES_CAN_USE_FTRACE as it
>>> is done in its callsites.
>>
>> Thank you for reporting and fixing :)
> 
> Why is a forward declation needed, why isn't the function in the 
> proper spot in the file to begin with?

There is no reason, ftrace-related things can also be moved on top.
Borislav, could you move it?

Thanks,

-- 
Masami HIRAMATSU
Software Platform Research Dept. Linux Technology Center
Hitachi, Ltd., Yokohama Research Laboratory
E-mail: masami.hiramatsu.pt@hitachi.com



^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] x86, kprobes: Hide skip_singlestep forward declaration properly
  2012-09-20  9:36     ` Masami Hiramatsu
@ 2012-09-20  9:40       ` Ingo Molnar
  2012-09-20 10:33         ` Borislav Petkov
  2012-09-24 10:28         ` [PATCH] x86, kprobes: Hide skip_singlestep forward declaration properly Masami Hiramatsu
  0 siblings, 2 replies; 10+ messages in thread
From: Ingo Molnar @ 2012-09-20  9:40 UTC (permalink / raw)
  To: Masami Hiramatsu
  Cc: Borislav Petkov, Ingo Molnar, LKML, X86-ML, Borislav Petkov,
	Steven Rostedt


* Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com> wrote:

> (2012/09/20 18:25), Ingo Molnar wrote:
> > 
> > * Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com> wrote:
> > 
> >> (2012/09/19 22:46), Borislav Petkov wrote:
> >>> From: Borislav Petkov <borislav.petkov@amd.com>
> >>>
> >>> I get
> >>>
> >>> arch/x86/kernel/kprobes.c:544:23: warning: ‘skip_singlestep’ declared ‘static’ but never defined [-Wunused-function]
> >>>
> >>> on tip/auto-latest.
> >>>
> >>> Hide the forward declaration in the KPROBES_CAN_USE_FTRACE as it
> >>> is done in its callsites.
> >>
> >> Thank you for reporting and fixing :)
> > 
> > Why is a forward declation needed, why isn't the function in the 
> > proper spot in the file to begin with?
> 
> There is no reason, ftrace-related things can also be moved on top.
> Borislav, could you move it?

Could be moved into a separate file as well - we could have 
arch/x86/kprobes/, with core.c, opt.c, ftrace.c and common.h in 
it - possibly more in the future.

Thanks,

	Ingo

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] x86, kprobes: Hide skip_singlestep forward declaration properly
  2012-09-20  9:40       ` Ingo Molnar
@ 2012-09-20 10:33         ` Borislav Petkov
  2012-09-20 11:05           ` Ingo Molnar
  2012-09-24 10:28         ` [PATCH] x86, kprobes: Hide skip_singlestep forward declaration properly Masami Hiramatsu
  1 sibling, 1 reply; 10+ messages in thread
From: Borislav Petkov @ 2012-09-20 10:33 UTC (permalink / raw)
  To: Ingo Molnar
  Cc: Masami Hiramatsu, Borislav Petkov, Ingo Molnar, LKML, X86-ML,
	Steven Rostedt

On Thu, Sep 20, 2012 at 11:40:17AM +0200, Ingo Molnar wrote:
> Could be moved into a separate file as well - we could have
> arch/x86/kprobes/, with core.c, opt.c, ftrace.c and common.h in it -
> possibly more in the future.

So, I'm guessing I should move only the function now and let Masami do
the proper splitup later, correct?

-- 
Regards/Gruss,
Boris.

Advanced Micro Devices GmbH
Einsteinring 24, 85609 Dornach
GM: Alberto Bozzo
Reg: Dornach, Landkreis Muenchen
HRB Nr. 43632 WEEE Registernr: 129 19551

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] x86, kprobes: Hide skip_singlestep forward declaration properly
  2012-09-20 10:33         ` Borislav Petkov
@ 2012-09-20 11:05           ` Ingo Molnar
  2012-09-20 12:43             ` [PATCH -v2] x86, kprobes: Move skip_singlestep up Borislav Petkov
  0 siblings, 1 reply; 10+ messages in thread
From: Ingo Molnar @ 2012-09-20 11:05 UTC (permalink / raw)
  To: Borislav Petkov
  Cc: Masami Hiramatsu, Ingo Molnar, LKML, X86-ML, Steven Rostedt


* Borislav Petkov <bp@amd64.org> wrote:

> On Thu, Sep 20, 2012 at 11:40:17AM +0200, Ingo Molnar wrote:
>
> > Could be moved into a separate file as well - we could have 
> > arch/x86/kprobes/, with core.c, opt.c, ftrace.c and common.h 
> > in it - possibly more in the future.
> 
> So, I'm guessing I should move only the function now and let 
> Masami do the proper splitup later, correct?

Yeah, I guess.

Thanks,

	Ingo

^ permalink raw reply	[flat|nested] 10+ messages in thread

* [PATCH -v2] x86, kprobes: Move skip_singlestep up
  2012-09-20 11:05           ` Ingo Molnar
@ 2012-09-20 12:43             ` Borislav Petkov
  2012-09-20 14:15               ` [tip:perf/core] kprobes/x86: " tip-bot for Borislav Petkov
  0 siblings, 1 reply; 10+ messages in thread
From: Borislav Petkov @ 2012-09-20 12:43 UTC (permalink / raw)
  To: Ingo Molnar
  Cc: Masami Hiramatsu, Steven Rostedt, X86-ML, LKML, Borislav Petkov

From: Borislav Petkov <borislav.petkov@amd.com>

I get

arch/x86/kernel/kprobes.c:544:23: warning: ‘skip_singlestep’ declared ‘static’ but never defined [-Wunused-function]

on tip/auto-latest.

Put the skip_singlestep function declaration up, in KPROBES_CAN_USE_FTRACE
and drop the superfluous forward declaration.

Cc: Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>
Cc: Steven Rostedt <rostedt@goodmis.org>
Signed-off-by: Borislav Petkov <borislav.petkov@amd.com>
---
 arch/x86/kernel/kprobes.c | 32 ++++++++++++++++----------------
 1 file changed, 16 insertions(+), 16 deletions(-)

diff --git a/arch/x86/kernel/kprobes.c b/arch/x86/kernel/kprobes.c
index b7c2a85d1926..57916c0d3cf6 100644
--- a/arch/x86/kernel/kprobes.c
+++ b/arch/x86/kernel/kprobes.c
@@ -541,8 +541,23 @@ reenter_kprobe(struct kprobe *p, struct pt_regs *regs, struct kprobe_ctlblk *kcb
 	return 1;
 }
 
+#ifdef KPROBES_CAN_USE_FTRACE
 static void __kprobes skip_singlestep(struct kprobe *p, struct pt_regs *regs,
-				      struct kprobe_ctlblk *kcb);
+				      struct kprobe_ctlblk *kcb)
+{
+	/*
+	 * Emulate singlestep (and also recover regs->ip)
+	 * as if there is a 5byte nop
+	 */
+	regs->ip = (unsigned long)p->addr + MCOUNT_INSN_SIZE;
+	if (unlikely(p->post_handler)) {
+		kcb->kprobe_status = KPROBE_HIT_SSDONE;
+		p->post_handler(p, regs, 0);
+	}
+	__this_cpu_write(current_kprobe, NULL);
+}
+#endif
+
 /*
  * Interrupts are disabled on entry as trap3 is an interrupt gate and they
  * remain disabled throughout this function.
@@ -1061,21 +1076,6 @@ int __kprobes longjmp_break_handler(struct kprobe *p, struct pt_regs *regs)
 }
 
 #ifdef KPROBES_CAN_USE_FTRACE
-static void __kprobes skip_singlestep(struct kprobe *p, struct pt_regs *regs,
-				      struct kprobe_ctlblk *kcb)
-{
-	/*
-	 * Emulate singlestep (and also recover regs->ip)
-	 * as if there is a 5byte nop
-	 */
-	regs->ip = (unsigned long)p->addr + MCOUNT_INSN_SIZE;
-	if (unlikely(p->post_handler)) {
-		kcb->kprobe_status = KPROBE_HIT_SSDONE;
-		p->post_handler(p, regs, 0);
-	}
-	__this_cpu_write(current_kprobe, NULL);
-}
-
 /* Ftrace callback handler for kprobes */
 void __kprobes kprobe_ftrace_handler(unsigned long ip, unsigned long parent_ip,
 				     struct ftrace_ops *ops, struct pt_regs *regs)
-- 
1.7.11.rc1


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [tip:perf/core] kprobes/x86: Move skip_singlestep up
  2012-09-20 12:43             ` [PATCH -v2] x86, kprobes: Move skip_singlestep up Borislav Petkov
@ 2012-09-20 14:15               ` tip-bot for Borislav Petkov
  0 siblings, 0 replies; 10+ messages in thread
From: tip-bot for Borislav Petkov @ 2012-09-20 14:15 UTC (permalink / raw)
  To: linux-tip-commits
  Cc: linux-kernel, hpa, mingo, masami.hiramatsu.pt, rostedt, tglx,
	borislav.petkov

Commit-ID:  50a011f6409e888d5f41343024d24885281f048c
Gitweb:     http://git.kernel.org/tip/50a011f6409e888d5f41343024d24885281f048c
Author:     Borislav Petkov <borislav.petkov@amd.com>
AuthorDate: Thu, 20 Sep 2012 14:43:54 +0200
Committer:  Ingo Molnar <mingo@kernel.org>
CommitDate: Thu, 20 Sep 2012 14:48:16 +0200

kprobes/x86: Move skip_singlestep up

I get this warning:

  arch/x86/kernel/kprobes.c:544:23: warning: ‘skip_singlestep’ declared ‘static’ but never defined

on tip/auto-latest.

Put the skip_singlestep function declaration up, in
KPROBES_CAN_USE_FTRACE and drop the superfluous forward
declaration.

Signed-off-by: Borislav Petkov <borislav.petkov@amd.com>
Acked-by: Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>
Cc: Steven Rostedt <rostedt@goodmis.org>
Link: http://lkml.kernel.org/r/1348145034-16603-1-git-send-email-bp@amd64.org
Signed-off-by: Ingo Molnar <mingo@kernel.org>
---
 arch/x86/kernel/kprobes.c |   32 ++++++++++++++++----------------
 1 files changed, 16 insertions(+), 16 deletions(-)

diff --git a/arch/x86/kernel/kprobes.c b/arch/x86/kernel/kprobes.c
index b7c2a85..57916c0 100644
--- a/arch/x86/kernel/kprobes.c
+++ b/arch/x86/kernel/kprobes.c
@@ -541,8 +541,23 @@ reenter_kprobe(struct kprobe *p, struct pt_regs *regs, struct kprobe_ctlblk *kcb
 	return 1;
 }
 
+#ifdef KPROBES_CAN_USE_FTRACE
 static void __kprobes skip_singlestep(struct kprobe *p, struct pt_regs *regs,
-				      struct kprobe_ctlblk *kcb);
+				      struct kprobe_ctlblk *kcb)
+{
+	/*
+	 * Emulate singlestep (and also recover regs->ip)
+	 * as if there is a 5byte nop
+	 */
+	regs->ip = (unsigned long)p->addr + MCOUNT_INSN_SIZE;
+	if (unlikely(p->post_handler)) {
+		kcb->kprobe_status = KPROBE_HIT_SSDONE;
+		p->post_handler(p, regs, 0);
+	}
+	__this_cpu_write(current_kprobe, NULL);
+}
+#endif
+
 /*
  * Interrupts are disabled on entry as trap3 is an interrupt gate and they
  * remain disabled throughout this function.
@@ -1061,21 +1076,6 @@ int __kprobes longjmp_break_handler(struct kprobe *p, struct pt_regs *regs)
 }
 
 #ifdef KPROBES_CAN_USE_FTRACE
-static void __kprobes skip_singlestep(struct kprobe *p, struct pt_regs *regs,
-				      struct kprobe_ctlblk *kcb)
-{
-	/*
-	 * Emulate singlestep (and also recover regs->ip)
-	 * as if there is a 5byte nop
-	 */
-	regs->ip = (unsigned long)p->addr + MCOUNT_INSN_SIZE;
-	if (unlikely(p->post_handler)) {
-		kcb->kprobe_status = KPROBE_HIT_SSDONE;
-		p->post_handler(p, regs, 0);
-	}
-	__this_cpu_write(current_kprobe, NULL);
-}
-
 /* Ftrace callback handler for kprobes */
 void __kprobes kprobe_ftrace_handler(unsigned long ip, unsigned long parent_ip,
 				     struct ftrace_ops *ops, struct pt_regs *regs)

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* Re: [PATCH] x86, kprobes: Hide skip_singlestep forward declaration properly
  2012-09-20  9:40       ` Ingo Molnar
  2012-09-20 10:33         ` Borislav Petkov
@ 2012-09-24 10:28         ` Masami Hiramatsu
  1 sibling, 0 replies; 10+ messages in thread
From: Masami Hiramatsu @ 2012-09-24 10:28 UTC (permalink / raw)
  To: Ingo Molnar
  Cc: Borislav Petkov, Ingo Molnar, LKML, X86-ML, Borislav Petkov,
	Steven Rostedt

(2012/09/20 18:40), Ingo Molnar wrote:
> 
> * Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com> wrote:
> 
>> (2012/09/20 18:25), Ingo Molnar wrote:
>>>
>>> * Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com> wrote:
>>>
>>>> (2012/09/19 22:46), Borislav Petkov wrote:
>>>>> From: Borislav Petkov <borislav.petkov@amd.com>
>>>>>
>>>>> I get
>>>>>
>>>>> arch/x86/kernel/kprobes.c:544:23: warning: ‘skip_singlestep’ declared ‘static’ but never defined [-Wunused-function]
>>>>>
>>>>> on tip/auto-latest.
>>>>>
>>>>> Hide the forward declaration in the KPROBES_CAN_USE_FTRACE as it
>>>>> is done in its callsites.
>>>>
>>>> Thank you for reporting and fixing :)
>>>
>>> Why is a forward declation needed, why isn't the function in the 
>>> proper spot in the file to begin with?
>>
>> There is no reason, ftrace-related things can also be moved on top.
>> Borislav, could you move it?
> 
> Could be moved into a separate file as well - we could have 
> arch/x86/kprobes/, with core.c, opt.c, ftrace.c and common.h in 
> it - possibly more in the future.

OK, I'll break it into those pieces :)

Thank you,

-- 
Masami HIRAMATSU
Software Platform Research Dept. Linux Technology Center
Hitachi, Ltd., Yokohama Research Laboratory
E-mail: masami.hiramatsu.pt@hitachi.com



^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2012-09-24 10:28 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2012-09-19 13:46 [PATCH] x86, kprobes: Hide skip_singlestep forward declaration properly Borislav Petkov
2012-09-20  9:01 ` Masami Hiramatsu
2012-09-20  9:25   ` Ingo Molnar
2012-09-20  9:36     ` Masami Hiramatsu
2012-09-20  9:40       ` Ingo Molnar
2012-09-20 10:33         ` Borislav Petkov
2012-09-20 11:05           ` Ingo Molnar
2012-09-20 12:43             ` [PATCH -v2] x86, kprobes: Move skip_singlestep up Borislav Petkov
2012-09-20 14:15               ` [tip:perf/core] kprobes/x86: " tip-bot for Borislav Petkov
2012-09-24 10:28         ` [PATCH] x86, kprobes: Hide skip_singlestep forward declaration properly Masami Hiramatsu

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.