Xen devel - Jun 2013 - [PATCH RFC] suggestion on the inflight irq order

Since inflight irq is only used for keeping track of on-going irqs, 
I don''t think it should be sorted by priority. (Only used for cleaing 
all inflight irqs and debug-print all inflight irqs) For IRQ processing 
at xen, we can save some time by not keeping it in order.

Signed-off-by: Jaeyong Yoo <jaeyong.yoo@samsung.com>
---
 xen/arch/arm/vgic.c |   11 +----------
 1 file changed, 1 insertion(+), 10 deletions(-)

diff --git a/xen/arch/arm/vgic.c b/xen/arch/arm/vgic.c
index 7eaccb7..47cc2a0 100644
--- a/xen/arch/arm/vgic.c
+++ b/xen/arch/arm/vgic.c
@@ -666,7 +666,7 @@ void vgic_vcpu_inject_irq(struct vcpu *v, unsigned int irq,
int virtual)
     int idx = irq >> 2, byte = irq & 0x3;
     uint8_t priority;
     struct vgic_irq_rank *rank = vgic_irq_rank(v, 8, idx);
-    struct pending_irq *iter, *n = irq_to_pending(v, irq);
+    struct pending_irq *n = irq_to_pending(v, irq);
     unsigned long flags;
     bool_t running;
 
@@ -692,16 +692,7 @@ void vgic_vcpu_inject_irq(struct vcpu *v, unsigned int irq,
int virtual)
     if ( rank->ienable & (1 << (irq % 32)) )
         gic_set_guest_irq(v, irq, GICH_LR_PENDING, priority);
 
-    list_for_each_entry ( iter, &v->arch.vgic.inflight_irqs, inflight )
-    {
-        if ( iter->priority > priority )
-        {
-            list_add_tail(&n->inflight, &iter->inflight);
-            goto out;
-        }
-    }
     list_add_tail(&n->inflight, &v->arch.vgic.inflight_irqs);
-out:
     spin_unlock_irqrestore(&v->arch.vgic.lock, flags);
     /* we have a new higher priority irq, inject it into the guest */
     running = v->is_running;
-- 
1.7.9.5

On Fri, 2013-06-14 at 20:07 +0900, Jaeyong Yoo wrote:
> Since inflight irq is only used for keeping track of on-going irqs, 
> I don''t think it should be sorted by priority. (Only used for
cleaing
> all inflight irqs and debug-print all inflight irqs) For IRQ processing 
> at xen, we can save some time by not keeping it in order.
It does seem to be the case that only the lr_pending list needs to be
ordered, but I''ve always had trouble following the flow of interrupts
through these lists so I''ll have to defer to Stefano on this one.
> Signed-off-by: Jaeyong Yoo <jaeyong.yoo@samsung.com>
> ---
>  xen/arch/arm/vgic.c |   11 +----------
>  1 file changed, 1 insertion(+), 10 deletions(-)
> 
> diff --git a/xen/arch/arm/vgic.c b/xen/arch/arm/vgic.c
> index 7eaccb7..47cc2a0 100644
> --- a/xen/arch/arm/vgic.c
> +++ b/xen/arch/arm/vgic.c
> @@ -666,7 +666,7 @@ void vgic_vcpu_inject_irq(struct vcpu *v, unsigned int
irq, int virtual)
>      int idx = irq >> 2, byte = irq & 0x3;
>      uint8_t priority;
>      struct vgic_irq_rank *rank = vgic_irq_rank(v, 8, idx);
> -    struct pending_irq *iter, *n = irq_to_pending(v, irq);
> +    struct pending_irq *n = irq_to_pending(v, irq);
>      unsigned long flags;
>      bool_t running;
>  
> @@ -692,16 +692,7 @@ void vgic_vcpu_inject_irq(struct vcpu *v, unsigned int
irq, int virtual)
>      if ( rank->ienable & (1 << (irq % 32)) )
>          gic_set_guest_irq(v, irq, GICH_LR_PENDING, priority);
>  
> -    list_for_each_entry ( iter, &v->arch.vgic.inflight_irqs,
inflight )
> -    {
> -        if ( iter->priority > priority )
> -        {
> -            list_add_tail(&n->inflight, &iter->inflight);
> -            goto out;
> -        }
> -    }
>      list_add_tail(&n->inflight,
&v->arch.vgic.inflight_irqs);
> -out:
>      spin_unlock_irqrestore(&v->arch.vgic.lock, flags);
>      /* we have a new higher priority irq, inject it into the guest */
>      running = v->is_running;

On Fri, 14 Jun 2013, Jaeyong Yoo wrote:
> Since inflight irq is only used for keeping track of on-going irqs, 
> I don''t think it should be sorted by priority. (Only used for
cleaing
> all inflight irqs and debug-print all inflight irqs) For IRQ processing 
> at xen, we can save some time by not keeping it in order.
> 
> Signed-off-by: Jaeyong Yoo <jaeyong.yoo@samsung.com>
It''s true that at the moment it''s not useful to order inflight
irqs by
priority but one day when we''ll actually support irq priorities
properly
and we''ll be able to replace an irq in an LR register with an higher
priority irq then it is probably going to be needed.

However we''ll likely have to make so many changes to make that work
that
we might as well reintroduce the order by priority in the inflight queue
too.

>  xen/arch/arm/vgic.c |   11 +----------
>  1 file changed, 1 insertion(+), 10 deletions(-)
> 
> diff --git a/xen/arch/arm/vgic.c b/xen/arch/arm/vgic.c
> index 7eaccb7..47cc2a0 100644
> --- a/xen/arch/arm/vgic.c
> +++ b/xen/arch/arm/vgic.c
> @@ -666,7 +666,7 @@ void vgic_vcpu_inject_irq(struct vcpu *v, unsigned int
irq, int virtual)
>      int idx = irq >> 2, byte = irq & 0x3;
>      uint8_t priority;
>      struct vgic_irq_rank *rank = vgic_irq_rank(v, 8, idx);
> -    struct pending_irq *iter, *n = irq_to_pending(v, irq);
> +    struct pending_irq *n = irq_to_pending(v, irq);
>      unsigned long flags;
>      bool_t running;
>  
> @@ -692,16 +692,7 @@ void vgic_vcpu_inject_irq(struct vcpu *v, unsigned int
irq, int virtual)
>      if ( rank->ienable & (1 << (irq % 32)) )
>          gic_set_guest_irq(v, irq, GICH_LR_PENDING, priority);
>  
> -    list_for_each_entry ( iter, &v->arch.vgic.inflight_irqs,
inflight )
> -    {
> -        if ( iter->priority > priority )
> -        {
> -            list_add_tail(&n->inflight, &iter->inflight);
> -            goto out;
> -        }
> -    }
>      list_add_tail(&n->inflight,
&v->arch.vgic.inflight_irqs);
> -out:
>      spin_unlock_irqrestore(&v->arch.vgic.lock, flags);
>      /* we have a new higher priority irq, inject it into the guest */
>      running = v->is_running;
> -- 
> 1.7.9.5
> 
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xen.org
> http://lists.xen.org/xen-devel
>