On 5/27/2014 9:47 AM, Ralf Kirchner wrote: > Enabling and disabling preemption as done for single core will not work for > SMP. > In the bdbuf initialization preemption handling can be avoided in general by > using pthred_once().
I am ok with this in concept but have problems with some implementation details. Are there disable critical section points left? > --- > cpukit/libblock/src/bdbuf.c | 79 > +++++++++++++++++++++++++++---------------- > 1 Datei geändert, 49 Zeilen hinzugefügt(+), 30 Zeilen entfernt(-) > > diff --git a/cpukit/libblock/src/bdbuf.c b/cpukit/libblock/src/bdbuf.c > index 07f479f..1e4887c 100644 > --- a/cpukit/libblock/src/bdbuf.c > +++ b/cpukit/libblock/src/bdbuf.c > @@ -35,6 +35,7 @@ > #include <stdio.h> > #include <string.h> > #include <inttypes.h> > +#include <pthread.h> > > #include <rtems.h> > #include <rtems/error.h> > @@ -137,8 +138,6 @@ typedef struct rtems_bdbuf_cache > rtems_id read_ahead_task; /**< Read-ahead task */ > rtems_chain_control read_ahead_chain; /**< Read-ahead request chain */ > bool read_ahead_enabled; /**< Read-ahead enabled */ > - > - bool initialised; /**< Initialised state. */ > } rtems_bdbuf_cache; > > typedef enum { > @@ -171,6 +170,20 @@ typedef enum { > RTEMS_BDBUF_FATAL_WAIT_TRANS_EVNT > } rtems_bdbuf_fatal_code; > > +typedef struct { > + pthread_once_t is_initialized; > + rtems_status_code return_status; > +} rtems_bdbuf_init_data_type; > + > +#define RTEMS_BDBUF_INIT_DATA_TYPE_INITIALIZER( \ > +) \ > +{ \ > + PTHREAD_ONCE_INIT, \ > + RTEMS_SUCCESSFUL \ > +} > + > +static rtems_bdbuf_init_data_type rtems_bdbuf_data = > RTEMS_BDBUF_INIT_DATA_TYPE_INITIALIZER(); > + > /** > * The events used in this code. These should be system events rather than > * application events. > @@ -1388,10 +1401,9 @@ rtems_bdbuf_read_request_size (uint32_t transfer_count) > /** > * Initialise the cache. > * > - * @return rtems_status_code The initialisation status. > */ > -rtems_status_code > -rtems_bdbuf_init (void) > +static void > +rtems_bdbuf_init_once (void) > { > rtems_bdbuf_group* group; > rtems_bdbuf_buffer* bd; > @@ -1399,41 +1411,30 @@ rtems_bdbuf_init (void) > size_t b; > size_t cache_aligment; > rtems_status_code sc; > - rtems_mode prev_mode; > > if (rtems_bdbuf_tracer) > printf ("bdbuf:init\n"); > > - if (rtems_interrupt_is_in_progress()) > - return RTEMS_CALLED_FROM_ISR; > + if (rtems_interrupt_is_in_progress()) { > + rtems_bdbuf_data.return_status = RTEMS_CALLED_FROM_ISR; > + goto end; > + } > You add a goto to avoid an early out with no work being done on the goto destination. Leave the early out. Applies multiple times. > /* > * Check the configuration table values. > */ > > - if ((bdbuf_config.buffer_max % bdbuf_config.buffer_min) != 0) > - return RTEMS_INVALID_NUMBER; > + if ((bdbuf_config.buffer_max % bdbuf_config.buffer_min) != 0) { > + rtems_bdbuf_data.return_status = RTEMS_INVALID_NUMBER; > + goto end; > + } > > if (rtems_bdbuf_read_request_size (bdbuf_config.max_read_ahead_blocks) > - > RTEMS_MINIMUM_STACK_SIZE / 8U) > - return RTEMS_INVALID_NUMBER; > - > - /* > - * We use a special variable to manage the initialisation incase we have > - * completing threads doing this. You may get errors if the another thread > - * makes a call and we have not finished initialisation. > - */ > - prev_mode = rtems_bdbuf_disable_preemption (); > - if (bdbuf_cache.initialised) > - { > - rtems_bdbuf_restore_preemption (prev_mode); > - return RTEMS_RESOURCE_IN_USE; > + > RTEMS_MINIMUM_STACK_SIZE / 8U) { > + rtems_bdbuf_data.return_status = RTEMS_INVALID_NUMBER; > + goto end; > } > > - memset(&bdbuf_cache, 0, sizeof(bdbuf_cache)); > - bdbuf_cache.initialised = true; > - rtems_bdbuf_restore_preemption (prev_mode); > - > /* > * For unspecified cache alignments we use the CPU alignment. > */ > @@ -1605,7 +1606,8 @@ rtems_bdbuf_init (void) > > rtems_bdbuf_unlock_cache (); > > - return RTEMS_SUCCESSFUL; > + rtems_bdbuf_data.return_status = RTEMS_SUCCESSFUL; > + goto end; > > error: > > @@ -1650,9 +1652,26 @@ error: > rtems_semaphore_delete (bdbuf_cache.lock); > } > > - bdbuf_cache.initialised = false; > + rtems_bdbuf_data.return_status = RTEMS_UNSATISFIED; > + > +end:; > +} > > - return RTEMS_UNSATISFIED; > +/** > + * Initialise the cache. > + * > + * @return rtems_status_code The initialisation status. > + */ > +rtems_status_code > +rtems_bdbuf_init (void) > +{ > + int eno = pthread_once (&rtems_bdbuf_data.is_initialized, > rtems_bdbuf_init_once); > + > + if (eno != 0) { > + return RTEMS_UNSATISFIED; > + } else { > + return rtems_bdbuf_data.return_status; > + } > } And then you code having early outs. This is inconsistent. > > static void -- Joel Sherrill, Ph.D. Director of Research & Development joel.sherr...@oarcorp.com On-Line Applications Research Ask me about RTEMS: a free RTOS Huntsville AL 35805 Support Available (256) 722-9985 _______________________________________________ rtems-devel mailing list rtems-devel@rtems.org http://www.rtems.org/mailman/listinfo/rtems-devel