diff --git a/docs/src/man/man1/halcmd.1.adoc b/docs/src/man/man1/halcmd.1.adoc index 7b527b87066..82a8b2369f5 100644 --- a/docs/src/man/man1/halcmd.1.adoc +++ b/docs/src/man/man1/halcmd.1.adoc @@ -203,7 +203,9 @@ _pinname_ *=* _value_:: the order in which they were added. *stop*:: Stops execution of realtime threads. The threads will no longer call - their functions. + their functions. *stop* returns once every thread has finished the + functions it was running, so they can be unloaded. It fails if a + thread has not finished them within 1000 of its periods. *show* [_item_]:: Prints HAL items to _stdout_ in human readable format. _item_ can be one of "*comp*" (components), "*pin*", "*sig*" (signals), "*param*" diff --git a/docs/src/man/man3/hal_start_threads.3.adoc b/docs/src/man/man3/hal_start_threads.3.adoc index c13ecab07a4..e150fb3ce0c 100644 --- a/docs/src/man/man3/hal_start_threads.3.adoc +++ b/docs/src/man/man3/hal_start_threads.3.adoc @@ -23,10 +23,13 @@ This is the point at which realtime functions start being called. The *hal_stop_threads()* function stops all threads that were previously started by *hal_start_threads()*. It should be called before any component that is part of a system exits. +Called from a non-realtime process, it returns once every thread has finished the functions it was running, so their code can be unloaded. +Called from realtime code, it does not wait. == RETURN VALUE Returns a HAL status code. +*hal_stop_threads()* returns -ETIMEDOUT when a thread has not finished its functions within 1000 of its periods. == SEE ALSO diff --git a/src/hal/hal_lib.c b/src/hal/hal_lib.c index 227cf6ad629..d5d84107e8c 100644 --- a/src/hal/hal_lib.c +++ b/src/hal/hal_lib.c @@ -165,6 +165,9 @@ static void free_funct_entry_struct(hal_funct_entry_t * funct_entry); static hal_list_t *funct_entry_unlink(hal_list_t * entry); static int funct_entry_release(hal_thread_t * thread, hal_funct_entry_t * funct_entry); +#ifdef ULAPI +static int threads_wait_idle(void); +#endif /* ULAPI */ #ifdef RTAPI static void free_thread_struct(hal_thread_t * thread); #endif /* RTAPI */ @@ -2776,6 +2779,12 @@ int hal_stop_threads(void) } hal_data->threads_running = 0; +#ifdef ULAPI + /* a thread may still be in the pass it started before */ + if (threads_wait_idle() != 0) { + return -ETIMEDOUT; + } +#endif /* ULAPI */ rtapi_print_msg(RTAPI_MSG_DBG, "HAL: threads stopped\n"); return 0; } @@ -3275,6 +3284,11 @@ static void thread_task(void *arg) thread = arg; while (1) { + /* busy is set before threads_running is read, and + hal_stop_threads() clears threads_running before it reads + busy: either this pass sees the stop or the stop sees the + pass and waits for it */ + atomic_store_explicit(&thread->busy, 1, memory_order_seq_cst); if (hal_data->threads_running > 0 && !thread->init_done) { /* special init cycle: run init_funct_list once with no timing measurement, re-anchor the period so the long @@ -3343,6 +3357,8 @@ static void thread_task(void *arg) } hal_set_sint(thread->threadbeat, ++thread->beatcnt); } + /* the functions are no longer used until the next pass */ + atomic_store_explicit(&thread->busy, 0, memory_order_release); /* wait until next period */ rtapi_wait(); } @@ -3708,6 +3724,7 @@ static hal_thread_t *alloc_thread_struct(void) list_init_entry(&(p->funct_list)); list_init_entry(&(p->init_funct_list)); p->init_done = 0; + atomic_init(&p->busy, 0); p->name[0] = '\0'; } return p; @@ -4002,11 +4019,54 @@ static hal_list_t *funct_entry_unlink(hal_list_t * entry) starved; delf then fails instead of waiting forever. */ #define QUIESCENT_TIMEOUT_PERIODS 1000 +/* Wait until 'thread' is between two passes. With the threads stopped + it then does not start another one. Returns 0, or -ETIMEDOUT. */ +static int thread_wait_idle(hal_thread_t * thread) +{ + int n; + + for (n = 0; atomic_load_explicit(&thread->busy, memory_order_seq_cst); n++) { + if (n == QUIESCENT_TIMEOUT_PERIODS) { + return -ETIMEDOUT; + } + rtapi_delay(thread->period); + } + return 0; +} + +/* Called by hal_stop_threads() after it cleared threads_running. A + thread may still be in the pass it started before. Wait until every + thread has finished it, so that once the threads are stopped no + function runs and functions can be removed and unloaded. Returns 0, + or -ETIMEDOUT. */ +static int threads_wait_idle(void) +{ + int next; + hal_thread_t *thread; + int retval = 0; + + halpr_mutex_acquire(); + next = hal_data->thread_list_ptr; + while (next != 0) { + thread = SHMPTR(next); + if (thread_wait_idle(thread) != 0) { + rtapi_print_msg(RTAPI_MSG_ERR, + "HAL: ERROR: thread '%s' did not finish its functions within" + " %d periods\n", thread->name, QUIESCENT_TIMEOUT_PERIODS); + retval = -ETIMEDOUT; + } + next = thread->next_ptr; + } + halpr_mutex_release(); + return retval; +} + /* Called with the HAL mutex held, after a funct entry was unlinked from 'thread'. The realtime thread walks its funct_list without the mutex, so it may still be on the unlinked entry. Wait until the thread's threadbeat advanced by two (the pass that could see the entry has - ended) or the threads are stopped. Returns 0, or -ETIMEDOUT. + ended) or, once the threads are stopped, until the thread has + finished its pass. Returns 0, or -ETIMEDOUT. thread->threadbeat is only valid in the context that created the thread (pin addressing depends on the context), so the pin is looked @@ -4018,6 +4078,9 @@ static int thread_wait_quiescent(hal_thread_t * thread) rtapi_sint start; int n, rv; + if (hal_data->threads_running == 0) { + return thread_wait_idle(thread); + } rtapi_snprintf(name, sizeof(name), "%s.threadbeat", thread->name); q.name = name; q.qtype = HAL_QTYPE_PIN; @@ -4035,43 +4098,47 @@ static int thread_wait_quiescent(hal_thread_t * thread) start = hal_get_sint(q.pp.ref.s); for (n = 0; n < QUIESCENT_TIMEOUT_PERIODS; n++) { rtapi_delay(thread->period); - if (hal_data->threads_running == 0 - || hal_get_sint(q.pp.ref.s) - start >= 2) { + if (hal_get_sint(q.pp.ref.s) - start >= 2) { return 0; } + if (hal_data->threads_running == 0) { + return thread_wait_idle(thread); + } } return -ETIMEDOUT; } #endif /* ULAPI */ /* Free a funct entry that was unlinked from 'thread' with - funct_entry_unlink(). While the threads run, the thread may still be - on the entry. delf (ULAPI) waits until it has left. Code that runs in + funct_entry_unlink(). The thread may still be on the entry, while + the threads run or, after they were stopped, until it has finished + its pass. delf (ULAPI) waits until it has left. Code that runs in RTAPI (a component's exit, unloadrt) must not wait; there the entry - is not recycled and an error is reported, the threads should be - stopped or the function removed with delf before unloading. */ + is not recycled and an error is reported if the threads run, they + should be stopped or the function removed with delf before + unloading. */ static int funct_entry_release(hal_thread_t * thread, hal_funct_entry_t * funct_entry) { hal_funct_t *funct = SHMPTR(funct_entry->funct_ptr); - if (hal_data->threads_running > 0) { #ifdef ULAPI - if (thread_wait_quiescent(thread) != 0) { - rtapi_print_msg(RTAPI_MSG_ERR, - "HAL: ERROR: thread '%s' did not leave function '%s' within" - " %d periods; the function must not be unloaded\n", - thread->name, funct->name, QUIESCENT_TIMEOUT_PERIODS); - return -ETIMEDOUT; - } + if (thread_wait_quiescent(thread) != 0) { + rtapi_print_msg(RTAPI_MSG_ERR, + "HAL: ERROR: thread '%s' did not leave function '%s' within" + " %d periods; the function must not be unloaded\n", + thread->name, funct->name, QUIESCENT_TIMEOUT_PERIODS); + return -ETIMEDOUT; + } #else + if (hal_data->threads_running > 0) { rtapi_print_msg(RTAPI_MSG_ERR, "HAL: ERROR: function '%s' removed from running thread '%s';" " stop the threads or delf the function before unloading\n", funct->name, thread->name); return -EBUSY; -#endif } +#endif free_funct_entry_struct(funct_entry); return 0; } diff --git a/src/hal/hal_priv.h b/src/hal/hal_priv.h index 8692aae9463..78247c78699 100644 --- a/src/hal/hal_priv.h +++ b/src/hal/hal_priv.h @@ -100,6 +100,7 @@ #include #include +#include /* IMPORTANT: If any of the structures in this file are changed, the version code (HAL_VER) must be incremented, to ensure that @@ -124,7 +125,7 @@ */ #define HAL_KEY 0x48414C32 /* key used to open HAL shared memory */ -#define HAL_VER 0x00000015 /* version code */ +#define HAL_VER 0x00000016 /* version code */ #define HAL_SIZE (2*256*4096) /* These pointers are set by hal_init() to point to the shmem block @@ -286,7 +287,7 @@ typedef struct hal_data_t { SHMFIELD(hal_funct_t) funct_list_ptr; // root of linked list of functions SHMFIELD(hal_thread_t) thread_list_ptr; // root of linked list of threads long base_period; // timer period for realtime tasks - int threads_running; // non-zero if threads are started + rtapi_atomic_int threads_running; // non-zero if threads are started SHMFIELD(hal_oldname_t) oldname_free_ptr; // list of free oldname structs SHMFIELD(hal_comp_t) comp_free_ptr; // list of free component structs SHMFIELD(hal_pin_t) pin_free_ptr; // list of free pin structs @@ -430,6 +431,7 @@ struct hal_thread_t { hal_list_t funct_list; // list of functions to run hal_list_t init_funct_list; // list of init functions, run once before first cyclic cycle int init_done; // 0 = init pending, 1 = init cycle has executed + rtapi_atomic_int busy; // non-zero while the thread may run functions char name[HAL_NAME_LEN + 1]; // thread name int comp_id; }; diff --git a/tests/hal-delf-live/README b/tests/hal-delf-live/README index 281b5465749..9e1e8c3f473 100644 --- a/tests/hal-delf-live/README +++ b/tests/hal-delf-live/README @@ -26,7 +26,10 @@ waits there in rtapi_delay(), which sleeps in uspace, so delftest.py and halcmd also run on the CPU of the thread. When delf returns, pin inside must be false; after unloadrt t.threadbeat must keep advancing. Then a second instance is held inside its function and -unloadrt is run without delf; the thread must keep running. Last, the -usual stop, unloadrt, start sequence is checked. Pins are +unloadrt is run without delf; the thread must keep running. Last, a +third instance is held inside its function and the usual stop, +unloadrt, start sequence is run: when stop returns, pin inside must be +false, since with the threads stopped unloadrt removes the function at +once and unloads its code. Pins are accessed with hal.set_p() and hal.get_p(); there are no nets. A wait that makes no progress for 10 s aborts the test. diff --git a/tests/hal-delf-live/delftest.py b/tests/hal-delf-live/delftest.py index 681f598268e..df1d677ab7d 100755 --- a/tests/hal-delf-live/delftest.py +++ b/tests/hal-delf-live/delftest.py @@ -2,7 +2,9 @@ # Hold the realtime thread inside a function and delf the function from # another process. delf must not return before the thread has left the # function. unloadrt of a component whose function is in the running -# thread removes the function the same way first; it works after stop too. +# thread removes the function the same way first. stop must not return +# before the thread has left the function either; unloadrt after stop +# then works too. # # Every wait is on a HAL state. A wait that takes more than 10 s of wall # time means the machine is dying; the test then aborts. @@ -62,11 +64,19 @@ def wait_beat(n): wait_beat(2) print("ok: unloadrt waits for the thread to leave the function") -# usual path: stop, then unload with the function still in the thread +# usual path: stop, then unload with the function still in the thread. +# stop must not return while the thread is inside the function: with +# the threads stopped, unloadrt removes the function at once and +# unloads its code halcmd("loadrt", "delfvictim", "names=w") halcmd("addf", "w", "t") wait_beat(2) +hal.set_p("w.hold", True) +wait(lambda: hal.get_p("w.inside")) halcmd("stop") +if hal.get_p("w.inside"): + sys.exit("FAIL: stop returned while the thread was inside w") +print("ok: stop waits for the thread to leave the function") halcmd("unloadrt", "delfvictim") halcmd("start") wait_beat(2)