Skip to contentSkip to conversation

powerpc: Fix call to flush_ptrace_hw_breakpoint()

PATCH In review Benjamin Herrenschmidt 2 participants 6 messages Feb 6, 2011 – Feb 10, 2011 PowerPC

lore
Share & tools
lore
#0 lore raw

PATCH 1/1 powerpc: Fix call to flush_ptrace_hw_breakpoint()

A typo in the #ifdef statement makes us never call it in flush_thread()

---

arch/powerpc/kernel/process.c +3 -3
index 7a1d5cb..c4e4eab 100644
@@ -670,11 +670,11 @@ void flush_thread(void)
 {
 	discard_lazy_cpu_state();
 
-#ifdef CONFIG_HAVE_HW_BREAKPOINTS
+#ifdef CONFIG_HAVE_HW_BREAKPOINT
 	flush_ptrace_hw_breakpoint(current);
-#else /* CONFIG_HAVE_HW_BREAKPOINTS */
+#else /* CONFIG_HAVE_HW_BREAKPOINT */
 	set_debug_reg_defaults(&current->thread);
-#endif /* CONFIG_HAVE_HW_BREAKPOINTS */
+#endif /* CONFIG_HAVE_HW_BREAKPOINT */
 }
 
 void
Acked-by on v1 1/1
#1 lore raw
Quoted 3 lines · Benjamin Herrenschmidt

A typo in the #ifdef statement makes us never call it in flush_thread()

I wish it never compiled for such typos :-)

Quoted 1 line
Quoted 23 lines

---

arch/powerpc/kernel/process.c +3 -3
index 7a1d5cb..c4e4eab 100644
@@ -670,11 +670,11 @@ void flush_thread(void)
 {
 	discard_lazy_cpu_state();
 
-#ifdef CONFIG_HAVE_HW_BREAKPOINTS
+#ifdef CONFIG_HAVE_HW_BREAKPOINT
 	flush_ptrace_hw_breakpoint(current);
-#else /* CONFIG_HAVE_HW_BREAKPOINTS */
+#else /* CONFIG_HAVE_HW_BREAKPOINT */
 	set_debug_reg_defaults(&current->thread);
-#endif /* CONFIG_HAVE_HW_BREAKPOINTS */
+#endif /* CONFIG_HAVE_HW_BREAKPOINT */
 }
 
 void
#2 lore raw
Quoted 10 lines · K.Prasad
Quoted 3 lines · Benjamin Herrenschmidt

A typo in the #ifdef statement makes us never call it in flush_thread()

I wish it never compiled for such typos :-)

Quoted 1 line

Interestingly, that 'fix' now breaks the build:

cc1: warnings being treated as errors /home/benh/linux-powerpc-test/arch/powerpc/kernel/process.c:356: error:
'set_debug_reg_defaults' defined but not used

This file is is becoming an absolute mess of ifdef's in large part due to the new BookE debug stuff and your HW breakpoint stuff... Any chance you and Shaggy see if you can improve that situation a bit ?

Cheers,
Ben.

#3 lore raw
Quoted 18 lines · Benjamin Herrenschmidt
Quoted 10 lines · K.Prasad
Quoted 3 lines · Benjamin Herrenschmidt

A typo in the #ifdef statement makes us never call it in flush_thread()

I wish it never compiled for such typos :-)

Quoted 1 line

Interestingly, that 'fix' now breaks the build:

cc1: warnings being treated as errors /home/benh/linux-powerpc-test/arch/powerpc/kernel/process.c:356: error:
'set_debug_reg_defaults' defined but not used

Quoted 8 lines

This file is is becoming an absolute mess of ifdef's in large part due to the new BookE debug stuff and your HW breakpoint stuff... Any chance you and Shaggy see if you can improve that situation a bit ?

Cheers,
Ben.

Okay! Another wrapper of "#ifndef CONFIG_HAVE_HW_BREAKPOINT" around the definition of 'set_debug_reg_defaults'.

There's indeed too much sprinkling of #ifdefs in the code, but most of it would go away when the BookE code also uses the generic hw-breakpoint interfaces. Given the advanced debug features that BookE supports, it's unfortunately not that straight-forward (needs additions to generic hw-breakpoint infrastructure).

Thanks,
K.Prasad

#4 lore raw
Quoted 3 lines · K.Prasad

Okay! Another wrapper of "#ifndef CONFIG_HAVE_HW_BREAKPOINT" around the
definition of 'set_debug_reg_defaults'.

Can you send a patch ?

Quoted 3 lines

There's indeed too much sprinkling of #ifdefs in the code, but most of it would go away when the BookE code also uses the generic hw-breakpoint interfaces.

What's your status for those patches ?

Quoted 3 lines

Given the advanced debug features that BookE supports, it's unfortunately not that straight-forward (needs additions to generic hw-breakpoint infrastructure).

Might want to move a lot of that code to a separate set of files maybe and just call a single hook from the various process.c locations ?

Cheers,
Ben.

#5 lore raw
Quoted 7 lines · Benjamin Herrenschmidt
Quoted 3 lines · K.Prasad

Okay! Another wrapper of "#ifndef CONFIG_HAVE_HW_BREAKPOINT" around the
definition of 'set_debug_reg_defaults'.

Can you send a patch ?

Pasted below.

Quoted 6 lines
Quoted 3 lines

There's indeed too much sprinkling of #ifdefs in the code, but most of it would go away when the BookE code also uses the generic hw-breakpoint interfaces.

What's your status for those patches ?

The last I checked, I knew that the version of patchset I sent out (linuxppc-dev message id: [email protected]) didn't actually work when tested on hardware (it was just compile tested before sending). It needs substantial amount of time to get it in shape, which I'm unfortunately lacking now (plenty of priority items in hand that needs dedicated attention).

Thanks,
K.Prasad

Fix the error in spelling the config option for hw-breakpoints and fix the build issue that follows.

Signed-off by: K.Prasad <[email protected]>

---
 arch/powerpc/kernel/process.c |    8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)
arch/powerpc/kernel/process.c +5 -3
Index: linux-2.6.set_hwdebug/arch/powerpc/kernel/process.c
===================================================================
@@ -353,6 +353,7 @@ static void switch_booke_debug_regs(stru
 			prime_debug_regs(new_thread);
 }
 #else	/* !CONFIG_PPC_ADV_DEBUG_REGS */
+#ifndef CONFIG_HAVE_HW_BREAKPOINT
 static void set_debug_reg_defaults(struct thread_struct *thread)
 {
 	if (thread->dabr) {
@@ -360,6 +361,7 @@ static void set_debug_reg_defaults(struc
 		set_dabr(0);
 	}
 }
+#endif /* !CONFIG_HAVE_HW_BREAKPOINT */
 #endif	/* CONFIG_PPC_ADV_DEBUG_REGS */
 
 int set_dabr(unsigned long dabr)
@@ -670,11 +672,11 @@ void flush_thread(void)
 {
 	discard_lazy_cpu_state();
 
-#ifdef CONFIG_HAVE_HW_BREAKPOINTS
+#ifdef CONFIG_HAVE_HW_BREAKPOINT
 	flush_ptrace_hw_breakpoint(current);
-#else /* CONFIG_HAVE_HW_BREAKPOINTS */
+#else /* CONFIG_HAVE_HW_BREAKPOINT */
 	set_debug_reg_defaults(&current->thread);
-#endif /* CONFIG_HAVE_HW_BREAKPOINTS */
+#endif /* CONFIG_HAVE_HW_BREAKPOINT */
 }
 
 void