Re: [RFC IRQ] genirq: fix handle_nested_irq for lazy disable

[Abhijeet Dharmapurikar <adharmap@codeaurora.org>]

Thomas Gleixner wrote:
> On Thu, 28 Oct 2010, Abhijeet Dharmapurikar wrote:
>> Thomas Gleixner wrote:
>>> Aside of that this wont work for edge triggered interrupts, as you'd
>>> loose the edge, so this needs more thought and a thorough look at the
>>> users of handle_nested_irq().
>> I didn't understand this though. This patch will mask the interrupt in the
>> controller even if it were edge. My interrupt controller latches
>> edges and wants a mask (or an ack) to be executed to deactivate the line
>> summary line. Do you mean that I should mark the interrupt IRQ_PENDING if it
>> were an edge before masking it? If not, can you please explain.
> 
> See handle_edge_irq().
> 
> An edge is a one time event. Once you mask/ack it, it's gone. So now
> when you unmask it won't reissue the interrupt on the hardware
> level. Level interrupts do, as the mask does not affect that.

handle_edge_irq() needs to handle nested invocations and so it checks if 
the irq is in progress and does the right masking/unmasking and calling 
the handler again.
But I think we will never be executing an interrupt within an interrupt 
in the threaded irq controller case. By that I mean the irq controller 
thread wont call handle_nested_irq on the same interrupt until the first 
execution finishes. We don't need to worry about nested calls to 
handle_nested_irq().
If this is right, simply marking the edge interrupt IRQF_PENDING before 
masking it will suffice IMO.
Please let me know if this is closer to what you were suggesting?

diff --git a/kernel/irq/chip.c b/kernel/irq/chip.c
index baa5c4a..2dd0228 100644
--- a/kernel/irq/chip.c
+++ b/kernel/irq/chip.c
@@ -419,6 +419,7 @@ void handle_nested_irq(unsigned int irq)
  {
         struct irq_desc *desc = irq_to_desc(irq);
         struct irqaction *action;
+       int mask_this_irq = 0;
         irqreturn_t action_ret;

         might_sleep();
@@ -428,8 +429,12 @@ void handle_nested_irq(unsigned int irq)
         kstat_incr_irqs_this_cpu(irq, desc);

         action = desc->action;
-       if (unlikely(!action || (desc->status & IRQ_DISABLED)))
+       if (unlikely(!action || (desc->status & IRQ_DISABLED))) {
+               mask_this_irq = 1;
+               if (!(desc->status & IRQ_LEVEL))
+                       desc->status |= IRQ_PENDING;
                 goto out_unlock;
+       }

         desc->status |= IRQ_INPROGRESS;
         raw_spin_unlock_irq(&desc->lock);
@@ -443,6 +448,11 @@ void handle_nested_irq(unsigned int irq)

  out_unlock:
         raw_spin_unlock_irq(&desc->lock);
+       if (unlikely(mask_this_irq)) {
+               chip_bus_lock(desc);
+               mask_irq(irq);
+               chip_bus_sync_unlock(desc);
+       }
  }
  EXPORT_SYMBOL_GPL(handle_nested_irq);

-- 
Sent by an employee of the Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum.