* [Xenomai-help] How to fix t_delete call in psos skin?
@ 2009-11-04 9:20 Alexandre Coffignal
2009-11-04 10:50 ` [Xenomai-core] " Philippe Gerum
0 siblings, 1 reply; 4+ messages in thread
From: Alexandre Coffignal @ 2009-11-04 9:20 UTC (permalink / raw)
To: xenomai
[-- Attachment #1: Type: text/plain, Size: 3188 bytes --]
Hello,
I'm trying to use t_delete routine in psos skin (xenomai-2.4.7) from a
user space application .
I'm encountring two different behaviors:
-1- Calling t_delete(0) from a psos task seems to work perfectly (e.g.
task self deletes).
-2- Calling t_delete(task_id) with another task id (e.g. current task
tries to delete another task) doesn't result in task deletion but
returns 0 (success).
I've been through both psos skin (ksrc/psos+/task.c) and nucleus
(ksrc/nucleus/pod.c) source code.
xnpod_delete_thread() behaves differently depending on previously
described t_delete calls:
-1- First case (t_delete(0)):
Execution skip condition in ksrc/nucleus/pod.c at line 1173:
if (xnthread_user_task(thread) != NULL &&
!xnthread_test_state(thread, XNDORMANT) &&
!xnpod_current_p(thread)) {
if (!xnpod_userspace_p())
xnshadow_send_sig(thread, SIGKILL, 1);
/*
* Otherwise, assume the interface library has issued
* pthread_cancel on the target thread, which should
* cause the current service to be called for
* self-deletion of that thread.
*/
goto unlock_and_exit;
}
#endif /* CONFIG_XENO_OPT_PERVASIVE */
and continues to run after it. This results in a successful deletion.
-2- Second case:
Execution enters one of the first conditions (if
(xnthread_user_task(thread))...) but skip the second one :
xnshadow_send_sig(thread, SIGKILL, 1) is not executed;
goto unlock_and_exit; instruction is then executed.
->Our thread is never deleted.
-3-
Modifing actual source code by commenting goto intruction or moving it
into second condition (if (!xnpod_userspace_p()) mentioned above)
results in a working t_delete (task_id).
- I'm not sure of the possible side effects of such a modification?
- Is it correct to do so?
As mentioned in the source code comment, it is assumed "the interface
library has issued pthread_cancel" ...
I assume it might be missing somewhere?
Please can you help on this topic?
Thanks in advance.
Please find a patch regarding this modification.
diff --git a/ksrc/nucleus/pod.c b/ksrc/nucleus/pod.c
index 9348ce1..ee362df 100644
--- a/ksrc/nucleus/pod.c
+++ b/ksrc/nucleus/pod.c
@@ -1177,14 +1177,16 @@ void xnpod_delete_thread(xnthread_t *thread)
!xnthread_test_state(thread, XNDORMANT) &&
!xnpod_current_p(thread)) {
if (!xnpod_userspace_p())
+ {
xnshadow_send_sig(thread, SIGKILL, 1);
+ goto unlock_and_exit;
+ }
/*
* Otherwise, assume the interface library has issued
* pthread_cancel on the target thread, which should
* cause the current service to be called for
* self-deletion of that thread.
*/
- goto unlock_and_exit;
}
#endif /* CONFIG_XENO_OPT_PERVASIVE */
Fabrice.
--------------------------------
CénoSYS <http://www.cenosys.com>
10, Rue Xavier Bichat
F-72000 Le MANS
--------------------------------
[-- Attachment #2: pod.patch --]
[-- Type: text/x-patch, Size: 675 bytes --]
diff --git a/ksrc/nucleus/pod.c b/ksrc/nucleus/pod.c
index 9348ce1..ee362df 100644
--- a/ksrc/nucleus/pod.c
+++ b/ksrc/nucleus/pod.c
@@ -1177,14 +1177,16 @@ void xnpod_delete_thread(xnthread_t *thread)
!xnthread_test_state(thread, XNDORMANT) &&
!xnpod_current_p(thread)) {
if (!xnpod_userspace_p())
+ {
xnshadow_send_sig(thread, SIGKILL, 1);
+ goto unlock_and_exit;
+ }
/*
* Otherwise, assume the interface library has issued
* pthread_cancel on the target thread, which should
* cause the current service to be called for
* self-deletion of that thread.
*/
- goto unlock_and_exit;
}
#endif /* CONFIG_XENO_OPT_PERVASIVE */
^ permalink raw reply related [flat|nested] 4+ messages in thread* Re: [Xenomai-core] [Xenomai-help] How to fix t_delete call in psos skin? 2009-11-04 9:20 [Xenomai-help] How to fix t_delete call in psos skin? Alexandre Coffignal @ 2009-11-04 10:50 ` Philippe Gerum 2009-11-05 10:54 ` Alexandre Coffignal 0 siblings, 1 reply; 4+ messages in thread From: Philippe Gerum @ 2009-11-04 10:50 UTC (permalink / raw) To: Alexandre Coffignal; +Cc: xenomai On Wed, 2009-11-04 at 10:20 +0100, Alexandre Coffignal wrote: > Hello, > > I'm trying to use t_delete routine in psos skin (xenomai-2.4.7) from a > user space application . > I'm encountring two different behaviors: > -1- Calling t_delete(0) from a psos task seems to work perfectly (e.g. > task self deletes). > -2- Calling t_delete(task_id) with another task id (e.g. current task > tries to delete another task) doesn't result in task deletion but > returns 0 (success). > > I've been through both psos skin (ksrc/psos+/task.c) and nucleus > (ksrc/nucleus/pod.c) source code. > > xnpod_delete_thread() behaves differently depending on previously > described t_delete calls: > > -1- First case (t_delete(0)): > > Execution skip condition in ksrc/nucleus/pod.c at line 1173: > > if (xnthread_user_task(thread) != NULL && > !xnthread_test_state(thread, XNDORMANT) && > !xnpod_current_p(thread)) { > if (!xnpod_userspace_p()) > xnshadow_send_sig(thread, SIGKILL, 1); > /* > * Otherwise, assume the interface library has issued > * pthread_cancel on the target thread, which should > * cause the current service to be called for > * self-deletion of that thread. > */ > goto unlock_and_exit; > } > #endif /* CONFIG_XENO_OPT_PERVASIVE */ > > and continues to run after it. This results in a successful deletion. > > > -2- Second case: > > Execution enters one of the first conditions (if > (xnthread_user_task(thread))...) but skip the second one : > xnshadow_send_sig(thread, SIGKILL, 1) is not executed; > goto unlock_and_exit; instruction is then executed. > ->Our thread is never deleted. > > -3- > Modifing actual source code by commenting goto intruction or moving it > into second condition (if (!xnpod_userspace_p()) mentioned above) > results in a working t_delete (task_id). > > - I'm not sure of the possible side effects of such a modification? > - Is it correct to do so? > Nope, this would defeat the purpose of testing those conditions, and end up breaking the nucleus. The implicit logic behind such block is that a real-time task should always exit on behalf of its own context, so that its Linux side never survives the removal of the mated Xenomai shadow TCB. Otherwise, you would get a real-time Xenomai task running without any Xenomai TCB. The fix has to happen in userland, and only there, by implementing what the comment suggests, which is still missing from the psos skin. i.e. have the t_delete() wrapper issue pthread_cancel() under the hood, in src/skins/psos/task.c. The native and POSIX APIs illustrate what is needed in this respect, albeit the way they implement this is not 100% portable to the psos skin context. PS: this discussion should rather happen on xenomai-core. Redirecting now. > > As mentioned in the source code comment, it is assumed "the interface > library has issued pthread_cancel" ... > I assume it might be missing somewhere? > > Please can you help on this topic? > > Thanks in advance. > > Please find a patch regarding this modification. > > diff --git a/ksrc/nucleus/pod.c b/ksrc/nucleus/pod.c > index 9348ce1..ee362df 100644 > --- a/ksrc/nucleus/pod.c > +++ b/ksrc/nucleus/pod.c > @@ -1177,14 +1177,16 @@ void xnpod_delete_thread(xnthread_t *thread) > !xnthread_test_state(thread, XNDORMANT) && > !xnpod_current_p(thread)) { > if (!xnpod_userspace_p()) > + { > xnshadow_send_sig(thread, SIGKILL, 1); > + goto unlock_and_exit; > + } > /* > * Otherwise, assume the interface library has issued > * pthread_cancel on the target thread, which should > * cause the current service to be called for > * self-deletion of that thread. > */ > - goto unlock_and_exit; > } > #endif /* CONFIG_XENO_OPT_PERVASIVE */ > > > > Fabrice. > > > -------------------------------- > CénoSYS <http://www.cenosys.com> > 10, Rue Xavier Bichat > F-72000 Le MANS > -------------------------------- > > > _______________________________________________ > Xenomai-help mailing list > Xenomai-help@domain.hid > https://mail.gna.org/listinfo/xenomai-help -- Philippe. ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [Xenomai-core] [Xenomai-help] How to fix t_delete call in psos skin? 2009-11-04 10:50 ` [Xenomai-core] " Philippe Gerum @ 2009-11-05 10:54 ` Alexandre Coffignal 2009-11-06 11:28 ` Philippe Gerum 0 siblings, 1 reply; 4+ messages in thread From: Alexandre Coffignal @ 2009-11-05 10:54 UTC (permalink / raw) To: Philippe Gerum, xenomai [-- Attachment #1: Type: text/plain, Size: 5469 bytes --] Philippe Gerum a écrit : > On Wed, 2009-11-04 at 10:20 +0100, Alexandre Coffignal wrote: > >> Hello, >> >> I'm trying to use t_delete routine in psos skin (xenomai-2.4.7) from a >> user space application . >> I'm encountring two different behaviors: >> -1- Calling t_delete(0) from a psos task seems to work perfectly (e.g. >> task self deletes). >> -2- Calling t_delete(task_id) with another task id (e.g. current task >> tries to delete another task) doesn't result in task deletion but >> returns 0 (success). >> >> I've been through both psos skin (ksrc/psos+/task.c) and nucleus >> (ksrc/nucleus/pod.c) source code. >> >> xnpod_delete_thread() behaves differently depending on previously >> described t_delete calls: >> >> -1- First case (t_delete(0)): >> >> Execution skip condition in ksrc/nucleus/pod.c at line 1173: >> >> if (xnthread_user_task(thread) != NULL && >> !xnthread_test_state(thread, XNDORMANT) && >> !xnpod_current_p(thread)) { >> if (!xnpod_userspace_p()) >> xnshadow_send_sig(thread, SIGKILL, 1); >> /* >> * Otherwise, assume the interface library has issued >> * pthread_cancel on the target thread, which should >> * cause the current service to be called for >> * self-deletion of that thread. >> */ >> goto unlock_and_exit; >> } >> #endif /* CONFIG_XENO_OPT_PERVASIVE */ >> >> and continues to run after it. This results in a successful deletion. >> >> >> -2- Second case: >> >> Execution enters one of the first conditions (if >> (xnthread_user_task(thread))...) but skip the second one : >> xnshadow_send_sig(thread, SIGKILL, 1) is not executed; >> goto unlock_and_exit; instruction is then executed. >> ->Our thread is never deleted. >> >> -3- >> Modifing actual source code by commenting goto intruction or moving it >> into second condition (if (!xnpod_userspace_p()) mentioned above) >> results in a working t_delete (task_id). >> >> - I'm not sure of the possible side effects of such a modification? >> - Is it correct to do so? >> >> > > Nope, this would defeat the purpose of testing those conditions, and end > up breaking the nucleus. The implicit logic behind such block is that a > real-time task should always exit on behalf of its own context, so that > its Linux side never survives the removal of the mated Xenomai shadow > TCB. Otherwise, you would get a real-time Xenomai task running without > any Xenomai TCB. > > The fix has to happen in userland, and only there, by implementing what > the comment suggests, which is still missing from the psos skin. i.e. > have the t_delete() wrapper issue pthread_cancel() under the hood, in > src/skins/psos/task.c. The native and POSIX APIs illustrate what is > needed in this respect, albeit the way they implement this is not 100% > portable to the psos skin context. > > PS: this discussion should rather happen on xenomai-core. Redirecting > now. > > Hello, Thank you for your response. I tried to adopt the same philosophy as in the native API. Please find attached a patch that modifies psos skin this way: in /src/skins/psos+/task.c A reference on pthread is kept at task creation time (in psos_task_trampoline and t_shadow routines) to be retrieved at deletion time (in t_delete routine, when tid isn't 0). When t_delete is called with tid != 0, pthread_cancel() is issued with this reference. include/psos+/task.h Pthread reference has been added in psostask struct (kernel side) and is passed to it by using SKINCALL at creation time. ksrc/skins/psos+/syscall.c & /include/psos+/syscall.h Pthread reference is passed to it by using SKINCALL at creation time (in __t_create). An additional skin call (__t_get_pthread) allow to retreive pthread reference from user space Does this fix seem more suitable to you? Do you see someting else missing? Please advice. Fabrice Gasnier >> As mentioned in the source code comment, it is assumed "the interface >> library has issued pthread_cancel" ... >> I assume it might be missing somewhere? >> >> Please can you help on this topic? >> >> Thanks in advance. >> >> Please find a patch regarding this modification. >> >> diff --git a/ksrc/nucleus/pod.c b/ksrc/nucleus/pod.c >> index 9348ce1..ee362df 100644 >> --- a/ksrc/nucleus/pod.c >> +++ b/ksrc/nucleus/pod.c >> @@ -1177,14 +1177,16 @@ void xnpod_delete_thread(xnthread_t *thread) >> !xnthread_test_state(thread, XNDORMANT) && >> !xnpod_current_p(thread)) { >> if (!xnpod_userspace_p()) >> + { >> xnshadow_send_sig(thread, SIGKILL, 1); >> + goto unlock_and_exit; >> + } >> /* >> * Otherwise, assume the interface library has issued >> * pthread_cancel on the target thread, which should >> * cause the current service to be called for >> * self-deletion of that thread. >> */ >> - goto unlock_and_exit; >> } >> #endif /* CONFIG_XENO_OPT_PERVASIVE */ >> >> > > >> Fabrice. >> >> >> -------------------------------- >> CénoSYS <http://www.cenosys.com> >> 10, Rue Xavier Bichat >> F-72000 Le MANS >> -------------------------------- >> >> >> _______________________________________________ >> Xenomai-help mailing list >> Xenomai-help@domain.hid >> https://mail.gna.org/listinfo/xenomai-help >> > > > [-- Attachment #2: patch_psos_t_delete --] [-- Type: text/plain, Size: 6826 bytes --] Index: xenomai-2.4.7_bugless/include/psos+/syscall.h =================================================================== --- xenomai-2.4.7_bugless.orig/include/psos+/syscall.h 2009-11-05 09:12:29.000000000 +0100 +++ xenomai-2.4.7_bugless/include/psos+/syscall.h 2009-11-05 09:13:14.000000000 +0100 @@ -73,6 +73,7 @@ #define __psos_as_send 45 /* Xenomai extension: get raw count of jiffies */ #define __psos_tm_getc 46 +#define __psos_t_get_pthread 47 /* get hidden pthread_t identifier. */ #ifdef __KERNEL__ Index: xenomai-2.4.7_bugless/ksrc/skins/psos+/syscall.c =================================================================== --- xenomai-2.4.7_bugless.orig/ksrc/skins/psos+/syscall.c 2009-11-05 09:16:48.000000000 +0100 +++ xenomai-2.4.7_bugless/ksrc/skins/psos+/syscall.c 2009-11-05 09:16:57.000000000 +0100 @@ -67,29 +67,37 @@ xncompletion_t __user *u_completion; u_long prio, flags, tid, err; char name[XNOBJECT_NAME_LEN]; + struct arg_bulk5 bulk; psostask_t *task; - if (!__xn_access_ok(curr, VERIFY_READ, __xn_reg_arg1(regs), 4)) + if (!__xn_access_ok(curr, VERIFY_READ, __xn_reg_arg1(regs), sizeof(bulk))) + return -EFAULT; + + __xn_copy_from_user(curr, &bulk, (void __user *)__xn_reg_arg1(regs), + sizeof(bulk)); + + if (!__xn_access_ok(curr, VERIFY_READ, bulk.a1, sizeof(name))) { return -EFAULT; + } /* Get task name. */ - __xn_strncpy_from_user(curr, name, (const char __user *)__xn_reg_arg1(regs), + __xn_strncpy_from_user(curr, name, (const char __user *)bulk.a1, sizeof(name) - 1); name[sizeof(name) - 1] = '\0'; strncpy(curr->comm, name, sizeof(curr->comm)); curr->comm[sizeof(curr->comm) - 1] = '\0'; if (!__xn_access_ok - (curr, VERIFY_WRITE, __xn_reg_arg4(regs), sizeof(tid))) + (curr, VERIFY_WRITE, bulk.a4, sizeof(tid))) return -EFAULT; /* Task priority. */ - prio = __xn_reg_arg2(regs); + prio = bulk.a2; /* Task flags. Force FPU support in user-space. This will lead to a no-op if the platform does not support it. */ - flags = __xn_reg_arg3(regs) | T_SHADOW | T_FPU; + flags = bulk.a3 | T_SHADOW | T_FPU; /* Completion descriptor our parent thread is pending on. */ - u_completion = (xncompletion_t __user *)__xn_reg_arg5(regs); + u_completion = (xncompletion_t __user *)__xn_reg_arg2(regs); err = t_create(name, prio, 0, 0, flags, &tid); @@ -99,7 +107,8 @@ * about the new thread id, so we can manipulate its * TCB pointer freely. */ tid = xnthread_handle(&task->threadbase); - __xn_copy_to_user(curr, (void __user *)__xn_reg_arg4(regs), &tid, + task->opaque2 = bulk.a5; /* hidden pthread_t identifier. */ + __xn_copy_to_user(curr, (void __user *) bulk.a4, &tid, sizeof(tid)); err = xnshadow_map(&task->threadbase, u_completion); /* May be NULL */ } else { @@ -1443,6 +1452,36 @@ return as_send((u_long)task, signals); } + +/* + * int __t_get_pthread(u_long tid, u_long *pthread) + */ +static int __t_get_pthread(struct task_struct *curr, struct pt_regs *regs) +{ + xnhandle_t handle; + psostask_t *task; + u_long pthread; + + handle = __xn_reg_arg1(regs); + + if (!__xn_access_ok(curr, VERIFY_READ, __xn_reg_arg2(regs), sizeof(u_long))) + return -EFAULT; + + if (handle) + task = (psostask_t *)xnregistry_fetch(handle); + else + task = __psos_task_current(curr); + + if (!task) + return ERR_OBJID; + + pthread = task->opaque2; /* hidden pthread_t identifier. */ + + __xn_copy_to_user(curr, (void __user *) __xn_reg_arg2(regs), &pthread, sizeof(u_long)); + + return SUCCESS; +} + static void *psos_shadow_eventcb(int event, void *data) { struct psos_resource_holder *rh; @@ -1524,6 +1563,7 @@ [__psos_tm_signal] = {&__tm_signal, __xn_exec_primary}, [__psos_as_send] = {&__as_send, __xn_exec_conforming}, [__psos_tm_getc] = {&__tm_getc, __xn_exec_any}, + [__psos_t_get_pthread] = {&__t_get_pthread, __xn_exec_any}, }; extern xntbase_t *psos_tbase; Index: xenomai-2.4.7_bugless/src/skins/psos+/task.c =================================================================== --- xenomai-2.4.7_bugless.orig/src/skins/psos+/task.c 2009-11-05 09:17:31.000000000 +0100 +++ xenomai-2.4.7_bugless/src/skins/psos+/task.c 2009-11-05 09:17:56.000000000 +0100 @@ -26,6 +26,15 @@ #include <memory.h> #include <psos+/psos.h> +struct arg_bulk5{ + + u_long a1; + u_long a2; + u_long a3; + u_long a4; + u_long a5; +}; + extern int __psos_muxid; struct psos_task_iargs { @@ -73,6 +82,7 @@ struct sched_param param; int policy; long err; + struct arg_bulk5 bulk; policy = psos_task_set_posix_priority(iargs->prio, ¶m); pthread_setschedparam(pthread_self(), policy, ¶m); @@ -81,10 +91,14 @@ old_sigharden_handler = signal(SIGHARDEN, &psos_task_sigharden); - err = XENOMAI_SKINCALL5(__psos_muxid, - __psos_t_create, - iargs->name, iargs->prio, iargs->flags, - iargs->tid_r, iargs->completionp); + bulk.a1 = (u_long)iargs->name; + bulk.a2 = (u_long)iargs->prio; + bulk.a3 = (u_long)iargs->flags; + bulk.a4 = (u_long)iargs->tid_r; + bulk.a5 = (u_long)pthread_self(); + + err = XENOMAI_SKINCALL2(__psos_muxid, __psos_t_create, &bulk, iargs->completionp); + if (err) goto fail; @@ -172,14 +186,19 @@ u_long flags, u_long *tid_r) { + struct arg_bulk5 bulk; + pthread_setcanceltype(PTHREAD_CANCEL_ASYNCHRONOUS, NULL); old_sigharden_handler = signal(SIGHARDEN, &psos_task_sigharden); - return XENOMAI_SKINCALL5(__psos_muxid, - __psos_t_create, - name, prio, flags, - tid_r, NULL); + bulk.a1 = (u_long)name; + bulk.a2 = (u_long)prio; + bulk.a3 = (u_long)flags; + bulk.a4 = (u_long)tid_r; + bulk.a5 = (u_long)pthread_self(); + + return XENOMAI_SKINCALL2(__psos_muxid, __psos_t_create, &bulk, NULL); } u_long t_start(u_long tid, @@ -196,6 +215,18 @@ u_long t_delete(u_long tid) { + long err; + u_long pthread; + + if(tid) + { + err = XENOMAI_SKINCALL2(__psos_muxid, __psos_t_get_pthread, tid, &pthread); + if (err) + return err; + err = pthread_cancel((pthread_t)pthread); + if (err) + return err; + } return XENOMAI_SKINCALL1(__psos_muxid, __psos_t_delete, tid); } Index: xenomai-2.4.7_bugless/include/psos+/task.h =================================================================== --- xenomai-2.4.7_bugless.orig/include/psos+/task.h 2009-11-05 09:19:49.000000000 +0100 +++ xenomai-2.4.7_bugless/include/psos+/task.h 2009-11-05 09:19:58.000000000 +0100 @@ -26,6 +26,16 @@ #define PSOS_TASK_MAGIC 0x81810101 +struct arg_bulk5{ + + u_long a1; + u_long a2; + u_long a3; + u_long a4; + u_long a5; +}; + + typedef struct psostask { unsigned magic; /* Magic code - must be first */ @@ -64,6 +74,8 @@ } waitargs; + u_long opaque2; /* hidden pthread_t identifier. */ + } psostask_t; static inline psostask_t *thread2psostask (xnthread_t *t) ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [Xenomai-core] [Xenomai-help] How to fix t_delete call in psos skin? 2009-11-05 10:54 ` Alexandre Coffignal @ 2009-11-06 11:28 ` Philippe Gerum 0 siblings, 0 replies; 4+ messages in thread From: Philippe Gerum @ 2009-11-06 11:28 UTC (permalink / raw) To: Alexandre Coffignal; +Cc: xenomai On Thu, 2009-11-05 at 11:54 +0100, Alexandre Coffignal wrote: > Does this fix seem more suitable to you? > Do you see someting else missing? > The logic is fine, but the implementation should get closer to what is being done in the -head branch. Preliminary remarks: - Please follow the kernel CodingStyle guidelines for anything that is Xenomai-related, including userland code. Keeping a single code style throughout our code makes things easier for reviewing, and keeps visual pollution to the lowest possible level. - Please rebase your work on 2.4.10 for pushing those changes to Xenomai mainline. There is no point for us to consider changes to legacy releases which are not being actively maintained anymore; you can always backport those changes to 2.4.7 locally if you really fancy dealing with bugs solved between 2.4.7 and 2.4.10... - Please tell your CVS/SVN diff tool to use -p, when formatting patches. > Please advice. > > plain text document attachment (patch_psos_t_delete) > Index: xenomai-2.4.7_bugless/include/psos+/syscall.h > =================================================================== > --- xenomai-2.4.7_bugless.orig/include/psos+/syscall.h 2009-11-05 09:12:29.000000000 +0100 > +++ xenomai-2.4.7_bugless/include/psos+/syscall.h 2009-11-05 09:13:14.000000000 +0100 > @@ -73,6 +73,7 @@ > #define __psos_as_send 45 > /* Xenomai extension: get raw count of jiffies */ > #define __psos_tm_getc 46 > +#define __psos_t_get_pthread 47 /* get hidden pthread_t identifier. */ psos_t_getpth is enough. To be really, really pSOS-conformant when introducing a routine, we should either use: 1) a preposterously long argument list or, at the very least: 2) an absolutely unpronounceable C identifier __psos_t_getpth happily qualifies for #2. > > #ifdef __KERNEL__ > > Index: xenomai-2.4.7_bugless/ksrc/skins/psos+/syscall.c > =================================================================== > --- xenomai-2.4.7_bugless.orig/ksrc/skins/psos+/syscall.c 2009-11-05 09:16:48.000000000 +0100 > +++ xenomai-2.4.7_bugless/ksrc/skins/psos+/syscall.c 2009-11-05 09:16:57.000000000 +0100 > @@ -67,29 +67,37 @@ > xncompletion_t __user *u_completion; > u_long prio, flags, tid, err; > char name[XNOBJECT_NAME_LEN]; > + struct arg_bulk5 bulk; Please have a look at what is being done in the -head branch, for the very same routine (in the absence of -p passed to diff, seems to be __t_create). An arg bulk has been introduced there, so you just need to backport this. > psostask_t *task; > > - if (!__xn_access_ok(curr, VERIFY_READ, __xn_reg_arg1(regs), 4)) > + if (!__xn_access_ok(curr, VERIFY_READ, __xn_reg_arg1(regs), sizeof(bulk))) > + return -EFAULT; > + > + __xn_copy_from_user(curr, &bulk, (void __user *)__xn_reg_arg1(regs), > + sizeof(bulk)); > + > + if (!__xn_access_ok(curr, VERIFY_READ, bulk.a1, sizeof(name))) { > return -EFAULT; > + } > > /* Get task name. */ > - __xn_strncpy_from_user(curr, name, (const char __user *)__xn_reg_arg1(regs), > + __xn_strncpy_from_user(curr, name, (const char __user *)bulk.a1, > sizeof(name) - 1); > name[sizeof(name) - 1] = '\0'; > strncpy(curr->comm, name, sizeof(curr->comm)); > curr->comm[sizeof(curr->comm) - 1] = '\0'; > > if (!__xn_access_ok > - (curr, VERIFY_WRITE, __xn_reg_arg4(regs), sizeof(tid))) > + (curr, VERIFY_WRITE, bulk.a4, sizeof(tid))) > return -EFAULT; > > /* Task priority. */ > - prio = __xn_reg_arg2(regs); > + prio = bulk.a2; > /* Task flags. Force FPU support in user-space. This will lead > to a no-op if the platform does not support it. */ > - flags = __xn_reg_arg3(regs) | T_SHADOW | T_FPU; > + flags = bulk.a3 | T_SHADOW | T_FPU; > /* Completion descriptor our parent thread is pending on. */ > - u_completion = (xncompletion_t __user *)__xn_reg_arg5(regs); > + u_completion = (xncompletion_t __user *)__xn_reg_arg2(regs); > > err = t_create(name, prio, 0, 0, flags, &tid); > > @@ -99,7 +107,8 @@ > * about the new thread id, so we can manipulate its > * TCB pointer freely. */ > tid = xnthread_handle(&task->threadbase); > - __xn_copy_to_user(curr, (void __user *)__xn_reg_arg4(regs), &tid, > + task->opaque2 = bulk.a5; /* hidden pthread_t identifier. */ > + __xn_copy_to_user(curr, (void __user *) bulk.a4, &tid, > sizeof(tid)); > err = xnshadow_map(&task->threadbase, u_completion); /* May be NULL */ > } else { > @@ -1443,6 +1452,36 @@ > return as_send((u_long)task, signals); > } > > + > +/* > + * int __t_get_pthread(u_long tid, u_long *pthread) > + */ > +static int __t_get_pthread(struct task_struct *curr, struct pt_regs *regs) > +{ > + xnhandle_t handle; > + psostask_t *task; > + u_long pthread; > + > + handle = __xn_reg_arg1(regs); > + > + if (!__xn_access_ok(curr, VERIFY_READ, __xn_reg_arg2(regs), sizeof(u_long))) > + return -EFAULT; > + > + if (handle) > + task = (psostask_t *)xnregistry_fetch(handle); > + else > + task = __psos_task_current(curr); > + > + if (!task) > + return ERR_OBJID; Critical section enforcement is missing. What if the caller gets preempted after the handle is fetched, and the non-current tid gets killed by a concurrent thread right after? See t_ident for an illustration. If the deletion happens after the critical section, well, we would just have to hope that pthread_cancel() does a good job at validating its handle, but at least we would not have referred to stale memory on our end. > + > + pthread = task->opaque2; /* hidden pthread_t identifier. */ > + > + __xn_copy_to_user(curr, (void __user *) __xn_reg_arg2(regs), &pthread, sizeof(u_long)); > + > + return SUCCESS; > +} > + > static void *psos_shadow_eventcb(int event, void *data) > { > struct psos_resource_holder *rh; > @@ -1524,6 +1563,7 @@ > [__psos_tm_signal] = {&__tm_signal, __xn_exec_primary}, > [__psos_as_send] = {&__as_send, __xn_exec_conforming}, > [__psos_tm_getc] = {&__tm_getc, __xn_exec_any}, > + [__psos_t_get_pthread] = {&__t_get_pthread, __xn_exec_any}, > }; > > extern xntbase_t *psos_tbase; > Index: xenomai-2.4.7_bugless/src/skins/psos+/task.c > =================================================================== > --- xenomai-2.4.7_bugless.orig/src/skins/psos+/task.c 2009-11-05 09:17:31.000000000 +0100 > +++ xenomai-2.4.7_bugless/src/skins/psos+/task.c 2009-11-05 09:17:56.000000000 +0100 > @@ -26,6 +26,15 @@ > #include <memory.h> > #include <psos+/psos.h> > > +struct arg_bulk5{ > + > + u_long a1; > + u_long a2; > + u_long a3; > + u_long a4; > + u_long a5; > +}; Rather use psos_arg_bulk like in -head tree. > + > extern int __psos_muxid; > > struct psos_task_iargs { > @@ -73,6 +82,7 @@ > struct sched_param param; > int policy; > long err; > + struct arg_bulk5 bulk; > > policy = psos_task_set_posix_priority(iargs->prio, ¶m); > pthread_setschedparam(pthread_self(), policy, ¶m); > @@ -81,10 +91,14 @@ > > old_sigharden_handler = signal(SIGHARDEN, &psos_task_sigharden); > > - err = XENOMAI_SKINCALL5(__psos_muxid, > - __psos_t_create, > - iargs->name, iargs->prio, iargs->flags, > - iargs->tid_r, iargs->completionp); > + bulk.a1 = (u_long)iargs->name; > + bulk.a2 = (u_long)iargs->prio; > + bulk.a3 = (u_long)iargs->flags; > + bulk.a4 = (u_long)iargs->tid_r; > + bulk.a5 = (u_long)pthread_self(); > + > + err = XENOMAI_SKINCALL2(__psos_muxid, __psos_t_create, &bulk, iargs->completionp); > + > if (err) > goto fail; > > @@ -172,14 +186,19 @@ > u_long flags, > u_long *tid_r) > { > + struct arg_bulk5 bulk; > + > pthread_setcanceltype(PTHREAD_CANCEL_ASYNCHRONOUS, NULL); > > old_sigharden_handler = signal(SIGHARDEN, &psos_task_sigharden); > > - return XENOMAI_SKINCALL5(__psos_muxid, > - __psos_t_create, > - name, prio, flags, > - tid_r, NULL); > + bulk.a1 = (u_long)name; > + bulk.a2 = (u_long)prio; > + bulk.a3 = (u_long)flags; > + bulk.a4 = (u_long)tid_r; > + bulk.a5 = (u_long)pthread_self(); > + > + return XENOMAI_SKINCALL2(__psos_muxid, __psos_t_create, &bulk, NULL); > } > > u_long t_start(u_long tid, > @@ -196,6 +215,18 @@ > > u_long t_delete(u_long tid) > { > + long err; > + u_long pthread; > + > + if(tid) > + { > + err = XENOMAI_SKINCALL2(__psos_muxid, __psos_t_get_pthread, tid, &pthread); > + if (err) > + return err; > + err = pthread_cancel((pthread_t)pthread); > + if (err) > + return err; > + } We should handle the self-deletion case specifically: + else { + /* Silently migrate to avoid raising SIGXCPU. */ + XENOMAI_SYSCALL1(__xn_sys_migrate, XENOMAI_LINUX_DOMAIN); + pthread_exit(NULL); + } Actually, we should also test for the case when a non-zero tid ends up pointing at the current thread as well, e.g. if (tid == 0) goto self_delete; err = XENOMAI_SKINCALL2(__psos_muxid, __psos_t_getpth, tid, &ptid); if (err) return err; if ((pthread_t)ptid == pthread_self()) goto self_delete; err = pthread_cancel((pthread_t)ptid); if (err) return -err; /* differentiate from pSOS codes */ return XENOMAI_SKINCALL1(__psos_muxid, __psos_t_delete, tid); self_delete: /* Silently migrate to avoid raising SIGXCPU. */ XENOMAI_SYSCALL1(__xn_sys_migrate, XENOMAI_LINUX_DOMAIN); pthread_exit(NULL); return SUCCESS; /* not reached */ > > > return XENOMAI_SKINCALL1(__psos_muxid, __psos_t_delete, tid); > } > > Index: xenomai-2.4.7_bugless/include/psos+/task.h > =================================================================== > --- xenomai-2.4.7_bugless.orig/include/psos+/task.h 2009-11-05 09:19:49.000000000 +0100 > +++ xenomai-2.4.7_bugless/include/psos+/task.h 2009-11-05 09:19:58.000000000 +0100 > @@ -26,6 +26,16 @@ > > #define PSOS_TASK_MAGIC 0x81810101 > > +struct arg_bulk5{ > + > + u_long a1; > + u_long a2; > + u_long a3; > + u_long a4; > + u_long a5; > +}; > + > + > typedef struct psostask { > > unsigned magic; /* Magic code - must be first */ > @@ -64,6 +74,8 @@ > > } waitargs; > > + u_long opaque2; /* hidden pthread_t identifier. */ No need to make this more opaque than required to the reviewer; besides, we only need this field when user-space support is compiled in. - u_long opaque2; /* hidden pthread_t identifier. */ + #ifdef CONFIG_XENO_OPT_PERVASIVE + u_long pthread; + #endif > + > } psostask_t; > > static inline psostask_t *thread2psostask (xnthread_t *t) -- Philippe. ^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2009-11-06 11:28 UTC | newest] Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2009-11-04 9:20 [Xenomai-help] How to fix t_delete call in psos skin? Alexandre Coffignal 2009-11-04 10:50 ` [Xenomai-core] " Philippe Gerum 2009-11-05 10:54 ` Alexandre Coffignal 2009-11-06 11:28 ` 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.