Messages in this thread | | | Date | Tue, 10 Mar 2009 10:01:17 +0100 | From | Andreas Herrmann <> | Subject | Re: [git-pull -tip] x86: cpu architecture debug code |
| |
On Mon, Mar 09, 2009 at 12:42:49PM +0530, Jaswinder Singh Rajput wrote: > On Sun, 2009-03-08 at 12:19 +0530, Jaswinder Singh Rajput wrote: > > The following changes since commit cb553adb4e83bc5314b5a0c397e546a215b19307: > > Ingo Molnar (1): > > Merge branch 'tracing/printk' > > > > are available in the git repository at: > > > > git://git.kernel.org/pub/scm/linux/kernel/git/jaswinder/linux-2.6-tip-cpu.git master > > > > Jaswinder Singh Rajput (1): > > x86: cpu architecture debug code > >
I still doubt that this is going to be useful (I don't see use cases for it). Nevertheless I have some comments.
> diff --git a/arch/x86/kernel/cpu/Makefile b/arch/x86/kernel/cpu/Makefile > index c381330..6427a84 100644 > --- a/arch/x86/kernel/cpu/Makefile > +++ b/arch/x86/kernel/cpu/Makefile > @@ -9,7 +9,7 @@ endif > > obj-y := intel_cacheinfo.o addon_cpuid_features.o > obj-y += proc.o capflags.o powerflags.o common.o > -obj-y += vmware.o hypervisor.o > +obj-y += vmware.o hypervisor.o cpu_debug.o > > obj-$(CONFIG_X86_32) += bugs.o cmpxchg.o > obj-$(CONFIG_X86_64) += bugs_64.o
There is no kernel config option and this code will always be compiled in. Why?
> diff --git a/arch/x86/kernel/cpu/cpu_debug.c b/arch/x86/kernel/cpu/cpu_debug.c > new file mode 100755 > index 0000000..e50d73b > --- /dev/null > +++ b/arch/x86/kernel/cpu/cpu_debug.c > @@ -0,0 +1,692 @@
...
> +static int cpu_seq_show(struct seq_file *seq, void *v) > +{ > + struct cpu_private *priv = seq->private; > + > + if (priv == NULL) > + return -EINVAL; > + > + switch (cpu_base[priv->type].flag) { > + case CPU_TSS: > + smp_call_function_single(priv->cpu, print_stdreg, seq, 1); > + break; > + case CPU_CR: > + smp_call_function_single(priv->cpu, print_cr, seq, 1); > + break; > + case CPU_DT: > + smp_call_function_single(priv->cpu, print_dt, seq, 1); > + break; > + default: > + print_msr(seq, priv->cpu, cpu_base[priv->type].flag); > + break; > + } > + seq_printf(seq, "\n");
(As one example.) Please don't print empty lines.
General thoughts/questions:
Your code is not cpu-hotpluggable. The complete debugfs subtree shows up for all CPUs - including CPUs that are offline. Was this your intention?
How does this (especially dumping of register state) fit into other kernel debugging facilities, ie. - kgdb - sysrq
Regards, Andreas
-- Operating | Advanced Micro Devices GmbH System | Karl-Hammerschmidt-Str. 34, 85609 Dornach b. München, Germany Research | Geschäftsführer: Jochen Polster, Thomas M. McCoy, Giuliano Meroni Center | Sitz: Dornach, Gemeinde Aschheim, Landkreis München (OSRC) | Registergericht München, HRB Nr. 43632
-- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
| |