Changeset f0fcb04 in mainline

Timestamp:

2012-07-29T19:26:32Z (13 years ago)

Author:

Adam Hraska <adam.hraska+hos@…>

Branches:

lfn, master, serial, ticket/834-toolchain-update, topic/msim-upgrade, topic/simplify-dev-export

Children:

d4d36f9

Parents:

5b03a72

Message:

rcu: Replaced checking three variables to see if the detector needs to be notified in rcu_read_unlock() with a single bool cpu.signal_unlock.

Location:

kernel/generic

Files:

• include/synch/rcu.h (modified) (1 diff)
• include/synch/rcu_types.h (modified) (1 diff)
• src/synch/rcu.c (modified) (8 diffs)

kernel/generic/include/synch/rcu.h

@@ -200 +200 @@
                 /* 
                  * The thread was preempted while in a critical section or 
-                 * the detector is eagerly waiting for this cpu's reader 
-                 * to finish. 
-                 * 
-                 * Note that THREAD may be 0 in scheduler() and not just during boot.
+                 * the detector is eagerly waiting for this cpu's reader to finish. 
                  */
-                if ((THREAD && THREAD->rcu.was_preempted) || CPU->rcu.is_delaying_gp) {
+                if (CPU->rcu.signal_unlock) {
                         /* Rechecks with disabled interrupts. */
                         _rcu_signal_read_unlock();
                 }
         }
-
         
         preemption_enable();

kernel/generic/include/synch/rcu_types.h

@@ -107 +107 @@
         bool is_delaying_gp;
         
+        /** True if we should signal the detector that we exited a reader section.
+         * 
+         * Equal to (THREAD->rcu.was_preempted || CPU->rcu.is_delaying_gp).
+         */
+        bool signal_unlock;
+        
         /** Positive if there are callbacks pending in arriving_cbs. */
         semaphore_t arrived_flag;

kernel/generic/src/synch/rcu.c

@@ -239 +239 @@
         
         CPU->rcu.is_delaying_gp = false;
+        CPU->rcu.signal_unlock = false;
         
         semaphore_initialize(&CPU->rcu.arrived_flag, 0);
@@ -410 +411 @@
         ASSERT(PREEMPTION_DISABLED || interrupts_disabled());
         
+        /* todo: make NMI safe with cpu-local atomic ops. */
+        
         /*
          * We have to disable interrupts in order to make checking
@@ -457 +460 @@
                 irq_spinlock_unlock(&rcu.preempt_lock, false);
         }
+        
+        /* If there was something to signal to the detector we have done so. */
+        CPU->rcu.signal_unlock = false;
+        
         interrupts_restore(ipl);
 }
@@ -1201 +1208 @@
                         /* Note to notify the detector from rcu_read_unlock(). */
                         CPU->rcu.is_delaying_gp = true;
+                        /* 
+                         * Set signal_unlock only after setting is_delaying_gp so
+                         * that NMI handlers do not accidentally clear it in unlock()
+                         * before seeing and acting upon is_delaying_gp.
+                         */
+                        compiler_barrier();
+                        CPU->rcu.signal_unlock = true;
+                        
                         atomic_inc(&rcu.delaying_cpu_cnt);
                 } else {
@@ -1274 +1289 @@
 {
         ASSERT(interrupts_disabled());
-        
-        /* Save the thread's nesting count when its not running. */
+        /* todo: make is_delaying_gp and was_preempted NMI safe via local atomics.*/
+
+        /* 
+         * Prevent NMI handlers from interfering. The detector will be notified
+         * here if CPU->rcu.is_delaying_gp and the current thread is no longer 
+         * running so there is nothing to signal to the detector.
+         */
+        CPU->rcu.signal_unlock = false;
+        /* Separates clearing of .signal_unlock from CPU->rcu.nesting_cnt = 0. */
+        compiler_barrier();
+        
+        /* Save the thread's nesting count when it is not running. */
         THREAD->rcu.nesting_cnt = CPU->rcu.nesting_cnt;
         /* Interrupt handlers might use RCU while idle in scheduler(). */
@@ -1300 +1325 @@
         }
         
+        
         /* 
          * The preempted reader has been noted globally. There are therefore
@@ -1317 +1343 @@
                 semaphore_up(&rcu.remaining_readers);
         }
-        
+
         /* 
          * Forcefully associate the detector with the highest priority
@@ -1354 +1380 @@
         /* Load the thread's saved nesting count from before it was preempted. */
         CPU->rcu.nesting_cnt = THREAD->rcu.nesting_cnt;
+        /* 
+         * In the unlikely event that a NMI occurs between the loading of the 
+         * variables and setting signal_unlock, the NMI handler may invoke 
+         * rcu_read_unlock() and clear signal_unlock. In that case we will
+         * incorrectly overwrite signal_unlock from false to true. This event
+         * situation benign and the next rcu_read_unlock() will at worst 
+         * needlessly invoke _rcu_signal_unlock().
+         */
+        CPU->rcu.signal_unlock = THREAD->rcu.was_preempted || CPU->rcu.is_delaying_gp;
 }