Jan Beulich
2008-Jun-12 15:02 UTC
[Xen-devel] [PATCH] reduce useless output from ''d'' console command
Instead of printing context of the interrupt/IPI that invoked the output function, print just the interrupted context, and avoid printing the same context twice if the interrupted context was in guest space. This not only makes analyzing the output easier, it also helps reduce the amount of output in some cases, which is pretty significant given how easily the ring buffer overruns. (IA64 part only compile tested.) Signed-off-by: Jan Beulich <jbeulich@novell.com> Index: 2008-06-12/xen/arch/ia64/linux-xen/smp.c ==================================================================--- 2008-06-12.orig/xen/arch/ia64/linux-xen/smp.c 2008-06-12 16:46:04.000000000 +0200 +++ 2008-06-12/xen/arch/ia64/linux-xen/smp.c 2008-06-11 08:37:37.000000000 +0200 @@ -175,7 +175,7 @@ handle_IPI (int irq, void *dev_id, struc * At this point the structure may be gone unless * wait is true. */ - (*func)(info); + (*func)(info ?: regs); /* Notify the sending CPU that the task is done. */ mb(); Index: 2008-06-12/xen/arch/x86/smp.c ==================================================================--- 2008-06-12.orig/xen/arch/x86/smp.c 2008-06-12 16:46:04.000000000 +0200 +++ 2008-06-12/xen/arch/x86/smp.c 2008-06-11 08:35:19.000000000 +0200 @@ -357,7 +357,7 @@ fastcall void smp_call_function_interrup if ( call_data->wait ) { - (*func)(info); + (*func)(info ?: regs); mb(); atomic_inc(&call_data->finished); } @@ -365,7 +365,7 @@ fastcall void smp_call_function_interrup { mb(); atomic_inc(&call_data->started); - (*func)(info); + (*func)(info ?: regs); } irq_exit(); Index: 2008-06-12/xen/common/keyhandler.c ==================================================================--- 2008-06-12.orig/xen/common/keyhandler.c 2008-06-12 16:46:04.000000000 +0200 +++ 2008-06-12/xen/common/keyhandler.c 2008-06-12 16:46:54.000000000 +0200 @@ -91,14 +91,25 @@ static void show_handlers(unsigned char key_table[i].desc); } -static void __dump_execstate(void *unused) +static void __dump_execstate(void *_regs) { - dump_execution_state(); - printk("*** Dumping CPU%d guest state: ***\n", smp_processor_id()); + struct cpu_user_regs *regs = _regs; + unsigned int cpu = smp_processor_id(); + + if ( !guest_mode(regs) ) + { + printk("\n*** Dumping CPU%u host state: ***\n", cpu); + show_execution_state(regs); + } if ( is_idle_vcpu(current) ) - printk("No guest context (CPU is idle).\n"); + printk("No guest context (CPU%u is idle).\n", cpu); else + { + printk("*** Dumping CPU%u guest state (d%d:v%d): ***\n", + smp_processor_id(), current->domain->domain_id, + current->vcpu_id); show_execution_state(guest_cpu_user_regs()); + } } static void dump_registers(unsigned char key, struct cpu_user_regs *regs) @@ -108,14 +119,12 @@ static void dump_registers(unsigned char printk("''%c'' pressed -> dumping registers\n", key); /* Get local execution state out immediately, in case we get stuck. */ - printk("\n*** Dumping CPU%d host state: ***\n", smp_processor_id()); - __dump_execstate(NULL); + __dump_execstate(regs); for_each_online_cpu ( cpu ) { if ( cpu == smp_processor_id() ) continue; - printk("\n*** Dumping CPU%d host state: ***\n", cpu); on_selected_cpus(cpumask_of_cpu(cpu), __dump_execstate, NULL, 1, 1); } Index: 2008-06-12/xen/include/asm-ia64/linux-xen/asm/ptrace.h ==================================================================--- 2008-06-12.orig/xen/include/asm-ia64/linux-xen/asm/ptrace.h 2008-06-12 16:46:04.000000000 +0200 +++ 2008-06-12/xen/include/asm-ia64/linux-xen/asm/ptrace.h 2008-06-11 08:57:07.000000000 +0200 @@ -278,7 +278,7 @@ struct switch_stack { # define ia64_task_regs(t) (((struct pt_regs *) ((char *) (t) + IA64_STK_OFFSET)) - 1) # define ia64_psr(regs) ((struct ia64_psr *) &(regs)->cr_ipsr) #ifdef XEN -# define guest_mode(regs) (ia64_psr(regs)->cpl != 0) +# define guest_mode(regs) (ia64_psr(regs)->cpl && !ia64_psr(regs)->vm) # define guest_kernel_mode(regs) (ia64_psr(regs)->cpl == CONFIG_CPL0_EMUL) # define vmx_guest_kernel_mode(regs) (ia64_psr(regs)->cpl == 0) # define regs_increment_iip(regs) \ _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Keir Fraser
2008-Jun-12 15:31 UTC
Re: [Xen-devel] [PATCH] reduce useless output from ''d'' console command
I quite like it as it is. The output for a CPU in guest mode is quite short, and outputting printk("CPU%d blah") before doing the IPI also lets us see very clearly if a CPU is probably not responding to IPI. I think changing console_start_log_everything() to console_start_sync() would be a good idea, at least for key ''d''. -- Keir On 12/6/08 16:02, "Jan Beulich" <jbeulich@novell.com> wrote:> Instead of printing context of the interrupt/IPI that invoked the > output function, print just the interrupted context, and avoid > printing the same context twice if the interrupted context was in guest > space. This not only makes analyzing the output easier, it also helps > reduce the amount of output in some cases, which is pretty significant > given how easily the ring buffer overruns. (IA64 part only compile > tested.) > > Signed-off-by: Jan Beulich <jbeulich@novell.com> > > Index: 2008-06-12/xen/arch/ia64/linux-xen/smp.c > ==================================================================> --- 2008-06-12.orig/xen/arch/ia64/linux-xen/smp.c 2008-06-12 > 16:46:04.000000000 +0200 > +++ 2008-06-12/xen/arch/ia64/linux-xen/smp.c 2008-06-11 08:37:37.000000000 > +0200 > @@ -175,7 +175,7 @@ handle_IPI (int irq, void *dev_id, struc > * At this point the structure may be gone unless > * wait is true. > */ > - (*func)(info); > + (*func)(info ?: regs); > > /* Notify the sending CPU that the task is done. */ > mb(); > Index: 2008-06-12/xen/arch/x86/smp.c > ==================================================================> --- 2008-06-12.orig/xen/arch/x86/smp.c 2008-06-12 16:46:04.000000000 +0200 > +++ 2008-06-12/xen/arch/x86/smp.c 2008-06-11 08:35:19.000000000 +0200 > @@ -357,7 +357,7 @@ fastcall void smp_call_function_interrup > > if ( call_data->wait ) > { > - (*func)(info); > + (*func)(info ?: regs); > mb(); > atomic_inc(&call_data->finished); > } > @@ -365,7 +365,7 @@ fastcall void smp_call_function_interrup > { > mb(); > atomic_inc(&call_data->started); > - (*func)(info); > + (*func)(info ?: regs); > } > > irq_exit(); > Index: 2008-06-12/xen/common/keyhandler.c > ==================================================================> --- 2008-06-12.orig/xen/common/keyhandler.c 2008-06-12 16:46:04.000000000 > +0200 > +++ 2008-06-12/xen/common/keyhandler.c 2008-06-12 16:46:54.000000000 +0200 > @@ -91,14 +91,25 @@ static void show_handlers(unsigned char > key_table[i].desc); > } > > -static void __dump_execstate(void *unused) > +static void __dump_execstate(void *_regs) > { > - dump_execution_state(); > - printk("*** Dumping CPU%d guest state: ***\n", smp_processor_id()); > + struct cpu_user_regs *regs = _regs; > + unsigned int cpu = smp_processor_id(); > + > + if ( !guest_mode(regs) ) > + { > + printk("\n*** Dumping CPU%u host state: ***\n", cpu); > + show_execution_state(regs); > + } > if ( is_idle_vcpu(current) ) > - printk("No guest context (CPU is idle).\n"); > + printk("No guest context (CPU%u is idle).\n", cpu); > else > + { > + printk("*** Dumping CPU%u guest state (d%d:v%d): ***\n", > + smp_processor_id(), current->domain->domain_id, > + current->vcpu_id); > show_execution_state(guest_cpu_user_regs()); > + } > } > > static void dump_registers(unsigned char key, struct cpu_user_regs *regs) > @@ -108,14 +119,12 @@ static void dump_registers(unsigned char > printk("''%c'' pressed -> dumping registers\n", key); > > /* Get local execution state out immediately, in case we get stuck. */ > - printk("\n*** Dumping CPU%d host state: ***\n", smp_processor_id()); > - __dump_execstate(NULL); > + __dump_execstate(regs); > > for_each_online_cpu ( cpu ) > { > if ( cpu == smp_processor_id() ) > continue; > - printk("\n*** Dumping CPU%d host state: ***\n", cpu); > on_selected_cpus(cpumask_of_cpu(cpu), __dump_execstate, NULL, 1, 1); > } > > Index: 2008-06-12/xen/include/asm-ia64/linux-xen/asm/ptrace.h > ==================================================================> --- 2008-06-12.orig/xen/include/asm-ia64/linux-xen/asm/ptrace.h 2008-06-12 > 16:46:04.000000000 +0200 > +++ 2008-06-12/xen/include/asm-ia64/linux-xen/asm/ptrace.h 2008-06-11 > 08:57:07.000000000 +0200 > @@ -278,7 +278,7 @@ struct switch_stack { > # define ia64_task_regs(t) (((struct pt_regs *) ((char *) (t) + > IA64_STK_OFFSET)) - 1) > # define ia64_psr(regs) ((struct ia64_psr *) &(regs)->cr_ipsr) > #ifdef XEN > -# define guest_mode(regs) (ia64_psr(regs)->cpl != 0) > +# define guest_mode(regs) (ia64_psr(regs)->cpl && !ia64_psr(regs)->vm) > # define guest_kernel_mode(regs) (ia64_psr(regs)->cpl == CONFIG_CPL0_EMUL) > # define vmx_guest_kernel_mode(regs) (ia64_psr(regs)->cpl == 0) > # define regs_increment_iip(regs) \ > > > > _______________________________________________ > Xen-devel mailing list > Xen-devel@lists.xensource.com > http://lists.xensource.com/xen-devel_______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Jan Beulich
2008-Jun-12 15:50 UTC
Re: [Xen-devel] [PATCH] reduce useless output from ''d'' consolecommand
>>> Keir Fraser <keir.fraser@eu.citrix.com> 12.06.08 17:31 >>> >I quite like it as it is. The output for a CPU in guest mode is quite short, >and outputting printk("CPU%d blah") before doing the IPI also lets us see >very clearly if a CPU is probably not responding to IPI. I think changing >console_start_log_everything() to console_start_sync() would be a good idea, >at least for key ''d''.The part about placement of the printk() is certainly reasonable, but the guest mode output being short I have to question - its stack trace can be as long as the hypervisor''s. And clearly printing hypervisor context when guest execution was interrupted is pointless. Jan _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Keir Fraser
2008-Jun-12 15:54 UTC
Re: [Xen-devel] [PATCH] reduce useless output from ''d'' consolecommand
On 12/6/08 16:50, "Jan Beulich" <jbeulich@novell.com> wrote:>>>> Keir Fraser <keir.fraser@eu.citrix.com> 12.06.08 17:31 >>> >> I quite like it as it is. The output for a CPU in guest mode is quite short, >> and outputting printk("CPU%d blah") before doing the IPI also lets us see >> very clearly if a CPU is probably not responding to IPI. I think changing >> console_start_log_everything() to console_start_sync() would be a good idea, >> at least for key ''d''. > > The part about placement of the printk() is certainly reasonable, but > the guest mode output being short I have to question - its stack trace > can be as long as the hypervisor''s. And clearly printing hypervisor > context when guest execution was interrupted is pointless.I added console_start/end_sync() to the key handler, so that at least reduces this from an issue resulting in annoying loss of critical debug data down to a minor quibble. And now you''re *guaranteed* not to lose any debug data (so long as all CPUs are alive) which is even better than thinning down the output and hoping for the best. -- Keir _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel