I''m having some difficulty understanding why these two need to be distinguished: Depending on the code location, an IRQ passed in from the guest may be checked against NR_PIRQS (map_domain_pirq() as called from PHYSDEVOP_alloc_irq_vector) or NR_IRQS (PHYSDEVOP_irq_status_query, PHYSDEVOP_map_pirq), despite it having the same source. Also, tying NR_IRQS to NR_VECTORS seems bogus - even with current code I can''t see why we shouldn''t be able to support a higher NR_IRQS without immediately doing the more involved code changes needed to also grow NR_VECTORS. After all, NR_IRQS is directly tied to the number of IO-APIC pins we can support - in order to support a device, its cumulative pin number (being the irq) must be below NR_IRQS. But since very likely not all pins are connected to devices, NR_VECTORS is much less of a limiting factor. Thanks, Jan _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
PIRQs are actually a different namespace. They aren''t necessarily 1:1 mapped. Hence NR_PIRQS and NR_IRQS not really same thing. Yes, I''m sure with a bit of finessing we could have NR_IRQS != NR_VECTORS. I''m sure there''ll be some barking NUMA box down the road that will require something like that, but thankfully not so far. -- Keir On 13/11/08 16:59, "Jan Beulich" <jbeulich@novell.com> wrote:> I''m having some difficulty understanding why these two need to be > distinguished: Depending on the code location, an IRQ passed in from the > guest may be checked against NR_PIRQS (map_domain_pirq() as called > from PHYSDEVOP_alloc_irq_vector) or NR_IRQS (PHYSDEVOP_irq_status_query, > PHYSDEVOP_map_pirq), despite it having the same source. > > Also, tying NR_IRQS to NR_VECTORS seems bogus - even with current > code I can''t see why we shouldn''t be able to support a higher NR_IRQS > without immediately doing the more involved code changes needed to > also grow NR_VECTORS. After all, NR_IRQS is directly tied to the number > of IO-APIC pins we can support - in order to support a device, its > cumulative pin number (being the irq) must be below NR_IRQS. But since > very likely not all pins are connected to devices, NR_VECTORS is much > less of a limiting factor. > > Thanks, Jan > > > _______________________________________________ > Xen-devel mailing list > Xen-devel@lists.xensource.com > http://lists.xensource.com/xen-devel_______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
On 13/11/08 18:41, "Keir Fraser" <keir.fraser@eu.citrix.com> wrote:> PIRQs are actually a different namespace. They aren''t necessarily 1:1 > mapped. Hence NR_PIRQS and NR_IRQS not really same thing.However I''ll agree that in some cases it''s assumed that the namespaces are the same size (PHYSDEVOP_alloc_irq_vector pretty much does that). And they *are* being used rather interchangeably already... So yes, actually perhaps we should kill off NR_PIRQS. It seems not worth cleaning up usage of NR_IRQS vs NR_PIRQS to make the distinction clean and correct.> Yes, I''m sure with a bit of finessing we could have NR_IRQS != NR_VECTORS. > I''m sure there''ll be some barking NUMA box down the road that will require > something like that, but thankfully not so far.I agree with keeping this naming distinction of course, although I think allowing NR_IRQS > NR_VECTORS right now is not very useful. But maybe you have a box in mind that needs it? -- Keir> -- Keir > > On 13/11/08 16:59, "Jan Beulich" <jbeulich@novell.com> wrote: > >> I''m having some difficulty understanding why these two need to be >> distinguished: Depending on the code location, an IRQ passed in from the >> guest may be checked against NR_PIRQS (map_domain_pirq() as called >> from PHYSDEVOP_alloc_irq_vector) or NR_IRQS (PHYSDEVOP_irq_status_query, >> PHYSDEVOP_map_pirq), despite it having the same source. >> >> Also, tying NR_IRQS to NR_VECTORS seems bogus - even with current >> code I can''t see why we shouldn''t be able to support a higher NR_IRQS >> without immediately doing the more involved code changes needed to >> also grow NR_VECTORS. After all, NR_IRQS is directly tied to the number >> of IO-APIC pins we can support - in order to support a device, its >> cumulative pin number (being the irq) must be below NR_IRQS. But since >> very likely not all pins are connected to devices, NR_VECTORS is much >> less of a limiting factor. >> >> Thanks, Jan >> >> >> _______________________________________________ >> Xen-devel mailing list >> Xen-devel@lists.xensource.com >> http://lists.xensource.com/xen-devel > > > > _______________________________________________ > Xen-devel mailing list > Xen-devel@lists.xensource.com > http://lists.xensource.com/xen-devel_______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
>>> Keir Fraser <keir.fraser@eu.citrix.com> 13.11.08 20:19 >>> >On 13/11/08 18:41, "Keir Fraser" <keir.fraser@eu.citrix.com> wrote: >> Yes, I''m sure with a bit of finessing we could have NR_IRQS != NR_VECTORS. >> I''m sure there''ll be some barking NUMA box down the road that will require >> something like that, but thankfully not so far. > >I agree with keeping this naming distinction of course, although I think >allowing NR_IRQS > NR_VECTORS right now is not very useful. But maybe you >have a box in mind that needs it?I had sent a mail a few days ago on this, where IBM was testing 96 CPU support (4-node system), and it crashing because of a PIRQ ending up in DYNIRQ space (kernel perspective), because there being 300+ IO-APIC pins. While the crash ought to be fixed with the subsequent patch, it''s clear that none of the devices with an accumulated pin number greater than 255 will actually work on that system. Jan _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
On 14/11/08 07:48, "Jan Beulich" <jbeulich@novell.com> wrote:>>> Yes, I''m sure with a bit of finessing we could have NR_IRQS != NR_VECTORS. >>> I''m sure there''ll be some barking NUMA box down the road that will require >>> something like that, but thankfully not so far. >> >> I agree with keeping this naming distinction of course, although I think >> allowing NR_IRQS > NR_VECTORS right now is not very useful. But maybe you >> have a box in mind that needs it? > > I had sent a mail a few days ago on this, where IBM was testing 96 CPU > support (4-node system), and it crashing because of a PIRQ ending up in > DYNIRQ space (kernel perspective), because there being 300+ IO-APIC > pins. While the crash ought to be fixed with the subsequent patch, it''s > clear that none of the devices with an accumulated pin number greater > than 255 will actually work on that system.Oh dear. :-D -- Keir _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
On 14/11/08 07:54, "Keir Fraser" <keir.fraser@eu.citrix.com> wrote:>>> I agree with keeping this naming distinction of course, although I think >>> allowing NR_IRQS > NR_VECTORS right now is not very useful. But maybe you >>> have a box in mind that needs it? >> >> I had sent a mail a few days ago on this, where IBM was testing 96 CPU >> support (4-node system), and it crashing because of a PIRQ ending up in >> DYNIRQ space (kernel perspective), because there being 300+ IO-APIC >> pins. While the crash ought to be fixed with the subsequent patch, it''s >> clear that none of the devices with an accumulated pin number greater >> than 255 will actually work on that system. > > Oh dear. :-DIs fixing this actually any harder than just bumping NR_IRQS/NR_PIRQS in Xen and NR_PIRQS in Linux? Have IRQS and VECTORS got somehow accidentally tied together in Xen? These parameters should probably be build-time configurable. -- Keir _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
>>> Keir Fraser <keir.fraser@eu.citrix.com> 14.11.08 09:00 >>> >On 14/11/08 07:54, "Keir Fraser" <keir.fraser@eu.citrix.com> wrote: > >>>> I agree with keeping this naming distinction of course, although I think >>>> allowing NR_IRQS > NR_VECTORS right now is not very useful. But maybe you >>>> have a box in mind that needs it? >>> >>> I had sent a mail a few days ago on this, where IBM was testing 96 CPU >>> support (4-node system), and it crashing because of a PIRQ ending up in >>> DYNIRQ space (kernel perspective), because there being 300+ IO-APIC >>> pins. While the crash ought to be fixed with the subsequent patch, it''s >>> clear that none of the devices with an accumulated pin number greater >>> than 255 will actually work on that system. >> >> Oh dear. :-D > >Is fixing this actually any harder than just bumping NR_IRQS/NR_PIRQS in Xen >and NR_PIRQS in Linux? Have IRQS and VECTORS got somehow accidentally tied >together in Xen?Perhaps not, but I only started checking (on the Xen side - the kernel side has no issues, already bumped the value there). I''ll continue as time permits.>These parameters should probably be build-time configurable.That''d certainly be nice for NR_IRQS (it seems we agreed to get rid of NR_PIRQS). I can''t the same being valid for NR_VECTORS, though. Jan _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
On 14/11/08 08:19, "Jan Beulich" <jbeulich@novell.com> wrote:>> Is fixing this actually any harder than just bumping NR_IRQS/NR_PIRQS in Xen >> and NR_PIRQS in Linux? Have IRQS and VECTORS got somehow accidentally tied >> together in Xen? > > Perhaps not, but I only started checking (on the Xen side - the kernel side > has no issues, already bumped the value there). I''ll continue as time permits. > >> These parameters should probably be build-time configurable. > > That''d certainly be nice for NR_IRQS (it seems we agreed to get rid of > NR_PIRQS). I can''t the same being valid for NR_VECTORS, though.I can''t really disagree regarding NR_VECTORS! -- Keir _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Keir Fraser wrote:> Is fixing this actually any harder than just bumping NR_IRQS/NR_PIRQS in Xen > and NR_PIRQS in Linux? Have IRQS and VECTORS got somehow accidentally tied > together in Xen? >The pvops dom0 kernel allocates all irqs dynamically without having reserved ranges for particular classes of irq, so I think it all comes down to whether Xen can handle it. J _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
>>> Keir Fraser <keir.fraser@eu.citrix.com> 14.11.08 09:00 >>> >On 14/11/08 07:54, "Keir Fraser" <keir.fraser@eu.citrix.com> wrote: > >>>> I agree with keeping this naming distinction of course, although I think >>>> allowing NR_IRQS > NR_VECTORS right now is not very useful. But maybe you >>>> have a box in mind that needs it? >>> >>> I had sent a mail a few days ago on this, where IBM was testing 96 CPU >>> support (4-node system), and it crashing because of a PIRQ ending up in >>> DYNIRQ space (kernel perspective), because there being 300+ IO-APIC >>> pins. While the crash ought to be fixed with the subsequent patch, it''s >>> clear that none of the devices with an accumulated pin number greater >>> than 255 will actually work on that system. >> >> Oh dear. :-D > >Is fixing this actually any harder than just bumping NR_IRQS/NR_PIRQS in Xen >and NR_PIRQS in Linux? Have IRQS and VECTORS got somehow accidentally tied >together in Xen?Unfortunately there was some mix-up. But I think I got this fixed (running a NR_IRQS=1024 hypervisor at the moment). I''d prefer to submit these patches only after your NR_PIRQS -> NR_IRQS change comes out of the staging tree though, to make sure it applies (and works) correctly on top of it.>These parameters should probably be build-time configurable.This too. Jan _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
On 19/11/08 15:55, "Jan Beulich" <jbeulich@novell.com> wrote:>> Is fixing this actually any harder than just bumping NR_IRQS/NR_PIRQS in Xen >> and NR_PIRQS in Linux? Have IRQS and VECTORS got somehow accidentally tied >> together in Xen? > > Unfortunately there was some mix-up. But I think I got this fixed (running > a NR_IRQS=1024 hypervisor at the moment). I''d prefer to submit these > patches only after your NR_PIRQS -> NR_IRQS change comes out of the > staging tree though, to make sure it applies (and works) correctly on top > of it.Okay, I manually synced the tree. I''ve been meaning to do so for a while. -- Keir _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
>>> Keir Fraser <keir.fraser@eu.citrix.com> 19.11.08 17:09 >>> >On 19/11/08 15:55, "Jan Beulich" <jbeulich@novell.com> wrote: > >>> Is fixing this actually any harder than just bumping NR_IRQS/NR_PIRQS in Xen >>> and NR_PIRQS in Linux? Have IRQS and VECTORS got somehow accidentally tied >>> together in Xen? >> >> Unfortunately there was some mix-up. But I think I got this fixed (running >> a NR_IRQS=1024 hypervisor at the moment). I''d prefer to submit these >> patches only after your NR_PIRQS -> NR_IRQS change comes out of the >> staging tree though, to make sure it applies (and works) correctly on top >> of it. > >Okay, I manually synced the tree. I''ve been meaning to do so for a while.Is there anything in particular that''s broken (and thus preventing it to progress automatically) that one should be aware of? Jan _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
On 19/11/08 16:23, "Jan Beulich" <jbeulich@novell.com> wrote:>>> Unfortunately there was some mix-up. But I think I got this fixed (running >>> a NR_IRQS=1024 hypervisor at the moment). I''d prefer to submit these >>> patches only after your NR_PIRQS -> NR_IRQS change comes out of the >>> staging tree though, to make sure it applies (and works) correctly on top >>> of it. >> >> Okay, I manually synced the tree. I''ve been meaning to do so for a while. > > Is there anything in particular that''s broken (and thus preventing it to > progress automatically) that one should be aware of?No, I think some part of the test infrastructure is down. -- Keir _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel