proc_forbid(): extensively document why we don't need locking

Also change the ASSERT() in proc_permit() to check for != 0, as a
workaround for CPUs like the AVR which are unable to write an int to
memory atomically.

A better fix would be changing the type of preempt_forbid_cnt to
cpu_stack_t or a new type cpu_atomic_t.

git-svn-id: https://src.develer.com/svnoss/bertos/trunk@1755 38d2e660-2303-0410-9eaa-f027e97ec537

diff --git a/bertos/kern/proc.h b/bertos/kern/proc.h
index 1a35834683c13c27db0bdc23a73bd178bd510b45..5c26465257f075b4ef71213095266f421e5396cd 100644 (file)
--- a/bertos/kern/proc.h
+++ b/bertos/kern/proc.h
@@ -95,6 +95,9 @@ const char *proc_currentName(void);
  * \note Calling functions that could sleep while task switching is disabled
  * is dangerous and unsupported.
  *
+ * \note calling proc_forbid() from within an interrupt is illegal and
+ * meaningless.
+ *
  * \note proc_permit() expands inline to 1-2 asm instructions, so it's a
  * very efficient locking primitive in simple but performance-critical
  * situations.  In all other cases, semaphores offer a more flexible and
@@ -106,7 +109,29 @@ INLINE void proc_forbid(void)
 {
        #if CONFIG_KERN_PREEMPT
                extern int _preempt_forbid_cnt;
-               // No need to protect against interrupts here.
+               /*
+                * We don't need to protect the counter against other processes.
+                * The reason why is a bit subtle.
+                *
+                * If a process gets here, preempt_forbid_cnt can be either 0,
+                * or != 0.  In the latter case, preemption is already disabled
+                * and no concurrency issues can occur.
+                *
+                * In the former case, we could be preempted just after reading the
+                * value 0 from memory, and a concurrent process might, in fact,
+                * bump the value of preempt_forbid_cnt under our nose!
+                *
+                * BUT: if this ever happens, then we won't get another chance to
+                * run until the other process calls proc_permit() to re-enable
+                * preemption.  At this point, the value of preempt_forbid_cnt
+                * must be back to 0, and thus what we had originally read from
+                * memory happens to be valid.
+                *
+                * No matter how hard you think about it, and how complicated you
+                * make your scenario, the above holds true as long as
+                * "preempt_forbid_cnt != 0" means that no task switching is
+                * possible.
+                */
                ++_preempt_forbid_cnt;
 
                /*
@@ -133,8 +158,8 @@ INLINE void proc_permit(void)
                MEMORY_BARRIER;
                extern int _preempt_forbid_cnt;
                /* No need to protect against interrupts here. */
+               ASSERT(_preempt_forbid_cnt != 0);
                --_preempt_forbid_cnt;
-               ASSERT(_preempt_forbid_cnt >= 0);
 
                /*
                 * This ensures _preempt_forbid_cnt is flushed to memory immediately