pop_ros_tf can handle missed IPIs
authorBarret Rhoden <brho@cs.berkeley.edu>
Sun, 11 Apr 2010 23:35:55 +0000 (16:35 -0700)
committerKevin Klues <klueska@cs.berkeley.edu>
Thu, 3 Nov 2011 00:35:42 +0000 (17:35 -0700)
The issue is that an IPI can arrive (and be ignored) after the queue is
checked and before notifications are enabled.  Check notif_pending and
self notify if this happens.  Check the documentation for more details.

Documentation/processes.txt
kern/arch/i686/ros/bits/syscall.h
kern/src/process.c
kern/src/syscall.c
tests/mhello.c
user/include/i686/hart.h
user/include/sparc/hart.h

index 47a7e90..c435258 100644 (file)
@@ -447,6 +447,22 @@ and then execute a return.  If an IPI arrives before the "function return", then
 when that context gets restarted, it will run the "return" with the appropriate
 value on the stack still.
 
+There is a further complication.  The kernel can send an IPI that the process
+wanted, but the vcore did not get truly interrupted since its notifs were
+disabled.  There is a race between checking the queue/bitmask and then enabling
+notifications.  The way we deal with it is that the kernel posts the
+message/bit, then sets notif_pending.  Then it sends the IPI, which may or may
+not be received (based on notif_enabled).  (Actually, the kernel only ought to
+send the IPI if notif_pending was 0 (atomically) and notif_enabled is 1).  When
+leaving the transition stack, userspace should clear the notif_pending, then
+check the queue do whatever, and then try to pop the tf.  When popping the tf,
+after enabling notifications, check notif_pending.  If it is still clear, return
+without fear of missing a notif.  If it is not clear, it needs to manually
+notify itself (sys_self_notify) so that it can process the notification that it
+missed and for which it wanted to receive an IPI.  Before it does this, it needs
+to clear notif_pending, so the kernel will send it an IPI.  These last parts are
+handled in pop_ros_tf().
+
 4.3: Preemption Specifics
 -------------------------------
 When a vcore is preempted, the kernel takes whatever context was running (which
index 3db26a6..92a121e 100644 (file)
@@ -68,6 +68,7 @@ static inline intreg_t __syscall_trap(uint16_t num, intreg_t a1,
        // potentially change the condition codes and arbitrary
        // memory locations.
 
+       /* If you change this, change pop_ros_tf() */
        asm volatile(""
                     " int %2"
                     : "=a" (ret),
index 088eeab..95a240b 100644 (file)
@@ -722,18 +722,20 @@ void do_notify(struct proc *p, uint32_t vcoreid, unsigned int notif,
         * briefly and the kernel immediately returns back once it realizes notifs
         * are masked.  To fix it, we'll need atomic_swapb() (right answer), or not
         * use a bool. (wrong answer). */
-       if (nm->flags & NOTIF_IPI && vcpd->notif_enabled && !vcpd->notif_pending) {
+       if (nm->flags & NOTIF_IPI && !vcpd->notif_pending) {
                vcpd->notif_pending = TRUE;
-               spin_lock_irqsave(&p->proc_lock);
-                       if ((p->state & PROC_RUNNING_M) && // TODO: (VC#) (_S state)
-                                     (p->procinfo->vcoremap[vcoreid].valid)) {
-                               printk("[kernel] sending notif to vcore %d\n", vcoreid);
-                               send_kernel_message(p->procinfo->vcoremap[vcoreid].pcoreid,
-                                                   __notify, p, 0, 0, KMSG_ROUTINE);
-                       } else { // TODO: think about this, fallback, etc
-                               warn("Vcore unmapped, not receiving an active notif");
-                       }
-               spin_unlock_irqsave(&p->proc_lock);
+               if (vcpd->notif_enabled) {
+                       spin_lock_irqsave(&p->proc_lock);
+                               if ((p->state & PROC_RUNNING_M) && // TODO: (VC#) (_S state)
+                                             (p->procinfo->vcoremap[vcoreid].valid)) {
+                                       printk("[kernel] sending notif to vcore %d\n", vcoreid);
+                                       send_kernel_message(p->procinfo->vcoremap[vcoreid].pcoreid,
+                                                           __notify, p, 0, 0, KMSG_ROUTINE);
+                               } else { // TODO: think about this, fallback, etc
+                                       warn("Vcore unmapped, not receiving an active notif");
+                               }
+                       spin_unlock_irqsave(&p->proc_lock);
+               }
        }
 }
 
index 5589b7c..7d1f396 100644 (file)
@@ -594,12 +594,14 @@ static int sys_notify(struct proc *p, int target_pid, unsigned int notif,
 }
 
 /* Will notify the calling process on the given vcore, independently of WANTED
- * or advertised vcoreid. */
+ * or advertised vcoreid.  If you change the parameters, change pop_ros_tf() */
 static int sys_self_notify(struct proc *p, uint32_t vcoreid, unsigned int notif,
                            struct notif_event *u_ne)
 {
        struct notif_event local_ne;
 
+       printd("[kernel] received self notify for vcoreid %d, notif %d, ne %08p\n",
+              vcoreid, notif, u_ne);
        /* if the user provided a notif_event, copy it in and use that */
        if (u_ne) {
                if (memcpy_from_user(p, &local_ne, u_ne, sizeof(struct notif_event))) {
index d91eff2..a369e81 100644 (file)
@@ -28,7 +28,7 @@ int main(int argc, char** argv)
 
        /* tell the kernel where and how we want to receive notifications */
        struct notif_method *nm;
-       for (int i = 1; i < MAX_NR_NOTIF; i++) {
+       for (int i = 0; i < MAX_NR_NOTIF; i++) {
                nm = &__procdata.notif_methods[i];
                nm->flags |= NOTIF_WANTED | NOTIF_MSG | NOTIF_IPI;
                nm->vcoreid = i % 2; // vcore0 or 1, keepin' it fresh.
@@ -95,6 +95,7 @@ int main(int argc, char** argv)
 void hart_entry(void)
 {
        uint32_t vcoreid = hart_self();
+       static bool first_time = TRUE;
 
        temp = 0xcafebabe;
 /* begin: stuff userspace needs to do to handle notifications */
@@ -118,9 +119,16 @@ void hart_entry(void)
         * entry for this vcore to point to the TCB of the new user-thread. */
        if (vcoreid == 0) {
                printf("restarting vcore0 from userspace\n");
+               /* Do one last check for notifs before clearing pending */
+               vcpd->notif_pending = 0;
+               if (first_time) { // testing for missing a notif
+                       first_time = FALSE;
+                       printf("setting pending, trying to renotify etc\n");
+                       vcpd->notif_pending = 1;
+               }
                set_tls_desc(core0_tls, 0);
                /* Load silly state (Floating point) too */
-               pop_ros_tf(&vcpd->notif_tf, &vcpd->notif_enabled);
+               pop_ros_tf(&vcpd->notif_tf, vcoreid);
                panic("should never see me!");
        }       
        /* unmask notifications once you can let go of the notif_tf and it is okay
index b54fc84..0b3876b 100644 (file)
  * A Userspace scheduler can call this when transitioning off the transition
  * stack.
  *
+ * Make sure you clear the notif_pending flag, and then check the queue before
+ * calling this.  If notif_pending is not clear, this will self_notify this
+ * core, since it should be because we missed a notification message while
+ * notifs were disabled. 
+ *
  * Basically, it sets up the future stack pointer to have extra stuff after it,
  * and then it pops the registers, then pops the new context's stack
  * pointer.  Then it uses the extra stuff (the new PC is on the stack, the
  * location of notif_enabled, and a clobbered work register) to enable notifs,
- * restore the work reg, and then "ret".
+ * make sure notif IPIs weren't pending, restore the work reg, and then "ret".
+ *
+ * This is what the target notif_tf's stack will look like (growing down):
+ *
+ * Target ESP -> |   u_thread's old stuff   |
+ *               |   new eip                |
+ *               |   eax save space         |
+ *               |   vcoreid                |
+ *               |   notif_pending_loc      |
+ *               |   notif_enabled_loc      |
  *
- * The important thing is that it can a notification after it enables
+ * The important thing is that it can handle a notification after it enables
  * notifications, and when it gets resumed it can ultimately run the new
  * context.  Enough state is saved in the running context and stack to continue
  * running. */
-static inline void pop_ros_tf(struct user_trapframe *tf, bool *notif_en_loc)
+static inline void pop_ros_tf(struct user_trapframe *tf, uint32_t vcoreid)
 {
+       struct preempt_data *vcpd = &__procdata.vcore_preempt_data[vcoreid];
        if (!tf->tf_cs) { /* sysenter TF.  esp and eip are in other regs. */
                tf->tf_esp = tf->tf_regs.reg_ebp;
                tf->tf_eip = tf->tf_regs.reg_edx;
        }
-       asm volatile ("movl %2,-4(%1);          " /* push the PC */
-                     "movl %3,-12(%1);         " /* leave room for eax, push loc */
-                     "movl %0,%%esp;           " /* pop the real tf */
-                     "popal;                   "
-                     "addl $0x20,%%esp;        " /* move to the eflags in the tf */
-                     "popfl;                   " /* restore eflags */
-                     "popl %%esp;              " /* change to the new %esp */
-                     "subl $0x4,%%esp;         " /* move esp to the slot for eax */
-                     "pushl %%eax;             " /* save eax, will clobber soon */
-                     "subl $0x4,%%esp;         " /* move to notif_en_loc slot */
-                     "popl %%eax;              "
-                     "movb $0x01,(%%eax);      " /* enable notifications */
-                     "popl %%eax;              " /* restore tf's %eax */
-                     "ret;                     " /* return to the new PC */
+       asm volatile ("movl %2,-0x04(%1);    " /* push the PC */
+                     "movl %3,-0x0c(%1);    " /* room for eax, push vcoreid */
+                     "movl %4,-0x10(%1);    " /* push notif_pending loc */
+                     "movl %5,-0x14(%1);    " /* push notif_enabled loc */
+                     "movl %0,%%esp;        " /* pop the real tf */
+                     "popal;                " /* restore normal registers */
+                     "addl $0x20,%%esp;     " /* move to the eflags in the tf */
+                     "popfl;                " /* restore eflags */
+                     "popl %%esp;           " /* change to the new %esp */
+                     "subl $0x4,%%esp;      " /* move esp to the slot for eax */
+                     "pushl %%eax;          " /* save eax, will clobber soon */
+                     "subl $0xc,%%esp;      " /* move to notif_en_loc slot */
+                     "popl %%eax;           " /* load notif_enabaled addr */
+                     "movb $0x01,(%%eax);   " /* enable notifications */
+                     "popl %%eax;           " /* get notif_pending status */
+                     "pushfl;               " /* save eflags */
+                     "testb $0x01,(%%eax);  " /* test if a notif is pending */
+                     "jz 1f;                " /* if not pending, skip syscall */
+                     "popfl;                " /* restore eflags */
+                     "movb $0x00,(%%eax);   " /* clear pending */
+                     "pushl %%edx;          " /* save edx, syscall arg1 */
+                     "pushl %%ecx;          " /* save ecx, syscall arg2 */
+                     "pushl %%ebx;          " /* save ebx, syscall arg3 */
+                     "pushl %%esi;          " /* will be clobbered for errno */
+                     "popl %%edx;           " /* vcoreid, arg1 */
+                     "movl $0x0,%%ecx;      " /* send the null notif, arg2 */
+                     "movl $0x0,%%ecx;      " /* no u_ne message, arg3 */
+                     "movl %6,%%eax;        " /* syscall num */
+                     "int %7;               " /* fire the syscall */
+                     "popl %%esi;           " /* restore regs after syscall */
+                     "popl %%ebx;           "
+                     "popl %%ecx;           "
+                     "popl %%edx;           "
+                     "jmp 2f;               " /* skip 1:, already popped */
+                     "1: popfl;             " /* restore eflags */
+                     "popl %%eax;           " /* discard vcoreid */
+                     "2: popl %%eax;        " /* restore tf's %eax */
+                     "ret;                  " /* return to the new PC */
                      :
-                     : "g"(tf), "r"(tf->tf_esp), "r"(tf->tf_eip), "r"(notif_en_loc)
+                     : "g"(tf), "r"(tf->tf_esp), "r"(tf->tf_eip), "r"(vcoreid),
+                       "r"(&vcpd->notif_pending), "r"(&vcpd->notif_enabled),
+                       "i"(SYS_self_notify), "i"(T_SYSCALL)
                      : "memory");
 }
 
index 346419a..d806ca4 100644 (file)
@@ -8,18 +8,16 @@
  * A Userspace scheduler can call this when transitioning off the transition
  * stack.
  *
- * Here's how x86 does it:
- * Basically, it sets up the future stack pointer to have extra stuff after it,
- * and then it pops the registers, then pops the new context's stack
- * pointer.  Then it uses the extra stuff (the new PC is on the stack, the
- * location of notif_enabled, and a clobbered work register) to enable notifs,
- * restore the work reg, and then "ret".
+ * Make sure you clear the notif_pending flag, and then check the queue before
+ * calling this.  If notif_pending is not clear, this will self_notify this
+ * core, since it should be because we missed a notification message while
+ * notifs were disabled. 
  *
  * The important thing is that it can a notification after it enables
  * notifications, and when it gets resumed it can ultimately run the new
  * context.  Enough state is saved in the running context and stack to continue
  * running. */
-static inline void pop_ros_tf(struct user_trapframe *tf, bool *notif_en_loc)
+static inline void pop_ros_tf(struct user_trapframe *tf, uint32_t vcoreid)
 {
        // TODO: whatever sparc needs.
 }