All the mail mirrored from lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] reduce useless output from 'd' console command
@ 2008-06-12 15:02 Jan Beulich
  2008-06-12 15:31 ` Keir Fraser
  0 siblings, 1 reply; 4+ messages in thread
From: Jan Beulich @ 2008-06-12 15:02 UTC (permalink / raw
  To: xen-devel; +Cc: xen-ia64-devel

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)					\

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] reduce useless output from 'd' console command
  2008-06-12 15:02 [PATCH] reduce useless output from 'd' console command Jan Beulich
@ 2008-06-12 15:31 ` Keir Fraser
  2008-06-12 15:50   ` [PATCH] reduce useless output from 'd' consolecommand Jan Beulich
  0 siblings, 1 reply; 4+ messages in thread
From: Keir Fraser @ 2008-06-12 15:31 UTC (permalink / raw
  To: Jan Beulich, xen-devel; +Cc: xen-ia64-devel

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

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] reduce useless output from 'd' consolecommand
  2008-06-12 15:31 ` Keir Fraser
@ 2008-06-12 15:50   ` Jan Beulich
  2008-06-12 15:54     ` Keir Fraser
  0 siblings, 1 reply; 4+ messages in thread
From: Jan Beulich @ 2008-06-12 15:50 UTC (permalink / raw
  To: Keir Fraser, xen-devel; +Cc: xen-ia64-devel

>>> 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

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] reduce useless output from 'd' consolecommand
  2008-06-12 15:50   ` [PATCH] reduce useless output from 'd' consolecommand Jan Beulich
@ 2008-06-12 15:54     ` Keir Fraser
  0 siblings, 0 replies; 4+ messages in thread
From: Keir Fraser @ 2008-06-12 15:54 UTC (permalink / raw
  To: Jan Beulich, xen-devel; +Cc: xen-ia64-devel

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

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2008-06-12 15:54 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2008-06-12 15:02 [PATCH] reduce useless output from 'd' console command Jan Beulich
2008-06-12 15:31 ` Keir Fraser
2008-06-12 15:50   ` [PATCH] reduce useless output from 'd' consolecommand Jan Beulich
2008-06-12 15:54     ` Keir Fraser

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.