On 09/09/2020 00:43, Joel Sherrill wrote:
On Wed, Sep 2, 2020 at 11:09 AM Sebastian Huber
<sebastian.hu...@embedded-brains.de
<mailto:sebastian.hu...@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.hu...@embedded-brains.de
PGP : Public key available on request.
Diese Nachricht ist keine geschäftliche Mitteilung im Sinne des EHUG.
_______________________________________________
devel mailing list
devel@rtems.org
http://lists.rtems.org/mailman/listinfo/devel
