linux-kernel - Re: [RFC IRQ] genirq: fix handle_nested

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <4CCA0F92.4070802@codeaurora.org>
Date:	Thu, 28 Oct 2010 17:04:34 -0700
From:	Abhijeet Dharmapurikar <adharmap@...eaurora.org>
To:	Thomas Gleixner <tglx@...utronix.de>
CC:	Ingo Molnar <mingo@...e.hu>, "H. Peter Anvin" <hpa@...or.com>,
	linux-kernel@...r.kernel.org, linux-arm-msm-owner@...r.kernel.org
Subject: Re: [RFC IRQ] genirq: fix handle_nested_irq for lazy disable

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.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/