[PATCH V3] Xen/Keyhandler: Rework process of nonirq keyhandler

[PATCH V3] Xen/Keyhandler: Rework process of nonirq keyhandler

[Lan Tianyu]

Keyhandler may run for a long time in serial port driver's
timer handler on the large machine with a lot of physical
cpus(e,g dump_timerq()) when serial port driver works in
the poll mode(via the exception mechanism).

If a timer handler runs a long time, it will block nmi_timer_fn()
to feed NMI watchdog and cause Xen hypervisor panic. Inserting
process_pending_softirqs() in timer handler will not help. when timer
interrupt arrives, timer subsystem calls all expired timer handlers
before programming next timer interrupt. There is no timer interrupt
arriving to trigger timer softirq during run a timer handler.

This patch is to fix the issue to make nonirq keyhandler run in
tasklet when receive debug key from serial port.

Signed-off-by: Lan Tianyu <tianyu.lan@intel.com>
---
 xen/common/keyhandler.c      |    8 +++++---
 xen/common/sysctl.c          |    2 +-
 xen/drivers/char/console.c   |    2 +-
 xen/include/xen/keyhandler.h |    4 +++-
 4 files changed, 10 insertions(+), 6 deletions(-)

diff --git a/xen/common/keyhandler.c b/xen/common/keyhandler.c
index 16de6e8..005ef99 100644
--- a/xen/common/keyhandler.c
+++ b/xen/common/keyhandler.c
@@ -75,19 +75,21 @@ static struct keyhandler {
 
 static void keypress_action(unsigned long unused)
 {
-    handle_keypress(keypress_key, NULL);
+    console_start_log_everything();
+    key_table[keypress_key].fn(keypress_key);
+    console_end_log_everything();
 }
 
 static DECLARE_TASKLET(keypress_tasklet, keypress_action, 0);
 
-void handle_keypress(unsigned char key, struct cpu_user_regs *regs)
+void handle_keypress(unsigned char key, struct cpu_user_regs *regs, bool force_tasklet)
 {
     struct keyhandler *h;
 
     if ( key >= ARRAY_SIZE(key_table) || !(h = &key_table[key])->fn )
         return;
 
-    if ( !in_irq() || h->irq_callback )
+    if ( h->irq_callback || !force_tasklet )
     {
         console_start_log_everything();
         h->irq_callback ? h->irq_fn(key, regs) : h->fn(key);
diff --git a/xen/common/sysctl.c b/xen/common/sysctl.c
index 8aea6ef..1eb7bad 100644
--- a/xen/common/sysctl.c
+++ b/xen/common/sysctl.c
@@ -136,7 +136,7 @@ long do_sysctl(XEN_GUEST_HANDLE_PARAM(xen_sysctl_t) u_sysctl)
         {
             if ( copy_from_guest_offset(&c, op->u.debug_keys.keys, i, 1) )
                 goto out;
-            handle_keypress(c, guest_cpu_user_regs());
+            handle_keypress(c, guest_cpu_user_regs(), false);
         }
         ret = 0;
         copyback = 0;
diff --git a/xen/drivers/char/console.c b/xen/drivers/char/console.c
index 55ae31a..b0f74ce 100644
--- a/xen/drivers/char/console.c
+++ b/xen/drivers/char/console.c
@@ -347,7 +347,7 @@ static void switch_serial_input(void)
 static void __serial_rx(char c, struct cpu_user_regs *regs)
 {
     if ( xen_rx )
-        return handle_keypress(c, regs);
+        return handle_keypress(c, regs, !in_irq());
 
     /* Deliver input to guest buffer, unless it is already full. */
     if ( (serial_rx_prod-serial_rx_cons) != SERIAL_RX_SIZE )
diff --git a/xen/include/xen/keyhandler.h b/xen/include/xen/keyhandler.h
index 06c05c8..e9595bd 100644
--- a/xen/include/xen/keyhandler.h
+++ b/xen/include/xen/keyhandler.h
@@ -46,7 +46,9 @@ void register_irq_keyhandler(unsigned char key,
                              bool_t diagnostic);
 
 /* Inject a keypress into the key-handling subsystem. */
-extern void handle_keypress(unsigned char key, struct cpu_user_regs *regs);
+extern void handle_keypress(unsigned char key,
+			    struct cpu_user_regs *regs,
+			    bool async);
 
 /* Scratch space is available for use of any keyhandler. */
 extern char keyhandler_scratch[1024];
-- 
1.7.1


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [PATCH V3] Xen/Keyhandler: Rework process of nonirq keyhandler

[Jan Beulich]

>>> On 13.10.16 at 12:06, <tianyu.lan@intel.com> wrote:
> Keyhandler may run for a long time in serial port driver's
> timer handler on the large machine with a lot of physical
> cpus(e,g dump_timerq()) when serial port driver works in
> the poll mode(via the exception mechanism).
> 
> If a timer handler runs a long time, it will block nmi_timer_fn()
> to feed NMI watchdog and cause Xen hypervisor panic. Inserting
> process_pending_softirqs() in timer handler will not help. when timer
> interrupt arrives, timer subsystem calls all expired timer handlers
> before programming next timer interrupt. There is no timer interrupt
> arriving to trigger timer softirq during run a timer handler.
> 
> This patch is to fix the issue to make nonirq keyhandler run in
> tasklet when receive debug key from serial port.
> 
> Signed-off-by: Lan Tianyu <tianyu.lan@intel.com>

Reviewed-by: Jan Beulich <jbeulich@suse.com>
with ...

> --- a/xen/include/xen/keyhandler.h
> +++ b/xen/include/xen/keyhandler.h
> @@ -46,7 +46,9 @@ void register_irq_keyhandler(unsigned char key,
>                               bool_t diagnostic);
>  
>  /* Inject a keypress into the key-handling subsystem. */
> -extern void handle_keypress(unsigned char key, struct cpu_user_regs *regs);
> +extern void handle_keypress(unsigned char key,
> +			    struct cpu_user_regs *regs,
> +			    bool async);

... this also changed to force_tasklet. I guess the committer could,
easily do that, but otoh I'm not sure we want/need this for 4.8 - Wei?

Jan


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [PATCH V3] Xen/Keyhandler: Rework process of nonirq keyhandler

[Wei Liu]

On Thu, Oct 13, 2016 at 03:30:04AM -0600, Jan Beulich wrote:
> >>> On 13.10.16 at 12:06, <tianyu.lan@intel.com> wrote:
> > Keyhandler may run for a long time in serial port driver's
> > timer handler on the large machine with a lot of physical
> > cpus(e,g dump_timerq()) when serial port driver works in
> > the poll mode(via the exception mechanism).
> > 
> > If a timer handler runs a long time, it will block nmi_timer_fn()
> > to feed NMI watchdog and cause Xen hypervisor panic. Inserting
> > process_pending_softirqs() in timer handler will not help. when timer
> > interrupt arrives, timer subsystem calls all expired timer handlers
> > before programming next timer interrupt. There is no timer interrupt
> > arriving to trigger timer softirq during run a timer handler.
> > 
> > This patch is to fix the issue to make nonirq keyhandler run in
> > tasklet when receive debug key from serial port.
> > 
> > Signed-off-by: Lan Tianyu <tianyu.lan@intel.com>
> 
> Reviewed-by: Jan Beulich <jbeulich@suse.com>
> with ...
> 
> > --- a/xen/include/xen/keyhandler.h
> > +++ b/xen/include/xen/keyhandler.h
> > @@ -46,7 +46,9 @@ void register_irq_keyhandler(unsigned char key,
> >                               bool_t diagnostic);
> >  
> >  /* Inject a keypress into the key-handling subsystem. */
> > -extern void handle_keypress(unsigned char key, struct cpu_user_regs *regs);
> > +extern void handle_keypress(unsigned char key,
> > +			    struct cpu_user_regs *regs,
> > +			    bool async);
> 
> ... this also changed to force_tasklet. I guess the committer could,
> easily do that, but otoh I'm not sure we want/need this for 4.8 - Wei?
> 

I'm fine with putting this into 4.8. The tree is stable at the moment,
we can feed to few nice-to-have things.

Wei.

> Jan
> 

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel