Re: [Xenomai-help] kernel oopses when killing realtime task

[Jan Kiszka <jan.kiszka@domain.hid>]

[-- Attachment #1: Type: text/plain, Size: 3780 bytes --]

Am 28.10.2010 09:31, Jan Kiszka wrote:
> This sounds like it's best discussed based on patches.
> 

BTW, this is my current idea to make deregistration via
ipipe_virtualize_irq safe against ongoing IRQs, ie. the risk to invoke
a NULL handler:


From: Jan Kiszka <jan.kiszka@domain.hid>

ipipe: Catch ongoing IRQs while deregistering from the line

If ipipe_virtualize_irq is called for an IRQ that may be under
processing on a different CPU, we must not clear the handler to avoid
NULL pointer failures. However, this IRQ-on-the-fly already became
spurious before the caller invoked ipipe_virtualize_irq. So install a
dummy handler that swallows the event silently.

Signed-off-by: Jan Kiszka <jan.kiszka@domain.hid>
---
 kernel/ipipe/core.c |   30 ++++++++++++++++++++++++------
 1 files changed, 24 insertions(+), 6 deletions(-)

diff --git a/kernel/ipipe/core.c b/kernel/ipipe/core.c
index b3a8139..bf43bb8 100644
--- a/kernel/ipipe/core.c
+++ b/kernel/ipipe/core.c
@@ -277,6 +277,18 @@ void __init ipipe_init(void)
 	       IPIPE_VERSION_STRING);
 }
 
+static void ipipe_final_irq_handler(unsigned irq, struct irq_desc *desc)
+{
+	/*
+	 * De-virtualization of an IRQ may race with ongoing IRQ delivery on a
+	 * different CPU. This handler catches the delivery and ends the
+	 * request. It is assumed to be the final event as the caller of
+	 * ipipe_virtualize_irq() is supposed to have disabled any related IRQ
+	 * source at device level before deregistering the interrupt.
+	 */
+	desc->ipipe_end(irq, desc);
+}
+
 void __ipipe_init_stage(struct ipipe_domain *ipd)
 {
 	struct ipipe_percpu_domain_data *p;
@@ -292,7 +304,7 @@ void __ipipe_init_stage(struct ipipe_domain *ipd)
 
 	for (n = 0; n < IPIPE_NR_IRQS; n++) {
 		ipd->irqs[n].acknowledge = NULL;
-		ipd->irqs[n].handler = NULL;
+		ipd->irqs[n].handler = ipipe_final_irq_handler;
 		ipd->irqs[n].control = IPIPE_PASS_MASK;	/* Pass but don't handle */
 	}
 
@@ -924,14 +936,20 @@ int ipipe_virtualize_irq(struct ipipe_domain *ipd,
 		    ~(IPIPE_HANDLE_MASK | IPIPE_STICKY_MASK |
 		      IPIPE_EXCLUSIVE_MASK | IPIPE_WIRED_MASK);
 
-		ipd->irqs[irq].handler = NULL;
+		ipd->irqs[irq].handler = ipipe_final_irq_handler;
+		/*
+		 * Make sure we never call the old handler with an invalid
+		 * cookie. The dummy handler ignores the cookie. Set it first.
+		 */
+		smp_mb();
 		ipd->irqs[irq].cookie = NULL;
-		ipd->irqs[irq].acknowledge = NULL;
+		ipd->irqs[irq].acknowledge =
+			ipipe_root_domain->irqs[irq].acknowledge;
 		ipd->irqs[irq].control = modemask;
 
 		if (irq < NR_IRQS && !ipipe_virtual_irq_p(irq)) {
 			desc = irq_to_desc(irq);
-			if (old_handler && desc)
+			if (old_handler != ipipe_final_irq_handler && desc)
 				__ipipe_disable_irqdesc(ipd, irq);
 		}
 
@@ -941,7 +959,7 @@ int ipipe_virtualize_irq(struct ipipe_domain *ipd,
 	if (handler == IPIPE_SAME_HANDLER) {
 		cookie = ipd->irqs[irq].cookie;
 		handler = old_handler;
-		if (handler == NULL) {
+		if (handler == ipipe_final_irq_handler) {
 			ret = -EINVAL;
 			goto unlock_and_exit;
 		}
@@ -1024,7 +1042,7 @@ int ipipe_control_irq(struct ipipe_domain *ipd, unsigned int irq,
 		goto out;
 	}
 
-	if (ipd->irqs[irq].handler == NULL)
+	if (ipd->irqs[irq].handler == ipipe_final_irq_handler)
 		setmask &= ~(IPIPE_HANDLE_MASK | IPIPE_STICKY_MASK);
 
 	if ((setmask & IPIPE_STICKY_MASK) != 0)
-- 
1.7.1


I'm not your 100% happy with the fact that the old acknowledge handler
may be paired with ipipe_finale_irq_handler this way, but I see no
better approach so far.

So this patch just had to be paired with a barrier after the
deregistration (in Xenomai) to ensure all users of the old handlers and
cookies are done.

Jan


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 259 bytes --]