[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Xen-devel] [PATCH] reduce side effects of handling '*' debug key


  • To: Jan Beulich <JBeulich@xxxxxxxxxx>, "xen-devel@xxxxxxxxxxxxxxxxxxx" <xen-devel@xxxxxxxxxxxxxxxxxxx>
  • From: Keir Fraser <keir@xxxxxxx>
  • Date: Wed, 15 Dec 2010 10:45:55 +0000
  • Cc:
  • Delivery-date: Wed, 15 Dec 2010 02:46:53 -0800
  • Domainkey-signature: a=rsa-sha1; c=nofws; d=gmail.com; s=gamma; h=sender:user-agent:date:subject:from:to:message-id:thread-topic :thread-index:in-reply-to:mime-version:content-type :content-transfer-encoding; b=oSKnpk6gDDe4MgwEnEJ/C6M5nE689lyvjIqCcV8/y+O6tLO2ovkD9e3fMJ7a7aG9rt uXyLPihsPedJtdNcXLpgK7x3Kl/Bkn0nULWU/jP7T7qmkT3wDJfrTyqAqaC7SQEH16/8 dQWq8v2fzL8uGLINGNX+IifU8yrdwM0Ru/0Hw=
  • List-id: Xen developer discussion <xen-devel.lists.xensource.com>
  • Thread-index: AcucRT/2LCu8LWXQYUW5P9yY5jVw0g==
  • Thread-topic: [Xen-devel] [PATCH] reduce side effects of handling '*' debug key

On 14/12/2010 14:46, "Jan Beulich" <JBeulich@xxxxxxxxxx> wrote:

> NMI watchdog should be suppressed, and softirqs should be handled at
> least in the non-IRQ handler portion (they obviously must not be
> processed in IRQ context).

Why do you add calls to watchdog_disable() --- The process_softirqs() call
you add is intended to be sufficient to keep timers being handled in a
timely fashion, is it not?

I can see why you remove console_start_log_everything(), since that is
handled from handle_keypress(), but why do you add calls to
console_start_sync()?

Personally I think that if you have justifiable reason to place the console
in synchronous mode, and protect yourself from the NMI watchdog, these calls
should be added to handle_keypress() for all keyhandlers to enjoy.

 -- Keir

> The (slightly more involved) 4.0 variant of this patch is also
> attached.
> 
> Signed-off-by: Jan Beulich <jbeulich@xxxxxxxxxx>
> 
> --- a/xen/common/keyhandler.c
> +++ b/xen/common/keyhandler.c
> @@ -444,16 +444,21 @@ static void run_all_nonirq_keyhandlers(u
>      struct keyhandler *h;
>      int k;
>  
> -    console_start_log_everything();
> +    watchdog_disable();
> +    console_start_sync();
> +
>      for ( k = 0; k < ARRAY_SIZE(key_table); k++ )
>      {
> +        process_pending_softirqs();
>          h = key_table[k];
>          if ( (h == NULL) || !h->diagnostic || h->irq_callback )
>              continue;
>          printk("[%c: %s]\n", k, h->desc);
>          (*h->u.fn)(k);
>      }
> -    console_end_log_everything();
> +
> +    console_end_sync();
> +    watchdog_enable();
>  }
>  
>  static DECLARE_TASKLET(run_all_keyhandlers_tasklet,
> @@ -464,10 +469,12 @@ static void run_all_keyhandlers(unsigned
>      struct keyhandler *h;
>      int k;
>  
> +    watchdog_disable();
> +    console_start_sync();
> +
>      printk("'%c' pressed -> firing all diagnostic keyhandlers\n", key);
>  
>      /* Fire all the IRQ-context diangostic keyhandlers now */
> -    console_start_log_everything();
>      for ( k = 0; k < ARRAY_SIZE(key_table); k++ )
>      {
>          h = key_table[k];
> @@ -476,7 +483,9 @@ static void run_all_keyhandlers(unsigned
>          printk("[%c: %s]\n", k, h->desc);
>          (*h->u.irq_fn)(k, regs);
>      }
> -    console_end_log_everything();
> +
> +    console_end_sync();
> +    watchdog_enable();
>  
>      /* Trigger the others from a tasklet in non-IRQ context */
>      tasklet_schedule(&run_all_keyhandlers_tasklet);
> 
> 
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@xxxxxxxxxxxxxxxxxxx
> http://lists.xensource.com/xen-devel



_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel


 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.