Syscall return paths cleaned up
authorBarret Rhoden <brho@cs.berkeley.edu>
Fri, 19 Nov 2010 18:16:54 +0000 (10:16 -0800)
committerKevin Klues <klueska@cs.berkeley.edu>
Thu, 3 Nov 2011 00:35:56 +0000 (17:35 -0700)
The first syscall will return naturally via proc_restartcore() (which
does nothing if the core was already restarted and there is no
current_tf).

This also disables batching for now (not that anyone uses it, or the
asynchrony for that matter).  In the future, we can use kmsgs
(carefully) for batched syscalls, being careful of the *tf passed around
in the kmsg functions...

kern/arch/i686/trap.c
kern/arch/sparc/trap.c
kern/include/smp.h
kern/include/syscall.h
kern/src/smp.c
kern/src/syscall.c

index b294495..7a9ac9f 100644 (file)
@@ -238,13 +238,9 @@ trap_dispatch(trapframe_t *tf)
                case T_SYSCALL:
                        // check for userspace, for now
                        assert(tf->tf_cs != GD_KT);
-                       struct per_cpu_info* coreinfo = &per_cpu_info[core_id()];
-                       coreinfo->tf_retval_loc = &(tf->tf_regs.reg_eax);
                        /* Set up and run the async calls */
                        prep_syscalls(current, (struct syscall*)tf->tf_regs.reg_eax,
                                      tf->tf_regs.reg_edx);
-                       run_local_syscall();
-                       warn("No syscalls on a trap!");
                        break;
                default:
                        // Unexpected trap: The user process or the kernel has a bug.
@@ -399,16 +395,9 @@ void sysenter_callwrapper(struct trapframe *tf)
 
        if (in_kernel(tf))
                panic("sysenter from a kernel TF!!");
-       pcpui->tf_retval_loc = &(tf->tf_regs.reg_eax);
        /* Set up and run the async calls */
        prep_syscalls(current, (struct syscall*)tf->tf_regs.reg_eax,
                      tf->tf_regs.reg_esi);
-       run_local_syscall();            /* alternatively, we can call smp_idle() */
-       warn("No syscalls on a sysenter!");
-       /* careful here - we need to make sure that this current is the right
-        * process, which could be weird if the syscall blocked.  it would need to
-        * restore the proper value in current before returning to here.
-        * likewise, tf could be pointing to random gibberish. */
        proc_restartcore();
 }
 
@@ -508,6 +497,7 @@ void __kernel_message(struct trapframe *tf)
                                send_self_ipi(I_KERNEL_MSG);
                        /* Execute the kernel message */
                        assert(msg_cp.pc);
+                       /* TODO: when batching syscalls, this should be reread from cur_tf*/
                        msg_cp.pc(tf, msg_cp.srcid, msg_cp.arg0, msg_cp.arg1, msg_cp.arg2);
                }
        }
index 3c3f1fd..38e4d9c 100644 (file)
@@ -472,11 +472,7 @@ handle_syscall(trapframe_t* state)
 
        set_current_tf(pcpui, &state);
 
-       coreinfo->tf_retval_loc = &(state->gpr[8]);
-
        prep_syscalls(current, (struct syscall*)a1, a2);
-       run_local_syscall();
-       warn("No syscalls on a trap!");
 
        proc_restartcore();
 }
index c94a63b..1c36123 100644 (file)
@@ -29,12 +29,7 @@ struct per_cpu_info {
        struct trapframe *cur_tf;       /* user tf we came in on (can be 0) */
        struct trapframe actual_tf;     /* storage for cur_tf */
        struct kthread *spare;          /* useful when restarting */
-
-       /* Syscall management */
        struct syscall *cur_sysc;       /* ptr is into cur_proc's address space */
-       struct syscall *next_syscs;     /* other batched ones to do next */
-       unsigned int nr_syscs;
-       uintreg_t *tf_retval_loc;
 
 #ifdef __SHARC__
        // held spin-locks. this will have to go elsewhere if multiple kernel
index 0ca7a37..b07f4e0 100644 (file)
@@ -43,7 +43,6 @@ const static struct sys_table_entry syscall_table[];
 void prep_syscalls(struct proc *p, struct syscall *sysc, unsigned int nr_calls);
 intreg_t syscall(struct proc *p, uintreg_t sc_num, uintreg_t a0, uintreg_t a1,
                  uintreg_t a2, uintreg_t a3, uintreg_t a4, uintreg_t a5);
-void run_local_syscall(void);
 void set_errno(int errno);
 
 /* Tracing functions */
index 69efde1..517a738 100644 (file)
@@ -41,16 +41,11 @@ atomic_t outstanding_calls = 0;
 void smp_idle(void)
 {
        int8_t state = 0;
-       per_cpu_info_t *pcpui = &per_cpu_info[core_id()];
+       struct per_cpu_info *pcpui = &per_cpu_info[core_id()];
 
        /* There was a process running here, and we should return to it */
-       /* TODO: gut this */
        if (pcpui->cur_tf) {                    /* aka, current_tf */
                assert(pcpui->cur_proc);        /* aka, current */
-               /* We also check in run_local_syscall().  This is for sys_exec() */
-               if (pcpui->nr_syscs)
-                       run_local_syscall();
-               /* Now we're done, so return */
                proc_restartcore();
                assert(0);
        }
index 9e99c60..2cf2d8d 100644 (file)
@@ -437,7 +437,9 @@ static ssize_t sys_fork(env_t* e)
  * argv and envp are magically bundled in procinfo for now.  Keep in sync with
  * glibc's sysdeps/ros/execve.c.  Once past a certain point, this function won't
  * return.  It assumes (and checks) that it is current.  Don't give it an extra
- * refcnt'd *p (syscall won't do that). */
+ * refcnt'd *p (syscall won't do that). 
+ * Note: if someone batched syscalls with this call, they could clobber their
+ * old memory (and will likely PF and die).  Don't do it... */
 static int sys_exec(struct proc *p, char *path, size_t path_l,
                     struct procinfo *pi)
 {
@@ -518,10 +520,6 @@ success:
        schedule_proc(p);
        spin_unlock(&p->proc_lock);
 all_out:
-       /* When we idle, we don't want to try executing other syscalls.  If exec
-        * succeeded (or the proc was destroyed) it'd just be wrong. */
-       pcpui->next_syscs = 0;
-       pcpui->nr_syscs = 0;
        /* we can't return, since we'd write retvals to the old location of the
         * sycall struct (which has been freed and is in the old userspace) (or has
         * already been written to).*/
@@ -1380,50 +1378,35 @@ intreg_t syscall(struct proc *p, uintreg_t sc_num, uintreg_t a0, uintreg_t a1,
        return syscall_table[sc_num].call(p, a0, a1, a2, a3, a4, a5);
 }
 
-/* A process can trap and call this function, which will set up the core to
- * handle all the syscalls.  a.k.a. "sys_debutante(needs, wants)" */
-void prep_syscalls(struct proc *p, struct syscall *sysc, unsigned int nr_syscs)
+/* Execute the syscall on the local core */
+static void run_local_syscall(struct syscall *sysc)
 {
-       int retval;
        struct per_cpu_info *pcpui = &per_cpu_info[core_id()];
-       /* TODO: (UMEM) assert / pin the memory range sysc for nr_syscs.  Not sure
-        * how we'll know it is done to unpin it. (might need to handle it in
-        * abandon_core() or smp_idle(). */
-       user_mem_assert(p, sysc, nr_syscs * sizeof(struct syscall), PTE_USER_RW);
-       /* TEMP! */
-       if (nr_syscs != 1)
-               warn("Debutante calls: %d\n", nr_syscs);
-       /* Set up this core to process the local call */
-       *pcpui->tf_retval_loc = 0;      /* current_tf's retval says how many are done */
-       pcpui->next_syscs = sysc;
-       pcpui->nr_syscs = nr_syscs;
-}
 
-/* This returns if there are no syscalls to do, o/w it always calls smp_idle()
- * afterwards. */
-void run_local_syscall(void)
-{
-       struct per_cpu_info *pcpui = &per_cpu_info[core_id()];
-       struct syscall *sysc;
-
-       if (!pcpui->nr_syscs) {
-               /* TODO: UMEM - stop pinning their memory.  Note, we may need to do this
-                * in other places.  This also will be tricky with exec, which doesn't
-                * have the same memory map anymore */
-               return;
-       }
-       sysc = pcpui->next_syscs++;             /* get the next */
+       /* TODO: (UMEM) assert / pin the memory for the sysc */
+       user_mem_assert(pcpui->cur_proc, sysc, sizeof(struct syscall), PTE_USER_RW);
        pcpui->cur_sysc = sysc;                 /* let the core know which sysc it is */
-       pcpui->nr_syscs--;                              /* one less to do */
-       (*pcpui->tf_retval_loc)++;              /* one more started */
        sysc->retval = syscall(pcpui->cur_proc, sysc->num, sysc->arg0, sysc->arg1,
                               sysc->arg2, sysc->arg3, sysc->arg4, sysc->arg5);
        sysc->flags |= SC_DONE;
-       /* regardless of whether the call blocked or not, we smp_idle().  If it was
-        * the last call, it'll return to the process.  If there are more, it will
-        * do them. */
-       smp_idle();
-       assert(0);
+       /* Can unpin at this point */
+}
+
+/* A process can trap and call this function, which will set up the core to
+ * handle all the syscalls.  a.k.a. "sys_debutante(needs, wants)".  If there is
+ * at least one, it will run it directly. */
+void prep_syscalls(struct proc *p, struct syscall *sysc, unsigned int nr_syscs)
+{
+       int retval;
+       struct per_cpu_info *pcpui = &per_cpu_info[core_id()];
+       if (!nr_syscs)
+               return;
+       /* For all after the first call, send ourselves a KMSG (TODO). */
+       if (nr_syscs != 1)
+               warn("Only one supported (Debutante calls: %d)\n", nr_syscs);
+       /* Call the first one directly.  (we already checked to make sure there is
+        * 1) */
+       run_local_syscall(sysc);
 }
 
 /* Syscall tracing */