sched/wqueue/notifier: protect the work notifier with critical section

replace the semaphore to avoid the notifier holding the lock in the interrupt context

ASSERT:

libs/libc/assert/lib_assert.c:36   :_assert
sched/semphore/sem_wait.c:113      :nxsem_wait
sched/semphore/sem_wait.c:222      :nxsem_wait_uniterruptible
sched/wqueue/kwork_notifier.c:371  :work_notifier_signal
mm/iob/iob_free.c:188              :iob_free
drivers/syslog/syslog_stream.c:272 :syslogstream_destroy
...
sched/irq/irq_dispatch.c:183       :irq_dispatch

Signed-off-by: chao.an <anchao@xiaomi.com>
This commit is contained in:
chao.an 2021-01-28 21:51:39 +08:00 committed by David Sidrane
parent b288986ccd
commit 003b360d12

View File

@ -86,12 +86,6 @@ static dq_queue_t g_notifier_free;
static dq_queue_t g_notifier_pending; static dq_queue_t g_notifier_pending;
/* This semaphore is used as mutex to enforce mutually exclusive access to
* the notification data structures.
*/
static sem_t g_notifier_sem = SEM_INITIALIZER(1);
/* Used for lookup key generation */ /* Used for lookup key generation */
static uint16_t g_notifier_key; static uint16_t g_notifier_key;
@ -175,20 +169,21 @@ static void work_notifier_worker(FAR void *arg)
{ {
FAR struct work_notifier_entry_s *notifier = FAR struct work_notifier_entry_s *notifier =
(FAR struct work_notifier_entry_s *)arg; (FAR struct work_notifier_entry_s *)arg;
int ret; irqstate_t flags;
/* Forward to the real worker */ /* Forward to the real worker */
notifier->info.worker(notifier->info.arg); notifier->info.worker(notifier->info.arg);
/* Disable interrupts very briefly. */
flags = enter_critical_section();
/* Put the notification to the free list */ /* Put the notification to the free list */
ret = nxsem_wait_uninterruptible(&g_notifier_sem); dq_addlast((FAR dq_entry_t *)notifier, &g_notifier_free);
if (ret >= 0)
{ leave_critical_section(flags);
dq_addlast((FAR dq_entry_t *)notifier, &g_notifier_free);
nxsem_post(&g_notifier_sem);
}
} }
/**************************************************************************** /****************************************************************************
@ -217,24 +212,23 @@ static void work_notifier_worker(FAR void *arg)
int work_notifier_setup(FAR struct work_notifier_s *info) int work_notifier_setup(FAR struct work_notifier_s *info)
{ {
FAR struct work_notifier_entry_s *notifier; FAR struct work_notifier_entry_s *notifier;
irqstate_t flags;
int ret; int ret;
DEBUGASSERT(info != NULL && info->worker != NULL); DEBUGASSERT(info != NULL && info->worker != NULL);
DEBUGASSERT(info->qid == HPWORK || info->qid == LPWORK); DEBUGASSERT(info->qid == HPWORK || info->qid == LPWORK);
/* Get exclusive access to the notifier data structures */ /* Disable interrupts very briefly. */
ret = nxsem_wait(&g_notifier_sem); flags = enter_critical_section();
if (ret < 0)
{
return ret;
}
/* Try to get the entry from the free list */ /* Try to get the entry from the free list */
notifier = (FAR struct work_notifier_entry_s *) notifier = (FAR struct work_notifier_entry_s *)
dq_remfirst(&g_notifier_free); dq_remfirst(&g_notifier_free);
leave_critical_section(flags);
if (notifier == NULL) if (notifier == NULL)
{ {
/* Allocate a new notification entry */ /* Allocate a new notification entry */
@ -256,6 +250,10 @@ int work_notifier_setup(FAR struct work_notifier_s *info)
memcpy(&notifier->info, info, sizeof(struct work_notifier_s)); memcpy(&notifier->info, info, sizeof(struct work_notifier_s));
/* Disable interrupts very briefly. */
flags = enter_critical_section();
/* Generate a unique key for this notification */ /* Generate a unique key for this notification */
notifier->key = work_notifier_key(); notifier->key = work_notifier_key();
@ -270,9 +268,10 @@ int work_notifier_setup(FAR struct work_notifier_s *info)
dq_addlast((FAR dq_entry_t *)notifier, &g_notifier_pending); dq_addlast((FAR dq_entry_t *)notifier, &g_notifier_pending);
ret = notifier->key; ret = notifier->key;
leave_critical_section(flags);
} }
nxsem_post(&g_notifier_sem);
return ret; return ret;
} }
@ -298,17 +297,14 @@ int work_notifier_setup(FAR struct work_notifier_s *info)
int work_notifier_teardown(int key) int work_notifier_teardown(int key)
{ {
FAR struct work_notifier_entry_s *notifier; FAR struct work_notifier_entry_s *notifier;
int ret; irqstate_t flags;
int ret = OK;
DEBUGASSERT(key > 0 && key <= INT16_MAX); DEBUGASSERT(key > 0 && key <= INT16_MAX);
/* Get exclusive access to the notifier data structures */ /* Disable interrupts very briefly. */
ret = nxsem_wait(&g_notifier_sem); flags = enter_critical_section();
if (ret < 0)
{
return ret;
}
/* Find the entry matching this PID in the g_notifier_pending list. We /* Find the entry matching this PID in the g_notifier_pending list. We
* assume that there is only one. * assume that there is only one.
@ -330,10 +326,9 @@ int work_notifier_teardown(int key)
/* Put the notification to the free list */ /* Put the notification to the free list */
dq_addlast((FAR dq_entry_t *)notifier, &g_notifier_free); dq_addlast((FAR dq_entry_t *)notifier, &g_notifier_free);
ret = OK;
} }
nxsem_post(&g_notifier_sem); leave_critical_section(flags);
return ret; return ret;
} }
@ -364,22 +359,13 @@ void work_notifier_signal(enum work_evtype_e evtype,
FAR struct work_notifier_entry_s *notifier; FAR struct work_notifier_entry_s *notifier;
FAR dq_entry_t *entry; FAR dq_entry_t *entry;
FAR dq_entry_t *next; FAR dq_entry_t *next;
int ret; irqstate_t flags;
/* Get exclusive access to the notifier data structure */
ret = nxsem_wait_uninterruptible(&g_notifier_sem);
if (ret < 0)
{
serr("ERROR: nxsem_wait_uninterruptible failed: %d\n", ret);
return;
}
/* Don't let any newly started threads block this thread until all of /* Don't let any newly started threads block this thread until all of
* the notifications and been sent. * the notifications and been sent.
*/ */
sched_lock(); flags = enter_critical_section();
/* Process the notification at the head of the pending list until the /* Process the notification at the head of the pending list until the
* pending list is empty * pending list is empty
@ -422,8 +408,7 @@ void work_notifier_signal(enum work_evtype_e evtype,
} }
} }
sched_unlock(); leave_critical_section(flags);
nxsem_post(&g_notifier_sem);
} }
#endif /* CONFIG_WQUEUE_NOTIFIER */ #endif /* CONFIG_WQUEUE_NOTIFIER */