Ian Campbell
2010-Feb-26 10:59 UTC
[Xen-devel] [PATCH] xen: fix off-by-one error in find_unbound_irq
e459de95 "Find an unbound irq number in reverse order (high to low)" introduced an off by one error which would cause repeated allocations of the nr_irq''th IRQ if there are no spare interrupts (i.e. get_nr_hw_irqs() == nr_irqs). Signed-off-by: Ian Campbell <ian.campbell@citrix.com> Cc: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com> Cc: Jeremy Fitzhardinge <jeremy.fitzhardinge@citrix.com> --- drivers/xen/events.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/drivers/xen/events.c b/drivers/xen/events.c index 99f2b2a..5c64e1d 100644 --- a/drivers/xen/events.c +++ b/drivers/xen/events.c @@ -377,7 +377,7 @@ static int find_unbound_irq(void) if (irq_info[irq].type == IRQT_UNBOUND) break; - if (irq == start || irq == nr_irqs) + if (irq == start || irq == nr_irqs - 1) panic("No available IRQ to bind to: increase nr_irqs!\n"); desc = irq_to_desc_alloc_node(irq, 0); -- 1.5.6.5 _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Ian Campbell
2010-Feb-26 11:22 UTC
[Xen-devel] Re: [PATCH] xen: fix off-by-one error in find_unbound_irq
BTW, this is against xen/master, the original patch isn''t in xen/next. On Fri, 2010-02-26 at 10:59 +0000, Ian Campbell wrote:> e459de95 "Find an unbound irq number in reverse order (high to low)" introduced > an off by one error which would cause repeated allocations of the nr_irq''th IRQ > if there are no spare interrupts (i.e. get_nr_hw_irqs() == nr_irqs). > > Signed-off-by: Ian Campbell <ian.campbell@citrix.com> > Cc: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com> > Cc: Jeremy Fitzhardinge <jeremy.fitzhardinge@citrix.com> > --- > drivers/xen/events.c | 2 +- > 1 files changed, 1 insertions(+), 1 deletions(-) > > diff --git a/drivers/xen/events.c b/drivers/xen/events.c > index 99f2b2a..5c64e1d 100644 > --- a/drivers/xen/events.c > +++ b/drivers/xen/events.c > @@ -377,7 +377,7 @@ static int find_unbound_irq(void) > if (irq_info[irq].type == IRQT_UNBOUND) > break; > > - if (irq == start || irq == nr_irqs) > + if (irq == start || irq == nr_irqs - 1) > panic("No available IRQ to bind to: increase nr_irqs!\n"); > > desc = irq_to_desc_alloc_node(irq, 0);_______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Jeremy Fitzhardinge
2010-Feb-26 20:13 UTC
Re: [Xen-devel] Re: [PATCH] xen: fix off-by-one error in find_unbound_irq
On 02/26/2010 03:22 AM, Ian Campbell wrote:> BTW, this is against xen/master, the original patch isn''t in xen/next. >Looks like it came from the pcifront branch which isn''t in xen/next yet. J> On Fri, 2010-02-26 at 10:59 +0000, Ian Campbell wrote: > >> e459de95 "Find an unbound irq number in reverse order (high to low)" introduced >> an off by one error which would cause repeated allocations of the nr_irq''th IRQ >> if there are no spare interrupts (i.e. get_nr_hw_irqs() == nr_irqs). >> >> Signed-off-by: Ian Campbell<ian.campbell@citrix.com> >> Cc: Konrad Rzeszutek Wilk<konrad.wilk@oracle.com> >> Cc: Jeremy Fitzhardinge<jeremy.fitzhardinge@citrix.com> >> --- >> drivers/xen/events.c | 2 +- >> 1 files changed, 1 insertions(+), 1 deletions(-) >> >> diff --git a/drivers/xen/events.c b/drivers/xen/events.c >> index 99f2b2a..5c64e1d 100644 >> --- a/drivers/xen/events.c >> +++ b/drivers/xen/events.c >> @@ -377,7 +377,7 @@ static int find_unbound_irq(void) >> if (irq_info[irq].type == IRQT_UNBOUND) >> break; >> >> - if (irq == start || irq == nr_irqs) >> + if (irq == start || irq == nr_irqs - 1) >> panic("No available IRQ to bind to: increase nr_irqs!\n"); >> >> desc = irq_to_desc_alloc_node(irq, 0); >> > > > _______________________________________________ > 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
Ian Campbell
2010-Feb-26 21:21 UTC
Re: [Xen-devel] Re: [PATCH] xen: fix off-by-one error in find_unbound_irq
On Fri, 2010-02-26 at 20:13 +0000, Jeremy Fitzhardinge wrote:> On 02/26/2010 03:22 AM, Ian Campbell wrote: > > BTW, this is against xen/master, the original patch isn''t in xen/next. > > > > Looks like it came from the pcifront branch which isn''t in xen/next yet.Makes sense. I don''t think my fix is right though, exiting that loop with irq =nr_irqs - 1 can be valid if the test in first iteration succeeds and we break out The error case is when start == nr_irqs which means we do no iterations of the loop at all but still exit with irq == nr_irqs - 1. Ian.> > J > > > On Fri, 2010-02-26 at 10:59 +0000, Ian Campbell wrote: > > > >> e459de95 "Find an unbound irq number in reverse order (high to low)" introduced > >> an off by one error which would cause repeated allocations of the nr_irq''th IRQ > >> if there are no spare interrupts (i.e. get_nr_hw_irqs() == nr_irqs). > >> > >> Signed-off-by: Ian Campbell<ian.campbell@citrix.com> > >> Cc: Konrad Rzeszutek Wilk<konrad.wilk@oracle.com> > >> Cc: Jeremy Fitzhardinge<jeremy.fitzhardinge@citrix.com> > >> --- > >> drivers/xen/events.c | 2 +- > >> 1 files changed, 1 insertions(+), 1 deletions(-) > >> > >> diff --git a/drivers/xen/events.c b/drivers/xen/events.c > >> index 99f2b2a..5c64e1d 100644 > >> --- a/drivers/xen/events.c > >> +++ b/drivers/xen/events.c > >> @@ -377,7 +377,7 @@ static int find_unbound_irq(void) > >> if (irq_info[irq].type == IRQT_UNBOUND) > >> break; > >> > >> - if (irq == start || irq == nr_irqs) > >> + if (irq == start || irq == nr_irqs - 1) > >> panic("No available IRQ to bind to: increase nr_irqs!\n"); > >> > >> desc = irq_to_desc_alloc_node(irq, 0); > >> > > > > > > _______________________________________________ > > 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
Ian Campbell
2010-Mar-01 13:07 UTC
Re: [Xen-devel] Re: [PATCH] xen: fix off-by-one error in find_unbound_irq
On Fri, 2010-02-26 at 21:21 +0000, Ian Campbell wrote:> On Fri, 2010-02-26 at 20:13 +0000, Jeremy Fitzhardinge wrote: > > On 02/26/2010 03:22 AM, Ian Campbell wrote: > > > BTW, this is against xen/master, the original patch isn''t in xen/next. > > > > > > > Looks like it came from the pcifront branch which isn''t in xen/next yet. > > Makes sense. > > I don''t think my fix is right though, exiting that loop with irq => nr_irqs - 1 can be valid if the test in first iteration succeeds and we > break out > > The error case is when start == nr_irqs which means we do no iterations > of the loop at all but still exit with irq == nr_irqs - 1.I think this fixes the issue in xen/master: The following changes since commit 862b30f532edc893275874fed6352a07653356f7: Jeremy Fitzhardinge (1): Merge branch ''xen/dom0/backend/blktap2'' into xen/master are available in the git repository at: git://xenbits.xensource.com/people/ianc/linux-2.6.git for-jeremy/irq Ian Campbell (1): xen: fix error handling in in find_unbound_irq drivers/xen/events.c | 10 ++++++++-- 1 files changed, 8 insertions(+), 2 deletions(-) Ian. --->From 716645983e03118d11924cc245cd63fd67c6bfa8 Mon Sep 17 00:00:00 2001From: Ian Campbell <ian.campbell@citrix.com> Date: Mon, 1 Mar 2010 12:06:15 +0000 Subject: [PATCH] xen: fix error handling in in find_unbound_irq 68458a36 "fix off-by-one error in find_unbound_irq" introduced an issue with the error handling in this function. It incorrectly assumed that exiting the searhc loop with irq == nr_irqs - 1 was an error case when in fact it is prefectly possible for irq == nr_irqs - 1 to be an available IRQ. The actual error condition which 68458a36 tried to fix is when start =nr_irqs, IOW when there is literaly no interrupts which aren''t already h/w interrupts. Signed-off-by: Ian Campbell <ian.campbell@citrix.com> --- drivers/xen/events.c | 10 ++++++++-- 1 files changed, 8 insertions(+), 2 deletions(-) diff --git a/drivers/xen/events.c b/drivers/xen/events.c index 5c64e1d..ef7f00c 100644 --- a/drivers/xen/events.c +++ b/drivers/xen/events.c @@ -372,13 +372,16 @@ static int find_unbound_irq(void) struct irq_desc *desc; int start = get_nr_hw_irqs(); + if (start == nr_irqs) + goto no_irqs; + /* nr_irqs is a magic value. Must not use it.*/ for (irq = nr_irqs-1; irq > start; irq--) if (irq_info[irq].type == IRQT_UNBOUND) break; - if (irq == start || irq == nr_irqs - 1) - panic("No available IRQ to bind to: increase nr_irqs!\n"); + if (irq == start) + goto no_irqs; desc = irq_to_desc_alloc_node(irq, 0); if (WARN_ON(desc == NULL)) @@ -387,6 +390,9 @@ static int find_unbound_irq(void) dynamic_irq_init(irq); return irq; + +no_irqs: + panic("No available IRQ to bind to: increase nr_irqs!\n"); } static bool identity_mapped_irq(unsigned irq) -- 1.5.6.5 _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel