From 4cb61bd8c2877532840b3b55a604c1e6fb94257b Mon Sep 17 00:00:00 2001 From: Gregory Nutt Date: Mon, 10 Sep 2018 06:29:51 -0600 Subject: [PATCH] sched/wqueue: Notifier design cleanup. The original concept used pre-allocated notification data structures. However, the notification dat must persist for an indeterminate amount of time. So the design was modified to use dynamically allocted data structures. This commit simplifies the design by removed some residual 'machinery' that is no longer needed. --- sched/Kconfig | 11 --- sched/init/os_start.c | 6 -- sched/wqueue/Make.defs | 3 +- sched/wqueue/kwork_initialize.c | 73 --------------- sched/wqueue/kwork_notifier.c | 153 ++++++-------------------------- sched/wqueue/wqueue.h | 16 ---- 6 files changed, 26 insertions(+), 236 deletions(-) delete mode 100644 sched/wqueue/kwork_initialize.c diff --git a/sched/Kconfig b/sched/Kconfig index 7c94c2a130..2a7807eca1 100644 --- a/sched/Kconfig +++ b/sched/Kconfig @@ -1469,17 +1469,6 @@ config WQUEUE_NOTIFIER notifier, but was developed specifically to support poll() logic where the poll must wait for an resources to become available. -config WQUEUE_NOTIFIER_NWAITERS - int "Number of notifications" - default 16 - range 1 32766 - depends on WQUEUE_NOTIFIER - ---help--- - If CONFIG_WQUEUE_NOTIFIER is selected, then a pre-allocated pool of - notification structures will be used to hold information about - pending notifications. This then determines an upper limit for the - number of waiters that can be supported. - endif # SCHED_HPWORK config SCHED_LPWORK diff --git a/sched/init/os_start.c b/sched/init/os_start.c index 9f0d62d82a..e5a65ea997 100644 --- a/sched/init/os_start.c +++ b/sched/init/os_start.c @@ -705,12 +705,6 @@ void os_start(void) net_setup(); #endif -#ifdef CONFIG_SCHED_WORKQUEUE - /* Initialize work queue data structures */ - - work_initialize(); -#endif - /* The processor specific details of running the operating system * will be handled here. Such things as setting up interrupt * service routines and starting the clock are some of the things diff --git a/sched/wqueue/Make.defs b/sched/wqueue/Make.defs index e72b221ede..8b065ec2da 100644 --- a/sched/wqueue/Make.defs +++ b/sched/wqueue/Make.defs @@ -37,8 +37,7 @@ ifeq ($(CONFIG_SCHED_WORKQUEUE),y) -CSRCS += kwork_initialize.c kwork_queue.c kwork_process.c kwork_cancel.c -CSRCS += kwork_signal.c +CSRCS += kwork_queue.c kwork_process.c kwork_cancel.c kwork_signal.c # Add high priority work queue files diff --git a/sched/wqueue/kwork_initialize.c b/sched/wqueue/kwork_initialize.c deleted file mode 100644 index d87a825f59..0000000000 --- a/sched/wqueue/kwork_initialize.c +++ /dev/null @@ -1,73 +0,0 @@ -/**************************************************************************** - * sched/wqueue/kwork_initialize.c - * - * Copyright (C) 2018 Gregory Nutt. All rights reserved. - * Author: Gregory Nutt - * - * Redistribution and use in source and binary forms, with or without - * modification, are permitted provided that the following conditions - * are met: - * - * 1. Redistributions of source code must retain the above copyright - * notice, this list of conditions and the following disclaimer. - * 2. Redistributions in binary form must reproduce the above copyright - * notice, this list of conditions and the following disclaimer in - * the documentation and/or other materials provided with the - * distribution. - * 3. Neither the name NuttX nor the names of its contributors may be - * used to endorse or promote products derived from this software - * without specific prior written permission. - * - * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS - * "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT - * LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS - * FOR A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE - * COPYRIGHT OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, - * INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, - * BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS - * OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED - * AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT - * LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN - * ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE - * POSSIBILITY OF SUCH DAMAGE. - * - ****************************************************************************/ - -/**************************************************************************** - * Included Files - ****************************************************************************/ - -#include - -#include "wqueue/wqueue.h" - -#ifdef CONFIG_SCHED_WORKQUEUE - -/**************************************************************************** - * Public Functions - ****************************************************************************/ - -/**************************************************************************** - * Name: work_initialize - * - * Description: - * One time initialization of the work queue data structures. - * - * Input Parameters: - * None - * - * Returned Value: - * None - * - ****************************************************************************/ - -void work_initialize(void) -{ -#ifdef CONFIG_WQUEUE_NOTIFIER - /* Initialize work queue notifier data structures */ - - work_notifier_initialize(); -#endif -} - -#endif /* CONFIG_SCHED_WORKQUEUE */ diff --git a/sched/wqueue/kwork_notifier.c b/sched/wqueue/kwork_notifier.c index 55caf48b46..2da97589ca 100644 --- a/sched/wqueue/kwork_notifier.c +++ b/sched/wqueue/kwork_notifier.c @@ -59,23 +59,23 @@ * Private Types ****************************************************************************/ -/* The full allocated notification will hold additional information. The - * allocated notification information persist until the work is executed - * and must be freed using kmm_free() by the work. +/* This structure describes one notification list entry. It is cast- + * compatible with struct work_notifier_s. This structure is an allocated + * container for the user notification data. It is allocated because it + * must persist until the work is executed and must be freed using + * kmm_free() by the work. */ -struct work_notifier_alloc_s -{ - struct work_notifier_s info; /* The notification info */ - struct work_s work; /* Used for scheduling the work */ -}; - -/* This structure describes one notification list entry */ - struct work_notifier_entry_s { + /* User notification information */ + + struct work_notifier_s info; /* The notification info */ + + /* Additional payload needed to manage the notification */ + FAR struct work_notifier_entry_s *flink; - FAR struct work_notifier_alloc_s *alloc; /* Allocated notification info copy */ + struct work_s work; /* Used for scheduling the work */ int16_t key; /* Unique ID for the notification */ }; @@ -83,15 +83,6 @@ struct work_notifier_entry_s * Private Data ****************************************************************************/ -/* This is a statically allocated pool of notification structures */ - -static struct work_notifier_entry_s - g_notifier_pool[CONFIG_WQUEUE_NOTIFIER_NWAITERS]; - -/* This is a list of free notification structures */ - -static FAR struct work_notifier_entry_s *g_notifier_free; - /* This is a singly linked list of pending notifications. When an event * occurs available, *all* of the waiters for that event in this list will * be signaled and the entry will be freed. If there are multiple waiters @@ -106,7 +97,7 @@ static FAR struct work_notifier_entry_s *g_notifier_pending; * the notification data structures. */ -static sem_t g_notifier_sem; +static sem_t g_notifier_sem = SEM_INITIALIZER(1); /* Used for lookup key generation */ @@ -116,44 +107,6 @@ static uint16_t g_notifier_key; * Private Functions ****************************************************************************/ -/**************************************************************************** - * Name: work_notifier_alloc - * - * Description: - * Allocate a notification structure by removing it from the free list. - * - ****************************************************************************/ - -static FAR struct work_notifier_entry_s *work_notifier_alloc(void) -{ - FAR struct work_notifier_entry_s *notifier; - - notifier = g_notifier_free; - if (notifier != NULL) - { - g_notifier_free = notifier->flink; - notifier->flink = NULL; - } - - return notifier; -} - -/**************************************************************************** - * Name: work_notifier_free - * - * Description: - * Free a notification structure by returning it to the head of the free - * list. - * - ****************************************************************************/ - -static inline void - work_notifier_free(FAR struct work_notifier_entry_s *notifier) -{ - notifier->flink = g_notifier_free; - g_notifier_free = notifier; -} - /**************************************************************************** * Name: work_notifier_find * @@ -225,37 +178,6 @@ static int16_t work_notifier_key(void) * Public Functions ****************************************************************************/ -/**************************************************************************** - * Name: work_notifier_initialize - * - * Description: - * Set up the notification data structures for normal operation. - * - ****************************************************************************/ - -void work_notifier_initialize(void) -{ - int i; - - /* Add each notification structure to the free list */ - - for (i = 0; i < CONFIG_WQUEUE_NOTIFIER_NWAITERS; i++) - { - FAR struct work_notifier_entry_s *notifier = &g_notifier_pool[i]; - - /* Add the pre-allocated notification to the head of the free list */ - - notifier->flink = g_notifier_free; - g_notifier_free = notifier; - } - - /* Initialize the semaphore that enforces mutually exclusive access to the - * notification data structures. - */ - - nxsem_init(&g_notifier_sem, 0, 1); -} - /**************************************************************************** * Name: work_notifier_setup * @@ -277,6 +199,7 @@ void work_notifier_initialize(void) int work_notifier_setup(FAR struct work_notifier_s *info) { + FAR struct work_notifier_entry_s *notifier; int ret; /* Get exclusive access to the notifier data structures */ @@ -287,32 +210,22 @@ int work_notifier_setup(FAR struct work_notifier_s *info) return ret; } - /* Allocate a new notification */ + /* Allocate a new notification entry */ - FAR struct work_notifier_entry_s *notifier = work_notifier_alloc(); + notifier = kmm_malloc(sizeof(struct work_notifier_entry_s)); if (notifier == NULL) { - ret = -ENOSPC; + ret = -ENOMEM; } else { - FAR struct work_notifier_alloc_s *alloc; - int16_t key; - /* Duplicate the notification info */ - alloc = kmm_malloc(sizeof(struct work_notifier_alloc_s)); - if (alloc == NULL) - { - work_notifier_free(notifier); - return -ENOMEM; - } - - memcpy(&alloc->info, info, sizeof(struct work_notifier_s)); + memcpy(¬ifier->info, info, sizeof(struct work_notifier_s)); /* Generate a unique key for this notification */ - key = work_notifier_key(); + notifier->key = work_notifier_key(); /* Add the notification to the head of the pending list * @@ -323,11 +236,9 @@ int work_notifier_setup(FAR struct work_notifier_s *info) */ notifier->flink = g_notifier_pending; - notifier->alloc = alloc; - notifier->key = key; - g_notifier_pending = notifier; - ret = key; + + ret = work_notifier_key(); } (void)nxsem_post(&g_notifier_sem); @@ -391,14 +302,9 @@ int work_notifier_teardown(int key) prev->flink = notifier->flink; } - /* Free the contained information */ + /* Free the notification */ - DEBUGASSERT(notifier->alloc != NULL); - kmm_free(notifier->alloc); - - /* And return the notification to the free list */ - - work_notifier_free(notifier); + kmm_free(notifier); ret = OK; } @@ -458,7 +364,6 @@ void work_notifier_signal(enum work_evtype_e evtype, notifier != NULL; notifier = next) { - FAR struct work_notifier_alloc_s *alloc; FAR struct work_notifier_s *info; /* Set up for the next time through the loop (in case the entry is @@ -466,18 +371,14 @@ void work_notifier_signal(enum work_evtype_e evtype, */ next = notifier->flink; + info = ¬ifier->info; /* Check if this is the a notification request for the event that * just occurred. */ - alloc = notifier->alloc; - DEBUGASSERT(alloc != NULL); - info = &alloc->info; - if (info->evtype == evtype && info->qualifier == qualifier) { - /* Yes.. Remove the notification from the pending list */ if (prev == NULL) @@ -491,11 +392,7 @@ void work_notifier_signal(enum work_evtype_e evtype, /* Schedule the work */ - (void)work_queue(HPWORK, &alloc->work, info->worker, info, 0); - - /* Free the notification by returning it to the free list */ - - work_notifier_free(notifier); + (void)work_queue(HPWORK, ¬ifier->work, info->worker, info, 0); } else { diff --git a/sched/wqueue/wqueue.h b/sched/wqueue/wqueue.h index 5446d7e493..7f79fc488c 100644 --- a/sched/wqueue/wqueue.h +++ b/sched/wqueue/wqueue.h @@ -129,22 +129,6 @@ extern struct lp_wqueue_s g_lpwork; * Public Function Prototypes ****************************************************************************/ -/**************************************************************************** - * Name: work_initialize - * - * Description: - * One time initialization of the work queue data structures. - * - * Input Parameters: - * None - * - * Returned Value: - * None - * - ****************************************************************************/ - -void work_initialize(void); - /**************************************************************************** * Name: work_hpstart *