Module: xenomai-3 Branch: next Commit: e1b0d84ce7ab5a49bd4c5553bb9060055ebd2adb URL: http://git.xenomai.org/?p=xenomai-3.git;a=commit;h=e1b0d84ce7ab5a49bd4c5553bb9060055ebd2adb
Author: Philippe Gerum <r...@xenomai.org> Date: Wed Jun 29 09:54:12 2016 +0200 cobalt/thread: fix race on accessing the global thread list nkthreadq is only used by the /proc interface handlers these days, to get the list of active Cobalt threads. Adding an emerging user-space thread to that list should not be done before it is actually mated to a Cobalt shadow, otherwise the /proc handlers might find such thread before its host_task pointer is known, causing invalid memory accesses. A simple way to reproduce the issue involves running a (user) thread creation loop while continuously reading from /proc/xenomai/sched/* in parallel. The fix postpones the insertion of emerging threads into the global thread queue until their TCB is fully built. --- kernel/cobalt/thread.c | 84 +++++++++++++++++++++++++++--------------------- 1 file changed, 47 insertions(+), 37 deletions(-) diff --git a/kernel/cobalt/thread.c b/kernel/cobalt/thread.c index cabc6b8..435cd4b 100644 --- a/kernel/cobalt/thread.c +++ b/kernel/cobalt/thread.c @@ -88,6 +88,13 @@ static void periodic_handler(struct xntimer *timer) fixup_ptimer_affinity(thread); } +static inline void enlist_new_thread(struct xnthread *thread) +{ /* nklock held, irqs off */ + list_add_tail(&thread->glink, &nkthreadq); + cobalt_nrthreads++; + xnvfile_touch_tag(&nkthreadlist_tag); +} + struct kthread_arg { struct xnthread *thread; struct completion *done; @@ -573,10 +580,10 @@ void __xnthread_discard(struct xnthread *thread) * Initializes a new thread. The thread is left dormant until it is * actually started by xnthread_start(). * - * @param thread The address of a thread descriptor the nucleus will - * use to store the thread-specific data. This descriptor must always - * be valid while the thread is active therefore it must be allocated - * in permanent memory. @warning Some architectures may require the + * @param thread The address of a thread descriptor Cobalt will use to + * store the thread-specific data. This descriptor must always be + * valid while the thread is active therefore it must be allocated in + * permanent memory. @warning Some architectures may require the * descriptor to be properly aligned in memory; this is an additional * reason for descriptors not to be laid in the program stack where * alignement constraints might not always be satisfied. @@ -587,14 +594,13 @@ void __xnthread_discard(struct xnthread *thread) * * - name: An ASCII string standing for the symbolic name of the * thread. This name is copied to a safe place into the thread - * descriptor. This name might be used in various situations by the - * nucleus for issuing human-readable diagnostic messages, so it is - * usually a good idea to provide a sensible value here. NULL is fine - * though and means "anonymous". + * descriptor. This name might be used in various situations by Cobalt + * for issuing human-readable diagnostic messages, so it is usually a + * good idea to provide a sensible value here. NULL is fine though + * and means "anonymous". * * - flags: A set of creation flags affecting the operation. The - * following flags can be part of this bitmask, each of them affecting - * the nucleus behaviour regarding the created thread: + * following flags can be part of this bitmask: * * - XNSUSP creates the thread in a suspended state. In such a case, * the thread shall be explicitly resumed using the xnthread_resume() @@ -606,8 +612,8 @@ void __xnthread_discard(struct xnthread *thread) * user-space task. Otherwise, a new kernel host task is created, then * paired with the new Xenomai thread. * - * - XNFPU (enable FPU) tells the nucleus that the new thread may use - * the floating-point unit. XNFPU is implicitly assumed for user-space + * - XNFPU (enable FPU) tells Cobalt that the new thread may use the + * floating-point unit. XNFPU is implicitly assumed for user-space * threads even if not set in @a flags. * * - affinity: The processor affinity of this thread. Passing @@ -636,7 +642,6 @@ int xnthread_init(struct xnthread *thread, { struct xnsched *sched; cpumask_t affinity; - spl_t s; int ret; if (attr->flags & ~(XNFPU | XNUSER | XNSUSP)) @@ -659,12 +664,6 @@ int xnthread_init(struct xnthread *thread, trace_cobalt_thread_init(thread, attr, sched_class); - xnlock_get_irqsave(&nklock, s); - list_add_tail(&thread->glink, &nkthreadq); - cobalt_nrthreads++; - xnvfile_touch_tag(&nkthreadlist_tag); - xnlock_put_irqrestore(&nklock, s); - return 0; } EXPORT_SYMBOL_GPL(xnthread_init); @@ -684,9 +683,8 @@ EXPORT_SYMBOL_GPL(xnthread_init); * execution properties of the new thread. Members of this structure * are defined as follows: * - * - mode: The initial thread mode. The following flags can - * be part of this bitmask, each of them affecting the nucleus - * behaviour regarding the started thread: + * - mode: The initial thread mode. The following flags can be part of + * this bitmask: * * - XNLOCK causes the thread to lock the scheduler when it starts. * The target thread will have to call the xnsched_unlock() @@ -701,7 +699,7 @@ EXPORT_SYMBOL_GPL(xnthread_init); * - entry: The address of the thread's body routine. In other words, * it is the thread entry point. * - * - cookie: A user-defined opaque cookie the nucleus will pass to the + * - cookie: A user-defined opaque cookie Cobalt will pass to the * emerging thread as the sole argument of its entry point. * * @retval 0 if @a thread could be started ; @@ -728,6 +726,14 @@ int xnthread_start(struct xnthread *thread, if (attr->mode & XNLOCK) thread->lock_count = 1; + /* + * A user-space thread starts immediately Cobalt-wise since we + * already have an underlying Linux context for it, so we can + * enlist it now to make it visible from the /proc interface. + */ + if (xnthread_test_state(thread, XNUSER)) + enlist_new_thread(thread); + trace_cobalt_thread_start(thread); xnthread_resume(thread, XNDORMANT); @@ -1000,20 +1006,20 @@ void xnthread_suspend(struct xnthread *thread, int mask, * current thread. * * The net effect is that we are attempting to stop the - * shadow thread at the nucleus level, whilst this thread is - * actually running some code under the control of the Linux - * scheduler (i.e. it's relaxed). + * shadow thread for Cobalt, whilst this thread is actually + * running some code under the control of the Linux scheduler + * (i.e. it's relaxed). * * To make this possible, we force the target Linux task to * migrate back to the Xenomai domain by sending it a * SIGSHADOW signal the interface libraries trap for this * specific internal purpose, whose handler is expected to - * call back the nucleus's migration service. + * call back Cobalt's migration service. * - * By forcing this migration, we make sure that the real-time - * nucleus controls, hence properly stops, the target thread - * according to the requested suspension condition. Otherwise, - * the shadow thread in secondary mode would just keep running + * By forcing this migration, we make sure that Cobalt + * controls, hence properly stops, the target thread according + * to the requested suspension condition. Otherwise, the + * shadow thread in secondary mode would just keep running * into the Linux domain, thus breaking the most common * assumptions regarding suspended threads. * @@ -1878,10 +1884,10 @@ void xnthread_migrate_passive(struct xnthread *thread, struct xnsched *sched) * * Changes the base scheduling policy and paramaters of a thread. If * the thread is currently blocked, waiting in priority-pending mode - * (XNSYNCH_PRIO) for a synchronization object to be signaled, the - * nucleus will attempt to reorder the object's wait queue so that it - * reflects the new sleeper's priority, unless the XNSYNCH_DREORD flag - * has been set for the pended object. + * (XNSYNCH_PRIO) for a synchronization object to be signaled, Cobalt + * will attempt to reorder the object's wait queue so that it reflects + * the new sleeper's priority, unless the XNSYNCH_DREORD flag has been + * set for the pended object. * * @param thread The descriptor address of the affected thread. See * note. @@ -2551,8 +2557,8 @@ static inline void init_kthread_info(struct xnthread *thread) * @internal * @brief Create a shadow thread context over a kernel task. * - * This call maps a nucleus thread to the "current" Linux task running - * in kernel space. The priority and scheduling class of the + * This call maps a Cobalt core thread to the "current" Linux task + * running in kernel space. The priority and scheduling class of the * underlying Linux task are not affected; it is assumed that the * caller did set them appropriately before issuing the shadow mapping * request. @@ -2631,7 +2637,10 @@ int xnthread_map(struct xnthread *thread, struct completion *done) xnthread_resume(thread, XNDORMANT); ret = xnthread_harden(); wakeup_parent(done); + xnlock_get_irqsave(&nklock, s); + + enlist_new_thread(thread); /* * Make sure xnthread_start() did not slip in from another CPU * while we were back from wakeup_parent(). @@ -2639,6 +2648,7 @@ int xnthread_map(struct xnthread *thread, struct completion *done) if (thread->entry == NULL) xnthread_suspend(thread, XNDORMANT, XN_INFINITE, XN_RELATIVE, NULL); + xnlock_put_irqrestore(&nklock, s); xnthread_test_cancel(); _______________________________________________ Xenomai-git mailing list Xenomai-git@xenomai.org https://xenomai.org/mailman/listinfo/xenomai-git