* [Xenomai-help] -110 error on rt_task_send... bug?
@ 2006-08-08 1:50 Vincent Levesque
2006-08-08 22:12 ` Jan Kiszka
0 siblings, 1 reply; 16+ messages in thread
From: Vincent Levesque @ 2006-08-08 1:50 UTC (permalink / raw)
To: xenomai; +Cc: andrewg
Hello,
I'm having some problems with rt_task_send/receive/reply in the native
skin. The code attached at the end of this email works fine on Xenomai
2.1 but rt_task_send() fails with error message -110 on Xenomai 2.2.0
and today's subversion trunk. I checked the error values of all other
calls (removed for clarity) and everything seems to be ok. The -110
error code is returned either when rt_task_send() times out or when
rt_task_reply() is called, whichever comes first. If I set it to
TM_INFINITE, rt_task_send() never returns and the system hangs when I
press control-c. I've reproduced this behavior on two computers.
Am I doing something wrong that was tolerated by 2.1? Is this a bug in
Xenomai 2.2?
Thank you,
Vincent Levesque
vleves@domain.hid
gcc -o test test.c `/usr/xenomai/bin/xeno-config --xeno-cflags` -O2
`/usr/xenomai/bin/xeno-config --xeno-ldflags` -lnative
--------------------------------------------------------
test.c
--------------------------------------------------------
#include <stdio.h>
#include <sys/mman.h>
#include <native/task.h>
void recv_task(void *arg)
{
RT_TASK_MCB mcb_receive;
int flowid;
int data;
mcb_receive.data = (caddr_t)&data;
mcb_receive.size = sizeof(int);
flowid = rt_task_receive(&mcb_receive,TM_INFINITE);
if (mcb_receive.opcode == 1)
{
printf("receiving %d\n", data);
rt_task_reply(flowid, NULL);
}
}
int main (int argc, char *argv[])
{
int data, rv;
RT_TASK_MCB mcb_send;
RT_TASK peer;
RT_TASK task;
mlockall(MCL_CURRENT | MCL_FUTURE);
rt_task_spawn(&peer, "recv_task", 0, 99, T_FPU, &recv_task, NULL);
rt_task_shadow(&task, "send_task", 50, 0);
data = 13;
mcb_send.opcode = 1;
mcb_send.data = (caddr_t)&data;
mcb_send.size = sizeof(int);
printf("sending %d\n", data);
rv = rt_task_send(&peer,&mcb_send,NULL,1E9);
if (rv < 0)
{
switch(rv)
{
case -ENOBUFS: printf("ENOBUFS\n");
case -EWOULDBLOCK: printf("EWOULDBLOCK\n");
case -EIDRM: printf("EIDRM\n");
case -EINTR: printf("EINTR\n");
case -EPERM: printf("EPERM\n");
default: printf("unknown error: %d\n", rv);
}
}
rt_task_delete(&peer);
return 0;
}
^ permalink raw reply [flat|nested] 16+ messages in thread* Re: [Xenomai-help] -110 error on rt_task_send... bug? 2006-08-08 1:50 [Xenomai-help] -110 error on rt_task_send... bug? Vincent Levesque @ 2006-08-08 22:12 ` Jan Kiszka 2006-08-09 9:35 ` Dmitry Adamushko 0 siblings, 1 reply; 16+ messages in thread From: Jan Kiszka @ 2006-08-08 22:12 UTC (permalink / raw) To: Vincent Levesque; +Cc: xenomai, andrewg [-- Attachment #1: Type: text/plain, Size: 1828 bytes --] Vincent Levesque wrote: > Hello, > > I'm having some problems with rt_task_send/receive/reply in the native > skin. The code attached at the end of this email works fine on Xenomai > 2.1 but rt_task_send() fails with error message -110 on Xenomai 2.2.0 > and today's subversion trunk. I checked the error values of all other > calls (removed for clarity) and everything seems to be ok. The -110 > error code is returned either when rt_task_send() times out or when > rt_task_reply() is called, whichever comes first. If I set it to > TM_INFINITE, rt_task_send() never returns and the system hangs when I > press control-c. I've reproduced this behavior on two computers. > > Am I doing something wrong that was tolerated by 2.1? Is this a bug in > Xenomai 2.2? Nothing. This looks like a good catch of a regression in 2.2. I'm not that deep into rt_task_send/receive design, but this problem seems to be related to some heavy disagree between xnsynch_sleep_on and rt_task_send about the correct ownership of the receiver's send queue. Reading this comment [1] and then looking at that piece of code [2], something must go utterly wrong wrt/ message passing in Xenomai 2.2. [1] says the owner of task->msendq will always be "task" and never "sender" (the current thread), so the test in [2] will always fail. The lock-steeling code was introduced with 2.2, so this effect did not occur with 2.1. I have no idea yet how to solve it cleanly. Maybe Philippe finds some time to comment on this before he'll be away for the next week. In the meantime message passing users of the native skin may better stay on 2.1.x. Jan [1]http://www.rts.uni-hannover.de/xenomai/lxr/source/ksrc/skins/native/task.c#L1750 [2]http://www.rts.uni-hannover.de/xenomai/lxr/source/ksrc/nucleus/synch.c#L212 [-- Attachment #2: OpenPGP digital signature --] [-- Type: application/pgp-signature, Size: 249 bytes --] ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [Xenomai-help] -110 error on rt_task_send... bug? 2006-08-08 22:12 ` Jan Kiszka @ 2006-08-09 9:35 ` Dmitry Adamushko 2006-08-09 11:42 ` [Xenomai-core] " Dmitry Adamushko 2006-08-10 22:13 ` Philippe Gerum 0 siblings, 2 replies; 16+ messages in thread From: Dmitry Adamushko @ 2006-08-09 9:35 UTC (permalink / raw) To: Jan Kiszka; +Cc: xenomai, andrewg [-- Attachment #1: Type: text/plain, Size: 1460 bytes --] Hello, I'm not that deep into rt_task_send/receive design, but this problem > seems to be related to some heavy disagree between xnsynch_sleep_on and > rt_task_send about the correct ownership of the receiver's send queue. Not transfering the ownership causes this problem, yes. But looking briefly at the code... there is even yet another problem (if I haven't overlooked something) that also exists in 2.1. rt_task_reply() wakes up a sender with : [code] if (receiver->wait_args.mps.mcb_s.flowid == flowid) { /* Note that the following will cause the receiver to be unblocked without transferring the ownership of the msendq object, since we want the sender to keep it. */ xnpod_resume_thread(&receiver->thread_base, XNPEND); break; [/code] Now let's suppose : sender.prio > recv.prio so recv.prio is boosted when sender executes xnsynch_sleep_on(). The "recv" task (the one that calls rt_task_receive() and rt_task_reply()) doesn't use xnsynch_* functions (like xnsynch_wakeup_this_sleeper() ) to actually wake up the sender. Therefore xnsynch_clear_boost() is never called and "recv" keeps temporarily inhereted priority as a permanent one from now on. No? I have a few ideas on how to fix it intelegently so if Philippe will not come up with a quick solution, I may take a look? -- Best regards, Dmitry Adamushko [-- Attachment #2: Type: text/html, Size: 2427 bytes --] ^ permalink raw reply [flat|nested] 16+ messages in thread
* [Xenomai-core] Re: [Xenomai-help] -110 error on rt_task_send... bug? 2006-08-09 9:35 ` Dmitry Adamushko @ 2006-08-09 11:42 ` Dmitry Adamushko 2006-08-09 15:09 ` Ulrich Schwab 2006-08-10 22:13 ` Philippe Gerum 1 sibling, 1 reply; 16+ messages in thread From: Dmitry Adamushko @ 2006-08-09 11:42 UTC (permalink / raw) To: Jan Kiszka; +Cc: xenomai, andrewg, xenomai [-- Attachment #1.1: Type: text/plain, Size: 513 bytes --] So could anyone test this patch and let me know if it works? Note : I haven't compiled it even as I don't have a proper environment. But the changes are pretty simple so it should be ok. There was actually yet another problem mmm... who cares to delete a sender from the msendq? Now should be ok or maybe I'm completely wrong and see the things which do not exist at all :) p.s. I cc'ed "xenomai-core" as it may involve further discussions and it's the right place indeed. -- Best regards, Dmitry Adamushko [-- Attachment #1.2: Type: text/html, Size: 593 bytes --] [-- Attachment #2: synch-fowner.patch --] [-- Type: text/x-patch, Size: 4022 bytes --] diff -urp xenomai-a/include/nucleus/synch.h xenomai-b/include/nucleus/synch.h --- xenomai-a/include/nucleus/synch.h 2006-07-20 11:09:01.000000000 +0200 +++ xenomai-b/include/nucleus/synch.h 2006-08-09 12:53:37.044217000 +0200 @@ -28,6 +28,7 @@ #define XNSYNCH_NOPIP 0x0 #define XNSYNCH_PIP 0x2 #define XNSYNCH_DREORD 0x4 +#define XNSYNCH_FOWNER 0x20 /* Fixed owner */ #if defined(__KERNEL__) || defined(__XENO_SIM__) diff -urp xenomai-a/ksrc/nucleus/synch.c xenomai-b/ksrc/nucleus/synch.c --- xenomai-a/ksrc/nucleus/synch.c 2006-06-15 14:15:25.000000000 +0200 +++ xenomai-b/ksrc/nucleus/synch.c 2006-08-09 13:28:55.199081000 +0200 @@ -37,6 +37,14 @@ #include <nucleus/module.h> #include <nucleus/ltt.h> +/* temporarily here */ +static inline void xnsynch_set_owner_internal(xnsynch_t *synch, xnthread_t *thread) +{ + if (!testbits(synch->status, XNSYNCH_FOWNER)) + synch->owner = thread; +} + + /*! * \fn void xnsynch_init(xnsynch_t *synch, xnflags_t flags); * \brief Initialize a synchronization object. @@ -181,7 +189,7 @@ void xnsynch_sleep_on(xnsynch_t *synch, if (testbits(synch->status, XNSYNCH_PENDING)) { /* Ownership is still pending, steal the resource. */ - synch->owner = thread; + xnsynch_set_owner_internal(synch, thread); __clrbits(thread->status, XNRMID | XNTIMEO | XNBREAK); goto grab_ownership; @@ -209,7 +217,7 @@ void xnsynch_sleep_on(xnsynch_t *synch, xnpod_suspend_thread(thread, XNPEND, timeout, synch); - if (unlikely(synch->owner != thread)) { + if (!testbits(synch->status, XNSYNCH_OWNER) && unlikely(synch->owner != thread)) { /* Somebody stole us the ownership while we were ready to run, waiting for the CPU: we need to wait again for the resource. */ @@ -362,12 +370,12 @@ xnthread_t *xnsynch_wakeup_one_sleeper(x if (holder) { thread = link2thread(holder, plink); thread->wchan = NULL; - synch->owner = thread; + xnsynch_set_owner_internal(synch, thread); __setbits(synch->status, XNSYNCH_PENDING); xnltt_log_event(xeno_ev_wakeup1, thread->name, synch); xnpod_resume_thread(thread, XNPEND); } else { - synch->owner = NULL; + xnsynch_set_owner_internal(synch, thread); __clrbits(synch->status, XNSYNCH_PENDING); } @@ -435,7 +443,7 @@ xnpholder_t *xnsynch_wakeup_this_sleeper nholder = poppq(&synch->pendq, holder); thread = link2thread(holder, plink); thread->wchan = NULL; - synch->owner = thread; + xnsynch_set_owner_internal(synch, thread); __setbits(synch->status, XNSYNCH_PENDING); xnltt_log_event(xeno_ev_wakeupx, thread->name, synch); xnpod_resume_thread(thread, XNPEND); @@ -523,7 +531,7 @@ int xnsynch_flush(xnsynch_t *synch, xnfl status = XNSYNCH_RESCHED; } - synch->owner = NULL; + xnsynch_set_owner_internal(synch, NULL); __clrbits(synch->status, XNSYNCH_PENDING); xnlock_put_irqrestore(&nklock, s); diff -urp xenomai-a/ksrc/skins/native/task.c xenomai-b/ksrc/skins/native/task.c --- xenomai-a/ksrc/skins/native/task.c 2006-07-30 10:50:49.000000000 +0200 +++ xenomai-b/ksrc/skins/native/task.c 2006-08-09 13:32:21.917770000 +0200 @@ -262,7 +262,7 @@ int rt_task_create(RT_TASK *task, #ifdef CONFIG_XENO_OPT_NATIVE_MPS xnsynch_init(&task->mrecv, XNSYNCH_FIFO); - xnsynch_init(&task->msendq, XNSYNCH_PRIO | XNSYNCH_PIP); + xnsynch_init(&task->msendq, XNSYNCH_PRIO | XNSYNCH_PIP | XNSYNCH_FOWNER); xnsynch_set_owner(&task->msendq, &task->thread_base); task->flowgen = 0; #endif /* CONFIG_XENO_OPT_NATIVE_MPS */ @@ -2057,10 +2057,7 @@ int rt_task_reply(int flowid, RT_TASK_MC identifier from other senders wrt to a given receiver. */ if (receiver->wait_args.mps.mcb_s.flowid == flowid) { - /* Note that the following will cause the receiver to be - unblocked without transferring the ownership of the - msendq object, since we want the sender to keep it. */ - xnpod_resume_thread(&receiver->thread_base, XNPEND); + xnsynch_wakeup_this_sleeper(&sender->msendq, holder); break; } } ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [Xenomai-core] Re: [Xenomai-help] -110 error on rt_task_send... bug? 2006-08-09 11:42 ` [Xenomai-core] " Dmitry Adamushko @ 2006-08-09 15:09 ` Ulrich Schwab 2006-08-09 15:42 ` Dmitry Adamushko 2006-08-10 8:25 ` Jan Kiszka 0 siblings, 2 replies; 16+ messages in thread From: Ulrich Schwab @ 2006-08-09 15:09 UTC (permalink / raw) To: xenomai; +Cc: andrewg, Jan Kiszka [-- Attachment #1: Type: text/plain, Size: 1017 bytes --] On Wednesday 09 August 2006 13:42, Dmitry Adamushko wrote: > So could anyone test this patch and let me know if it works? > > Note : I haven't compiled it even as I don't have a proper environment. But > the changes are pretty simple so it should be ok. I just did run the test program supplied by Vincent after applying Your patch (against 2.2.0) It did work ! Only one small typo was in the patch, it is fixed in the attached patch version. > > There was actually yet another problem mmm... who cares to delete a sender > from the msendq? > > Now should be ok or maybe I'm completely wrong and see the things which do > not exist at all :) > > p.s. I cc'ed "xenomai-core" as it may involve further discussions and it's > the right place indeed. -- ==================================================== inmess GmbH Frankfurter Str. 74 D - 64521 Gross-Gerau Phone: +49 6152 97790 Fax : +49 6152 977920 mail : info@domain.hid web: www.inmess.de ==================================================== [-- Attachment #2: synch-fowner.patch --] [-- Type: text/x-diff, Size: 4023 bytes --] diff -urp xenomai-a/include/nucleus/synch.h xenomai-b/include/nucleus/synch.h --- xenomai-a/include/nucleus/synch.h 2006-07-20 11:09:01.000000000 +0200 +++ xenomai-b/include/nucleus/synch.h 2006-08-09 12:53:37.044217000 +0200 @@ -28,6 +28,7 @@ #define XNSYNCH_NOPIP 0x0 #define XNSYNCH_PIP 0x2 #define XNSYNCH_DREORD 0x4 +#define XNSYNCH_FOWNER 0x20 /* Fixed owner */ #if defined(__KERNEL__) || defined(__XENO_SIM__) diff -urp xenomai-a/ksrc/nucleus/synch.c xenomai-b/ksrc/nucleus/synch.c --- xenomai-a/ksrc/nucleus/synch.c 2006-06-15 14:15:25.000000000 +0200 +++ xenomai-b/ksrc/nucleus/synch.c 2006-08-09 13:28:55.199081000 +0200 @@ -37,6 +37,14 @@ #include <nucleus/module.h> #include <nucleus/ltt.h> +/* temporarily here */ +static inline void xnsynch_set_owner_internal(xnsynch_t *synch, xnthread_t *thread) +{ + if (!testbits(synch->status, XNSYNCH_FOWNER)) + synch->owner = thread; +} + + /*! * \fn void xnsynch_init(xnsynch_t *synch, xnflags_t flags); * \brief Initialize a synchronization object. @@ -181,7 +189,7 @@ void xnsynch_sleep_on(xnsynch_t *synch, if (testbits(synch->status, XNSYNCH_PENDING)) { /* Ownership is still pending, steal the resource. */ - synch->owner = thread; + xnsynch_set_owner_internal(synch, thread); __clrbits(thread->status, XNRMID | XNTIMEO | XNBREAK); goto grab_ownership; @@ -209,7 +217,7 @@ void xnsynch_sleep_on(xnsynch_t *synch, xnpod_suspend_thread(thread, XNPEND, timeout, synch); - if (unlikely(synch->owner != thread)) { + if (!testbits(synch->status, XNSYNCH_FOWNER) && unlikely(synch->owner != thread)) { /* Somebody stole us the ownership while we were ready to run, waiting for the CPU: we need to wait again for the resource. */ @@ -362,12 +370,12 @@ xnthread_t *xnsynch_wakeup_one_sleeper(x if (holder) { thread = link2thread(holder, plink); thread->wchan = NULL; - synch->owner = thread; + xnsynch_set_owner_internal(synch, thread); __setbits(synch->status, XNSYNCH_PENDING); xnltt_log_event(xeno_ev_wakeup1, thread->name, synch); xnpod_resume_thread(thread, XNPEND); } else { - synch->owner = NULL; + xnsynch_set_owner_internal(synch, thread); __clrbits(synch->status, XNSYNCH_PENDING); } @@ -435,7 +443,7 @@ xnpholder_t *xnsynch_wakeup_this_sleeper nholder = poppq(&synch->pendq, holder); thread = link2thread(holder, plink); thread->wchan = NULL; - synch->owner = thread; + xnsynch_set_owner_internal(synch, thread); __setbits(synch->status, XNSYNCH_PENDING); xnltt_log_event(xeno_ev_wakeupx, thread->name, synch); xnpod_resume_thread(thread, XNPEND); @@ -523,7 +531,7 @@ int xnsynch_flush(xnsynch_t *synch, xnfl status = XNSYNCH_RESCHED; } - synch->owner = NULL; + xnsynch_set_owner_internal(synch, NULL); __clrbits(synch->status, XNSYNCH_PENDING); xnlock_put_irqrestore(&nklock, s); diff -urp xenomai-a/ksrc/skins/native/task.c xenomai-b/ksrc/skins/native/task.c --- xenomai-a/ksrc/skins/native/task.c 2006-07-30 10:50:49.000000000 +0200 +++ xenomai-b/ksrc/skins/native/task.c 2006-08-09 13:32:21.917770000 +0200 @@ -262,7 +262,7 @@ int rt_task_create(RT_TASK *task, #ifdef CONFIG_XENO_OPT_NATIVE_MPS xnsynch_init(&task->mrecv, XNSYNCH_FIFO); - xnsynch_init(&task->msendq, XNSYNCH_PRIO | XNSYNCH_PIP); + xnsynch_init(&task->msendq, XNSYNCH_PRIO | XNSYNCH_PIP | XNSYNCH_FOWNER); xnsynch_set_owner(&task->msendq, &task->thread_base); task->flowgen = 0; #endif /* CONFIG_XENO_OPT_NATIVE_MPS */ @@ -2057,10 +2057,7 @@ int rt_task_reply(int flowid, RT_TASK_MC identifier from other senders wrt to a given receiver. */ if (receiver->wait_args.mps.mcb_s.flowid == flowid) { - /* Note that the following will cause the receiver to be - unblocked without transferring the ownership of the - msendq object, since we want the sender to keep it. */ - xnpod_resume_thread(&receiver->thread_base, XNPEND); + xnsynch_wakeup_this_sleeper(&sender->msendq, holder); break; } } ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [Xenomai-core] Re: [Xenomai-help] -110 error on rt_task_send... bug? 2006-08-09 15:09 ` Ulrich Schwab @ 2006-08-09 15:42 ` Dmitry Adamushko 2006-08-10 18:14 ` Vincent Levesque 2006-08-10 8:25 ` Jan Kiszka 1 sibling, 1 reply; 16+ messages in thread From: Dmitry Adamushko @ 2006-08-09 15:42 UTC (permalink / raw) To: Ulrich Schwab; +Cc: xenomai, andrewg, Jan Kiszka, xenomai [-- Attachment #1: Type: text/plain, Size: 952 bytes --] > > So could anyone test this patch and let me know if it works? > > > > Note : I haven't compiled it even as I don't have a proper environment. > But > > the changes are pretty simple so it should be ok. > I just did run the test program supplied by Vincent after applying Your > patch > (against 2.2.0) > It did work ! > That's good. Thanks for your assistance. So synch. message passing interface (IOW, rt_task_send/receive/reply) is mgmglmgmm... slightly broken at the moment for all versions. The ones who need it may use this patch so far. I'll think a bit more on it in the mean time but I tend to think that "Fixed Ownership" (expressed by XNSYNCH_FOWNER) should really be a property of the synch. object indeed (as in the current patch). > Only one small typo was in the patch, it is fixed in the attached patch > version. Ok, now I need to run diff -u old-patch new-patch to find out this typo :) -- Best regards, Dmitry Adamushko [-- Attachment #2: Type: text/html, Size: 1231 bytes --] ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [Xenomai-core] Re: [Xenomai-help] -110 error on rt_task_send... bug? 2006-08-09 15:42 ` Dmitry Adamushko @ 2006-08-10 18:14 ` Vincent Levesque 2006-08-10 18:18 ` Dmitry Adamushko 0 siblings, 1 reply; 16+ messages in thread From: Vincent Levesque @ 2006-08-10 18:14 UTC (permalink / raw) To: xenomai; +Cc: andrewg The patch works here too. Thanks for looking into this so quickly. I'll switch back to 2.1 until this is resolved. Vincent Levesque vleves@domain.hid Dmitry Adamushko wrote: > > > So could anyone test this patch and let me know if it works? > > > > Note : I haven't compiled it even as I don't have a proper > environment. But > > the changes are pretty simple so it should be ok. > I just did run the test program supplied by Vincent after applying > Your patch > (against 2.2.0) > It did work ! > > > > That's good. Thanks for your assistance. > > So synch. message passing interface (IOW, rt_task_send/receive/reply) > is mgmglmgmm... slightly broken at the moment for all versions. > > The ones who need it may use this patch so far. I'll think a bit more > on it in the mean time but I tend to think that "Fixed Ownership" > (expressed by XNSYNCH_FOWNER) should really be a property of the > synch. object indeed (as in the current patch). > > > > > Only one small typo was in the patch, it is fixed in the attached patch > > version. > > Ok, now I need to run diff -u old-patch new-patch to find out this typo :) > > > -- > Best regards, > Dmitry Adamushko > ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [Xenomai-core] Re: [Xenomai-help] -110 error on rt_task_send... bug? 2006-08-10 18:14 ` Vincent Levesque @ 2006-08-10 18:18 ` Dmitry Adamushko 0 siblings, 0 replies; 16+ messages in thread From: Dmitry Adamushko @ 2006-08-10 18:18 UTC (permalink / raw) To: Vincent Levesque; +Cc: xenomai, andrewg, xenomai [-- Attachment #1: Type: text/plain, Size: 437 bytes --] The patch works here too. Thanks for looking into this so quickly. I'll > switch back to 2.1 until this is resolved. > It's also broken in 2.1. There are 2 problems there not releted to recently introduced ownership-stealing part. That said, you may safely (well, you remember this "WITHOUT ANY WARRANTY..." but indeed :) apply this patch so far if you really need synch. message passing mechanism. -- Best regards, Dmitry Adamushko [-- Attachment #2: Type: text/html, Size: 719 bytes --] ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [Xenomai-core] Re: [Xenomai-help] -110 error on rt_task_send... bug? 2006-08-09 15:09 ` Ulrich Schwab 2006-08-09 15:42 ` Dmitry Adamushko @ 2006-08-10 8:25 ` Jan Kiszka 2006-08-10 10:11 ` Dmitry Adamushko 1 sibling, 1 reply; 16+ messages in thread From: Jan Kiszka @ 2006-08-10 8:25 UTC (permalink / raw) To: Dmitry Adamushko; +Cc: andrewg, xenomai Hi Dmitry, some comments on the patch, but I still haven't run it. The basic idea is sane, but we have to wait for Philippe's comments of course. Ulrich Schwab wrote: > On Wednesday 09 August 2006 13:42, Dmitry Adamushko wrote: >> So could anyone test this patch and let me know if it works? >> >> Note : I haven't compiled it even as I don't have a proper environment. But >> the changes are pretty simple so it should be ok. > I just did run the test program supplied by Vincent after applying Your patch > (against 2.2.0) > It did work ! > Only one small typo was in the patch, it is fixed in the attached patch > version. > >> There was actually yet another problem mmm... who cares to delete a sender >> from the msendq? >> >> Now should be ok or maybe I'm completely wrong and see the things which do >> not exist at all :) >> >> p.s. I cc'ed "xenomai-core" as it may involve further discussions and it's >> the right place indeed. > > > ------------------------------------------------------------------------ > > diff -urp xenomai-a/include/nucleus/synch.h xenomai-b/include/nucleus/synch.h > --- xenomai-a/include/nucleus/synch.h 2006-07-20 11:09:01.000000000 +0200 > +++ xenomai-b/include/nucleus/synch.h 2006-08-09 12:53:37.044217000 +0200 > @@ -28,6 +28,7 @@ > #define XNSYNCH_NOPIP 0x0 > #define XNSYNCH_PIP 0x2 > #define XNSYNCH_DREORD 0x4 > +#define XNSYNCH_FOWNER 0x20 /* Fixed owner */ > > #if defined(__KERNEL__) || defined(__XENO_SIM__) > > diff -urp xenomai-a/ksrc/nucleus/synch.c xenomai-b/ksrc/nucleus/synch.c > --- xenomai-a/ksrc/nucleus/synch.c 2006-06-15 14:15:25.000000000 +0200 > +++ xenomai-b/ksrc/nucleus/synch.c 2006-08-09 13:28:55.199081000 +0200 > @@ -37,6 +37,14 @@ > #include <nucleus/module.h> > #include <nucleus/ltt.h> > > +/* temporarily here */ > +static inline void xnsynch_set_owner_internal(xnsynch_t *synch, xnthread_t *thread) > +{ > + if (!testbits(synch->status, XNSYNCH_FOWNER)) > + synch->owner = thread; > +} > + > + I don't think we should use this inline function after the changes I'm suggesting below. There will be too many exceptions from this pattern, and the code is more readable with this unrolled. > /*! > * \fn void xnsynch_init(xnsynch_t *synch, xnflags_t flags); > * \brief Initialize a synchronization object. > @@ -181,7 +189,7 @@ void xnsynch_sleep_on(xnsynch_t *synch, > > if (testbits(synch->status, XNSYNCH_PENDING)) { > /* Ownership is still pending, steal the resource. */ > - synch->owner = thread; > + xnsynch_set_owner_internal(synch, thread); > __clrbits(thread->status, > XNRMID | XNTIMEO | XNBREAK); > goto grab_ownership; Mmm, given the fixed ownership, should we ever enter this code path under XNSYNCH_FOWNER? Is there a scenario with XNSYNCH_FOWNER | XNSYNCH_PENDING? If yes, I would say: fix that one. Then the change above is no longer required. > @@ -209,7 +217,7 @@ void xnsynch_sleep_on(xnsynch_t *synch, > > xnpod_suspend_thread(thread, XNPEND, timeout, synch); > > - if (unlikely(synch->owner != thread)) { > + if (!testbits(synch->status, XNSYNCH_FOWNER) && unlikely(synch->owner != thread)) { > /* Somebody stole us the ownership while we were ready to > run, waiting for the CPU: we need to wait again for the > resource. */ (synch->owner != thread) is more likely to be unlikely ;). I would reorder it, i.e. put the owner check to the front again. > @@ -362,12 +370,12 @@ xnthread_t *xnsynch_wakeup_one_sleeper(x > if (holder) { > thread = link2thread(holder, plink); > thread->wchan = NULL; > - synch->owner = thread; > + xnsynch_set_owner_internal(synch, thread); > __setbits(synch->status, XNSYNCH_PENDING); Here we are: I think we shouldn't set XNSYNCH_PENDING if XNSYNCH_FOWNER is set. > xnltt_log_event(xeno_ev_wakeup1, thread->name, synch); > xnpod_resume_thread(thread, XNPEND); > } else { > - synch->owner = NULL; > + xnsynch_set_owner_internal(synch, thread); > __clrbits(synch->status, XNSYNCH_PENDING); > } > > @@ -435,7 +443,7 @@ xnpholder_t *xnsynch_wakeup_this_sleeper > nholder = poppq(&synch->pendq, holder); > thread = link2thread(holder, plink); > thread->wchan = NULL; > - synch->owner = thread; > + xnsynch_set_owner_internal(synch, thread); > __setbits(synch->status, XNSYNCH_PENDING); ...and here again. > xnltt_log_event(xeno_ev_wakeupx, thread->name, synch); > xnpod_resume_thread(thread, XNPEND); > @@ -523,7 +531,7 @@ int xnsynch_flush(xnsynch_t *synch, xnfl > status = XNSYNCH_RESCHED; > } > > - synch->owner = NULL; > + xnsynch_set_owner_internal(synch, NULL); > __clrbits(synch->status, XNSYNCH_PENDING); > > xnlock_put_irqrestore(&nklock, s); > diff -urp xenomai-a/ksrc/skins/native/task.c xenomai-b/ksrc/skins/native/task.c > --- xenomai-a/ksrc/skins/native/task.c 2006-07-30 10:50:49.000000000 +0200 > +++ xenomai-b/ksrc/skins/native/task.c 2006-08-09 13:32:21.917770000 +0200 > @@ -262,7 +262,7 @@ int rt_task_create(RT_TASK *task, > > #ifdef CONFIG_XENO_OPT_NATIVE_MPS > xnsynch_init(&task->mrecv, XNSYNCH_FIFO); > - xnsynch_init(&task->msendq, XNSYNCH_PRIO | XNSYNCH_PIP); > + xnsynch_init(&task->msendq, XNSYNCH_PRIO | XNSYNCH_PIP | XNSYNCH_FOWNER); > xnsynch_set_owner(&task->msendq, &task->thread_base); > task->flowgen = 0; > #endif /* CONFIG_XENO_OPT_NATIVE_MPS */ > @@ -2057,10 +2057,7 @@ int rt_task_reply(int flowid, RT_TASK_MC > identifier from other senders wrt to a given receiver. */ > > if (receiver->wait_args.mps.mcb_s.flowid == flowid) { > - /* Note that the following will cause the receiver to be > - unblocked without transferring the ownership of the > - msendq object, since we want the sender to keep it. */ > - xnpod_resume_thread(&receiver->thread_base, XNPEND); > + xnsynch_wakeup_this_sleeper(&sender->msendq, holder); Yeah, this somehow looks cleaner to me. > break; > } > } > Jan ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [Xenomai-core] Re: [Xenomai-help] -110 error on rt_task_send... bug? 2006-08-10 8:25 ` Jan Kiszka @ 2006-08-10 10:11 ` Dmitry Adamushko 0 siblings, 0 replies; 16+ messages in thread From: Dmitry Adamushko @ 2006-08-10 10:11 UTC (permalink / raw) To: Jan Kiszka; +Cc: andrewg, xenomai [-- Attachment #1: Type: text/plain, Size: 806 bytes --] Hello, if you take a look e.g. at how synch->owner is used in the current SVN codebase then you will notice that this field is actually usefull only for PIP objects but xnsynch_wakeup_* and friends update it for all cases. No, I don't want to say that's why let's do the same with PENDING. This is a preliminary patch and, that said, I'm going to take a closer look at the synch.c. Regarding PENDING : not sure at the moment if it would be helpful (have to think about possible use cases), but with the help of PENDING it's possible to support "ownership"-stealing for FOWNER objects. Sounds funny? :) if (testbits(synch->status, XNSYNCH_FOWNER | XNSYNCH_PENDING) == XNSYNCH_FOWNER) { // the object has been stolen while a thread was waiting to be scheduled on } -- Best regards, Dmitry Adamushko [-- Attachment #2: Type: text/html, Size: 903 bytes --] ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [Xenomai-help] -110 error on rt_task_send... bug? 2006-08-09 9:35 ` Dmitry Adamushko 2006-08-09 11:42 ` [Xenomai-core] " Dmitry Adamushko @ 2006-08-10 22:13 ` Philippe Gerum 2006-08-11 7:50 ` Dmitry Adamushko 2006-08-12 10:33 ` Dmitry Adamushko 1 sibling, 2 replies; 16+ messages in thread From: Philippe Gerum @ 2006-08-10 22:13 UTC (permalink / raw) To: Dmitry Adamushko; +Cc: xenomai, andrewg, Jan Kiszka On Wed, 2006-08-09 at 11:35 +0200, Dmitry Adamushko wrote: > > Hello, > > > I'm not that deep into rt_task_send/receive design, but this > problem > seems to be related to some heavy disagree between > xnsynch_sleep_on and > rt_task_send about the correct ownership of the receiver's > send queue. > > > Not transfering the ownership causes this problem, yes. But looking > briefly at the code... there is even yet another problem (if I haven't > overlooked something) that also exists in 2.1. > > rt_task_reply() wakes up a sender with : > > [code] > > if (receiver->wait_args.mps.mcb_s.flowid == flowid) { > /* Note that the following will cause the > receiver to be > unblocked without transferring the > ownership of the > msendq object, since we want the sender to > keep it. */ > xnpod_resume_thread(&receiver->thread_base, > XNPEND); > break; > > [/code] > > > Now let's suppose : sender.prio > recv.prio > > so recv.prio is boosted when sender executes xnsynch_sleep_on(). > > The "recv" task (the one that calls rt_task_receive() and > rt_task_reply()) doesn't use xnsynch_* functions (like > xnsynch_wakeup_this_sleeper() ) to actually wake up the sender. > Therefore xnsynch_clear_boost() is never called and "recv" keeps > temporarily inhereted priority as a permanent one from now on. > No? > No. xnpod_resume_thread() detects that the sender is pending on the msendq, and eventually calls xnsynch_forget_sleeper(), which removes the sender from the pend queue, then clears the boost. The problem is triggered by synch->owner being cleared in xnsynch_flush(), as a result of the receiver task to exit (called from the native task deletion hook, i.e. xnsynch_destroy(&task->msendq)). The problem to fix is indeed in xnsynch_sleep_on(), but it's rather a matter of making sure that the awakened thread is not going to check the owner field of a synch object that has just been destroyed. > I have a few ideas on how to fix it intelegently so if Philippe will > not come up with a quick solution, I may take a look? > > > -- > Best regards, > Dmitry Adamushko > > _______________________________________________ > Xenomai-help mailing list > Xenomai-help@domain.hid > https://mail.gna.org/listinfo/xenomai-help -- Philippe. ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [Xenomai-help] -110 error on rt_task_send... bug? 2006-08-10 22:13 ` Philippe Gerum @ 2006-08-11 7:50 ` Dmitry Adamushko 2006-08-12 10:33 ` Dmitry Adamushko 1 sibling, 0 replies; 16+ messages in thread From: Dmitry Adamushko @ 2006-08-11 7:50 UTC (permalink / raw) To: rpm; +Cc: xenomai, andrewg, Jan Kiszka [-- Attachment #1: Type: text/plain, Size: 808 bytes --] No. xnpod_resume_thread() detects that the sender is pending on the > msendq, and eventually calls xnsynch_forget_sleeper(), which removes the > sender from the pend queue, then clears the boost. I overlooked it indeed. The problem is triggered by synch->owner being cleared in > xnsynch_flush(), as a result of the receiver task to exit (called from > the native task deletion hook, i.e. xnsynch_destroy(&task->msendq)). > The problem to fix is indeed in xnsynch_sleep_on(), but it's rather a > matter of making sure that the awakened thread is not going to check the > owner field of a synch object that has just been destroyed. > > and moreover I was completely wrong. So the reputation of 2.1 is restored and that's more important indeed than err... mine own :o) -- Best regards, Dmitry Adamushko [-- Attachment #2: Type: text/html, Size: 1213 bytes --] ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [Xenomai-help] -110 error on rt_task_send... bug? 2006-08-10 22:13 ` Philippe Gerum 2006-08-11 7:50 ` Dmitry Adamushko @ 2006-08-12 10:33 ` Dmitry Adamushko 2006-08-12 21:14 ` Philippe Gerum 1 sibling, 1 reply; 16+ messages in thread From: Dmitry Adamushko @ 2006-08-12 10:33 UTC (permalink / raw) To: rpm; +Cc: xenomai, Jan Kiszka [-- Attachment #1: Type: text/plain, Size: 731 bytes --] > > > The problem is triggered by synch->owner being cleared in > xnsynch_flush(), as a result of the receiver task to exit (called from > the native task deletion hook, i.e. xnsynch_destroy(&task->msendq)). As I understood it didn't work in all cases and not only when a receiver exits. What's wrong with the case Jan revealed? I mean rt_task_reply() makes use of a direct call to xnpod_resume_thread() to avoid a change of the owner while a sender finds itself being woken up in xnsynch_sleep_on() - the PRIO + PIP branch - and the first thing it does is a check for (synch->owner != thread) and it's obviously that the sender is not the owner (but the receiver). No? > -- > > Philippe. -- Best regards, Dmitry Adamushko [-- Attachment #2: Type: text/html, Size: 1054 bytes --] ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [Xenomai-help] -110 error on rt_task_send... bug? 2006-08-12 10:33 ` Dmitry Adamushko @ 2006-08-12 21:14 ` Philippe Gerum 2006-08-13 14:47 ` Philippe Gerum 0 siblings, 1 reply; 16+ messages in thread From: Philippe Gerum @ 2006-08-12 21:14 UTC (permalink / raw) To: Dmitry Adamushko; +Cc: xenomai, Jan Kiszka On Sat, 2006-08-12 at 12:33 +0200, Dmitry Adamushko wrote: > > The problem is triggered by synch->owner being cleared in > xnsynch_flush(), as a result of the receiver task to exit > (called from > the native task deletion hook, i.e. > xnsynch_destroy(&task->msendq)). > > > As I understood it didn't work in all cases and not only when a > receiver exits. What's wrong with the case Jan revealed? > > I mean rt_task_reply() makes use of a direct call to > xnpod_resume_thread() to avoid a change of the owner while a sender > finds itself being woken up in xnsynch_sleep_on() - the PRIO + PIP > branch - and the first thing it does is a check for (synch->owner != > thread) and it's obviously that the sender is not the owner (but the > receiver). > > No? Yes, but there is even more than this, which will require some fixing. There is a design issue regarding how we deal with the lifetime of the RT_TASK::msendq member: if the embodying task (i.e. the server task) exits after the client task has been replied to, but before such task has had a chance to return from xnsynch_sleep_on(), then the latter routine would use a _destroyed_ synch. object inside the return path. Due to the object stealing feature, xnsynch_sleep_on() now relies on the assumption that the pended synch object still exists upon return from xnpod_suspend_thread(), and this is really, really bad. The only thing that may be accessed safely on this return path is the resuming thread's control block itself (obviously), and _not_ what it was pending on. It's the caller's business to find out whether the awaited resource is still usable, but the nucleus should not rely on this assumption, given that a rescheduling is likely to have taken place in xnsynch_sleep_on(), and lots of things might have happended since then which are way outside of its knowledge. IOW, a construct like "if (synch-> ...." when returning from suspension is the root of all evil. We must not depend on anything requiring such dereference when resuming the thread. A way to fix this would be to introduce a new thread status flag which gets raised whenever some object ownership is stolen, so that we would not have to inspect the synch object anymore to know about such situation, and rely on potentially wrong information from the ->owner field. We would only have to inspect the status flags of the thread that got "robbed" (i.e. the one that resumes) and act accordingly. I will post a patch implementing this, after I have solved all the related issues. PS: note that in the particular case of the message passing services, object stealing cannot happen, since we never call synch wakeup services that transfer ownership but rather resume the thread bluntly, so the PENDING bit never gets raised, thus preventing any stealing. > > > > -- > > > > Philippe. > > -- > Best regards, > Dmitry Adamushko -- Philippe. ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [Xenomai-help] -110 error on rt_task_send... bug? 2006-08-12 21:14 ` Philippe Gerum @ 2006-08-13 14:47 ` Philippe Gerum 2006-08-13 20:24 ` Philippe Gerum 0 siblings, 1 reply; 16+ messages in thread From: Philippe Gerum @ 2006-08-13 14:47 UTC (permalink / raw) To: Dmitry Adamushko; +Cc: xenomai, Jan Kiszka On Sat, 2006-08-12 at 23:14 +0200, Philippe Gerum wrote: > IOW, a construct like "if (synch-> ...." when returning from suspension > is the root of all evil. We must not depend on anything requiring such > dereference when resuming the thread. A way to fix this would be to > introduce a new thread status flag which gets raised whenever some > object ownership is stolen, so that we would not have to inspect the > synch object anymore to know about such situation, and rely on > potentially wrong information from the ->owner field. We would only have > to inspect the status flags of the thread that got "robbed" (i.e. the > one that resumes) and act accordingly. I will post a patch implementing > this, after I have solved all the related issues. The patch below provides a more general fix to the issues introduced by the resource stealing feature. There is still one case where this code could attempt to reuse a stolen resource unsafely, but the calling code would have to be quite fragile in the first place. Normal use cases should be ok. Already checked on the simulator, but please give this a try. Index: ksrc/nucleus/pod.c =================================================================== --- ksrc/nucleus/pod.c (revision 1420) +++ ksrc/nucleus/pod.c (working copy) @@ -1419,7 +1419,7 @@ __clrbits(thread->status, XNREADY); } - __clrbits(thread->status, XNRMID | XNTIMEO | XNBREAK); + __clrbits(thread->status, XNRMID | XNTIMEO | XNBREAK | XNWAKEN | XNROBBED); } __setbits(thread->status, mask); Index: ksrc/nucleus/synch.c =================================================================== --- ksrc/nucleus/synch.c (revision 1420) +++ ksrc/nucleus/synch.c (working copy) @@ -92,7 +92,7 @@ if (flags & XNSYNCH_PIP) flags |= XNSYNCH_PRIO; /* Obviously... */ - synch->status = flags & ~(XNSYNCH_CLAIMED | XNSYNCH_PENDING); + synch->status = flags & ~XNSYNCH_CLAIMED; synch->owner = NULL; synch->cleanup = NULL; /* Only works for PIP-enabled objects. */ initpq(&synch->pendq, xnpod_get_qdir(nkpod), @@ -169,79 +169,74 @@ xnltt_log_event(xeno_ev_sleepon, thread->name, synch); - if (testbits(synch->status, XNSYNCH_PRIO)) { + if (!testbits(synch->status, XNSYNCH_PRIO)) { /* i.e. FIFO */ + appendpq(&synch->pendq, &thread->plink); + xnpod_suspend_thread(thread, XNPEND, timeout, synch); + goto unlock_and_exit; + } - if (testbits(synch->status, XNSYNCH_PIP)) { - redo: - owner = synch->owner; + if (!testbits(synch->status, XNSYNCH_PIP)) { /* i.e. no ownership */ + insertpqf(&synch->pendq, &thread->plink, thread->cprio); + xnpod_suspend_thread(thread, XNPEND, timeout, synch); + goto unlock_and_exit; + } - if (owner - && xnpod_compare_prio(thread->cprio, - owner->cprio) > 0) { +redo: + owner = synch->owner; - if (testbits(synch->status, XNSYNCH_PENDING)) { - /* Ownership is still pending, steal the resource. */ - synch->owner = thread; - __clrbits(thread->status, - XNRMID | XNTIMEO | XNBREAK); - goto grab_ownership; - } + if (!owner) { + synch->owner = thread; + __clrbits(thread->status, XNRMID | XNTIMEO | XNBREAK); + goto unlock_and_exit; + } - if (!testbits(owner->status, XNBOOST)) { - owner->bprio = owner->cprio; - __setbits(owner->status, XNBOOST); - } + if (xnpod_compare_prio(thread->cprio, owner->cprio) > 0) { - if (testbits(synch->status, XNSYNCH_CLAIMED)) - removepq(&owner->claimq, &synch->link); - else - __setbits(synch->status, - XNSYNCH_CLAIMED); + if (testbits(owner->status, XNWAKEN)) { + /* Ownership is still pending, steal the resource. */ + synch->owner = thread; + __clrbits(thread->status, XNRMID | XNTIMEO | XNBREAK); + __setbits(owner->status, XNROBBED); + goto unlock_and_exit; + } - insertpqf(&synch->pendq, &thread->plink, - thread->cprio); - insertpqf(&owner->claimq, &synch->link, - thread->cprio); - xnsynch_renice_thread(owner, thread->cprio); - } else - insertpqf(&synch->pendq, &thread->plink, - thread->cprio); + if (!testbits(owner->status, XNBOOST)) { + owner->bprio = owner->cprio; + __setbits(owner->status, XNBOOST); + } - xnpod_suspend_thread(thread, XNPEND, timeout, synch); + if (testbits(synch->status, XNSYNCH_CLAIMED)) + removepq(&owner->claimq, &synch->link); + else + __setbits(synch->status, XNSYNCH_CLAIMED); - if (unlikely(testbits(thread->status, XNRMID | XNTIMEO | XNBREAK))) - goto unlock_and_exit; + insertpqf(&synch->pendq, &thread->plink, thread->cprio); + insertpqf(&owner->claimq, &synch->link, thread->cprio); + xnsynch_renice_thread(owner, thread->cprio); + } else + insertpqf(&synch->pendq, &thread->plink, thread->cprio); - if (unlikely(synch->owner != thread)) { - /* Somebody stole us the ownership while we were ready to - run, waiting for the CPU: we need to wait again for the - resource. */ - if (timeout == XN_INFINITE) - goto redo; - timeout = xnthread_timeout(thread); - if (timeout > 1) /* Otherwise, it's too late, time elapsed. */ - goto redo; - __setbits(thread->status, XNTIMEO); - goto unlock_and_exit; - } - } else { - insertpqf(&synch->pendq, &thread->plink, thread->cprio); - xnpod_suspend_thread(thread, XNPEND, timeout, synch); - } - } else { /* otherwise FIFO */ - appendpq(&synch->pendq, &thread->plink); - xnpod_suspend_thread(thread, XNPEND, timeout, synch); - if (unlikely(testbits(thread->status, XNRMID | XNTIMEO | XNBREAK))) - goto unlock_and_exit; - } + xnpod_suspend_thread(thread, XNPEND, timeout, synch); - grab_ownership: + if (testbits(thread->status, XNRMID | XNTIMEO | XNBREAK)) + goto unlock_and_exit; - /* Now the resource is truely owned by the caller. */ - __clrbits(synch->status, XNSYNCH_PENDING); + if (testbits(thread->status, XNROBBED)) { + /* Somebody stole us the ownership while we were ready + to run, waiting for the CPU: we need to wait again + for the resource. */ + if (timeout == XN_INFINITE) + goto redo; + timeout = xnthread_timeout(thread); + if (timeout > 1) /* Otherwise, it's too late. */ + goto redo; + __setbits(thread->status, XNTIMEO); + } unlock_and_exit: + __clrbits(thread->status, XNWAKEN); + xnlock_put_irqrestore(&nklock, s); } @@ -368,13 +363,11 @@ thread = link2thread(holder, plink); thread->wchan = NULL; synch->owner = thread; - __setbits(synch->status, XNSYNCH_PENDING); + __setbits(thread->status, XNWAKEN); xnltt_log_event(xeno_ev_wakeup1, thread->name, synch); xnpod_resume_thread(thread, XNPEND); - } else { + } else synch->owner = NULL; - __clrbits(synch->status, XNSYNCH_PENDING); - } if (testbits(synch->status, XNSYNCH_CLAIMED)) xnsynch_clear_boost(synch, lastowner); @@ -441,7 +434,7 @@ thread = link2thread(holder, plink); thread->wchan = NULL; synch->owner = thread; - __setbits(synch->status, XNSYNCH_PENDING); + __setbits(thread->status, XNWAKEN); xnltt_log_event(xeno_ev_wakeupx, thread->name, synch); xnpod_resume_thread(thread, XNPEND); @@ -467,7 +460,7 @@ * is cleared, such as before the resource is deleted. * * @param synch The descriptor address of the synchronization object - * whose ownership is changed. + * to be flushed. * * @param reason Some flags to set in the status mask of every * unblocked thread. Zero is an acceptable value. The following bits @@ -528,9 +521,6 @@ status = XNSYNCH_RESCHED; } - synch->owner = NULL; - __clrbits(synch->status, XNSYNCH_PENDING); - xnlock_put_irqrestore(&nklock, s); xnarch_post_graph_if(synch, 0, emptypq_p(&synch->pendq)); Index: include/nucleus/synch.h =================================================================== --- include/nucleus/synch.h (revision 1420) +++ include/nucleus/synch.h (working copy) @@ -32,7 +32,6 @@ #if defined(__KERNEL__) || defined(__XENO_SIM__) #define XNSYNCH_CLAIMED 0x8 /* Claimed by other thread(s) w/ PIP */ -#define XNSYNCH_PENDING 0x10 /* Pending ownership -- may be stolen */ /* Spare flags usable by upper interfaces */ #define XNSYNCH_SPARE0 0x01000000 @@ -86,13 +85,11 @@ void xnsynch_init(xnsynch_t *synch, xnflags_t flags); -#define xnsynch_destroy(synch) \ -xnsynch_flush(synch,XNRMID) +#define xnsynch_destroy(synch) xnsynch_flush(synch,XNRMID) static inline void xnsynch_set_owner (xnsynch_t *synch, struct xnthread *thread) { synch->owner = thread; - __clrbits(synch->status, XNSYNCH_PENDING); } static inline void xnsynch_register_cleanup (xnsynch_t *synch, void (*handler)(xnsynch_t *)) Index: include/nucleus/thread.h =================================================================== --- include/nucleus/thread.h (revision 1420) +++ include/nucleus/thread.h (working copy) @@ -53,6 +53,8 @@ #define XNSHADOW 0x00800000 /* Shadow thread */ #define XNROOT 0x01000000 /* Root thread (i.e. Linux/IDLE) */ #define XNINVPS 0x02000000 /* Using inverted priority scale */ +#define XNWAKEN 0x04000000 /* Thread waken up upon resource availability */ +#define XNROBBED 0x08000000 /* Robbed from resource ownership */ /* Must follow the declaration order of the above bits. Status symbols @@ -73,12 +75,12 @@ 'o' -> Priority coupling off. 'f' -> FPU enabled (for kernel threads). */ -#define XNTHREAD_SLABEL_INIT { \ - 'S', 'W', 'D', 'R', 'U', \ - '.', '.', '.', 'X', 'H', \ - '.', '.', '.', '.', 'b', 'T', \ - 'l', 'r', '.', 's', 't', 'o', \ - 'f', '.', '.', '.' \ +#define XNTHREAD_SLABEL_INIT { \ + 'S', 'W', 'D', 'R', 'U', \ + '.', '.', '.', 'X', 'H', \ + '.', '.', '.', '.', 'b', 'T', \ + 'l', 'r', '.', 's', 't', 'o', \ + 'f', '.', '.', '.', '.', '.' \ } #define XNTHREAD_BLOCK_BITS (XNSUSP|XNPEND|XNDELAY|XNDORMANT|XNRELAX|XNHELD) -- Philippe. ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [Xenomai-help] -110 error on rt_task_send... bug? 2006-08-13 14:47 ` Philippe Gerum @ 2006-08-13 20:24 ` Philippe Gerum 0 siblings, 0 replies; 16+ messages in thread From: Philippe Gerum @ 2006-08-13 20:24 UTC (permalink / raw) To: Dmitry Adamushko; +Cc: xenomai, Jan Kiszka On Sun, 2006-08-13 at 16:47 +0200, Philippe Gerum wrote: > On Sat, 2006-08-12 at 23:14 +0200, Philippe Gerum wrote: > > > IOW, a construct like "if (synch-> ...." when returning from suspension > > is the root of all evil. We must not depend on anything requiring such > > dereference when resuming the thread. A way to fix this would be to > > introduce a new thread status flag which gets raised whenever some > > object ownership is stolen, so that we would not have to inspect the > > synch object anymore to know about such situation, and rely on > > potentially wrong information from the ->owner field. We would only have > > to inspect the status flags of the thread that got "robbed" (i.e. the > > one that resumes) and act accordingly. I will post a patch implementing > > this, after I have solved all the related issues. > > The patch below provides a more general fix to the issues introduced by > the resource stealing feature. There is still one case where this code > could attempt to reuse a stolen resource unsafely, More precisely, a stolen then deleted resource, and those events must happen while the caller of xnsynch_sleep_on has been readied from suspension, but not yet switched in. Deletion is the issue there, meaning that the resource might be destroyed immediately after it has been released, without any attempt to synchronize with the next consumer of such resource. -- Philippe. ^ permalink raw reply [flat|nested] 16+ messages in thread
end of thread, other threads:[~2006-08-13 20:24 UTC | newest] Thread overview: 16+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2006-08-08 1:50 [Xenomai-help] -110 error on rt_task_send... bug? Vincent Levesque 2006-08-08 22:12 ` Jan Kiszka 2006-08-09 9:35 ` Dmitry Adamushko 2006-08-09 11:42 ` [Xenomai-core] " Dmitry Adamushko 2006-08-09 15:09 ` Ulrich Schwab 2006-08-09 15:42 ` Dmitry Adamushko 2006-08-10 18:14 ` Vincent Levesque 2006-08-10 18:18 ` Dmitry Adamushko 2006-08-10 8:25 ` Jan Kiszka 2006-08-10 10:11 ` Dmitry Adamushko 2006-08-10 22:13 ` Philippe Gerum 2006-08-11 7:50 ` Dmitry Adamushko 2006-08-12 10:33 ` Dmitry Adamushko 2006-08-12 21:14 ` Philippe Gerum 2006-08-13 14:47 ` Philippe Gerum 2006-08-13 20:24 ` Philippe Gerum
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.