Re: [Kgdb-bugreport] 2.6.23-rc3-mm1: kgdb build failure on powerpc
-BEGIN PGP SIGNED MESSAGE- Hash: SHA1 Jason Wessel wrote: Andrew Morton wrote: On Wed, 22 Aug 2007 17:44:12 -0500 Jason Wessel [EMAIL PROTECTED] wrote: +while (!atomic_read(debugger_active)); eek. We're in the process of hunting down and eliminating exactly this construct. There have been cases where the compiler cached the atomic_read() result in a register, turning the above into an infinite loop. Plus we should never add power-burners like that into the kernel anyway. That loop should have a cpu_relax() in it. Which will also fix the compiler problem described above. Agreed, and fixed with a cpu_relax. Thirdly, please always add a newline when coding statements like that: while (expr()) ; The other instances I found of the same problem in the kgdb core are fixed too. I merged all the changes into the for_mm branch in the kgdb git tree. Where is the kgdb git tree? - -piet Thanks, Jason. - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/ -BEGIN PGP SIGNATURE- Version: GnuPG v1.4.7 (GNU/Linux) Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org iD8DBQFG1gS/JICwm/rv3hoRAhfRAJ42F3QlzGwG4aQbs9hHVMI4kJ9SWQCfXrku UGo97ByKsB9yhyIu5c+2Jh0= =welB -END PGP SIGNATURE- ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: [Kgdb-bugreport] 2.6.23-rc3-mm1: kgdb build failure on powerpc
-BEGIN PGP SIGNED MESSAGE- Hash: SHA1 Pete/Piet Delaney wrote: Jason Wessel wrote: Andrew Morton wrote: On Wed, 22 Aug 2007 17:44:12 -0500 Jason Wessel [EMAIL PROTECTED] wrote: +while (!atomic_read(debugger_active)); eek. We're in the process of hunting down and eliminating exactly this construct. There have been cases where the compiler cached the atomic_read() result in a register, turning the above into an infinite loop. Plus we should never add power-burners like that into the kernel anyway. That loop should have a cpu_relax() in it. Which will also fix the compiler problem described above. Agreed, and fixed with a cpu_relax. Thirdly, please always add a newline when coding statements like that: while (expr()) ; The other instances I found of the same problem in the kgdb core are fixed too. I merged all the changes into the for_mm branch in the kgdb git tree. Where is the kgdb git tree? Trying: git clone http://master.kernel.org/pub/scm/linux/kernel/git/jwessel/linux-2.6-kgdb.git - -piet -piet Thanks, Jason. - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/ - - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/ -BEGIN PGP SIGNATURE- Version: GnuPG v1.4.7 (GNU/Linux) Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org iD8DBQFG1gnFJICwm/rv3hoRApOoAJ9BHXLsIuxDiOCaAFRfAZGwrDXATQCeLL3O bxtr3qz0soPRghPmtSZgOqc= =kQd1 -END PGP SIGNATURE- ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: [Kgdb-bugreport] 2.6.23-rc3-mm1: kgdb build failure on powerpc
Pete/Piet Delaney wrote: We are getting a problem with VMware where kernel text is the schedler is getting wacked with four null bytes into the code. Thought I'd use the current linux-2.6-kgdb.git tree and possible the CONFIG_DEBUG_RODATA patch to make kernel text readonly: https://www.x86-64.org/pipermail/patches/2007-March/003666.html I thought the kernel text was RO and gdb had to disable it to insert a breakpoint. If you are going to make all the kernel text RO, then you are going to have to add some code to the kgdb write memory so as to unprotect a given page or all the breakpoint writes are going to fail. Alternatively you can use HW breakpoints. But, I have no idea if your VM Ware simulated HW emulate HW breakpoint registers or not. Jason. ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: [Kgdb-bugreport] 2.6.23-rc3-mm1: kgdb build failure on powerpc
On Wed, 22 Aug 2007 17:44:12 -0500 Jason Wessel [EMAIL PROTECTED] wrote: Perhaps there is a cleaner way to do the same thing and avoid the cmpxchg all together. I used the attached patch to eliminate the cmpxchg operation. Jason. [kgdb_enter_atomic.patch text/plain (2.0KB)] Signed-off-by: Jason Wessel [EMAIL PROTECTED] --- kernel/kgdb.c | 18 -- 1 file changed, 16 insertions(+), 2 deletions(-) --- a/kernel/kgdb.c +++ b/kernel/kgdb.c @@ -121,6 +121,7 @@ struct task_struct *kgdb_usethread, *kgd int debugger_step; atomic_t debugger_active; +static atomic_t kgdb_sync = ATOMIC_INIT(-1); /* Our I/O buffers. */ static char remcom_in_buffer[BUFMAX]; @@ -638,8 +639,14 @@ static void kgdb_wait(struct pt_regs *re kgdb_info[processor].task = current; atomic_set(procindebug[processor], 1); + /* The master processor must be active to enter here, but this is + * gaurd in case the master processor had not been selected if + * this was an entry via nmi. + */ + while (!atomic_read(debugger_active)); eek. We're in the process of hunting down and eliminating exactly this construct. There have been cases where the compiler cached the atomic_read() result in a register, turning the above into an infinite loop. Plus we should never add power-burners like that into the kernel anyway. That loop should have a cpu_relax() in it. Which will also fix the compiler problem described above. Thirdly, please always add a newline when coding statements like that: while (expr()) ; ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: [Kgdb-bugreport] 2.6.23-rc3-mm1: kgdb build failure on powerpc
Andrew Morton wrote:
On Wed, 22 Aug 2007 21:04:28 +0200
Mariusz Kozlowski [EMAIL PROTECTED] wrote:
Hello,
Got that on imac g3.
CC kernel/kgdb.o
kernel/kgdb.c: In function 'kgdb_handle_exception':
kernel/kgdb.c:940: error: invalid lvalue in unary ''
kernel/kgdb.c:940: warning: type defaults to 'int' in declaration of '_o_'
kernel/kgdb.c:940: error: invalid lvalue in unary ''
kernel/kgdb.c:940: warning: type defaults to 'int' in declaration of '_n_'
kernel/kgdb.c:940: error: invalid lvalue in unary ''
kernel/kgdb.c:940: error: invalid lvalue in unary ''
kernel/kgdb.c:940: error: invalid lvalue in unary ''
kernel/kgdb.c:940: warning: type defaults to 'int' in declaration of 'type name'
make[1]: *** [kernel/kgdb.o] Blad 1
make: *** [kernel] Blad 2
Against the tip of the kernel + kgdb patches this config builds. I
wonder if is the compiler or the macros for atomic_read or cmpxchg have
changed for in the -mm tree. Perhaps it is not relevant though if you
read on.
I'm not surprised.
while (cmpxchg(atomic_read(debugger_active), 0, (procid + 1)) != 0) {
a) cmpxchg isn't available on all architectures
It was available for all the archs that the kgdb had been implemented on
at the time.
b) we can't just go and take the address of atomic_read()'s return value!
Perhaps yes, perhaps no I guess it depends on what actually gets
generated... In the past the intent of this was to guard for the race
to be the master processor and looked like some attempt to do it
atomically. This code had been in use for a number of years at this point.
c) that's pretty ugly-looking stuff anyway.
Perhaps there is a cleaner way to do the same thing and avoid the
cmpxchg all together. I used the attached patch to eliminate the
cmpxchg operation.
Jason.
Signed-off-by: Jason Wessel [EMAIL PROTECTED]
---
kernel/kgdb.c | 18 --
1 file changed, 16 insertions(+), 2 deletions(-)
--- a/kernel/kgdb.c
+++ b/kernel/kgdb.c
@@ -121,6 +121,7 @@ struct task_struct *kgdb_usethread, *kgd
int debugger_step;
atomic_t debugger_active;
+static atomic_t kgdb_sync = ATOMIC_INIT(-1);
/* Our I/O buffers. */
static char remcom_in_buffer[BUFMAX];
@@ -638,8 +639,14 @@ static void kgdb_wait(struct pt_regs *re
kgdb_info[processor].task = current;
atomic_set(procindebug[processor], 1);
+ /* The master processor must be active to enter here, but this is
+* gaurd in case the master processor had not been selected if
+* this was an entry via nmi.
+*/
+ while (!atomic_read(debugger_active));
+
/* Wait till master processor goes completely into the debugger.
-* FIXME: this looks racy */
+*/
while (!atomic_read(procindebug[atomic_read(debugger_active) - 1])) {
int i = 10; /* an arbitrary number */
@@ -973,8 +980,13 @@ int kgdb_handle_exception(int ex_vector,
/* Hold debugger_active */
procid = raw_smp_processor_id();
- while (cmpxchg(atomic_read(debugger_active), 0, (procid + 1)) != 0) {
+ while (1) {
int i = 25; /* an arbitrary number */
+ if (atomic_read(kgdb_sync) 0
+ atomic_inc_and_test(kgdb_sync)) {
+ atomic_set(debugger_active, procid + 1);
+ break;
+ }
while (--i)
cpu_relax();
@@ -991,6 +1003,7 @@ int kgdb_handle_exception(int ex_vector,
if (atomic_read(cpu_doing_single_step) != -1
atomic_read(cpu_doing_single_step) != procid) {
atomic_set(debugger_active, 0);
+ atomic_set(kgdb_sync, -1);
clocksource_touch_watchdog();
kgdb_softlock_skip[procid] = 1;
local_irq_restore(flags);
@@ -1557,6 +1570,7 @@ int kgdb_handle_exception(int ex_vector,
kgdb_restore:
/* Free debugger_active */
atomic_set(debugger_active, 0);
+ atomic_set(kgdb_sync, -1);
clocksource_touch_watchdog();
kgdb_softlock_skip[processor] = 1;
local_irq_restore(flags);
___
Linuxppc-dev mailing list
Linuxppc-dev@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: [Kgdb-bugreport] 2.6.23-rc3-mm1: kgdb build failure on powerpc
Andrew Morton wrote: On Wed, 22 Aug 2007 17:44:12 -0500 Jason Wessel [EMAIL PROTECTED] wrote: +while (!atomic_read(debugger_active)); eek. We're in the process of hunting down and eliminating exactly this construct. There have been cases where the compiler cached the atomic_read() result in a register, turning the above into an infinite loop. Plus we should never add power-burners like that into the kernel anyway. That loop should have a cpu_relax() in it. Which will also fix the compiler problem described above. Agreed, and fixed with a cpu_relax. Thirdly, please always add a newline when coding statements like that: while (expr()) ; The other instances I found of the same problem in the kgdb core are fixed too. I merged all the changes into the for_mm branch in the kgdb git tree. Thanks, Jason. ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
