[PATCH 1/1] rtems: Add rtems_interrupt_server_build()

Sebastian Huber sebastian.huber at embedded-brains.de
Fri Jul 31 15:59:24 UTC 2020


On 31/07/2020 17:27, Gedare Bloom wrote:

> reviewing for feature on 5/
>
> On Fri, Jul 31, 2020 at 5:43 AM Sebastian Huber
> <sebastian.huber at embedded-brains.de> wrote:
>> Add rtems_interrupt_server_destroy().
>>
>> Before this patch, the only way to create interrupt servers was
>> rtems_interrupt_server_initialize(). This function creates the default
>> interrupt server and in SMP configurations additional interrupt servers
>> for the additional processors. The interrupt server is heavily used by
>> libbsd. This includes the epoch based reclamation which performs time
>> consuming resource and memory deallocation work. This does not work well
>> with time critical services, for example an UART over SPI or I2C. One
>> approach to address this problem is to allow the application to create
>> custom interrupt servers with the right priority and task properties.
>> The interrupt server API accounted for this, however, it was not
>> implemented before this patch.
>>
>> Close #4033.
>> ---
>>   bsps/shared/irq/irq-server.c         | 360 ++++++++++++++++++---------
>>   cpukit/include/rtems/irq-extension.h | 124 +++++++--
>>   2 files changed, 355 insertions(+), 129 deletions(-)
>>
>> diff --git a/bsps/shared/irq/irq-server.c b/bsps/shared/irq/irq-server.c
>> index 93e2d144d8..aa8b8aa186 100644
>> --- a/bsps/shared/irq/irq-server.c
>> +++ b/bsps/shared/irq/irq-server.c
>> @@ -7,13 +7,7 @@
>>    */
>>
>>   /*
>> - * Copyright (c) 2009, 2019 embedded brains GmbH.  All rights reserved.
>> - *
>> - *  embedded brains GmbH
>> - *  Dornierstr. 4
>> - *  82178 Puchheim
>> - *  Germany
>> - *  <rtems at embedded-brains.de>
>> + * Copyright (C) 2009, 2020 embedded brains GmbH (http://www.embedded-brains.de)
>>    *
>>    * The license and distribution terms for this file may be
>>    * found in the file LICENSE in this distribution or at
>> @@ -21,6 +15,7 @@
>>    */
>>
>>   #include <stdlib.h>
>> +#include <string.h>
>>
>>   #include <rtems.h>
>>   #include <rtems/chain.h>
>> @@ -30,54 +25,43 @@
>>
>>   #define BSP_INTERRUPT_SERVER_MANAGEMENT_VECTOR (BSP_INTERRUPT_VECTOR_MAX + 1)
>>
>> -typedef struct {
>> -  RTEMS_INTERRUPT_LOCK_MEMBER(lock);
>> -  rtems_chain_control entries;
>> -  rtems_id server;
>> -  unsigned errors;
>> -} bsp_interrupt_server_context;
>> +static rtems_interrupt_server_control bsp_interrupt_server_default;
>>
>> -#if defined(RTEMS_SMP)
>> -static bsp_interrupt_server_context *bsp_interrupt_server_instances;
>> -#else
>> -static bsp_interrupt_server_context bsp_interrupt_server_instance;
>> -#endif
>> +static rtems_chain_control bsp_interrupt_server_chain =
>> +  RTEMS_CHAIN_INITIALIZER_EMPTY(bsp_interrupt_server_chain);
>>
>> -static bsp_interrupt_server_context *bsp_interrupt_server_get_context(
>> +static rtems_interrupt_server_control *bsp_interrupt_server_get_context(
>>     uint32_t server_index,
>>     rtems_status_code *sc
>>   )
>>   {
>> -#if defined(RTEMS_SMP)
>> -  if (bsp_interrupt_server_instances == NULL) {
>> -    *sc = RTEMS_INCORRECT_STATE;
>> -    return NULL;
>> -  }
>> -#else
>> -  if (bsp_interrupt_server_instance.server == RTEMS_ID_NONE) {
>> -    *sc = RTEMS_INCORRECT_STATE;
>> -    return NULL;
>> -  }
>> -#endif
>> +  rtems_chain_node *node;
>> +
>> +  bsp_interrupt_lock();
>> +  node = rtems_chain_first(&bsp_interrupt_server_chain);
>> +
>> +  while (node != rtems_chain_tail(&bsp_interrupt_server_chain)) {
>> +    rtems_interrupt_server_control *s;
>>
>> -  if (server_index >= rtems_scheduler_get_processor_maximum()) {
>> -    *sc = RTEMS_INVALID_ID;
>> -    return NULL;
>> +    s = RTEMS_CONTAINER_OF(node, rtems_interrupt_server_control, node);
>> +    if (s->index == server_index) {
>> +      bsp_interrupt_unlock();
> set sc = RTEMS_SUCCESSFUL?
Not needed, if s != NULL the status is unused.
>
>> +      return s;
>> +    }
>> +
>> +    node = rtems_chain_next(node);
>>     }
>>
>> -  *sc = RTEMS_SUCCESSFUL;
>> -#if defined(RTEMS_SMP)
>> -  return &bsp_interrupt_server_instances[server_index];
>> -#else
>> -  return &bsp_interrupt_server_instance;
>> -#endif
>> +  bsp_interrupt_unlock();
>> +  *sc = RTEMS_INVALID_ID;
>> +  return NULL;
>>   }
>>
>>   static void bsp_interrupt_server_trigger(void *arg)
>>   {
>>     rtems_interrupt_lock_context lock_context;
>>     rtems_interrupt_server_entry *e = arg;
>> -  bsp_interrupt_server_context *s = e->server;
>> +  rtems_interrupt_server_control *s = e->server;
>>
>>     if (bsp_interrupt_is_valid_vector(e->vector)) {
>>       bsp_interrupt_vector_disable(e->vector);
>> @@ -137,7 +121,7 @@ static rtems_interrupt_server_entry *bsp_interrupt_server_query_entry(
>>   }
>>
>>   typedef struct {
>> -  bsp_interrupt_server_context *server;
>> +  rtems_interrupt_server_control *server;
>>     rtems_vector_number vector;
>>     rtems_option options;
>>     rtems_interrupt_handler handler;
>> @@ -281,7 +265,7 @@ static void bsp_interrupt_server_remove_helper(void *arg)
>>   }
>>
>>   static rtems_status_code bsp_interrupt_server_call_helper(
>> -  bsp_interrupt_server_context *s,
>> +  rtems_interrupt_server_control *s,
>>     rtems_vector_number vector,
>>     rtems_option options,
>>     rtems_interrupt_handler handler,
>> @@ -314,7 +298,7 @@ static rtems_status_code bsp_interrupt_server_call_helper(
>>   }
>>
>>   static rtems_interrupt_server_entry *bsp_interrupt_server_get_entry(
>> -  bsp_interrupt_server_context *s
>> +  rtems_interrupt_server_control *s
>>   )
>>   {
>>     rtems_interrupt_lock_context lock_context;
>> @@ -337,7 +321,7 @@ static rtems_interrupt_server_entry *bsp_interrupt_server_get_entry(
>>
>>   static void bsp_interrupt_server_task(rtems_task_argument arg)
>>   {
>> -  bsp_interrupt_server_context *s = (bsp_interrupt_server_context *) arg;
>> +  rtems_interrupt_server_control *s = (rtems_interrupt_server_control *) arg;
>>
>>     while (true) {
>>       rtems_event_set events;
>> @@ -377,7 +361,7 @@ rtems_status_code rtems_interrupt_server_handler_install(
>>   )
>>   {
>>     rtems_status_code sc;
>> -  bsp_interrupt_server_context *s;
>> +  rtems_interrupt_server_control *s;
>>
>>     s = bsp_interrupt_server_get_context(server_index, &sc);
>>     if (s == NULL) {
>> @@ -402,7 +386,7 @@ rtems_status_code rtems_interrupt_server_handler_remove(
>>   )
>>   {
>>     rtems_status_code sc;
>> -  bsp_interrupt_server_context *s;
>> +  rtems_interrupt_server_control *s;
>>
>>     s = bsp_interrupt_server_get_context(server_index, &sc);
>>     if (s == NULL) {
>> @@ -464,7 +448,7 @@ rtems_status_code rtems_interrupt_server_handler_iterate(
>>   {
>>     rtems_status_code sc;
>>     bsp_interrupt_server_handler_iterate_helper_data hihd;
>> -  bsp_interrupt_server_context *s;
>> +  rtems_interrupt_server_control *s;
>>
>>     s = bsp_interrupt_server_get_context(server_index, &sc);
>>     if (s == NULL) {
>> @@ -487,103 +471,249 @@ rtems_status_code rtems_interrupt_server_handler_iterate(
>>     );
>>   }
>>
>> -rtems_status_code rtems_interrupt_server_initialize(
>> +static void bsp_interrupt_server_destroy_memset(
>> +  rtems_interrupt_server_control *s
>> +)
>> +{
>> +  memset(s, 0, sizeof(*s));
> should this also free?
No, the default server control is statically allocated. I will add a 
comment to explain this.
>
>> +}
>> +
>> +#if defined(RTEMS_SMP)
>> +static void bsp_interrupt_server_destroy_free(
>> +  rtems_interrupt_server_control *s
>> +)
>> +{
>> +  free(s);
>> +}
>> +#endif
>> +
>> +static rtems_status_code bsp_interrupt_server_create(
>> +  rtems_interrupt_server_control *s,
>>     rtems_task_priority priority,
>>     size_t stack_size,
>>     rtems_mode modes,
>>     rtems_attribute attributes,
>> -  uint32_t *server_count
>> +  uint32_t cpu_index
>>   )
>>   {
>> -  uint32_t cpu_index;
>> -  uint32_t cpu_count;
>> -  uint32_t dummy;
>> -  bsp_interrupt_server_context *instances;
>> +  rtems_status_code sc;
>> +#if defined(RTEMS_SMP)
>> +  rtems_id scheduler;
>> +  cpu_set_t cpu;
>> +#endif
>>
>> -  if (server_count == NULL) {
>> -    server_count = &dummy;
>> -  }
>> +  rtems_interrupt_lock_initialize(&s->lock, "Interrupt Server");
>> +  rtems_chain_initialize_empty(&s->entries);
>>
>> -  cpu_count = rtems_scheduler_get_processor_maximum();
>> +  sc = rtems_task_create(
>> +    rtems_build_name('I', 'R', 'Q', 'S'),
> this name needs to be documented as "reserved" in case the irq server is used
I will add this to the rtems_interrupt_server_initialize() 
documentation. I added the task name to the rtems_interrupt_server_config.
>
>> [...]
>> +/**
>> + * @brief Builds an interrupt server.
>> + *
>> + * This function may block.
>> + *
>> + * @param[out] control is the interrupt server control.  The ownership of this
>> + *   structure is transferred from the caller of this function to the interrupt
>> + *   server management.
>> + *
>> + * @param config is the interrupt server configuration.
>> + *
>> + * @param[out] server_index is the pointer to a server index variable.  The
>> + *   index of the built interrupt server will be stored in the referenced
>> + *   variable if the operation was successful.
>> + *
>> + * @retval RTEMS_SUCCESSFUL The operation was successful.
>> + *
>> + * @return The function uses rtems_task_create().  If this operation is not
>> + *   successful, then its status code is returned.
>> + *
>> + * @see rtems_interrupt_server_initialize() and
>> + *   rtems_interrupt_server_destroy().
>> + */
>> +rtems_status_code rtems_interrupt_server_build(
> name change to create is fine, update the @brief also.
Oh, yes.
>
>> +  rtems_interrupt_server_control      *control,
>> +  const rtems_interrupt_server_config *config,
>> +  uint32_t                            *server_index
>> +);
>> +
>> +/**
>> + * @brief Destroys an interrupt server.
>> + *
>> + * This function may block.
>> + *
>> + * The interrupt server deletes itself, so after the return of the function the
>> + * interrupt server may be still in the termination process depending on the
>> + * task priorities of the system.
>> + *
>> + * @param server_index is the index of the interrupt server to destroy.  Use
>> + *   ::RTEMS_INTERRUPT_SERVER_DEFAULT to specify the default server.
>> + *
>> + * @retval RTEMS_SUCCESSFUL The operation was successful.
>> + * @retval RTEMS_INVALID_ID The interrupt server index was invalid.
>> + *
>> + * @see rtems_interrupt_server_build()
>> + */
>> +rtems_status_code rtems_interrupt_server_destroy( uint32_t server_index );
>> +
>>   /**
>>    * @brief Installs the interrupt handler routine @a handler for the interrupt
>>    * vector with number @a vector on the server @a server.
> The rtems_interrupt_server* should get user-facing documentation
> (eventually). Please open a ticket for adding it.

https://devel.rtems.org/ticket/3269

It is on my TODO list to document it in the RTEMS Classic API Guide.



More information about the devel mailing list