On Thu, 2006-10-05 at 12:17 -0700, Tom Rini wrote: > On Thu, Oct 05, 2006 at 08:46:20AM -0700, George Anzinger wrote: > > Tom Rini wrote: > > >On Wed, Oct 04, 2006 at 08:42:04PM -0700, Piet Delaney wrote: > > > > > > > > >>Ton, George, Amit, et. al: > > >> > > >>If CONFIG_KGDB isn't defined, the kernel should be exactly the > > >>same as when kgdb isn't integrated. So shouldn't we should add > > >>#ifdef CONFIG_KGDB in the traps.c code where the traps are > > >>initialized and panic notify is registered. > > > > > > > > >No, because that makes the code look way too ugly.
I agree that it's ugly, but until stock kernel wants to
do it this way I suspect that we will have an easier time
in being assimilated into the kernel.org repository by
being non-invasive.
We can likely avoid uglyness with cpp macros.
> If there's no harm
> > >in doing something if kgdb is or isn't compiled in, we want to do it,
> > >such as in this case.
I agree, but you can't say to management that with KGDB un-configured
that there is absolutely no change with the patch applied. A binary diff
will show a change.
> > >
> > For what its worth, I a) agree with Piet, and b) disagree with active init
> > of the traps. In the last version of kgdb I built I coded a static init of
> > the trap notify. The value used was conditionally defined in a kgdb header
> > file thus resulting in clean trap.c code.
> >
> > This enabled kgdb to come up earlier than the running of the init code
> > (other things may be required...)
>
> The point of early_trap_init()s (more than i386 has it, with the KGDB
> patches) is that we can always call that as soon as we can. It's either
> a non-impact (!KGDB, etc) or helpful (KGDB)
Actually the ARM code has it 'right' and the call to early_trap_init()
within #if defined(CONFIG_KGDB).... #endif.
I'm not sure yet but I think ia64 also removes all traps.c changes.
Looks like mips has the same problem as i386. For example there
should be a #ifdef CONFIG_KGDB before line 982
975 void __init trap_init(void)
976 {
977 extern char except_vec3_generic, except_vec3_r4000;
978 extern char except_vec_ejtag_debug;
979 extern char except_vec4;
980 unsigned long i;
981
982 if (kgdb_early_setup)
983 return; /* Already done */
Mips looks odd, the first check of kgdb_early_setup isn't #ifdef'd
but the second one is. IMO both should be.
110 void __init init_IRQ(void)
111 {
112 int i;
113
114 if (kgdb_early_setup)
115 return;
116
117 for (i = 0; i < NR_IRQS; i++) {
118 irq_desc[i].status = IRQ_DISABLED;
119 irq_desc[i].action = NULL;
120 irq_desc[i].depth = 1;
121 irq_desc[i].handler = &no_irq_type;
122 spin_lock_init(&irq_desc[i].lock);
123 }
124
125 arch_init_irq();
126 #ifdef CONFIG_KGDB
127 /*
128 * We have been called before kgdb_arch_init(). Hence,
129 * we dont want the traps to be reinitialized
130 */
131 if (kgdb_early_setup == 0)
132 kgdb_early_setup = 1;
133 #endif
I think your right that it LIKELY has no impact; just think our
footprint on the kernel should be absolutely zero. Once kgdb
is assimilated into the kernel the trap.c guru's can decide if
they want to change trap.c to be cosmetically cleaner.
George's kgdb approach looks pretty nice to me. In 2.4 he had:
---------------------------------------------------------------
set_trap_gate(0,÷_error);
#ifndef CONFIG_KGDB
set_trap_gate(1,&debug);
set_intr_gate(2,&nmi);
set_system_gate(3,&int3); /* int3-5 can be called from all */
#else
set_intr_gate(1,&debug);
set_intr_gate(2,&nmi);
set_intr_usr_gate(3,&int3); /* int3-5 can be called from all */
#endif
---------------------------------------------------------------
In linux-2.6.12-mm1 George had:
----------------------------------------------------------------------------
set_trap_gate(0,÷_error);
set_intr_gate(1,&debug);
set_intr_gate(2,&nmi);
#ifndef CONFIG_KGDB
set_system_intr_gate(3, &int3); /* int3-5 can be called from all */
#else
set_intr_usr_gate(3,&int3); /* int3-5 can be called from all */
#endif
----------------------------------------------------------------------------
I didn't find the "static init of the trap notify";
George: where is that?
I like the idea of kgdb coming up earlier;
sounds like the right solution.
-piet
--
Piet Delaney Phone: (408) 200-5256
Blue Lane Technologies Fax: (408) 200-5299
10450 Bubb Rd.
Cupertino, Ca. 95014 Email: [EMAIL PROTECTED]
--- Begin Message ---Tom Rini wrote:For what its worth, I a) agree with Piet, and b) disagree with active init of the traps. In the last version of kgdb I built I coded a static init of the trap notify. The value used was conditionally defined in a kgdb header file thus resulting in clean trap.c code.On Wed, Oct 04, 2006 at 08:42:04PM -0700, Piet Delaney wrote:Ton, George, Amit, et. al: If CONFIG_KGDB isn't defined, the kernel should be exactly thesame as when kgdb isn't integrated. So shouldn't we should add #ifdef CONFIG_KGDB in the traps.c code where the traps are initialized and panic notify is registered.No, because that makes the code look way too ugly. If there's no harm in doing something if kgdb is or isn't compiled in, we want to do it, such as in this case.This enabled kgdb to come up earlier than the running of the init code (other things may be required...)-- George Anzinger [EMAIL PROTECTED]
--- End Message ---
signature.asc
Description: This is a digitally signed message part
------------------------------------------------------------------------- Take Surveys. Earn Cash. Influence the Future of IT Join SourceForge.net's Techsay panel and you'll get the chance to share your opinions on IT & business topics through brief surveys -- and earn cash http://www.techsay.com/default.php?page=join.php&p=sourceforge&CID=DEVDEV
_______________________________________________ Kgdb-bugreport mailing list [email protected] https://lists.sourceforge.net/lists/listinfo/kgdb-bugreport
