2011-02-05 20:08:58

by Thomas Gleixner

[permalink] [raw]
Subject: [patch 0/4] XEN: Interrupt cleanups

Jeremy,

the following series converts the interrupt chips of XEN to the new
chip functions and the last patches replace the xen private resume
hackery.

The first two patches have no dependencies.

The last two need the modification to the generic interrupt layer. I
could either carry them through the genirq tree with your
acked-tested-whatever-by or I provide you a branch to pull that change
from. I need the genirq change local as it conflicts with other patches
in the pipeline. Either way works fine.

Thanks,

tglx


2011-02-07 21:29:13

by Jeremy Fitzhardinge

[permalink] [raw]
Subject: Re: [patch 0/4] XEN: Interrupt cleanups

On 02/05/2011 12:08 PM, Thomas Gleixner wrote:
> the following series converts the interrupt chips of XEN to the new
> chip functions and the last patches replace the xen private resume
> hackery.
>
> The first two patches have no dependencies.
>
> The last two need the modification to the generic interrupt layer. I
> could either carry them through the genirq tree with your
> acked-tested-whatever-by or I provide you a branch to pull that change
> from. I need the genirq change local as it conflicts with other patches
> in the pipeline. Either way works fine.

Hi Thomas,

Thanks very much for looking at this. IanC also has some patches to
clean up the Xen interrupt stuff to get rid of the local hacks, so I'll
let him work out how to reconcile the two sets of work (ah, I see he's
already replied).

[Read patches]

It doesn't look like there's any functional overlap at all, since his
patches are concerned with using the core irq allocator rather than a
private one, so that's OK.

Anyway, I'll let Ian do the ack.

J

2011-02-07 21:34:01

by Thomas Gleixner

[permalink] [raw]
Subject: Re: [patch 0/4] XEN: Interrupt cleanups

Jeremy,

On Mon, 7 Feb 2011, Jeremy Fitzhardinge wrote:
> On 02/05/2011 12:08 PM, Thomas Gleixner wrote:
> > the following series converts the interrupt chips of XEN to the new
> > chip functions and the last patches replace the xen private resume
> > hackery.
> >
> > The first two patches have no dependencies.
> >
> > The last two need the modification to the generic interrupt layer. I
> > could either carry them through the genirq tree with your
> > acked-tested-whatever-by or I provide you a branch to pull that change
> > from. I need the genirq change local as it conflicts with other patches
> > in the pipeline. Either way works fine.
>
> Hi Thomas,
>
> Thanks very much for looking at this. IanC also has some patches to
> clean up the Xen interrupt stuff to get rid of the local hacks, so I'll
> let him work out how to reconcile the two sets of work (ah, I see he's
> already replied).
>
> [Read patches]
>
> It doesn't look like there's any functional overlap at all, since his
> patches are concerned with using the core irq allocator rather than a
> private one, so that's OK.
>
> Anyway, I'll let Ian do the ack.

Ok. The irq_chip conversion is mostly mechanical, but I'm really
concerned about that IRQ_SUSPENDED hackery. It'd be nice if you
resp. Ian could give that a test ride. That would allow me to cleanup
stuff in the core code.

Thanks,

tglx

2011-02-07 21:58:04

by Jeremy Fitzhardinge

[permalink] [raw]
Subject: Re: [patch 0/4] XEN: Interrupt cleanups

On 02/07/2011 01:33 PM, Thomas Gleixner wrote:
> Ok. The irq_chip conversion is mostly mechanical, but I'm really
> concerned about that IRQ_SUSPENDED hackery. It'd be nice if you
> resp. Ian could give that a test ride. That would allow me to cleanup
> stuff in the core code.

Ian notes: "tglx's 4 patch interrupt cleanup series on LKML causes some
oddities on PV migration. Will dig further tomorrow..."

So it looks like there's still something amiss.

J

2011-02-08 09:18:00

by Ian Campbell

[permalink] [raw]
Subject: Re: [patch 0/4] XEN: Interrupt cleanups

On Sat, 2011-02-05 at 20:08 +0000, Thomas Gleixner wrote:
> Jeremy,
>
> the following series converts the interrupt chips of XEN to the new
> chip functions and the last patches replace the xen private resume
> hackery.
>
> The first two patches have no dependencies.
>
> The last two need the modification to the generic interrupt layer. I
> could either carry them through the genirq tree with your
> acked-tested-whatever-by or I provide you a branch to pull that change
> from. I need the genirq change local as it conflicts with other patches
> in the pipeline. Either way works fine.

I definitely like IRQF_FORCE_RESUME bits, clearly much better than the
current solution.

I applied the complete series applied on top of current Linus tree (with
fixlet from my reply to 2/4 and a little reject fixup in irq.h from
3/4).

It appears that with all 4 patches applied IPIs aren't getting
resumed/unmasked/something else after a migration.

Adding an irq_enable hook to the relevant irq_chip didn't fix it and I
didn't get a chance to dig any deeper this evening.

I'll look a bit closer tomorrow and get back to you.

Ian.

2011-02-08 14:03:35

by Ian Campbell

[permalink] [raw]
Subject: Re: [patch 0/4] XEN: Interrupt cleanups

On Mon, 2011-02-07 at 13:57 -0800, Jeremy Fitzhardinge wrote:
> On 02/07/2011 01:33 PM, Thomas Gleixner wrote:
> > Ok. The irq_chip conversion is mostly mechanical, but I'm really
> > concerned about that IRQ_SUSPENDED hackery. It'd be nice if you
> > resp. Ian could give that a test ride. That would allow me to cleanup
> > stuff in the core code.
>
> Ian notes: "tglx's 4 patch interrupt cleanup series on LKML causes some
> oddities on PV migration. Will dig further tomorrow..."
>
> So it looks like there's still something amiss.

The patches missed an indirect use of IRQF_NO_SUSPEND pulled in via
IRQF_TIMER. The following fixed things for me (probably belongs in your
patch 4/4).

With this fixlet PV guest migration works just fine. I also booted the
entire series as a dom0 kernel and it appeared fine.

I also tested alongside the cleanup patches Jeremy mentioned before and
as expected there is no interaction.

So, with the fixes to 2/4 (irq_move_irq think from yesterday) and 4/4
(below), the entire series is:
Acked-by: Ian Campbell <[email protected]>

Ian.

diff --git a/arch/x86/xen/time.c b/arch/x86/xen/time.c
index 067759e..2e2d370 100644
--- a/arch/x86/xen/time.c
+++ b/arch/x86/xen/time.c
@@ -397,7 +397,9 @@ void xen_setup_timer(int cpu)
name = "<timer kasprintf failed>";

irq = bind_virq_to_irqhandler(VIRQ_TIMER, cpu, xen_timer_interrupt,
- IRQF_DISABLED|IRQF_PERCPU|IRQF_NOBALANCING|IRQF_TIMER,
+ IRQF_DISABLED|IRQF_PERCPU|
+ IRQF_NOBALANCING|IRQF_TIMER|
+ IRQF_FORCE_RESUME,
name, NULL);

evt = &per_cpu(xen_clock_events, cpu);

2011-02-08 14:55:10

by Thomas Gleixner

[permalink] [raw]
Subject: Re: [patch 0/4] XEN: Interrupt cleanups

On Tue, 8 Feb 2011, Ian Campbell wrote:
> On Mon, 2011-02-07 at 13:57 -0800, Jeremy Fitzhardinge wrote:
> > On 02/07/2011 01:33 PM, Thomas Gleixner wrote:
> > > Ok. The irq_chip conversion is mostly mechanical, but I'm really
> > > concerned about that IRQ_SUSPENDED hackery. It'd be nice if you
> > > resp. Ian could give that a test ride. That would allow me to cleanup
> > > stuff in the core code.
> >
> > Ian notes: "tglx's 4 patch interrupt cleanup series on LKML causes some
> > oddities on PV migration. Will dig further tomorrow..."
> >
> > So it looks like there's still something amiss.
>
> The patches missed an indirect use of IRQF_NO_SUSPEND pulled in via
> IRQF_TIMER. The following fixed things for me (probably belongs in your
> patch 4/4).
>
> With this fixlet PV guest migration works just fine. I also booted the
> entire series as a dom0 kernel and it appeared fine.
>
> I also tested alongside the cleanup patches Jeremy mentioned before and
> as expected there is no interaction.
>
> So, with the fixes to 2/4 (irq_move_irq think from yesterday) and 4/4
> (below), the entire series is:
> Acked-by: Ian Campbell <[email protected]>

Cool. So what's the best way to proceed ? That code is not yet in
linus tree, right ?

So I guess the best way is that I add the core changes to a rc-4 based
branch and you can pull it in and apply the whole xen stuff to your
own tree.

I base my pending patches on top of that so it wont be any problem
when merging the stuff together in next or linus later.

Thanks,

tglx

2011-02-08 15:05:11

by Ian Campbell

[permalink] [raw]
Subject: Re: [patch 0/4] XEN: Interrupt cleanups

On Tue, 2011-02-08 at 14:55 +0000, Thomas Gleixner wrote:
> On Tue, 8 Feb 2011, Ian Campbell wrote:
> > On Mon, 2011-02-07 at 13:57 -0800, Jeremy Fitzhardinge wrote:
> > > On 02/07/2011 01:33 PM, Thomas Gleixner wrote:
> > > > Ok. The irq_chip conversion is mostly mechanical, but I'm really
> > > > concerned about that IRQ_SUSPENDED hackery. It'd be nice if you
> > > > resp. Ian could give that a test ride. That would allow me to cleanup
> > > > stuff in the core code.
> > >
> > > Ian notes: "tglx's 4 patch interrupt cleanup series on LKML causes some
> > > oddities on PV migration. Will dig further tomorrow..."
> > >
> > > So it looks like there's still something amiss.
> >
> > The patches missed an indirect use of IRQF_NO_SUSPEND pulled in via
> > IRQF_TIMER. The following fixed things for me (probably belongs in your
> > patch 4/4).
> >
> > With this fixlet PV guest migration works just fine. I also booted the
> > entire series as a dom0 kernel and it appeared fine.
> >
> > I also tested alongside the cleanup patches Jeremy mentioned before and
> > as expected there is no interaction.
> >
> > So, with the fixes to 2/4 (irq_move_irq think from yesterday) and 4/4
> > (below), the entire series is:
> > Acked-by: Ian Campbell <[email protected]>
>
> Cool. So what's the best way to proceed ? That code is not yet in
> linus tree, right ?

Correct.

> So I guess the best way is that I add the core changes to a rc-4 based
> branch and you can pull it in and apply the whole xen stuff to your
> own tree.

My existing cleanup patches are in Konrad's tree (which is in linux-next
etc) so that probably makes most sense as a home for this series. So
unless Konrad has any objections I think it makes sense to pull your
core changes into that branch and then apply your Xen bits on top.

Konrad's branch with my stuff is:
git://git.kernel.org/pub/scm/linux/kernel/git/konrad/xen.git stable/irq.rework

Konrad, this thread starts at <[email protected]>
== http://thread.gmane.org/gmane.linux.kernel/1096437

> I base my pending patches on top of that so it wont be any problem
> when merging the stuff together in next or linus later.

I don't think there will be much trouble with overlap between these and
any Xen events.c changes for the next merge window but what you suggest
should remove the risk.

Ian.

2011-02-08 15:40:13

by Thomas Gleixner

[permalink] [raw]
Subject: Re: [patch 0/4] XEN: Interrupt cleanups

On Tue, 8 Feb 2011, Ian Campbell wrote:
> On Tue, 2011-02-08 at 14:55 +0000, Thomas Gleixner wrote:
> > > So, with the fixes to 2/4 (irq_move_irq think from yesterday) and 4/4
> > > (below), the entire series is:
> > > Acked-by: Ian Campbell <[email protected]>
> >
> > Cool. So what's the best way to proceed ? That code is not yet in
> > linus tree, right ?
>
> Correct.
>
> > So I guess the best way is that I add the core changes to a rc-4 based
> > branch and you can pull it in and apply the whole xen stuff to your
> > own tree.
>
> My existing cleanup patches are in Konrad's tree (which is in linux-next
> etc) so that probably makes most sense as a home for this series. So
> unless Konrad has any objections I think it makes sense to pull your
> core changes into that branch and then apply your Xen bits on top.
>
> Konrad's branch with my stuff is:
> git://git.kernel.org/pub/scm/linux/kernel/git/konrad/xen.git stable/irq.rework
>
> Konrad, this thread starts at <[email protected]>
> == http://thread.gmane.org/gmane.linux.kernel/1096437
>
> > I base my pending patches on top of that so it wont be any problem
> > when merging the stuff together in next or linus later.
>
> I don't think there will be much trouble with overlap between these and
> any Xen events.c changes for the next merge window but what you suggest
> should remove the risk.

Yes, and please talk to me next time before you hack around in the
guts of the interrupt code. I noticed just because I was skimming
-next, and that really conflicts with major cleanups I'm doing. If
there is a shortcoming in the generic code, then let me know.

Core change is in

git://git.kernel.org/pub/scm/linux/kernel/git/tip/linux-2.6-tip.git irq/for-xen

Thanks,

tglx

2011-02-08 16:22:52

by Konrad Rzeszutek Wilk

[permalink] [raw]
Subject: Re: [patch 0/4] XEN: Interrupt cleanups

On Tue, Feb 08, 2011 at 04:39:58PM +0100, Thomas Gleixner wrote:
> On Tue, 8 Feb 2011, Ian Campbell wrote:
> > On Tue, 2011-02-08 at 14:55 +0000, Thomas Gleixner wrote:
> > > > So, with the fixes to 2/4 (irq_move_irq think from yesterday) and 4/4
> > > > (below), the entire series is:
> > > > Acked-by: Ian Campbell <[email protected]>
> > >
> > > Cool. So what's the best way to proceed ? That code is not yet in
> > > linus tree, right ?
> >
> > Correct.
> >
> > > So I guess the best way is that I add the core changes to a rc-4 based
> > > branch and you can pull it in and apply the whole xen stuff to your
> > > own tree.
> >
> > My existing cleanup patches are in Konrad's tree (which is in linux-next
> > etc) so that probably makes most sense as a home for this series. So
> > unless Konrad has any objections I think it makes sense to pull your
> > core changes into that branch and then apply your Xen bits on top.

Ok. Pulled in these patches and stuck Ack-ed by Ian on them.
> >
> > Konrad's branch with my stuff is:
> > git://git.kernel.org/pub/scm/linux/kernel/git/konrad/xen.git stable/irq.rework
> >
> > Konrad, this thread starts at <[email protected]>
> > == http://thread.gmane.org/gmane.linux.kernel/1096437
> >
> > > I base my pending patches on top of that so it wont be any problem
> > > when merging the stuff together in next or linus later.
> >
> > I don't think there will be much trouble with overlap between these and
> > any Xen events.c changes for the next merge window but what you suggest
> > should remove the risk.
>
> Yes, and please talk to me next time before you hack around in the
> guts of the interrupt code. I noticed just because I was skimming
> -next, and that really conflicts with major cleanups I'm doing. If
> there is a shortcoming in the generic code, then let me know.

<scratches his head> The rework was in Xen code not in generic, and
the only generic changes that are in there .. are your code?

This is what I've in the stable/irq.rework and also in the linux-next
branch. Please tell me if I messed up.

Ian Campbell (7):
xen: handled remapped IRQs when enabling a pcifront PCI device.
xen:events: move find_unbound_irq inside CONFIG_PCI_MSI
xen: events: add xen_allocate_irq_{dynamic, gsi} and xen_free_irq
xen: events: allocate GSIs and dynamic IRQs from separate IRQ ranges.
xen: events: do not free legacy IRQs
xen: Fix compile error introduced by "switch to new irq_chip functions"
xen/timer: Missing IRQF_NO_SUSPEND in timer code broke suspend.

Konrad Rzeszutek Wilk (2):
xen/irq: Cleanup the find_unbound_irq
xen/irq: Don't fall over when nr_irqs_gsi > nr_irqs.

Thomas Gleixner (3):
xen: Remove stale irq_chip.end
xen: Switch to new irq_chip functions
genirq: Add IRQF_FORCE_RESUME

>
> Core change is in
>
> git://git.kernel.org/pub/scm/linux/kernel/git/tip/linux-2.6-tip.git irq/for-xen

<nods> Pulled that in my branch.
>
> Thanks,
>
> tglx
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at http://www.tux.org/lkml/

2011-02-08 16:25:52

by Konrad Rzeszutek Wilk

[permalink] [raw]
Subject: Re: [patch 0/4] XEN: Interrupt cleanups

> Core change is in
>
> git://git.kernel.org/pub/scm/linux/kernel/git/tip/linux-2.6-tip.git irq/for-xen

I just noticed that the patch you put in there differs from the
one you posted: "genirq: Add IRQF_FORCE_RESUME". You missed the
changes in include/linux/interrupt.h for the IRQF_MODIFY_MASK.

Was that intentional?

2011-02-08 16:26:35

by Ian Campbell

[permalink] [raw]
Subject: Re: [patch 0/4] XEN: Interrupt cleanups

On Tue, 2011-02-08 at 15:39 +0000, Thomas Gleixner wrote:
> On Tue, 8 Feb 2011, Ian Campbell wrote:
> > On Tue, 2011-02-08 at 14:55 +0000, Thomas Gleixner wrote:
> > > > So, with the fixes to 2/4 (irq_move_irq think from yesterday) and 4/4
> > > > (below), the entire series is:
> > > > Acked-by: Ian Campbell <[email protected]>
> > >
> > > Cool. So what's the best way to proceed ? That code is not yet in
> > > linus tree, right ?
> >
> > Correct.
> >
> > > So I guess the best way is that I add the core changes to a rc-4 based
> > > branch and you can pull it in and apply the whole xen stuff to your
> > > own tree.
> >
> > My existing cleanup patches are in Konrad's tree (which is in linux-next
> > etc) so that probably makes most sense as a home for this series. So
> > unless Konrad has any objections I think it makes sense to pull your
> > core changes into that branch and then apply your Xen bits on top.
> >
> > Konrad's branch with my stuff is:
> > git://git.kernel.org/pub/scm/linux/kernel/git/konrad/xen.git stable/irq.rework
> >
> > Konrad, this thread starts at <[email protected]>
> > == http://thread.gmane.org/gmane.linux.kernel/1096437
> >
> > > I base my pending patches on top of that so it wont be any problem
> > > when merging the stuff together in next or linus later.
> >
> > I don't think there will be much trouble with overlap between these and
> > any Xen events.c changes for the next merge window but what you suggest
> > should remove the risk.
>
> Yes, and please talk to me next time before you hack around in the
> guts of the interrupt code. I noticed just because I was skimming
> -next, and that really conflicts with major cleanups I'm doing. If
> there is a shortcoming in the generic code, then let me know.

Will do in the future, sorry about that.

Ian.

>
> Core change is in
>
> git://git.kernel.org/pub/scm/linux/kernel/git/tip/linux-2.6-tip.git irq/for-xen
>
> Thanks,
>
> tglx

2011-02-08 16:31:08

by Ian Campbell

[permalink] [raw]
Subject: Re: [patch 0/4] XEN: Interrupt cleanups

On Tue, 2011-02-08 at 16:20 +0000, Konrad Rzeszutek Wilk wrote:
> On Tue, Feb 08, 2011 at 04:39:58PM +0100, Thomas Gleixner wrote:
> > On Tue, 8 Feb 2011, Ian Campbell wrote:
> > > On Tue, 2011-02-08 at 14:55 +0000, Thomas Gleixner wrote:
> > > > > So, with the fixes to 2/4 (irq_move_irq think from yesterday) and 4/4
> > > > > (below), the entire series is:
> > > > > Acked-by: Ian Campbell <[email protected]>
> > > >
> > > > Cool. So what's the best way to proceed ? That code is not yet in
> > > > linus tree, right ?
> > >
> > > Correct.
> > >
> > > > So I guess the best way is that I add the core changes to a rc-4 based
> > > > branch and you can pull it in and apply the whole xen stuff to your
> > > > own tree.
> > >
> > > My existing cleanup patches are in Konrad's tree (which is in linux-next
> > > etc) so that probably makes most sense as a home for this series. So
> > > unless Konrad has any objections I think it makes sense to pull your
> > > core changes into that branch and then apply your Xen bits on top.
>
> Ok. Pulled in these patches and stuck Ack-ed by Ian on them.
> > >
> > > Konrad's branch with my stuff is:
> > > git://git.kernel.org/pub/scm/linux/kernel/git/konrad/xen.git stable/irq.rework
> > >
> > > Konrad, this thread starts at <[email protected]>
> > > == http://thread.gmane.org/gmane.linux.kernel/1096437
> > >
> > > > I base my pending patches on top of that so it wont be any problem
> > > > when merging the stuff together in next or linus later.
> > >
> > > I don't think there will be much trouble with overlap between these and
> > > any Xen events.c changes for the next merge window but what you suggest
> > > should remove the risk.
> >
> > Yes, and please talk to me next time before you hack around in the
> > guts of the interrupt code. I noticed just because I was skimming
> > -next, and that really conflicts with major cleanups I'm doing. If
> > there is a shortcoming in the generic code, then let me know.
>
> <scratches his head> The rework was in Xen code not in generic, and
> the only generic changes that are in there .. are your code?

I think Thomas was referring to the "unmask because the core didn't do
it for us" hack I added a while back which he is replacing with
IRQF_FORCE_RESUME.

Ian.

>
> This is what I've in the stable/irq.rework and also in the linux-next
> branch. Please tell me if I messed up.
>
> Ian Campbell (7):
> xen: handled remapped IRQs when enabling a pcifront PCI device.
> xen:events: move find_unbound_irq inside CONFIG_PCI_MSI
> xen: events: add xen_allocate_irq_{dynamic, gsi} and xen_free_irq
> xen: events: allocate GSIs and dynamic IRQs from separate IRQ ranges.
> xen: events: do not free legacy IRQs
> xen: Fix compile error introduced by "switch to new irq_chip functions"
> xen/timer: Missing IRQF_NO_SUSPEND in timer code broke suspend.
>
> Konrad Rzeszutek Wilk (2):
> xen/irq: Cleanup the find_unbound_irq
> xen/irq: Don't fall over when nr_irqs_gsi > nr_irqs.
>
> Thomas Gleixner (3):
> xen: Remove stale irq_chip.end
> xen: Switch to new irq_chip functions
> genirq: Add IRQF_FORCE_RESUME
>
> >
> > Core change is in
> >
> > git://git.kernel.org/pub/scm/linux/kernel/git/tip/linux-2.6-tip.git irq/for-xen
>
> <nods> Pulled that in my branch.
> >
> > Thanks,
> >
> > tglx
> > --
> > To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> > the body of a message to [email protected]
> > More majordomo info at http://vger.kernel.org/majordomo-info.html
> > Please read the FAQ at http://www.tux.org/lkml/

2011-02-08 17:33:33

by Thomas Gleixner

[permalink] [raw]
Subject: Re: [patch 0/4] XEN: Interrupt cleanups

on Tue, 8 Feb 2011, Konrad Rzeszutek Wilk wrote:

> On Tue, Feb 08, 2011 at 04:39:58PM +0100, Thomas Gleixner wrote:
> > On Tue, 8 Feb 2011, Ian Campbell wrote:
> > > On Tue, 2011-02-08 at 14:55 +0000, Thomas Gleixner wrote:
> > > > > So, with the fixes to 2/4 (irq_move_irq think from yesterday) and 4/4
> > > > > (below), the entire series is:
> > > > > Acked-by: Ian Campbell <[email protected]>
> > > >
> > > > Cool. So what's the best way to proceed ? That code is not yet in
> > > > linus tree, right ?
> > >
> > > Correct.
> > >
> > > > So I guess the best way is that I add the core changes to a rc-4 based
> > > > branch and you can pull it in and apply the whole xen stuff to your
> > > > own tree.
> > >
> > > My existing cleanup patches are in Konrad's tree (which is in linux-next
> > > etc) so that probably makes most sense as a home for this series. So
> > > unless Konrad has any objections I think it makes sense to pull your
> > > core changes into that branch and then apply your Xen bits on top.
>
> Ok. Pulled in these patches and stuck Ack-ed by Ian on them.
> > >
> > > Konrad's branch with my stuff is:
> > > git://git.kernel.org/pub/scm/linux/kernel/git/konrad/xen.git stable/irq.rework
> > >
> > > Konrad, this thread starts at <[email protected]>
> > > == http://thread.gmane.org/gmane.linux.kernel/1096437
> > >
> > > > I base my pending patches on top of that so it wont be any problem
> > > > when merging the stuff together in next or linus later.
> > >
> > > I don't think there will be much trouble with overlap between these and
> > > any Xen events.c changes for the next merge window but what you suggest
> > > should remove the risk.
> >
> > Yes, and please talk to me next time before you hack around in the
> > guts of the interrupt code. I noticed just because I was skimming
> > -next, and that really conflicts with major cleanups I'm doing. If
> > there is a shortcoming in the generic code, then let me know.
>
> <scratches his head> The rework was in Xen code not in generic, and
> the only generic changes that are in there .. are your code?

The point is:

If you play with generic irq code in Xen in some weird way then you
basically block me to cleanup something in the core code w/o breaking
Xen. I wanted to move IRQ_SUSPENDED to a different field and
accidentally noticed that Xen was fiddling with in -next.

So that's what I'm grumpy about. You hack away in Xen and claim it's
confined to your code, while in reality it is _NOT_.

Again, if there is a problem with the generic code then talk to me.

That's going to be impossible anyway when I'm done with the
encapsulation.

Thanks,

tglx

2011-02-08 17:38:11

by Thomas Gleixner

[permalink] [raw]
Subject: Re: [patch 0/4] XEN: Interrupt cleanups

On Tue, 8 Feb 2011, Konrad Rzeszutek Wilk wrote:

> > Core change is in
> >
> > git://git.kernel.org/pub/scm/linux/kernel/git/tip/linux-2.6-tip.git irq/for-xen
>
> I just noticed that the patch you put in there differs from the
> one you posted: "genirq: Add IRQF_FORCE_RESUME". You missed the
> changes in include/linux/interrupt.h for the IRQF_MODIFY_MASK.
>
> Was that intentional?

Yes, because irq.h is missing the IRQ_... flag as well. It's only
settable via: IRQF_... which is a flag for request/setup_irq.

Thanks,

tglx

2011-02-08 18:40:26

by Konrad Rzeszutek Wilk

[permalink] [raw]
Subject: Re: [patch 0/4] XEN: Interrupt cleanups

> > <scratches his head> The rework was in Xen code not in generic, and
> > the only generic changes that are in there .. are your code?
>
> The point is:
>
> If you play with generic irq code in Xen in some weird way then you
> basically block me to cleanup something in the core code w/o breaking
> Xen. I wanted to move IRQ_SUSPENDED to a different field and
> accidentally noticed that Xen was fiddling with in -next.

Aaaaah. I somehow thought the problems were with code proposed for
2.6.39, but you are referring to: "xen: events: do not unmask event channels
on resume" (6903591f314b8947d0e362bda7715e90eb9df75e) which was added way
back in November (post 2.6.37 time-frame) - and is already in Linus tree.

OK, now that I've that clear in my head (sorry about this confusion):
>
> So that's what I'm grumpy about. You hack away in Xen and claim it's
> confined to your code, while in reality it is _NOT_.

.. I can quite understand why you are unhappy about this.
>
> Again, if there is a problem with the generic code then talk to me.

Absolutely. We appreciate you being quite dilligient about other
sub-platforms and sorry for this screwup.

>
> That's going to be impossible anyway when I'm done with the
> encapsulation.

Any thoughts on how to fix that code Xen code for that issue?

>
> Thanks,
>
> tglx
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at http://www.tux.org/lkml/

2011-02-08 19:06:15

by Thomas Gleixner

[permalink] [raw]
Subject: Re: [patch 0/4] XEN: Interrupt cleanups

On Tue, 8 Feb 2011, Konrad Rzeszutek Wilk wrote:
> Aaaaah. I somehow thought the problems were with code proposed for
> 2.6.39, but you are referring to: "xen: events: do not unmask event channels
> on resume" (6903591f314b8947d0e362bda7715e90eb9df75e) which was added way
> back in November (post 2.6.37 time-frame) - and is already in Linus tree.

Ah, right. I was looking at 2.6.37 plain + pending irq changes and next :)

> > That's going to be impossible anyway when I'm done with the
> > encapsulation.
>
> Any thoughts on how to fix that code Xen code for that issue?

That's what the patch you pulled and Ian tested is about. It does the
unmask in the core code when that IRQF_FORCE_RESUME flag is set.

Thanks,

tglx

2011-02-09 09:27:16

by Ian Campbell

[permalink] [raw]
Subject: Re: [patch 0/4] XEN: Interrupt cleanups

On Tue, 2011-02-08 at 16:20 +0000, Konrad Rzeszutek Wilk wrote:

> This is what I've in the stable/irq.rework [...] Please tell me if I messed up.

I tested v2.6.38-rc4:
+= git://git.kernel.org/pub/scm/linux/kernel/git/tip/linux-2.6-tip irq/for-xen
+= git://git.kernel.org/pub/scm/linux/kernel/git/konrad/xen stable/irq.rework

and all seems fine.

Thanks,
Ian.

2011-02-09 09:46:18

by Thomas Gleixner

[permalink] [raw]
Subject: Re: [patch 0/4] XEN: Interrupt cleanups

B1;2401;0cOn Wed, 9 Feb 2011, Ian Campbell wrote:

> On Tue, 2011-02-08 at 16:20 +0000, Konrad Rzeszutek Wilk wrote:
>
> > This is what I've in the stable/irq.rework [...] Please tell me if I messed up.
>
> I tested v2.6.38-rc4:
> += git://git.kernel.org/pub/scm/linux/kernel/git/tip/linux-2.6-tip irq/for-xen
> += git://git.kernel.org/pub/scm/linux/kernel/git/konrad/xen stable/irq.rework
>
> and all seems fine.

stable/irq.rework still has the resume hackery inside.

Thanks,

tglx

2011-02-09 09:49:17

by Thomas Gleixner

[permalink] [raw]
Subject: Re: [patch 0/4] XEN: Interrupt cleanups

On Wed, 9 Feb 2011, Thomas Gleixner wrote:

> B1;2401;0cOn Wed, 9 Feb 2011, Ian Campbell wrote:
>
> > On Tue, 2011-02-08 at 16:20 +0000, Konrad Rzeszutek Wilk wrote:
> >
> > > This is what I've in the stable/irq.rework [...] Please tell me if I messed up.
> >
> > I tested v2.6.38-rc4:
> > += git://git.kernel.org/pub/scm/linux/kernel/git/tip/linux-2.6-tip irq/for-xen
> > += git://git.kernel.org/pub/scm/linux/kernel/git/konrad/xen stable/irq.rework
> >
> > and all seems fine.
>
> stable/irq.rework still has the resume hackery inside.

Also please check whether your stuff compiles and works with the
following patch. x86 should be fine, if not please yell.
You might need to disable drivers/gpio and drivers/mfd

Thanks,

tglx
---
arch/x86/Kconfig | 1 +
1 file changed, 1 insertion(+)

Index: linux-2.6/arch/x86/Kconfig
===================================================================
--- linux-2.6.orig/arch/x86/Kconfig
+++ linux-2.6/arch/x86/Kconfig
@@ -63,6 +63,7 @@ config X86
select HAVE_ARCH_JUMP_LABEL
select HAVE_TEXT_POKE_SMP
select HAVE_GENERIC_HARDIRQS
+ select GENERIC_HARDIRQS_NO_DEPRECATED
select HAVE_SPARSE_IRQ
select GENERIC_IRQ_PROBE
select GENERIC_PENDING_IRQ if SMP

2011-02-09 09:56:33

by Ian Campbell

[permalink] [raw]
Subject: Re: [patch 0/4] XEN: Interrupt cleanups

On Wed, 2011-02-09 at 09:46 +0000, Thomas Gleixner wrote:
> B1;2401;0cOn Wed, 9 Feb 2011, Ian Campbell wrote:
>
> > On Tue, 2011-02-08 at 16:20 +0000, Konrad Rzeszutek Wilk wrote:
> >
> > > This is what I've in the stable/irq.rework [...] Please tell me if I messed up.
> >
> > I tested v2.6.38-rc4:
> > += git://git.kernel.org/pub/scm/linux/kernel/git/tip/linux-2.6-tip irq/for-xen
> > += git://git.kernel.org/pub/scm/linux/kernel/git/konrad/xen stable/irq.rework
> >
> > and all seems fine.
>
> stable/irq.rework still has the resume hackery inside.

ACK, yes I missed that. Konrad -- looks looks you didn't pick up Thomas'
patch 4/4?

I added that one patch to the above set of merges and tested again --
all fine.

Ian.

2011-02-09 10:16:34

by Ian Campbell

[permalink] [raw]
Subject: Re: [patch 0/4] XEN: Interrupt cleanups

On Wed, 2011-02-09 at 09:48 +0000, Thomas Gleixner wrote:
> On Wed, 9 Feb 2011, Thomas Gleixner wrote:
>
> > B1;2401;0cOn Wed, 9 Feb 2011, Ian Campbell wrote:
> >
> > > On Tue, 2011-02-08 at 16:20 +0000, Konrad Rzeszutek Wilk wrote:
> > >
> > > > This is what I've in the stable/irq.rework [...] Please tell me if I messed up.
> > >
> > > I tested v2.6.38-rc4:
> > > += git://git.kernel.org/pub/scm/linux/kernel/git/tip/linux-2.6-tip irq/for-xen
> > > += git://git.kernel.org/pub/scm/linux/kernel/git/konrad/xen stable/irq.rework
> > >
> > > and all seems fine.
> >
> > stable/irq.rework still has the resume hackery inside.
>
> Also please check whether your stuff compiles and works with the
> following patch. x86 should be fine, if not please yell.

Applied on top of the above += your 4/4 patch.

Compiled and tested as dom0 and PV domU -- Works For Me (tm), incl PV
migration.

Thanks,
Ian.