[PATCH 2/2] privateenv: Use POSIX keys instead of task variables.
Gedare Bloom
gedare at rtems.org
Wed Mar 26 15:56:49 UTC 2014
On Wed, Mar 26, 2014 at 11:43 AM, Christian Mauderer
<Christian.Mauderer at embedded-brains.de> wrote:
>
>
> Am 26.03.2014 16:23, schrieb Gedare Bloom:
>> On Wed, Mar 26, 2014 at 8:29 AM, Christian Mauderer
>> <christian.mauderer at embedded-brains.de> wrote:
>>> From: Christian Mauderer <Christian.Mauderer at embedded-brains.de>
>>>
>>> ---
>>> cpukit/include/rtems/userenv.h | 20 +++++++++++---
>>> cpukit/libcsupport/include/rtems/libio_.h | 2 ++
>>> cpukit/libcsupport/src/__usrenv.c | 2 +-
>>> cpukit/libcsupport/src/libio_init.c | 12 +++++++++
>>> cpukit/libcsupport/src/privateenv.c | 37 +++++++++++++-------------
>>> cpukit/sapi/include/confdefs.h | 29 ++++++++++++--------
>>> doc/shell/file.t | 4 ++-
>>> testsuites/fstests/imfs_support/fs_support.c | 1 +
>>> testsuites/fstests/jffs2_support/fs_support.c | 2 ++
>>> testsuites/fstests/mdosfs_support/fs_support.c | 1 +
>>> testsuites/fstests/mimfs_support/fs_support.c | 1 +
>>> testsuites/fstests/mrfs_support/fs_support.c | 1 +
>>> testsuites/libtests/ftp01/init.c | 2 ++
>>> testsuites/psxtests/psxchroot01/main.c | 2 ++
>>> testsuites/psxtests/psxmount/main.c | 2 ++
>>> testsuites/sptests/Makefile.am | 2 +-
>>> testsuites/sptests/configure.ac | 1 +
>>> testsuites/sptests/spfatal27/Makefile.am | 21 +++++++++++++++
>>> testsuites/sptests/spfatal27/spfatal27.doc | 19 +++++++++++++
>>> testsuites/sptests/spfatal27/spfatal27.scn | 4 +++
>>> testsuites/sptests/spfatal27/testcase.h | 27 +++++++++++++++++++
>>> testsuites/sptests/spprivenv01/init.c | 14 +++++++---
>>> 22 files changed, 168 insertions(+), 38 deletions(-)
>>> create mode 100644 testsuites/sptests/spfatal27/Makefile.am
>>> create mode 100644 testsuites/sptests/spfatal27/spfatal27.doc
>>> create mode 100644 testsuites/sptests/spfatal27/spfatal27.scn
>>> create mode 100644 testsuites/sptests/spfatal27/testcase.h
>>>
>>> diff --git a/cpukit/include/rtems/userenv.h b/cpukit/include/rtems/userenv.h
>>> index 8a9a4fc..576636e 100644
>>> --- a/cpukit/include/rtems/userenv.h
>>> +++ b/cpukit/include/rtems/userenv.h
>>> @@ -27,6 +27,8 @@
>>> */
>>> #include <limits.h>
>>>
>>> +#include <pthread.h>
>>> +
>>> #include <rtems.h>
>>> #include <rtems/fs.h>
>>>
>>> @@ -66,8 +68,18 @@ typedef struct {
>>> pid_t pgrp; /* process group id */
>>> } rtems_user_env_t;
>>>
>>> -extern rtems_user_env_t * rtems_current_user_env;
>>> -extern rtems_user_env_t rtems_global_user_env;
>>> +extern rtems_user_env_t rtems_global_user_env;
>>> +extern pthread_key_t rtems_current_user_env_key;
>>> +
>>> +/**
>>> + * @brief Fetch the pointer to the current user environment.
>>> + *
>>> + * If the task has a private user environment the pointer to it will be
>>> + * returned. Otherwise the pointer to rtems_global_user_env will be returned.
>>> + */
>>> +rtems_user_env_t * rtems_current_user_env_get(void);
>>> +
>>> +#define rtems_current_user_env rtems_current_user_env_get()
>>>
>>> #define rtems_filesystem_current (rtems_current_user_env->current_directory)
>>> #define rtems_filesystem_root (rtems_current_user_env->root_directory)
>>> @@ -84,7 +96,9 @@ extern rtems_user_env_t rtems_global_user_env;
>>> *
>>> * If the task has already a private environment nothing will be changed. This
>>> * function must be called from normal thread context and may block on a mutex.
>>> - * Thread dispatching is disabled to protect some critical sections.
>>> + * Thread dispatching is disabled to protect some critical sections. Please note
>>> + * that a POSIX key value pair has to be configured for each private
>>> + * environment.
>>> *
>>> * @retval RTEMS_SUCCESSFUL Successful operation.
>>> * @retval RTEMS_NO_MEMORY Not enough memory.
>>> diff --git a/cpukit/libcsupport/include/rtems/libio_.h b/cpukit/libcsupport/include/rtems/libio_.h
>>> index 9960288..0347c5b 100644
>>> --- a/cpukit/libcsupport/include/rtems/libio_.h
>>> +++ b/cpukit/libcsupport/include/rtems/libio_.h
>>> @@ -238,6 +238,8 @@ void rtems_filesystem_location_free( rtems_filesystem_location_info_t *loc );
>>> */
>>> #include <rtems/userenv.h>
>>>
>>> +void rtems_libio_free_user_env( void *env );
>>> +
>>> static inline void rtems_libio_lock( void )
>>> {
>>> rtems_semaphore_obtain( rtems_libio_semaphore, RTEMS_WAIT, RTEMS_NO_TIMEOUT );
>>> diff --git a/cpukit/libcsupport/src/__usrenv.c b/cpukit/libcsupport/src/__usrenv.c
>>> index 3ce6a2d..71efda9 100644
>>> --- a/cpukit/libcsupport/src/__usrenv.c
>>> +++ b/cpukit/libcsupport/src/__usrenv.c
>>> @@ -257,4 +257,4 @@ rtems_user_env_t rtems_global_user_env = {
>>> .umask = S_IWGRP | S_IWOTH
>>> };
>>>
>>> -rtems_user_env_t *rtems_current_user_env = &rtems_global_user_env;
>>> +pthread_key_t rtems_current_user_env_key;
>>> diff --git a/cpukit/libcsupport/src/libio_init.c b/cpukit/libcsupport/src/libio_init.c
>>> index 4d705fb..e64ddd6 100644
>>> --- a/cpukit/libcsupport/src/libio_init.c
>>> +++ b/cpukit/libcsupport/src/libio_init.c
>>> @@ -46,6 +46,7 @@ void rtems_libio_init( void )
>>> rtems_status_code rc;
>>> uint32_t i;
>>> rtems_libio_t *iop;
>>> + int eno;
>>>
>>> if (rtems_libio_number_iops > 0)
>>> {
>>> @@ -61,6 +62,17 @@ void rtems_libio_init( void )
>>> }
>>>
>>> /*
>>> + * Create the posix key for user environment.
>>> + */
>>> + eno = pthread_key_create(
>>> + &rtems_current_user_env_key,
>>> + rtems_libio_free_user_env
>>> + );
>>> + if (eno != 0) {
>>> + rtems_fatal_error_occurred( RTEMS_UNSATISFIED );
>>> + }
>>> +
>>> + /*
>>> * Create the binary semaphore used to provide mutual exclusion
>>> * on the IOP Table.
>>> */
>>> diff --git a/cpukit/libcsupport/src/privateenv.c b/cpukit/libcsupport/src/privateenv.c
>>> index bee94c1..60207b0 100644
>>> --- a/cpukit/libcsupport/src/privateenv.c
>>> +++ b/cpukit/libcsupport/src/privateenv.c
>>> @@ -29,7 +29,16 @@
>>> * Instantiate a private user environment for the calling thread.
>>> */
>>>
>>> -static void free_user_env(void *arg)
>>> +rtems_user_env_t * rtems_current_user_env_get(void)
>>> +{
>>> + void *ptr = pthread_getspecific(rtems_current_user_env_key);
>>> + if (ptr == NULL) {
>>> + ptr = &rtems_global_user_env;
>>> + }
>>> + return (rtems_user_env_t *) ptr;
>>> +}
>>> +
>>> +void rtems_libio_free_user_env(void *arg)
>>> {
>>> rtems_user_env_t *env = arg;
>>> bool uses_global_env = env == &rtems_global_user_env;
>>> @@ -44,7 +53,7 @@ static void free_user_env(void *arg)
>>> static void free_user_env_protected(rtems_user_env_t *env)
>>> {
>>> _Thread_Disable_dispatch();
>>> - free_user_env(env);
>>> + rtems_libio_free_user_env(env);
>>> _Thread_Enable_dispatch();
>>> }
>>>
>>> @@ -68,14 +77,13 @@ rtems_status_code rtems_libio_set_private_env(void)
>>> !rtems_filesystem_global_location_is_null(new_env->root_directory)
>>> && !rtems_filesystem_global_location_is_null(new_env->current_directory)
>>> ) {
>>> - sc = rtems_task_variable_add(
>>> - RTEMS_SELF,
>>> - (void **) &rtems_current_user_env,
>>> - free_user_env
>>> + int eno = pthread_setspecific(
>>> + rtems_current_user_env_key,
>>> + new_env
>>> );
>>> - if (sc == RTEMS_SUCCESSFUL) {
>>> +
>>> + if (eno == 0) {
>>> free_user_env_protected(old_env);
>>> - rtems_current_user_env = new_env;
>>> } else {
>>> sc = RTEMS_TOO_MANY;
>>> }
>>> @@ -84,7 +92,7 @@ rtems_status_code rtems_libio_set_private_env(void)
>>> }
>>>
>>> if (sc != RTEMS_SUCCESSFUL) {
>>> - free_user_env(new_env);
>>> + rtems_libio_free_user_env(new_env);
>>> }
>>> } else {
>>> sc = RTEMS_NO_MEMORY;
>>> @@ -101,14 +109,7 @@ void rtems_libio_use_global_env(void)
>>> bool uses_private_env = env != &rtems_global_user_env;
>>>
>>> if (uses_private_env) {
>>> - sc = rtems_task_variable_delete(
>>> - RTEMS_SELF,
>>> - (void **) &rtems_current_user_env
>>> - );
>>> - if (sc != RTEMS_SUCCESSFUL) {
>>> - rtems_fatal_error_occurred(0xdeadbeef);
>>> - }
>>> -
>>> - rtems_current_user_env = &rtems_global_user_env;
>>> + free_user_env_protected(env);
>>> + pthread_setspecific(rtems_current_user_env_key, NULL);
>>> }
>>> }
>>> diff --git a/cpukit/sapi/include/confdefs.h b/cpukit/sapi/include/confdefs.h
>>> index aca99f0..5da3c2e 100644
>>> --- a/cpukit/sapi/include/confdefs.h
>>> +++ b/cpukit/sapi/include/confdefs.h
>>> @@ -148,6 +148,11 @@ const rtems_libio_helper rtems_fs_init_helper =
>>> */
>>> #define CONFIGURE_LIBIO_SEMAPHORES 1
>>>
>>> +/**
>>> + * POSIX key count used by the IO library.
>>> + */
>>> +#define CONFIGURE_LIBIO_POSIX_KEYS 1
>>> +
>>> #ifdef CONFIGURE_INIT
>>> /**
>>> * When instantiating the configuration tables, this variable is
>>> @@ -1728,16 +1733,18 @@ const rtems_libio_helper rtems_fs_init_helper =
>>> #include <rtems/posix/key.h>
>>>
>>> #ifndef CONFIGURE_MAXIMUM_POSIX_KEYS
>>> - #define CONFIGURE_MAXIMUM_POSIX_KEYS 0
>>> - #define CONFIGURE_MAXIMUM_POSIX_KEY_VALUE_PAIRS 0
>>> -#else
>>> - #ifndef CONFIGURE_MAXIMUM_POSIX_KEY_VALUE_PAIRS
>>> - #define CONFIGURE_MAXIMUM_POSIX_KEY_VALUE_PAIRS \
>>> - (CONFIGURE_MAXIMUM_POSIX_KEYS * \
>>> - (CONFIGURE_MAXIMUM_POSIX_THREADS + CONFIGURE_MAXIMUM_TASKS))
>>> - #endif
>>> + #define CONFIGURE_MAXIMUM_POSIX_KEYS 0
>>> +#endif
>>> +
>>> +#ifndef CONFIGURE_MAXIMUM_POSIX_KEY_VALUE_PAIRS
>>> + #define CONFIGURE_MAXIMUM_POSIX_KEY_VALUE_PAIRS \
>>> + (CONFIGURE_MAXIMUM_POSIX_KEYS * \
>>> + (CONFIGURE_MAXIMUM_POSIX_THREADS + CONFIGURE_MAXIMUM_TASKS))
>>> #endif
>>>
>>> +#define CONFIGURE_POSIX_KEYS \
>>> + (CONFIGURE_MAXIMUM_POSIX_KEYS + CONFIGURE_LIBIO_POSIX_KEYS)
>>> +
>> If you add another macro here like...
>> #define CONFIGURE_POSIX_KEY_VALUE_PAIRS
>> (CONFIGURE_MAXIMUM_POSIX_KEY_VALUE_PAIRS + (1 *
>> CONFIGURE_LIBIO_POSIX_KEYS))
>> and change the below to use the new macro, then the user does not need
>> to do any configuring of the posix keys for the private environment.
>>
> Thanks for the hint. I have thought about that and decided against it
> with the following reason:
>
> One key value pair would most likely not be the correct number of key
> value pairs. We can distinguish between three cases:
>
> One case would be that no private environment is used. In that case we
> would need no key and only waste resources.
>
> The second case would be that one thread uses a private environment. In
> that case the number would be correct.
>
> The last case is that multiple threads are using private environments.
> Then we would need multiple key value pairs (one per thread).
>
> At the moment the main use for a private environment seems to be the
> chroot function. If a program never uses this function it will most
> likely never use a private environment. Therefore I would prefer not to
> add this key value pair in default configuration.
>
OK. I think it would be good to write this down somewhere for future
reference. possibly in the same spot you documented the note about
needing to configure the system with the key-value.
-Gedare
More information about the devel
mailing list