<div dir="ltr"><div dir="ltr"><br></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Thu, Nov 5, 2020 at 11:45 AM Sebastian Huber <<a href="mailto:sebastian.huber@embedded-brains.de">sebastian.huber@embedded-brains.de</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">On 04/11/2020 19:38, Gedare Bloom wrote:<br>
<br>
>> Based on the above suggestions I tried to move the User_extensions_Iterator storage to the TCB by adding a new field to the structure, but that did not compile(userextimpl.h is not included with thread.h because of cyclic dependency).<br>
>> This made me try out a naive hack, I defined a structure similar to the User_extensions_Iterator and then added the field to the TCB. The next problem that I faced was during the creation of the idle thread. Since an idle thread is created during system<br>
>> initialization, the 'executing' pointer pointing the TCB is null, and hence referencing to the iterator placed in the TCB for the idle thread fails. This was resolved by separating the cases for an idle thread and other threads. After that I was able to successfully isolate more than two thread stacks.<br>
>> Can you please suggest a more acceptable approach for resolving this issue?<br>
>><br>
> At a high level the approach you took makes sense. You have moved the<br>
> user extensions to the TCB, and then avoid accessing it in case the<br>
> TCB is null (i.e., the initial context switch). I'd need to see the<br>
> code to make any further critical analysis. But it sounds acceptable<br>
> to me.<br>
The executing thread pointer can be NULL and there is already a check <br>
for this in _User_extensions_Iterate(). In this case you can use a <br>
member in the _Per_CPU_Information[].<br>
<br>
-- <br>
Sebastian Huber, embedded brains GmbH<br>
<br>
Address : Dornierstr. 4, D-82178 Puchheim, Germany<br>
Phone : +49 89 189 47 41-16<br>
Fax : +49 89 189 47 41-09<br>
E-Mail : <a href="mailto:sebastian.huber@embedded-brains.de" target="_blank">sebastian.huber@embedded-brains.de</a><br>
PGP : Public key available on request.<br>
<br>
Diese Nachricht ist keine geschäftliche Mitteilung im Sinne des EHUG.<br>
<br></blockquote><div><br></div><div>Based on your comments I have made the following changes in the user extension iterator scheme. Some of the code is still hackish( if-else conditional for handling TCB pointer when it is NULL ) and I would request your suggestions on the same.</div><div><br></div><div>diff --git a/cpukit/include/rtems/score/percpu.h b/cpukit/include/rtems/score/percpu.h<br>index 31bc2b0bff..6e15f4b1ba 100644<br>--- a/cpukit/include/rtems/score/percpu.h<br>+++ b/cpukit/include/rtems/score/percpu.h<br>@@ -25,7 +25,7 @@<br> #include <rtems/asm.h><br> #else<br> #include <rtems/score/assert.h><br>- #include <rtems/score/chain.h><br>+ #include <rtems/score/chainimpl.h><br> #include <rtems/score/isrlock.h><br> #include <rtems/score/smp.h><br> #include <rtems/score/timestamp.h><br>@@ -339,6 +339,20 @@ typedef enum {<br> PER_CPU_WATCHDOG_COUNT<br> } Per_CPU_Watchdog_index;<br> <br>+#if defined ( RTEMS_THREAD_STACK_PROTECTION )<br>+ /**<br>+ * @brief Per CPU user extensions iterator structure<br>+ * <br>+ * This structure is used to refer to the user extensions iterator when<br>+ * thread stack protection is configured.<br>+ */<br>+typedef struct Per_CPU_User_extensions_Iterator {<br>+ Chain_Iterator Iterator;<br>+ struct Per_CPU_User_extensions_Iterator *previous;<br>+} Per_CPU_User_extensions_Iterator;<br>+#endif<br>+<br>+<br> /**<br> * @brief Per CPU Core Structure<br> *<br>@@ -595,6 +609,10 @@ typedef struct Per_CPU_Control {<br> bool boot;<br> #endif<br> <br>+ #if defined (RTEMS_THREAD_STACK_PROTECTION)<br>+ Per_CPU_User_extensions_Iterator iter;<br>+ #endif<br>+<br> struct Record_Control *record;<br> <br> Per_CPU_Stats Stats;<br>diff --git a/cpukit/include/rtems/score/thread.h b/cpukit/include/rtems/score/thread.h<br>index ee0aee5b79..98b85a66af 100644<br>--- a/cpukit/include/rtems/score/thread.h<br>+++ b/cpukit/include/rtems/score/thread.h<br>@@ -636,6 +636,13 @@ struct Thread_Action {<br> Thread_Action_handler handler;<br> };<br> <br>+#if defined (RTEMS_THREAD_STACK_PROTECTION)<br>+ typedef struct Thread_User_extensions_Iterator {<br>+ Chain_Iterator Iterator;<br>+ struct Thread_User_extensions_Iterator *previous;<br>+ } Thread_User_extensions_Iterator;<br>+#endif<br>+<br> /**<br> * @brief Per-thread information for POSIX Keys.<br> */<br>@@ -864,6 +871,10 @@ struct _Thread_Control {<br> */<br> struct _pthread_cleanup_context *last_cleanup_context;<br> <br>+#if defined (RTEMS_THREAD_STACK_PROTECTION)<br>+ Thread_User_extensions_Iterator iter;<br>+#endif<br>+<br> /**<br> * @brief LIFO list of user extensions iterators.<br> */<br>diff --git a/cpukit/score/src/userextiterate.c b/cpukit/score/src/userextiterate.c<br>index 06665a2d7a..814e695018 100644<br>--- a/cpukit/score/src/userextiterate.c<br>+++ b/cpukit/score/src/userextiterate.c<br>@@ -181,22 +181,47 @@ void _User_extensions_Iterate(<br> <br> _User_extensions_Acquire( &lock_context );<br> <br>- _Chain_Iterator_initialize(<br>+ if ( executing != NULL ) {<br>+ executing->iter.previous = executing->last_user_extensions_iterator;<br>+ executing->last_user_extensions_iterator = &executing->iter;<br>+<br>+ _Chain_Iterator_initialize(<br> &_User_extensions_List.Active,<br> &_User_extensions_List.Iterators,<br>- &iter.Iterator,<br>+ &executing->iter.Iterator,<br> direction<br> );<br> <br>- if ( executing != NULL ) {<br>- iter.previous = executing->last_user_extensions_iterator;<br>- executing->last_user_extensions_iterator = &iter;<br>+ while ( ( node = _Chain_Iterator_next( &executing->iter.Iterator ) ) != end ) {<br>+ const User_extensions_Control *extension;<br>+<br>+ _Chain_Iterator_set_position( &executing->iter.Iterator, node );<br>+<br>+ _User_extensions_Release( &lock_context );<br>+<br>+ extension = (const User_extensions_Control *) node;<br>+ ( *visitor )( executing, arg, &extension->Callouts );<br>+<br>+ _User_extensions_Acquire( &lock_context );<br> }<br> <br>- while ( ( node = _Chain_Iterator_next( &iter.Iterator ) ) != end ) {<br>+ executing->last_user_extensions_iterator = executing->iter.previous;<br>+ <br>+ _Chain_Iterator_destroy( &executing->iter.Iterator );<br>+<br>+ } else {<br>+<br>+ _Chain_Iterator_initialize(<br>+ &_User_extensions_List.Active,<br>+ &_User_extensions_List.Iterators,<br>+ &_Per_CPU_Information[0].per_cpu.iter.Iterator,<br>+ direction<br>+ );<br>+<br>+ while ( ( node = _Chain_Iterator_next( &_Per_CPU_Information[0].per_cpu.iter.Iterator ) ) != end ) {<br> const User_extensions_Control *extension;<br> <br>- _Chain_Iterator_set_position( &iter.Iterator, node );<br>+ _Chain_Iterator_set_position( &_Per_CPU_Information[0].per_cpu.iter.Iterator, node );<br> <br> _User_extensions_Release( &lock_context );<br> <br>@@ -206,11 +231,9 @@ void _User_extensions_Iterate(<br> _User_extensions_Acquire( &lock_context );<br> }<br> <br>- if ( executing != NULL ) {<br>- executing->last_user_extensions_iterator = iter.previous;<br>- }<br>+ _Chain_Iterator_destroy( &_Per_CPU_Information->per_cpu.iter.Iterator );<br> <br>- _Chain_Iterator_destroy( &iter.Iterator );<br>+ }<br> <br> _User_extensions_Release( &lock_context );<br></div></div></div>