* [PATCH] xen: fix off-by-one error in find_unbound_irq @ 2010-02-26 10:59 Ian Campbell 2010-02-26 11:22 ` Ian Campbell 0 siblings, 1 reply; 5+ messages in thread From: Ian Campbell @ 2010-02-26 10:59 UTC (permalink / raw) To: xen-devel; +Cc: Jeremy Fitzhardinge, Ian Campbell, Konrad Rzeszutek Wilk 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 ^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH] xen: fix off-by-one error in find_unbound_irq 2010-02-26 10:59 [PATCH] xen: fix off-by-one error in find_unbound_irq Ian Campbell @ 2010-02-26 11:22 ` Ian Campbell 2010-02-26 20:13 ` Jeremy Fitzhardinge 0 siblings, 1 reply; 5+ messages in thread From: Ian Campbell @ 2010-02-26 11:22 UTC (permalink / raw) To: xen-devel@lists.xensource.com; +Cc: Jeremy Fitzhardinge, Konrad Rzeszutek Wilk 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); ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: Re: [PATCH] xen: fix off-by-one error in find_unbound_irq 2010-02-26 11:22 ` Ian Campbell @ 2010-02-26 20:13 ` Jeremy Fitzhardinge 2010-02-26 21:21 ` Ian Campbell 0 siblings, 1 reply; 5+ messages in thread From: Jeremy Fitzhardinge @ 2010-02-26 20:13 UTC (permalink / raw) To: Ian Campbell Cc: xen-devel@lists.xensource.com, Jeremy Fitzhardinge, Konrad Rzeszutek Wilk 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 > > ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: Re: [PATCH] xen: fix off-by-one error in find_unbound_irq 2010-02-26 20:13 ` Jeremy Fitzhardinge @ 2010-02-26 21:21 ` Ian Campbell 2010-03-01 13:07 ` Ian Campbell 0 siblings, 1 reply; 5+ messages in thread From: Ian Campbell @ 2010-02-26 21:21 UTC (permalink / raw) To: Jeremy Fitzhardinge Cc: xen-devel@lists.xensource.com, Jeremy, Fitzhardinge, Konrad Rzeszutek Wilk 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 > > > > > ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: Re: [PATCH] xen: fix off-by-one error in find_unbound_irq 2010-02-26 21:21 ` Ian Campbell @ 2010-03-01 13:07 ` Ian Campbell 0 siblings, 0 replies; 5+ messages in thread From: Ian Campbell @ 2010-03-01 13:07 UTC (permalink / raw) To: Jeremy Fitzhardinge Cc: xen-devel@lists.xensource.com, Jeremy, Fitzhardinge, Konrad Rzeszutek Wilk 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 2001 From: 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 ^ permalink raw reply related [flat|nested] 5+ messages in thread
end of thread, other threads:[~2010-03-01 13:07 UTC | newest] Thread overview: 5+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2010-02-26 10:59 [PATCH] xen: fix off-by-one error in find_unbound_irq Ian Campbell 2010-02-26 11:22 ` Ian Campbell 2010-02-26 20:13 ` Jeremy Fitzhardinge 2010-02-26 21:21 ` Ian Campbell 2010-03-01 13:07 ` Ian Campbell
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).