* [Xenomai-core] [PATCH] shared irqs v.3
@ 2006-01-15 8:50 Dmitry Adamushko
2006-01-17 13:32 ` Jan Kiszka
0 siblings, 1 reply; 3+ messages in thread
From: Dmitry Adamushko @ 2006-01-15 8:50 UTC (permalink / raw)
To: xenomai
[-- Attachment #1.1: Type: text/plain, Size: 2174 bytes --]
Hi,
here goes another implementation of shared irqs on the nucleus layer.
I have conducted a few tests and it seems to work. The test example is
attached.
There were 2 main issues concerning synchronization:
1) xnintr_attach() vs. xnintr_detach() (and each of them vs. itself)
The problem is that we can't use the "nklock" (nor any other lock + irq
off) as Gilles pointed out.
A possible solution:
o something lick xnlock_get/put()
There is no irqsave/restore -less interface of xnlock_get/put available.
For pure locking scheme
(without touching the irqs) the concept of _preemption_ (to prevent a
thread from being preempted
while in a locked section) must be introduced and, at the first glance,
that would be quite difficult
since it must be consistent across all domains (if only for the primary
- that's easy).
o rthal_critical_enter/exit()
This one is used currently.
2) xnintr_attach/detach() vs. xnintr_irq_handler()
The problem here is how to be sure that 1) the "xnintr_shirq_t" object
is valid (when dynamically allocated)
and 2) to be safe while iterating through the handlers list.
Currently, 1) is allowed by the static xnintr_shirq_t
xnshirqs[IPIPE_NR_IRQS]. Ok, it can be done lighter
when a one-way-list is used instead of xnqueue_t.
Beleive it or not, I have considered different ways to guarantee that a
passed "cookie" param is valid
(xnintr_detach() has not deleted it) and remains to be so while the
xnintr_irq_handler() is running.
And there are some obstacles there... I'll post them later if someone is
interested since I'm short of time now :)
...
There are a few ugly things in code, namely __IPIPE_NR_IRQS and definitions
of rthal_critical_enter/exit().
That code is compiled for the user-mode code also and the originals are not
available. So consider it a temp
solution for test purposes, I guess it's easily fixable.
test/shirq.c - is a test module.
SHIRQ_VECTOR must be the one used by Linux, e.g. I have used 12 that's used
by the trackball device.
--
Best regards,
Dmitry Adamushko
[-- Attachment #1.2: Type: text/html, Size: 2819 bytes --]
[-- Attachment #2: shirq-2_intr.c.patch --]
[-- Type: application/octet-stream, Size: 4907 bytes --]
--- intr-cvs.c 2005-12-02 19:56:20.000000000 +0100
+++ intr.c 2006-01-15 09:39:37.000000000 +0100
@@ -41,6 +41,62 @@ xnintr_t nkclock;
static void xnintr_irq_handler(unsigned irq,
void *cookie);
+typedef struct xnintr_shirq {
+
+ xnqueue_t handlers;
+
+#ifdef CONFIG_SMP
+ atomic_counter_t active;
+#endif /* CONFIG_SMP */
+
+} xnintr_shirq_t;
+
+#define __IPIPE_NR_IRQS 224
+
+static xnintr_shirq_t xnshirqs[__IPIPE_NR_IRQS];
+
+extern unsigned long rthal_critical_enter(void (*synch)(void));
+extern void rthal_critical_exit(unsigned long flags);
+
+#ifdef CONFIG_SMP
+
+static inline void xnintr_shirq_lock(xnintr_shirq_t *shirq)
+{
+ xnarch_atomic_inc(&shirq->active);
+}
+
+static inline void xnintr_shirq_unlock(xnintr_shirq_t *shirq)
+{
+ xnarch_atomic_dec(&shirq->active);
+}
+
+static inline void xnintr_shirq_spin(xnintr_shirq_t *shirq)
+{
+ while (xnarch_atomic_get(&shirq->active))
+ cpu_relax();
+}
+
+#else /* !CONFIG_SMP */
+
+static inline void xnintr_shirq_lock(xnintr_shirq_t *shirq) {}
+static inline void xnintr_shirq_unlock(xnintr_shirq_t *shirq) {}
+static inline void xnintr_shirq_spin(xnintr_shirq_t *shirq) {}
+
+#endif /* CONFIG_SMP */
+
+int xnintr_mount(void)
+{
+ int i;
+ for (i = 0; i < __IPIPE_NR_IRQS; ++i)
+ {
+ initq(&xnshirqs[i].handlers);
+#ifdef CONFIG_SMP
+ atomic_set(&xnshirqs[i].active,0);
+#endif /* CONFIG_SMP */
+ }
+ return 0;
+}
+
/*!
* \fn int xnintr_init (xnintr_t *intr,unsigned irq,xnisr_t isr,xniack_t iack,xnflags_t flags)
* \brief Initialize an interrupt object.
@@ -130,6 +186,8 @@ int xnintr_init (xnintr_t *intr,
intr->iack = iack;
intr->cookie = NULL;
intr->hits = 0;
+ intr->flags = flags;
+ inith(&intr->link);
return 0;
}
@@ -181,8 +239,7 @@ int xnintr_destroy (xnintr_t *intr)
*
* @param cookie A user-defined opaque value which is stored into the
* interrupt object descriptor for further retrieval by the ISR/ISR
- * handlers.
- *
+ * handlers. *
* @return 0 is returned on success. Otherwise, -EINVAL is returned if
* a low-level error occurred while attaching the interrupt. -EBUSY is
* specifically returned if the interrupt object was already attached.
@@ -204,11 +261,45 @@ int xnintr_destroy (xnintr_t *intr)
int xnintr_attach (xnintr_t *intr,
void *cookie)
{
+ xnintr_shirq_t *shirq = &xnshirqs[intr->irq];
+ int err = 0, setup = 0, n;
+ unsigned long flags;
+
intr->hits = 0;
intr->cookie = cookie;
- return xnarch_hook_irq(intr->irq,&xnintr_irq_handler,intr->iack,intr);
+
+ flags = rthal_critical_enter(NULL);
+
+ if (n = countq(&shirq->handlers))
+ {
+ xnintr_t *old = link2xnintr(getheadq(&shirq->handlers));
+
+ if (!(old->flags & intr->flags & XN_ISR_SHARED))
+ {
+ err = -EBUSY;
+ goto unlock_and_exit;
+ }
+ }
+ else
+ setup = 1;
+
+ appendq(&shirq->handlers,&intr->link);
+
+ if (setup)
+ {
+ err = xnarch_hook_irq(intr->irq,&xnintr_irq_handler,intr->iack,NULL);
+
+ if (err)
+ removeq(&shirq->handlers,&intr->link);
+ }
+
+unlock_and_exit:
+
+ rthal_critical_exit(flags);
+ return err;
}
+
/*!
* \fn int xnintr_detach (xnintr_t *intr)
* \brief Detach an interrupt object.
@@ -241,7 +332,27 @@ int xnintr_attach (xnintr_t *intr,
int xnintr_detach (xnintr_t *intr)
{
- return xnarch_release_irq(intr->irq);
+ xnintr_shirq_t *shirq = &xnshirqs[intr->irq];
+ unsigned long flags;
+ int err = 0;
+
+ flags = rthal_critical_enter(NULL);
+
+ removeq(&shirq->handlers,&intr->link);
+
+ if (!countq(&shirq->handlers))
+ err = xnarch_release_irq(intr->irq);
+
+ rthal_critical_exit(flags);
+
+ /* The idea here is to keep the removed xnintr_t element valid as long
+ as the corresponding irq handler is running. This is one of the requirements
+ to iterate over the xnintr_shirq_t::handlers list in xnintr_irq_handler()
+ in a lockless way. */
+
+ xnintr_shirq_spin(shirq);
+
+ return err;
}
/*!
@@ -350,17 +461,32 @@ static void xnintr_irq_handler (unsigned
{
xnsched_t *sched = xnpod_current_sched();
- xnintr_t *intr = (xnintr_t *)cookie;
- int s;
+ xnintr_shirq_t *shirq = &xnshirqs[irq];
+ xnholder_t *holder;
+ int s = 0;
xnarch_memory_barrier();
xnltt_log_event(xeno_ev_ienter,irq);
++sched->inesting;
- s = intr->isr(intr);
+
+ xnintr_shirq_lock(shirq);
+
+ holder = getheadq(&shirq->handlers);
+
+ while (holder)
+ {
+ xnintr_t *intr = link2xnintr(holder);
+ holder = nextq(&shirq->handlers,holder);
+
+ s |= intr->isr(intr);
+ ++intr->hits;
+ }
+
+ xnintr_shirq_unlock(shirq);
+
--sched->inesting;
- ++intr->hits;
if (s & XN_ISR_ENABLE)
xnarch_enable_irq(irq);
@@ -384,6 +510,7 @@ static void xnintr_irq_handler (unsigned
xnltt_log_event(xeno_ev_iexit,irq);
}
+
/*@}*/
EXPORT_SYMBOL(xnintr_attach);
[-- Attachment #3: shirq-2_intr.h.patch --]
[-- Type: application/octet-stream, Size: 1151 bytes --]
--- intr-cvs.h 2005-11-01 23:37:45.000000000 +0100
+++ intr.h 2006-01-14 20:13:19.000000000 +0100
@@ -26,21 +26,31 @@
#define XN_ISR_CHAINED 0x1
#define XN_ISR_ENABLE 0x2
+/* Creation flags. */
+#define XN_ISR_SHARED 0x1
+
#if defined(__KERNEL__) || defined(__XENO_UVM__) || defined(__XENO_SIM__)
struct xnintr;
typedef struct xnintr {
- unsigned irq; /* !< IRQ number. */
+ xnholder_t link;
+#define link2xnintr(laddr) \
+((xnintr_t *)(((char *)laddr) - (int)(&((xnintr_t *)0)->link)))
+
xnisr_t isr; /* !< Interrupt service routine. */
- xniack_t iack; /* !< Interrupt acknowledge routine. */
+ void *cookie; /* !< User-defined cookie value. */
unsigned long hits; /* !< Number of receipts (since attachment). */
- void *cookie; /* !< User-defined cookie value. */
+ xnflags_t flags; /* !< Creation flags. */
+
+ unsigned irq; /* !< IRQ number. */
+
+ xniack_t iack; /* !< Interrupt acknowledge routine. */
} xnintr_t;
@@ -50,6 +60,8 @@ extern xnintr_t nkclock;
extern "C" {
#endif
+int xnintr_mount(void);
+
void xnintr_clock_handler(void);
/* Public interface. */
[-- Attachment #4: shirq-2_module.c.patch --]
[-- Type: application/octet-stream, Size: 298 bytes --]
--- module-cvs.c 2006-01-15 09:33:10.000000000 +0100
+++ module.c 2006-01-14 20:02:51.000000000 +0100
@@ -714,6 +714,8 @@ int __init __xeno_sys_init (void)
xnpod_init_proc();
#endif /* CONFIG_PROC_FS */
+ xnintr_mount();
+
#ifdef CONFIG_LTT
xnltt_mount();
#endif /* CONFIG_LTT */
[-- Attachment #5: test-shirq.tgz --]
[-- Type: application/x-gzip, Size: 1110 bytes --]
^ permalink raw reply [flat|nested] 3+ messages in thread* Re: [Xenomai-core] [PATCH] shared irqs v.3
2006-01-15 8:50 [Xenomai-core] [PATCH] shared irqs v.3 Dmitry Adamushko
@ 2006-01-17 13:32 ` Jan Kiszka
2006-01-18 9:19 ` Dmitry Adamushko
0 siblings, 1 reply; 3+ messages in thread
From: Jan Kiszka @ 2006-01-17 13:32 UTC (permalink / raw)
To: Dmitry Adamushko; +Cc: xenomai
[-- Attachment #1: Type: text/plain, Size: 3655 bytes --]
Hi Dmitry,
Dmitry Adamushko wrote:
> Hi,
>
> here goes another implementation of shared irqs on the nucleus layer.
> I have conducted a few tests and it seems to work. The test example is
> attached.
>
> There were 2 main issues concerning synchronization:
>
> 1) xnintr_attach() vs. xnintr_detach() (and each of them vs. itself)
>
> The problem is that we can't use the "nklock" (nor any other lock + irq
> off) as Gilles pointed out.
>
> A possible solution:
>
> o something lick xnlock_get/put()
>
> There is no irqsave/restore -less interface of xnlock_get/put available.
> For pure locking scheme
> (without touching the irqs) the concept of _preemption_ (to prevent a
> thread from being preempted
> while in a locked section) must be introduced and, at the first glance,
> that would be quite difficult
> since it must be consistent across all domains (if only for the primary
> - that's easy).
>
> o rthal_critical_enter/exit()
>
> This one is used currently.
>
>
> 2) xnintr_attach/detach() vs. xnintr_irq_handler()
>
> The problem here is how to be sure that 1) the "xnintr_shirq_t" object
> is valid (when dynamically allocated)
> and 2) to be safe while iterating through the handlers list.
>
> Currently, 1) is allowed by the static xnintr_shirq_t
> xnshirqs[IPIPE_NR_IRQS]. Ok, it can be done lighter
> when a one-way-list is used instead of xnqueue_t.
As lighter may mean that reducing the structure size also reduces the
number of used cache lines, it might be a good idea. The additional
complexity for entry removal is negligible.
>
> Beleive it or not, I have considered different ways to guarantee that a
> passed "cookie" param is valid
> (xnintr_detach() has not deleted it) and remains to be so while the
> xnintr_irq_handler() is running.
> And there are some obstacles there... I'll post them later if someone is
> interested since I'm short of time now :)
> ...
I'm interested...
>
>
> There are a few ugly things in code, namely __IPIPE_NR_IRQS and definitions
> of rthal_critical_enter/exit().
I do understand the first issue but not what you mean with the second one.
> That code is compiled for the user-mode code also and the originals are not
> available. So consider it a temp
> solution for test purposes, I guess it's easily fixable.
>
> test/shirq.c - is a test module.
>
> SHIRQ_VECTOR must be the one used by Linux, e.g. I have used 12 that's used
> by the trackball device.
>
I haven't tried your code yet, but in the preparation of a real scenario
I stumbled over a problem in my serial driver regarding IRQ sharing: In
case you want to use xeno_16550A for ISA devices with shared IRQs, an
iteration over the list of registered handlers would be required /until/
no device reported that it handled something. This is required so that
the IRQ line gets released for a while and system obtains a chance to
detect a new /edge/ triggered IRQ - ISA oddity.
That's the way most serial drivers work, but they do it internally. So
the question arose for me if this edge-specific handling shouldn't be
moved to the nucleus as well (so that I don't have to fix my 16550A ;)).
Another optimisation idea, which I once also realised in my own shared
IRQ wrapper, is to use specialised trampolines at the nucleus level,
i.e. to not apply the full sharing logic with its locking and list
iterations for non-shared IRQs. What do you think? Worth it? Might be
when the ISA/edge handling adds further otherwise unneeded overhead.
Jan
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 250 bytes --]
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [Xenomai-core] [PATCH] shared irqs v.3
2006-01-17 13:32 ` Jan Kiszka
@ 2006-01-18 9:19 ` Dmitry Adamushko
0 siblings, 0 replies; 3+ messages in thread
From: Dmitry Adamushko @ 2006-01-18 9:19 UTC (permalink / raw)
To: Jan Kiszka; +Cc: xenomai
[-- Attachment #1: Type: text/plain, Size: 3700 bytes --]
Hi Jan,
> As lighter may mean that reducing the structure size also
> reduces the number of used cache lines, it might be a good
> idea. The additional complexity for entry removal is negligible.
My current working version is already lighter when it comes to the size of
additional data structures. It's implemented via the one-way linked list
instead of xnqueue_t. This way, it's 3 times lighter for UP and 2 times for
SMP systems.
I'll try to post it today later. The only problem remaining is the
compilation issues so I should fix it before posting, namely:
it looks like some code in kscr/nucleus (e.g. intr.c) is used for compiling
both kernel-mode code (of cousre) and user-mode (maybe for UVM, though I
haven't looked at it thoroughly yet). The link to ksrc/nucleus is created in
the src/ directory.
Both the IPIPE_NR_IRQS macro and rthal_critical_enter/exit() calls are
undefined when intr.c is compiled for the user-mode side. That's why it so
far contains those __IPIPE_NR_IRQS and "external int
rthal_critical_enter/exit()" definitions.
I hope that also answers your another question later on this mail.
> >
> > Beleive it or not, I have considered different ways to
> >guarantee that a passed "cookie" param is valid
> > (xnintr_detach() has not deleted it) and remains to be so
> > while the xnintr_irq_handler() is running.
> > And there are some obstacles there... I'll post them later if > >
someone is interested since I'm short of time now :)
> > ...
> I'm interested...
Ok. So I will have at least a reader :) Actually, I still hope to find out
some solution so to make use of the recently extended ipipe interface as it
was supposed to be used (then there is no need for any per-irq "xnshirqs"
array in intr.c).
Otherwise, I have to admit that my recent work with that ipipe extension (I
can ay it since I made it) is of no big avail.
Maybe we together will find out a solution.
> > That code is compiled for the user-mode code also and the
> >originals are not available. So consider it a temp
> > solution for test purposes, I guess it's easily fixable.
> >
> > test/shirq.c - is a test module.
> >
> > SHIRQ_VECTOR must be the one used by Linux, e.g. I have > >used 12
that's used
> > by the trackball device.
> >
> >
> I haven't tried your code yet, but in the preparation of a real
>scenario I stumbled over a problem in my serial driver regarding > IRQ
sharing: In case you want to use xeno_16550A for ISA >devices with shared
IRQs, an iteration over the list of registered >handlers would be required
/until/ no device reported that it >handled something. This is required so
that the IRQ line gets
> released for a while and system obtains a chance to
> detect a new /edge/ triggered IRQ - ISA oddity.
>
> That's the way most serial drivers work, but they do it internally.
> So the question arose for me if this edge-specific handling
> shouldn't be moved to the nucleus as well (so that I don't have > to fix
my 16550A ;)).
Brrr... frankly speaking, I haven't got it clearly so don't want to make
pure speculations. Probably I have to take a look at the xeno_16550A driver
keeping in mind your words.
> Another optimisation idea, which I once also realised in my
> own shared IRQ wrapper, is to use specialised trampolines at >the nucleus
level, i.e. to not apply the full sharing logic with its >locking and list
iterations for non-shared IRQs. What do you >think? Worth it? Might be when
the ISA/edge handling adds >further otherwise unneeded overhead.
Yep, maybe. But let's take something working first..
> Jan
--
Best regards,
Dmitry Adamushko
[-- Attachment #2: Type: text/html, Size: 4264 bytes --]
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2006-01-18 9:19 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2006-01-15 8:50 [Xenomai-core] [PATCH] shared irqs v.3 Dmitry Adamushko
2006-01-17 13:32 ` Jan Kiszka
2006-01-18 9:19 ` Dmitry Adamushko
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.