From mboxrd@z Thu Jan 1 00:00:00 1970 From: Keir Fraser Subject: Re: [PATCH] reduce side effects of handling '*' debug key Date: Wed, 15 Dec 2010 10:45:55 +0000 Message-ID: References: <4D07914F0200007800027D39@vpn.id2.novell.com> Mime-Version: 1.0 Content-Type: text/plain; charset="US-ASCII" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <4D07914F0200007800027D39@vpn.id2.novell.com> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xensource.com Errors-To: xen-devel-bounces@lists.xensource.com To: Jan Beulich , "xen-devel@lists.xensource.com" List-Id: xen-devel@lists.xenproject.org On 14/12/2010 14:46, "Jan Beulich" 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 > > --- 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@lists.xensource.com > http://lists.xensource.com/xen-devel