[Xen-devel] RE: [PATCH] fix irq_vector[] update of c/s 19419

["Wei, Gang" <gang.wei@xxxxxxxxx>]

All things you found are absolutely right.

I used to think some code paths may need a vector to irq converting, but I 
can't find a way to dynamically assign a irq, so I did the assignment manually.
Careful checking shows it is not necessary, so your correction should be ok. 

As to the assign_irq_vector() return code, it should be my incaution.

Thanks for this correction patch.

Jimmy

On Wednesday, April 01, 2009 5:34 PM, Jan Beulich wrote:
> The updating of irq_vector[] looks very suspicious - the array is IRQ-
> indexed, hence you potentially overwrite an already existing entry, or
> risk your entry to be overwritten later.
> 
> Also, what is the reason for overwriting the vector_irq[] initialization
> assign_irq_vector() already did?
> 
> Likewise you should be calling free_irq_vector() in the error path rather
> than doing any of the updates yourself.
> 
> Finally, assign_irq_vector() does not return zero on failure, but a negative
> error code.
> 
> If there is nothing I'm overlooking, here's a patch - untested, as I don't
> have the needed hardware.
> 
> Signed-off-by: Jan Beulich <jbeulich@xxxxxxxxxx>
> 
> --- 2009-03-27.orig/xen/arch/x86/hpet.c       2009-03-24 09:04:02.000000000 
> +0100
> +++ 2009-03-27/xen/arch/x86/hpet.c    2009-04-01 11:28:25.000000000 +0200
> @@ -346,17 +346,14 @@ static int hpet_assign_irq(struct hpet_e
>      unsigned int vector;
> 
>      vector = assign_irq_vector(AUTO_ASSIGN_IRQ);
> -    if ( !vector )
> -        return -EINVAL;
> +    if ( (int)vector < 0 )
> +        return vector;
> 
> -    irq_vector[vector] = vector;
> -    vector_irq[vector] = vector;
>      vector_channel[vector] = ch - &hpet_events[0];
> 
>      if ( hpet_setup_msi_irq(vector) )
>      {
> -        irq_vector[vector] = 0;
> -        vector_irq[vector] = FREE_TO_ASSIGN_IRQ;
> +        free_irq_vector(vector);
>          vector_channel[vector] = -1;
>          return -EINVAL;
>      }
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel