WARNING - OLD ARCHIVES

This is an archived copy of the Xen.org mailing list, which we have preserved to ensure that existing links to archives are not broken. The live archive, which contains the latest emails, can be found at http://lists.xen.org/
   
 
 
Xen 
 
Home Products Support Community News
 
   
 

xen-devel

[Xen-devel] Re: [PATCH] xen: always handle VIRQ_TIMER first.

To: Jeremy Fitzhardinge <jeremy@xxxxxxxx>, Ian Campbell <ian.campbell@xxxxxxxxxx>
Subject: [Xen-devel] Re: [PATCH] xen: always handle VIRQ_TIMER first.
From: Keir Fraser <keir@xxxxxxx>
Date: Fri, 15 Oct 2010 19:30:17 +0100
Cc: xen-devel@xxxxxxxxxxxxxxxxxxx
Delivery-date: Fri, 15 Oct 2010 11:31:14 -0700
Dkim-signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=gamma; h=domainkey-signature:received:received:sender:user-agent:date :subject:from:to:cc:message-id:thread-topic:thread-index:in-reply-to :mime-version:content-type:content-transfer-encoding; bh=0Lg8Ge24mv+T4ZIQrV9LRwNd3NQaviHC9wqg0TkzHKU=; b=LHqllW1IoU+wXMsNhtwI1QOZk+7rVIC3F5G2ZB5EK6K04qbc+8dKAn1HXuqRpswIYb FCs4xmLhgbEPLjvTX1dX+j33XLv8I1mJj2U/FXNsEmeRbOFOTU4nnhTJclUnlZqE2Xcj L1vEmu4sVXxlicwze/Ddy/RkHSTDCoo+VnqXs=
Domainkey-signature: a=rsa-sha1; c=nofws; d=gmail.com; s=gamma; h=sender:user-agent:date:subject:from:to:cc:message-id:thread-topic :thread-index:in-reply-to:mime-version:content-type :content-transfer-encoding; b=BYIq+1xjuw89N1m/di35cVf+LWEfocVd0/3br53GQdW+Mr5RZuy7D0NKuXalPL6bQR 2rXvMqgprzcOYW8Rqu/PTRA5rygSL5BDk1Ow2XkGrGj2rjfICFzYIFMPBPT2fpM8/Kdj gx5btSIk34ZQeWEzEwIbUqIZZsTWmkN/nNpJo=
Envelope-to: www-data@xxxxxxxxxxxxxxxxxxx
In-reply-to: <4CB88CCF.3040305@xxxxxxxx>
List-help: <mailto:xen-devel-request@lists.xensource.com?subject=help>
List-id: Xen developer discussion <xen-devel.lists.xensource.com>
List-post: <mailto:xen-devel@lists.xensource.com>
List-subscribe: <http://lists.xensource.com/mailman/listinfo/xen-devel>, <mailto:xen-devel-request@lists.xensource.com?subject=subscribe>
List-unsubscribe: <http://lists.xensource.com/mailman/listinfo/xen-devel>, <mailto:xen-devel-request@lists.xensource.com?subject=unsubscribe>
Sender: xen-devel-bounces@xxxxxxxxxxxxxxxxxxx
Thread-index: ActslwPPvIIApIrKZkaqAwk8gAloQw==
Thread-topic: [PATCH] xen: always handle VIRQ_TIMER first.
User-agent: Microsoft-Entourage/12.26.0.100708
On 15/10/2010 18:18, "Jeremy Fitzhardinge" <jeremy@xxxxxxxx> wrote:

>  On 10/15/2010 03:52 AM, Ian Campbell wrote:
>> This ensures that system is updated before calling any hard irq
>> handlers after a long period of ticklessness. If we do not do this
>> then hardirq will see a jiffies from before the period of ticklessness
>> and make intcorrect decisions regarding timer expiry etc.
>> 
>> This resolves issues e.g. with USB keyboard timer repeats.
>> 
>> Based on a patch by Keir Fraser.
> 
> I talked about this with James, and it makes no sense to me at all.

When guest resumes execution after a long period blocked, the unblocking
interrupt may be handled before the inevitable timer interrupt which
actually syncs up jiffies to current system time. The unblocking interrupt
sees old jiffies -- most hardirq handlers really don't care about time, but
it happens that USB keyboard repeat is handled at that level -- it sees the
key pressed at old jiffies and not released until new jiffies plus small
delta. The difference between old and new jiffies can easily be enough to
cause phantom key repeats.

One question of course is whether the same hardirq key repeat mechanism can
be foxed simply be involuntary preemption of the guest. I suppose it could,
but it's vastly more unlikely than the systematic deterministic race
introduced by resume-from-block. Also we would hope that a runnable guest
would not be descheduled for as long periods as a guest can be voluntarily
blocked (bit arm waving that one I'll admit ;-).

 -- Keir

>     J
> 
>> Signed-off-by: Ian Campbell <ian.campbell@xxxxxxxxxx>
>> Cc: keir@xxxxxxx
>> ---
>>  drivers/xen/events.c |   22 +++++++++++++++++++++-
>>  1 files changed, 21 insertions(+), 1 deletions(-)
>> 
>> diff --git a/drivers/xen/events.c b/drivers/xen/events.c
>> index c9d1d4a..1496ba5 100644
>> --- a/drivers/xen/events.c
>> +++ b/drivers/xen/events.c
>> @@ -1052,6 +1052,7 @@ static void __xen_evtchn_do_upcall(struct pt_regs
>> *regs)
>>  
>> do {
>> unsigned long pending_words;
>> +  int irq;
>>  
>> vcpu_info->evtchn_upcall_pending = 0;
>>  
>> @@ -1062,6 +1063,24 @@ static void __xen_evtchn_do_upcall(struct pt_regs
>> *regs)
>> /* Clear master flag /before/ clearing selector flag. */
>> wmb();
>>  #endif
>> +
>> +  /*
>> +   * Handle timer interrupts before all others, so that all
>> +   * hardirq handlers see an up-to-date system time even if we
>> +   * have just woken from a long idle period.
>> +   */
>> +  irq = percpu_read(virq_to_irq[VIRQ_TIMER]);
>> +  if (irq != -1) {
>> +   int word_idx;
>> +   int bit_idx;
>> +   int port = evtchn_from_irq(irq);
>> +   word_idx = port / BITS_PER_LONG;
>> +   bit_idx = port % BITS_PER_LONG;
>> +   if (VALID_EVTCHN(port) &&
>> +       (active_evtchns(cpu, s, word_idx) & (1UL<<bit_idx)))
>> +    (void)handle_irq(irq, regs);
>> +  }
>> +
>> pending_words = xchg(&vcpu_info->evtchn_pending_sel, 0);
>> while (pending_words != 0) {
>> unsigned long pending_bits;
>> @@ -1071,9 +1090,10 @@ static void __xen_evtchn_do_upcall(struct pt_regs
>> *regs)
>> while ((pending_bits = active_evtchns(cpu, s, word_idx)) != 0) {
>> int bit_idx = __ffs(pending_bits);
>> int port = (word_idx * BITS_PER_LONG) + bit_idx;
>> -    int irq = evtchn_to_irq[port];
>> struct irq_desc *desc;
>>  
>> +    irq = evtchn_to_irq[port];
>> +
>> mask_evtchn(port);
>> clear_evtchn(port);
>>  
> 



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