[PATCH v2] rtems: Add rtems_task_create_from_config()
Sebastian Huber
sebastian.huber at embedded-brains.de
Wed Sep 9 08:11:08 UTC 2020
On 09/09/2020 00:43, Joel Sherrill wrote:
>
>
> On Wed, Sep 2, 2020 at 11:09 AM Sebastian Huber
> <sebastian.huber at embedded-brains.de
> <mailto:sebastian.huber at embedded-brains.de>> wrote:
>
> In contrast to rtems_task_create() this function creates a task with a
> user-provided task storage area. The new create function uses a
> configuration structure instead of individual parameters.
>
> Add RTEMS_TASK_STORAGE_ALIGNMENT to define the recommended alignment of
> a task storage area.
>
> Add RTEMS_TASK_STORAGE_SIZE() to calculate the recommended size of a
> task storage area based on the task attributes and the size dedicated to
> the task stack and thread-local storage. This macro may allow future
> extensions without breaking the API.
>
> Update #3959.
> ---
>
> v2:
>
> Rename function from rtems_task_build() to
> rtems_task_create_from_config(). Add RTEMS_TASK_STORAGE_ALIGNMENT and
> RTEMS_TASK_STORAGE_SIZE(). Improve documentation.
>
> cpukit/Makefile.am | 1 +
> cpukit/include/rtems/rtems/tasks.h | 124 +++++++++++
> cpukit/include/rtems/rtems/tasksimpl.h | 11 +
> cpukit/rtems/src/taskcreate.c | 278 +++++-------------------
> cpukit/rtems/src/taskcreatefromconfig.c | 274 +++++++++++++++++++++++
> testsuites/sptests/sp01/init.c | 24 +-
> testsuites/sptests/sp01/sp01.doc | 1 +
> testsuites/sptests/sp01/system.h | 2 +-
> 8 files changed, 479 insertions(+), 236 deletions(-)
> create mode 100644 cpukit/rtems/src/taskcreatefromconfig.c
>
> diff --git a/cpukit/Makefile.am b/cpukit/Makefile.am
> index e5009e53c9..caa6a9efe6 100644
> --- a/cpukit/Makefile.am
> +++ b/cpukit/Makefile.am
> @@ -787,6 +787,7 @@ librtemscpu_a_SOURCES += rtems/src/statustoerrno.c
> librtemscpu_a_SOURCES += rtems/src/systemeventreceive.c
> librtemscpu_a_SOURCES += rtems/src/systemeventsend.c
> librtemscpu_a_SOURCES += rtems/src/taskcreate.c
> +librtemscpu_a_SOURCES += rtems/src/taskcreatefromconfig.c
> librtemscpu_a_SOURCES += rtems/src/taskdelete.c
> librtemscpu_a_SOURCES += rtems/src/taskexit.c
> librtemscpu_a_SOURCES += rtems/src/taskgetaffinity.c
> diff --git a/cpukit/include/rtems/rtems/tasks.h
> b/cpukit/include/rtems/rtems/tasks.h
> index 12c323e60e..a183dcafed 100644
> --- a/cpukit/include/rtems/rtems/tasks.h
> +++ b/cpukit/include/rtems/rtems/tasks.h
> @@ -21,6 +21,7 @@
> #include <rtems/rtems/attr.h>
> #include <rtems/rtems/status.h>
> #include <rtems/rtems/types.h>
> +#include <rtems/score/context.h>
> #include <rtems/score/smp.h>
>
> #ifdef __cplusplus
> @@ -164,6 +165,129 @@ rtems_status_code rtems_task_create(
> rtems_id *id
> );
>
> +/**
> + * @brief Returns the recommended task storage area size for the
> specified size
> + * and task attributes.
> + *
> + * @param _size is the size dedicated to the task stack and
> thread-local
> + * storage.
>
>
> How does the user get the TLS size?
>
> Need advice on that. Seems hard to get at compile time since it is a link
> time aggregation.
I think we need an update of the Key Concepts or the Task Manager
Background chapters so that general task storage considerations are
explained.
https://docs.rtems.org/branches/master/c-user/key_concepts.html
https://docs.rtems.org/branches/master/c-user/task/background.html
See also reply to Chris.
>
> + *
> + * @param _attributes is the attribute set of the task using the
> storage area.
> + *
> + * @return The recommended task storage area size is returned
> calculated from
> + * the input parameters.
> + *
> + * @see rtems_task_config
> + */
> +#define RTEMS_TASK_STORAGE_SIZE( _size, _attributes ) \
> + ( ( _size ) + \
> + ( ( ( _attributes ) & RTEMS_FLOATING_POINT ) != 0 ?
> CONTEXT_FP_SIZE : 0 ) )
>
>
> If the architecture requires all threads to be FP, I don't think this
> will work.
Oh, yes. I will fix this in v3.
>
> +
> +/**
> + * @brief This variable attribute defines the recommended alignment
> of a task
> + * storage area.
> + *
> + * @see rtems_task_config
> + */
> +#define RTEMS_TASK_STORAGE_ALIGNMENT RTEMS_ALIGNED(
> CPU_STACK_ALIGNMENT )
>
> Good. I assume the stack comes off the lower address range. Will
> the TLS and FP areas have sufficient alignment? Is that accounted for?
>
> I can't speak to generic TLS alignment but seems like cache alignment
> is what the linker script would aim for.
The TLS alignment is defined by the content. The linker script doesn't
define a particular alignment.
In the storage area size sanity check we have to take the alignment into
account.
>
> FP context may have to be double aligned on many architectures.
I guess CPU_STACK_ALIGNMENT accounts for alignment requirements of
double values.
>
> +/**
> + * @brief This structure defines the configuration of a task created by
> + * rtems_task_create_from_config().
> + */
> +typedef struct {
> + /**
> + * @brief This member defines the name of the task.
> + */
> + rtems_name name;
> +
> + /**
> + * @brief This member defines initial priority of the task.
> + */
> + rtems_task_priority initial_priority;
> +
> + /**
> + * @brief This member shall point to the task storage area begin.
> + *
> + * The task storage area will contain the task stack, the
> thread-local
> + * storage, and, on some architectures, the floating-point context.
>
>
> What does this mean? I think most architectures treat the areas as
> separate (FP not in integer context) and some require all tasks to be
> FP. This seems inaccurate.
On arm, riscv, and the recent powerpc, the FP and vector unit context is
included in the normal task context. You have to check the CPU port
documentation to know how the FP context is managed. I don't think we
should repeat this information here.
What about:
* The task storage area will contain the task stack, the thread-local
* storage, and the floating-point context on architectures with a
separate
* floating-point context.
>
> + *
> + * There are no alignment requirements for the task storage
> area. To avoid
> + * memory waste, use the ::RTEMS_TASK_STORAGE_ALIGNMENT variable
> attribute to
> + * enforce the recommended alignment of the task storage area.
> + */
> + void *storage_area;
> +
> + /**
> + * @brief This member defines size of the task storage area in bytes.
> + *
> + * Use the RTEMS_TASK_STORAGE_SIZE() macro to determine the
> recommended task
> + * storage area size.
> + */
> + size_t storage_size;
> +
> + /**
> + * @brief This member defines the optional handler to free the
> task storage
> + * area.
> + *
> + * It is called when the task building aborts due to a failed
> task create
> + * extension or the task is deleted. It is called from task
> context under
> + * protection of the object allocator lock. It is allowed to
> call free() in
> + * this handler. The handler may be NULL.
> + */
> + void ( *storage_free )( void * );
>
>
> Is it not called when the thread is deleted?
"It is called when the task building [...] or the task is deleted."
I will replace this task building with "task creation" in v3.
>
> +
> + /**
> + * @brief This member defines the initial modes of the task.
> + */
> + rtems_mode initial_modes;
> +
> + /**
> + * @brief This member defines the attributes of the task.
> + */
> + rtems_attribute attributes;
> +} rtems_task_config;
> +
> +/**
> + * @brief Creates a task according to the specified configuration.
> + *
> + * In contrast to tasks created by rtems_task_create(), the tasks
> created by
> + * this directive use a user-provided task storage area (contains
> the task
> + * stack).
>
>
> Drop () and say which contains the task stack.
* In contrast to tasks created by rtems_task_create(), the tasks
created by
* this directive use a user-provided task storage area. The task
storage area
* contains the task stack, the thread-local storage, and the
floating-point
* context on architectures with a separate floating-point context.
>
> + *
> + * It is not recommended to mix rtems_task_create() and
> + * rtems_task_create_from_config() in an application. This
> directive is
> + * intended for applications which do not want to use the RTEMS
> Workspace and
> + * instead statically allocate all operating system resources. The
> stack space
> + * estimate done by <rtems/confdefs.h> assumes that all tasks are
> created by
> + * rtems_task_create(). The estimate can be adjusted to take
> user-provided task
> + * storage areas into account through the ::CONFIGURE_MEMORY_OVERHEAD
> + * application configuration option or a custom task stack
> allocator, see
> + * ::CONFIGURE_TASK_STACK_ALLOCATOR.
>
>
> CONFIGURE_MEMORY_OVERHEAD would have to be a negative number to
> make this work. Is this explained or is there some magic I am missing?
>
> CONFIGURE_MEMORY_OVERHEAD is intended as a backdoor if the
> confdefs.h memory calculation is too low. It is not intended for general
> use like this.
>
> I would rather see something like "configure build tasks" and just subtract
> that from maximum threads before multiplying by minimum stack size.
> This avoids the user having to add up all their statically allocated stack
> sizes.
What about a new option CONFIGURE_TASKS_CREATED_FROM_CONFIG? See v3.
>
> + *
> + * @param config is the task configuration.
> + *
> + * @param[out] id is the pointer to an object identifier variable.
> The object
> + * identifier of the created task will be stored in this
> variable, in case of
> + * a successful operation.
> + *
> + * @retval RTEMS_SUCCESSFUL Successful operation.
> + *
> + * @retval RTEMS_INVALID_ADDRESS The id parameter is @c NULL.
> + *
> + * @retval RTEMS_INVALID_NAME The task name is invalid.
> + *
> + * @retval RTEMS_INVALID_PRIORITY The initial priority of the task
> is invalid.
> + *
> + * @retval RTEMS_TOO_MANY No task is available.
> + *
> + * @retval RTEMS_UNSATISFIED A task create extension failed.
> + */
> +rtems_status_code rtems_task_create_from_config(
> + const rtems_task_config *config,
> + rtems_id *id
> +);
> +
> /**
> * @brief RTEMS Task Name to Id
> *
> diff --git a/cpukit/include/rtems/rtems/tasksimpl.h
> b/cpukit/include/rtems/rtems/tasksimpl.h
> index c9544f8c27..a39113a283 100644
> --- a/cpukit/include/rtems/rtems/tasksimpl.h
> +++ b/cpukit/include/rtems/rtems/tasksimpl.h
> @@ -42,6 +42,17 @@ extern "C" {
> */
> void _RTEMS_tasks_Initialize_user_tasks( void );
>
> +typedef void ( *RTEMS_tasks_Prepare_stack )(
> + Thread_Configuration *,
> + const rtems_task_config *
> +);
> +
> +rtems_status_code _RTEMS_tasks_Create(
> + const rtems_task_config *config,
> + rtems_id *id,
> + RTEMS_tasks_Prepare_stack prepare_stack
> +);
> +
> RTEMS_INLINE_ROUTINE Thread_Control *_RTEMS_tasks_Allocate(void)
> {
> _Objects_Allocator_lock();
> diff --git a/cpukit/rtems/src/taskcreate.c
> b/cpukit/rtems/src/taskcreate.c
> index 5486ac9b6e..1d9a4546e8 100644
> --- a/cpukit/rtems/src/taskcreate.c
> +++ b/cpukit/rtems/src/taskcreate.c
> @@ -1,17 +1,36 @@
[...]
>
>
> Is all of this just turning rtems_task_create into a wrapper for the
> task create from config?
Yes.
[...]
> diff --git a/testsuites/sptests/sp01/init.c
> b/testsuites/sptests/sp01/init.c
> index 2719c84fc8..a0a332987d 100644
> --- a/testsuites/sptests/sp01/init.c
> +++ b/testsuites/sptests/sp01/init.c
> @@ -16,6 +16,19 @@
>
> const char rtems_test_name[] = "SP 1";
>
> +RTEMS_TASK_STORAGE_ALIGNMENT static char Task_1_storage[
> + RTEMS_TASK_STORAGE_SIZE( 2 * RTEMS_MINIMUM_STACK_SIZE,
> RTEMS_FLOATING_POINT )
>
>
> This line appears to be too long.
No, it its exactly 79 chars long.
>
> +];
> +
> +static const rtems_task_config Task_1_config = {
> + .name = rtems_build_name( 'T', 'A', '1', ' ' ),
> + .initial_priority = 1,
> + .storage_area = Task_1_storage,
> + .storage_size = sizeof( Task_1_storage ),
> + .initial_modes = RTEMS_DEFAULT_MODES,
> + .attributes = RTEMS_FLOATING_POINT
> +};
> +
> rtems_task Init(
> rtems_task_argument argument
> )
> @@ -30,15 +43,8 @@ rtems_task Init(
> status = rtems_clock_set( &time );
> directive_failed( status, "rtems_clock_set" );
>
> - status = rtems_task_create(
> - rtems_build_name( 'T', 'A', '1', ' ' ),
> - 1,
> - RTEMS_MINIMUM_STACK_SIZE * 2,
> - RTEMS_DEFAULT_MODES,
> - RTEMS_DEFAULT_ATTRIBUTES,
> - &id
> - );
> - directive_failed( status, "rtems_task_create of TA1" );
> + status = rtems_task_create_from_config( &Task_1_config, &id );
> + directive_failed( status, "rtems_task_create_from_config of TA1" );
>
> status = rtems_task_start( id, Task_1_through_3, 1 );
> directive_failed( status, "rtems_task_start of TA1" );
> diff --git a/testsuites/sptests/sp01/sp01.doc
> b/testsuites/sptests/sp01/sp01.doc
> index d7d9f5d902..62bbe956d3 100644
> --- a/testsuites/sptests/sp01/sp01.doc
> +++ b/testsuites/sptests/sp01/sp01.doc
> @@ -9,6 +9,7 @@
> test name: sp01
>
> directives:
> + rtems_task_build
> rtems_task_create
> rtems_task_start
> rtems_task_wake_after
> diff --git a/testsuites/sptests/sp01/system.h
> b/testsuites/sptests/sp01/system.h
> index bde5328aa9..e2047b4d3a 100644
> --- a/testsuites/sptests/sp01/system.h
> +++ b/testsuites/sptests/sp01/system.h
> @@ -28,7 +28,7 @@ rtems_task Task_1_through_3(
>
> #define CONFIGURE_RTEMS_INIT_TASKS_TABLE
>
> -#define CONFIGURE_EXTRA_TASK_STACKS (4 *
> RTEMS_MINIMUM_STACK_SIZE)
> +#define CONFIGURE_EXTRA_TASK_STACKS (2 *
> RTEMS_MINIMUM_STACK_SIZE)
> #define CONFIGURE_MAXIMUM_TASKS 4
>
>
> This seems to be missing lowering the computation for number of
> minimum stacks reserved
>
In v3, I changed it to use CONFIGURE_TASKS_CREATED_FROM_CONFIG.
--
Sebastian Huber, embedded brains GmbH
Address : Dornierstr. 4, D-82178 Puchheim, Germany
Phone : +49 89 189 47 41-16
Fax : +49 89 189 47 41-09
E-Mail : sebastian.huber at embedded-brains.de
PGP : Public key available on request.
Diese Nachricht ist keine geschäftliche Mitteilung im Sinne des EHUG.
More information about the devel
mailing list