From b0ba66e728f0885d53e7836898ea60902c818aa8 Mon Sep 17 00:00:00 2001 From: bernie Date: Wed, 3 Sep 2008 08:17:51 +0000 Subject: [PATCH] SCHED_ENQUEUE: tighten checks on locking Factor out the two different implementations and ensure modifications happen with inteerrupts disabled. This uncovered a latent bug in our semaphore sleep code. Also explicitly document our locking requirements for ProcReadyList. The comment previously claimed that proc_forbid() would offer enough protection, which is, of course, bullshit. git-svn-id: https://src.develer.com/svnoss/bertos/trunk@1773 38d2e660-2303-0410-9eaa-f027e97ec537 --- bertos/kern/proc_p.h | 45 +++++++++++++++++++++----------------------- bertos/kern/sem.c | 5 ++--- 2 files changed, 23 insertions(+), 27 deletions(-) diff --git a/bertos/kern/proc_p.h b/bertos/kern/proc_p.h index 9d61b531..d99d5fc1 100644 --- a/bertos/kern/proc_p.h +++ b/bertos/kern/proc_p.h @@ -104,34 +104,31 @@ extern REGISTER Process *CurrentProcess; /** * Track ready processes. * - * Access to this list must be protected with a proc_forbid() / proc_premit() - * pair, or with SCHED_ATOMIC() + * Access to this list must be performed with interrupts disabled */ extern REGISTER List ProcReadyList; #if CONFIG_KERN_PRI - /** - * Enqueue a task in the ready list. - * - * Always use this macro to instert a process in the ready list, as its - * might vary to implement a different scheduling algorithms. - * - * \note This macro is *NOT* protected against the scheduler. Access to - * this list must be performed with interrupts disabled. - */ - #define SCHED_ENQUEUE(proc) do { \ - LIST_ASSERT_VALID(&ProcReadyList); \ - LIST_ENQUEUE(&ProcReadyList, &(proc)->link); \ - } while (0) - -#else // !CONFIG_KERN_PRI - - #define SCHED_ENQUEUE(proc) do { \ - LIST_ASSERT_VALID(&ProcReadyList); \ - ADDTAIL(&ProcReadyList, &(proc)->link); \ - } while (0) - -#endif // !CONFIG_KERN_PRI + #define SCHED_ENQUEUE_INTERNAL(proc) LIST_ENQUEUE(&ProcReadyList, &(proc)->link) +#else + #define SCHED_ENQUEUE_INTERNAL(proc) ADDTAIL(&ProcReadyList, &(proc)->link) +#endif + +/** + * Enqueue a process in the ready list. + * + * Always use this macro to instert a process in the ready list, as its + * might vary to implement a different scheduling algorithms. + * + * \note Access to the scheduler ready list must be performed with + * interrupts disabled. + */ +#define SCHED_ENQUEUE(proc) do { \ + IRQ_ASSERT_DISABLED(); \ + LIST_ASSERT_VALID(&ProcReadyList); \ + SCHED_ENQUEUE_INTERNAL(proc); \ + } while (0) + /// Schedule another process *without* adding the current one to the ready list. void proc_switch(void); diff --git a/bertos/kern/sem.c b/bertos/kern/sem.c index cc9234aa..7d07f4d6 100644 --- a/bertos/kern/sem.c +++ b/bertos/kern/sem.c @@ -28,17 +28,16 @@ * * Copyright 2001, 2004 Develer S.r.l. (http://www.develer.com/) * Copyright 1999, 2000, 2001 Bernie Innocenti - * * --> * * \brief Semaphore based synchronization services. * * \version $Id$ - * * \author Bernie Innocenti */ #include "sem.h" +#include // ASSERT_IRQ_DISABLED() #include #include #include @@ -178,7 +177,7 @@ void sem_release(struct Semaphore *s) { s->nest_count = 1; s->owner = proc; - SCHED_ENQUEUE(proc); + ATOMIC(SCHED_ENQUEUE(proc)); } } -- 2.25.1