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/
Home Products Support Community News


[Xen-devel] RE: [PATCH v2 2/2] x86: don't unmask disabled irqs when migr

To: Thomas Gleixner <tglx@xxxxxxxxxxxxx>
Subject: [Xen-devel] RE: [PATCH v2 2/2] x86: don't unmask disabled irqs when migrating them
From: "Tian, Kevin" <kevin.tian@xxxxxxxxx>
Date: Fri, 6 May 2011 20:54:39 +0800
Accept-language: en-US
Acceptlanguage: en-US
Cc: Ian, "linux-kernel@xxxxxxxxxxxxxxx" <linux-kernel@xxxxxxxxxxxxxxx>, "JBeulich@xxxxxxxxxx" <JBeulich@xxxxxxxxxx>, Campbell <Ian.Campbell@xxxxxxxxxxxxx>, "mingo@xxxxxxxxxx" <mingo@xxxxxxxxxx>, "hpa@xxxxxxxxx" <hpa@xxxxxxxxx>, "xen-devel@xxxxxxxxxxxxxxxxxxx" <xen-devel@xxxxxxxxxxxxxxxxxxx>
Delivery-date: Fri, 06 May 2011 05:55:12 -0700
Envelope-to: www-data@xxxxxxxxxxxxxxxxxxx
In-reply-to: <alpine.LFD.2.02.1105061149330.3005@ionos>
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>
References: <625BA99ED14B2D499DC4E29D8138F1505C8ED7F7E3@xxxxxxxxxxxxxxxxxxxxxxxxxxxx> <alpine.LFD.2.02.1105061149330.3005@ionos>
Sender: xen-devel-bounces@xxxxxxxxxxxxxxxxxxx
Thread-index: AcwL1GyqnvmzZBFoTHyj0RqABKyCfgAE23UA
Thread-topic: [PATCH v2 2/2] x86: don't unmask disabled irqs when migrating them
> From: Thomas Gleixner
> Sent: Friday, May 06, 2011 6:00 PM
> On Fri, 6 May 2011, Tian, Kevin wrote:
> > x86: don't unmask disabled irqs when migrating them
> >
> > it doesn't make sense to mask/unmask a disabled irq when migrating it
> > from offlined cpu to another, because it's not expected to handle any
> > instance of it. Current mask/set_affinity/unmask steps may trigger
> > unexpected instance on disabled irq which then simply bug on when
> > there is no handler for it. One failing example is observed in Xen.
> > Xen pvops
> So there is no handler, why the heck is there an irq action?
>         if (!irq_has_action(irq) ....
>               continue;
> Should have caught an uninitialized interrupt. If Xen abuses interrupts that 
> way,
> then it rightfully explodes. And we do not fix it by magic somewhere else.

sorry that my bad description here. there does be a dummy handler registered
on such irqs which simply throws out a BUG_ON when hit. I should just say such 
injection is not expected instead of no handler. :-)

> > guest marks a special type of irqs as disabled, which are simply used
> As I explained before several times, IRQF_DISABLED has absolutely nothing to
> do with it and pvops _CANNOT_ mark an interrupt disabled.

I have to admit that I need more study about whole interrupt sub-system, to 
understand your explanation here. Also here again my description is not accurate
enough. I meant that Xen pvops request the special irq with below flags:
and then later explicitly disable it with disable_irq(). As you said that 
itself has nothing to do with it, and it's the later disable_irq() which takes 
effect because Xen event chip hooks this callback to mask the irq from the chip 

> >
> >             chip = irq_data_get_irq_chip(data);
> > -           if (!irqd_can_move_in_process_context(data) && chip->irq_mask)
> > +           do_mask = !irqd_irq_disabled(data) &&
> > +                   !irqd_can_move_in_process_context(data) && 
> > chip->irq_mask;
> > +           if (do_mask)
> >                     chip->irq_mask(data);
> This is completely wrong. irqd_irq_disabled() is a status information which 
> does
> not tell you whether the interrupt is actually masked at the hardware level
> because we do lazy interrupt hardware masking. So your change would keep
> the line unmasked at the hardware level for all interrupts which are in the 
> lazy
> disabled state.

Got it.

> The only conditional which is interesting is the unmask path and that's a 
> simple
> optimization and not a correctness problem.

So what's your suggestion based on my updated information? Is there any
interface I may take to differentiate above exception with normal case? 
in Xen usage we want such irqs permanently disabled at the chip level. Or
could we only do mask/unmask for irqs which are unmasked atm if as you said
it's just an optimization step? :-)


Xen-devel mailing list