On Thu, Nov 5, 2020 at 11:45 AM Sebastian Huber < sebastian.hu...@embedded-brains.de> wrote:
> On 04/11/2020 19:38, Gedare Bloom wrote: > > >> 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). > >> 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 > >> 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. > >> Can you please suggest a more acceptable approach for resolving this > issue? > >> > > At a high level the approach you took makes sense. You have moved the > > user extensions to the TCB, and then avoid accessing it in case the > > TCB is null (i.e., the initial context switch). I'd need to see the > > code to make any further critical analysis. But it sounds acceptable > > to me. > The executing thread pointer can be NULL and there is already a check > for this in _User_extensions_Iterate(). In this case you can use a > member in the _Per_CPU_Information[]. > > -- > 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. > > 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. diff --git a/cpukit/include/rtems/score/percpu.h b/cpukit/include/rtems/score/percpu.h index 31bc2b0bff..6e15f4b1ba 100644 --- a/cpukit/include/rtems/score/percpu.h +++ b/cpukit/include/rtems/score/percpu.h @@ -25,7 +25,7 @@ #include <rtems/asm.h> #else #include <rtems/score/assert.h> - #include <rtems/score/chain.h> + #include <rtems/score/chainimpl.h> #include <rtems/score/isrlock.h> #include <rtems/score/smp.h> #include <rtems/score/timestamp.h> @@ -339,6 +339,20 @@ typedef enum { PER_CPU_WATCHDOG_COUNT } Per_CPU_Watchdog_index; +#if defined ( RTEMS_THREAD_STACK_PROTECTION ) + /** + * @brief Per CPU user extensions iterator structure + * + * This structure is used to refer to the user extensions iterator when + * thread stack protection is configured. + */ +typedef struct Per_CPU_User_extensions_Iterator { + Chain_Iterator Iterator; + struct Per_CPU_User_extensions_Iterator *previous; +} Per_CPU_User_extensions_Iterator; +#endif + + /** * @brief Per CPU Core Structure * @@ -595,6 +609,10 @@ typedef struct Per_CPU_Control { bool boot; #endif + #if defined (RTEMS_THREAD_STACK_PROTECTION) + Per_CPU_User_extensions_Iterator iter; + #endif + struct Record_Control *record; Per_CPU_Stats Stats; diff --git a/cpukit/include/rtems/score/thread.h b/cpukit/include/rtems/score/thread.h index ee0aee5b79..98b85a66af 100644 --- a/cpukit/include/rtems/score/thread.h +++ b/cpukit/include/rtems/score/thread.h @@ -636,6 +636,13 @@ struct Thread_Action { Thread_Action_handler handler; }; +#if defined (RTEMS_THREAD_STACK_PROTECTION) + typedef struct Thread_User_extensions_Iterator { + Chain_Iterator Iterator; + struct Thread_User_extensions_Iterator *previous; + } Thread_User_extensions_Iterator; +#endif + /** * @brief Per-thread information for POSIX Keys. */ @@ -864,6 +871,10 @@ struct _Thread_Control { */ struct _pthread_cleanup_context *last_cleanup_context; +#if defined (RTEMS_THREAD_STACK_PROTECTION) + Thread_User_extensions_Iterator iter; +#endif + /** * @brief LIFO list of user extensions iterators. */ diff --git a/cpukit/score/src/userextiterate.c b/cpukit/score/src/userextiterate.c index 06665a2d7a..814e695018 100644 --- a/cpukit/score/src/userextiterate.c +++ b/cpukit/score/src/userextiterate.c @@ -181,22 +181,47 @@ void _User_extensions_Iterate( _User_extensions_Acquire( &lock_context ); - _Chain_Iterator_initialize( + if ( executing != NULL ) { + executing->iter.previous = executing->last_user_extensions_iterator; + executing->last_user_extensions_iterator = &executing->iter; + + _Chain_Iterator_initialize( &_User_extensions_List.Active, &_User_extensions_List.Iterators, - &iter.Iterator, + &executing->iter.Iterator, direction ); - if ( executing != NULL ) { - iter.previous = executing->last_user_extensions_iterator; - executing->last_user_extensions_iterator = &iter; + while ( ( node = _Chain_Iterator_next( &executing->iter.Iterator ) ) != end ) { + const User_extensions_Control *extension; + + _Chain_Iterator_set_position( &executing->iter.Iterator, node ); + + _User_extensions_Release( &lock_context ); + + extension = (const User_extensions_Control *) node; + ( *visitor )( executing, arg, &extension->Callouts ); + + _User_extensions_Acquire( &lock_context ); } - while ( ( node = _Chain_Iterator_next( &iter.Iterator ) ) != end ) { + executing->last_user_extensions_iterator = executing->iter.previous; + + _Chain_Iterator_destroy( &executing->iter.Iterator ); + + } else { + + _Chain_Iterator_initialize( + &_User_extensions_List.Active, + &_User_extensions_List.Iterators, + &_Per_CPU_Information[0].per_cpu.iter.Iterator, + direction + ); + + while ( ( node = _Chain_Iterator_next( &_Per_CPU_Information[0].per_cpu.iter.Iterator ) ) != end ) { const User_extensions_Control *extension; - _Chain_Iterator_set_position( &iter.Iterator, node ); + _Chain_Iterator_set_position( &_Per_CPU_Information[0].per_cpu.iter.Iterator, node ); _User_extensions_Release( &lock_context ); @@ -206,11 +231,9 @@ void _User_extensions_Iterate( _User_extensions_Acquire( &lock_context ); } - if ( executing != NULL ) { - executing->last_user_extensions_iterator = iter.previous; - } + _Chain_Iterator_destroy( &_Per_CPU_Information->per_cpu.iter.Iterator ); - _Chain_Iterator_destroy( &iter.Iterator ); + } _User_extensions_Release( &lock_context );
_______________________________________________ devel mailing list devel@rtems.org http://lists.rtems.org/mailman/listinfo/devel