WARNING - OLD ARCHIVES

This is an archived copy of the Xen.org mailing list, which we have preserved to ensure that existing links to archives are not broken. The live archive, which contains the latest emails, can be found at http://lists.xen.org/
   
 
 
Xen 
 
Home Products Support Community News
 
   
 

xen-devel

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

To: Jan Beulich <jbeulich@xxxxxxxxxx>, "xen-devel@xxxxxxxxxxxxxxxxxxx" <xen-devel@xxxxxxxxxxxxxxxxxxx>
Subject: [Xen-devel] RE: [PATCH] fix irq_vector[] update of c/s 19419
From: "Wei, Gang" <gang.wei@xxxxxxxxx>
Date: Wed, 1 Apr 2009 18:50:40 +0800
Accept-language: en-US
Acceptlanguage: en-US
Cc:
Delivery-date: Wed, 01 Apr 2009 03:51:19 -0700
Envelope-to: www-data@xxxxxxxxxxxxxxxxxxx
In-reply-to: <49D35114.76E4.0078.0@xxxxxxxxxx>
List-help: <mailto:xen-devel-request@lists.xensource.com?subject=help>
List-id: Xen developer discussion <xen-devel.lists.xensource.com>
List-post: <mailto:xen-devel@lists.xensource.com>
List-subscribe: <http://lists.xensource.com/mailman/listinfo/xen-devel>, <mailto:xen-devel-request@lists.xensource.com?subject=subscribe>
List-unsubscribe: <http://lists.xensource.com/mailman/listinfo/xen-devel>, <mailto:xen-devel-request@lists.xensource.com?subject=unsubscribe>
References: <49D35114.76E4.0078.0@xxxxxxxxxx>
Sender: xen-devel-bounces@xxxxxxxxxxxxxxxxxxx
Thread-index: AcmyriP6H0F3h1vWQgOobtD0bQnO3AABxT6g
Thread-topic: [PATCH] fix irq_vector[] update of c/s 19419
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

<Prev in Thread] Current Thread [Next in Thread>