Possible bug in 23rt3 preempt-irqs-core.patch

Possible bug in 23rt3 preempt-irqs-core.patch

[Paul Gortmaker]

I was looking over the patches in preempt-irqs-core.patch in the broken
out 23rt3 series and came across this chunk:

-----------
@@ -325,6 +349,11 @@ int setup_irq(unsigned int irq, struct i
        if (!shared) {
                irq_chip_set_defaults(desc->chip);
 
+       /*
+        * Propagate any possible IRQF_NODELAY flag into IRQ_NODELAY:
+        */
+       recalculate_desc_flags(desc);
+
 #if defined(CONFIG_IRQ_PER_CPU)
                if (new->flags & IRQF_PERCPU)
                        desc->status |= IRQ_PER_CPU;
-----------

Note the recalculate is actually contained within the "if (!shared)"
even though at a casual glance the indentation suggests otherwise.

Looking at older versions of the broken out patches leads me to believe
the if clause should be after the recalculate.  I've attached a patch to
do just that, but if you'd rather I just respin the preempt-irqs-core.patch
then I could just as easily do that as well.

Thanks,
Paul.


--- linux-2.6.23-rt3/kernel/irq/manage.c~
+++ linux-2.6.23-rt3/kernel/irq/manage.c
@@ -354,14 +354,14 @@
 	if (new->flags & IRQF_NOBALANCING)
 		desc->status |= IRQ_NO_BALANCING;
 
-	if (!shared) {
-		irq_chip_set_defaults(desc->chip);
-
 	/*
 	 * Propagate any possible IRQF_NODELAY flag into IRQ_NODELAY:
 	 */
 	recalculate_desc_flags(desc);
 
+	if (!shared) {
+		irq_chip_set_defaults(desc->chip);
+
 #if defined(CONFIG_IRQ_PER_CPU)
 		if (new->flags & IRQF_PERCPU)
 			desc->status |= IRQ_PER_CPU;

Re: Possible bug in 23rt3 preempt-irqs-core.patch

[Steven Rostedt]

--
On Thu, 25 Oct 2007, Paul Gortmaker wrote:

> I was looking over the patches in preempt-irqs-core.patch in the broken
> out 23rt3 series and came across this chunk:
>
> -----------
> @@ -325,6 +349,11 @@ int setup_irq(unsigned int irq, struct i
>         if (!shared) {
>                 irq_chip_set_defaults(desc->chip);
>
> +       /*
> +        * Propagate any possible IRQF_NODELAY flag into IRQ_NODELAY:
> +        */
> +       recalculate_desc_flags(desc);
> +
>  #if defined(CONFIG_IRQ_PER_CPU)
>                 if (new->flags & IRQF_PERCPU)
>                         desc->status |= IRQ_PER_CPU;
> -----------
>
> Note the recalculate is actually contained within the "if (!shared)"
> even though at a casual glance the indentation suggests otherwise.
>
> Looking at older versions of the broken out patches leads me to believe
> the if clause should be after the recalculate.  I've attached a patch to
> do just that, but if you'd rather I just respin the preempt-irqs-core.patch
> then I could just as easily do that as well.
>

Grumble,  That's what I get for keeping fuzzy patching turned on in quilt
:-(

>
> --- linux-2.6.23-rt3/kernel/irq/manage.c~
> +++ linux-2.6.23-rt3/kernel/irq/manage.c
> @@ -354,14 +354,14 @@
>  	if (new->flags & IRQF_NOBALANCING)
>  		desc->status |= IRQ_NO_BALANCING;
>
> -	if (!shared) {
> -		irq_chip_set_defaults(desc->chip);
> -
>  	/*
>  	 * Propagate any possible IRQF_NODELAY flag into IRQ_NODELAY:
>  	 */
>  	recalculate_desc_flags(desc);
>
> +	if (!shared) {
> +		irq_chip_set_defaults(desc->chip);
> +
>  #if defined(CONFIG_IRQ_PER_CPU)
>  		if (new->flags & IRQF_PERCPU)
>  			desc->status |= IRQ_PER_CPU;
>

Thanks,

Applied!

-- Steve

P.S. I have since turned off fuzzy logic for updating -rt (I may need to
go back and reapply old patches to see what else broke :-()

Re: Possible bug in 23rt3 preempt-irqs-core.patch

[Steven Rostedt]

--
On Fri, 26 Oct 2007, Steven Rostedt wrote:

>
> --
> On Thu, 25 Oct 2007, Paul Gortmaker wrote:
>
> > I was looking over the patches in preempt-irqs-core.patch in the broken
> > out 23rt3 series and came across this chunk:
> >
> > -----------
> > @@ -325,6 +349,11 @@ int setup_irq(unsigned int irq, struct i
> >         if (!shared) {
> >                 irq_chip_set_defaults(desc->chip);
> >
> > +       /*
> > +        * Propagate any possible IRQF_NODELAY flag into IRQ_NODELAY:
> > +        */
> > +       recalculate_desc_flags(desc);
> > +
> >  #if defined(CONFIG_IRQ_PER_CPU)
> >                 if (new->flags & IRQF_PERCPU)
> >                         desc->status |= IRQ_PER_CPU;
> > -----------
> >
> > Note the recalculate is actually contained within the "if (!shared)"
> > even though at a casual glance the indentation suggests otherwise.
> >
> > Looking at older versions of the broken out patches leads me to believe
> > the if clause should be after the recalculate.  I've attached a patch to
> > do just that, but if you'd rather I just respin the preempt-irqs-core.patch
> > then I could just as easily do that as well.
> >
>
> Grumble,  That's what I get for keeping fuzzy patching turned on in quilt
> :-(
>

Looking at the patch set I started with, the bug exists there too. I just
did a full -F0 rework of what I started with and didn't find anything else
that could have been caused by fuzzy logic. Seems this bug crept in before
I took over as patch monkey.

-- Steve