Re: [Xen-devel] [PATCH v2 1/2] Xen/Keyhandler: Rework process of nonirq keyhandler

2016-10-12 Thread Lan Tianyu 
On 2016年10月13日 00:03, Jan Beulich wrote:
 On 12.10.16 at 16:30,  wrote:
>>
>> Since the issue happens when handle_keypress() runs in a timer handler,
>> how about to name new parameter "intimer"? __serial_rx() is called in a 
>> timer handler or interrupt handler. Or do you have other suggestion?
> 
> I think "intimer" can be confusing (to be mixed up with timer interrupt).
> How about "force_tasklet"?

OK. I will update.
-- 
Best regards
Tianyu Lan

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

Re: [Xen-devel] [PATCH v2 1/2] Xen/Keyhandler: Rework process of nonirq keyhandler

2016-10-12 Thread Jan Beulich 
>>> On 12.10.16 at 16:30,  wrote:

> 
> On 10/12/2016 9:19 PM, Jan Beulich wrote:
> On 12.10.16 at 09:58,  wrote:
>>> --- 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, true);
>>
>> I think it would be nice to pass true here only when in polling mode,
>> unless you know or can deduce that the a similar problem also exists
>> in IRQ mode. Perhaps you could simply move the !in_irq() here?
> 
> That's a good idea. Thanks.
> 
>>(Of course the new function parameter would then want to be renamed.)
> 
> Since the issue happens when handle_keypress() runs in a timer handler,
> how about to name new parameter "intimer"? __serial_rx() is called in a 
> timer handler or interrupt handler. Or do you have other suggestion?

I think "intimer" can be confusing (to be mixed up with timer interrupt).
How about "force_tasklet"?

Jan


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

Re: [Xen-devel] [PATCH v2 1/2] Xen/Keyhandler: Rework process of nonirq keyhandler

2016-10-12 Thread Lan, Tianyu 



On 10/12/2016 9:19 PM, Jan Beulich wrote:

On 12.10.16 at 09:58,  wrote:

--- 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, true);


I think it would be nice to pass true here only when in polling mode,
unless you know or can deduce that the a similar problem also exists
in IRQ mode. Perhaps you could simply move the !in_irq() here?


That's a good idea. Thanks.


(Of course the new function parameter would then want to be renamed.)


Since the issue happens when handle_keypress() runs in a timer handler,
how about to name new parameter "intimer"? __serial_rx() is called in a 
timer handler or interrupt handler. Or do you have other suggestion?




Jan



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

Re: [Xen-devel] [PATCH v2 1/2] Xen/Keyhandler: Rework process of nonirq keyhandler

2016-10-12 Thread Jan Beulich 
>>> On 12.10.16 at 09:58,  wrote:
> --- 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, true);

I think it would be nice to pass true here only when in polling mode,
unless you know or can deduce that the a similar problem also exists
in IRQ mode. Perhaps you could simply move the !in_irq() here? (Of
course the new function parameter would then want to be renamed.)

Jan


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