From b0ba66e728f0885d53e7836898ea60902c818aa8 Mon Sep 17 00:00:00 2001
From: bernie <bernie@38d2e660-2303-0410-9eaa-f027e97ec537>
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 <bernie@codewiz.org>
- *
  * -->
  *
  * \brief Semaphore based synchronization services.
  *
  * \version $Id$
- *
  * \author Bernie Innocenti <bernie@codewiz.org>
  */
 
 #include "sem.h"
+#include <cpu/irq.h> // ASSERT_IRQ_DISABLED()
 #include <kern/proc.h>
 #include <kern/proc_p.h>
 #include <kern/signal.h>
@@ -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