proc_wakeup() replaces schedule_scp()
authorBarret Rhoden <brho@cs.berkeley.edu>
Wed, 25 Apr 2012 02:17:10 +0000 (19:17 -0700)
committerBarret Rhoden <brho@cs.berkeley.edu>
Wed, 25 Apr 2012 02:17:10 +0000 (19:17 -0700)
All places wanting to make sure a proc runs now can spam proc_wakeup().
This commit makes new-proc calls and yield-related calls that aren't
already in the ksched call proc_wakeup().

This should also have cleaned up the last of the deadlockable lock
orderings (ever since we had proc_destroy in the ksched, it was possible
to deadlock during a concurrent sched_scp()).

Finally, this just gets rid of schedule_scp(), which was in no way
related to the __schedule_scp().  Ugh.

kern/include/process.h
kern/include/schedule.h
kern/src/kthread.c
kern/src/monitor.c
kern/src/process.c
kern/src/schedule.c
kern/src/syscall.c
kern/src/testing.c
tests/spawn.c

index 0d28cff..02ce9bc 100644 (file)
@@ -73,7 +73,7 @@ void __proc_run_m(struct proc *p);
 void proc_restartcore(void);
 bool __proc_destroy(struct proc *p, uint32_t *pc_arr, uint32_t *nr_revoked);
 int __proc_change_to_m(struct proc *p);
-void __proc_yield_s(struct proc *p, struct trapframe *tf);
+void __proc_save_context_s(struct proc *p, struct trapframe *tf);
 void proc_yield(struct proc *SAFE p, bool being_nice);
 void proc_notify(struct proc *p, uint32_t vcoreid);
 void __proc_wakeup(struct proc *p);
index 04418c8..5005f90 100644 (file)
@@ -27,9 +27,6 @@ void schedule_init(void);
 /* Tell the ksched about the process, which it will track cradle-to-grave */
 void register_proc(struct proc *p);
 
-/* _S is now runnable, tell the ksched to try to run it. */
-void schedule_scp(struct proc *p);
-
 /* Makes sure p is runnable.  Callers include event delivery, SCP yield, and new
  * SCPs.  Will trigger the __sched_.cp_wakeup() callbacks. */
 void proc_wakeup(struct proc *p);
index 0427704..29020e4 100644 (file)
@@ -240,9 +240,10 @@ void __launch_kthread(struct trapframe *tf, uint32_t srcid, long a0, long a1,
                if (pcpui->owning_proc->state == PROC_RUNNING_S) {
                        spin_lock(&pcpui->owning_proc->proc_lock);
                        /* Wrap up / yield the _S proc */
-                       __proc_yield_s(pcpui->owning_proc, pcpui->cur_tf);
+                       __proc_set_state(pcpui->owning_proc, PROC_WAITING);
+                       __proc_save_context_s(pcpui->owning_proc, current_tf);
                        spin_unlock(&pcpui->owning_proc->proc_lock);
-                       schedule_scp(p);
+                       proc_wakeup(p);
                        abandon_core();
                        /* prob need to clear the owning proc?  this is some old shit, so
                         * don't just uncomment it. */
index d334e2c..03ff404 100644 (file)
@@ -297,11 +297,7 @@ int mon_bin_run(int argc, char *NTS *NT COUNT(argc) argv, trapframe_t *tf)
        char *p_envp[] = {"LD_LIBRARY_PATH=/lib", 0};
        struct proc *p = proc_create(program, p_argv, p_envp);
        kfree(p_argv);
-
-       spin_lock(&p->proc_lock);
-       __proc_set_state(p, PROC_RUNNABLE_S);
-       schedule_scp(p);
-       spin_unlock(&p->proc_lock);
+       proc_wakeup(p);
        proc_decref(p); /* let go of the reference created in proc_create() */
        kref_put(&program->f_kref);
        /* Make a scheduling decision.  You might not get the process you created,
index 25645a3..05a4fe0 100644 (file)
@@ -872,21 +872,13 @@ static uint32_t get_pcoreid(struct proc *p, uint32_t vcoreid)
 
 /* Helper: saves the SCP's tf state and unmaps vcore 0.  In the future, we'll
  * probably use vc0's space for env_tf and the silly state. */
-static void __proc_save_context_s(struct proc *p, struct trapframe *tf)
+void __proc_save_context_s(struct proc *p, struct trapframe *tf)
 {
        p->env_tf= *tf;
        env_push_ancillary_state(p);                    /* TODO: (HSS) */
        __unmap_vcore(p, 0);    /* VC# keep in sync with proc_run_s */
 }
 
-/* Helper function: yields / wraps up current_tf */
-void __proc_yield_s(struct proc *p, struct trapframe *tf)
-{
-       assert(p->state == PROC_RUNNING_S);
-       __proc_set_state(p, PROC_RUNNABLE_S);
-       __proc_save_context_s(p, tf);
-}
-
 /* Yields the calling core.  Must be called locally (not async) for now.
  * - If RUNNING_S, you just give up your time slice and will eventually return,
  *   possibly after WAITING on an event.
@@ -960,12 +952,16 @@ void proc_yield(struct proc *SAFE p, bool being_nice)
                                 * hasn't already written notif_pending will have seen WAITING,
                                 * and will be spinning while we do this. */
                                __proc_save_context_s(p, current_tf);
+                               spin_unlock(&p->proc_lock);     /* note irqs are not enabled yet */
                        } else {
-                               /* yielding to allow other processes to run */
-                               __proc_yield_s(p, current_tf);
-                               schedule_scp(p);
+                               /* yielding to allow other processes to run.  we're briefly
+                                * WAITING, til we are woken up */
+                               __proc_set_state(p, PROC_WAITING);
+                               __proc_save_context_s(p, current_tf);
+                               spin_unlock(&p->proc_lock);     /* note irqs are not enabled yet */
+                               /* immediately wake up the proc (makes it runnable) */
+                               proc_wakeup(p);
                        }
-                       spin_unlock(&p->proc_lock);     /* note that irqs are not enabled yet */
                        goto out_yield_core;
                case (PROC_RUNNING_M):
                        break;                          /* will handle this stuff below */
index 057acef..33faf8a 100644 (file)
@@ -85,12 +85,9 @@ static void __kalarm(struct alarm_waiter *waiter)
 
 void schedule_init(void)
 {
-       TAILQ_INIT(&runnable_scps);
-       TAILQ_INIT(&all_mcps);
        assert(!core_id());             /* want the alarm on core0 for now */
        init_awaiter(&ksched_waiter, __kalarm);
        set_ksched_alarm();
-
        /* Ghetto old idle core init */
        /* Init idle cores. Core 0 is the management core. */
        spin_lock(&idle_lock);
@@ -178,17 +175,6 @@ void register_proc(struct proc *p)
        spin_unlock(&sched_lock);
 }
 
-/* TODO: the proc lock is currently held for sched and register */
-/* sched_scp tells us to try and run the scp
- * TODO: change this horrible name */
-void schedule_scp(struct proc *p)
-{
-       spin_lock(&sched_lock);
-       printd("Scheduling PID: %d\n", p->pid);
-       switch_lists(p, &unrunnable_scps, &runnable_scps);
-       spin_unlock(&sched_lock);
-}
-
 /* Returns 0 if it succeeded, an error code otherwise. */
 int proc_change_to_m(struct proc *p)
 {
@@ -284,7 +270,11 @@ static bool __schedule_scp(void)
                if (pcpui->owning_proc) {
                        printd("Descheduled %d in favor of %d\n", pcpui->owning_proc->pid,
                               p->pid);
-                       __proc_yield_s(pcpui->owning_proc, pcpui->cur_tf);
+                       /* locking just to be safe */
+                       spin_lock(&p->proc_lock);
+                       __proc_set_state(pcpui->owning_proc, PROC_RUNNABLE_S);
+                       __proc_save_context_s(pcpui->owning_proc, pcpui->cur_tf);
+                       spin_unlock(&p->proc_lock);
                        /* round-robin the SCPs (inserts at the end of the queue) */
                        switch_lists(pcpui->owning_proc, &unrunnable_scps, &runnable_scps);
                        clear_owning_proc(pcoreid);
index d90cb46..ce4a258 100644 (file)
@@ -312,24 +312,26 @@ static error_t sys_proc_run(struct proc *p, unsigned pid)
        struct proc *target = pid2proc(pid);
        error_t retval = 0;
 
-       if (!target)
-               return -EBADPROC;
-       // note we can get interrupted here. it's not bad.
-       spin_lock(&p->proc_lock);
-       // make sure we have access and it's in the right state to be activated
+       if (!target) {
+               set_errno(EBADPROC);
+               return -1;
+       }
+       /* make sure we have access and it's in the right state to be activated */
        if (!proc_controls(p, target)) {
-               proc_decref(target);
-               retval = -EPERM;
+               set_errno(EPERM);
+               goto out_error;
        } else if (target->state != PROC_CREATED) {
-               proc_decref(target);
-               retval = -EINVAL;
-       } else {
-               __proc_set_state(target, PROC_RUNNABLE_S);
-               schedule_scp(target);
+               set_errno(EINVAL);
+               goto out_error;
        }
-       spin_unlock(&p->proc_lock);
+       /* Note a proc can spam this for someone it controls.  Seems safe - if it
+        * isn't we can change it. */
+       proc_wakeup(target);
        proc_decref(target);
-       return retval;
+       return 0;
+out_error:
+       proc_decref(target);
+       return -1;
 }
 
 /* Destroy proc pid.  If this is called by the dying process, it will never
@@ -446,9 +448,9 @@ static ssize_t sys_fork(env_t* e)
        #endif
 
        clone_files(&e->open_files, &env->open_files);
+       /* FYI: once we call ready, the proc is open for concurrent usage */
        __proc_ready(env);
-       __proc_set_state(env, PROC_RUNNABLE_S);
-       schedule_scp(env);
+       proc_wakeup(env);
 
        // don't decref the new process.
        // that will happen when the parent waits for it.
@@ -546,12 +548,12 @@ mid_error:
 early_error:
        finish_current_sysc(-1);
 success:
-       /* Here's how we'll restart the new (or old) process: */
+       /* Here's how we restart the new (on success) or old (on failure) proc: */
        spin_lock(&p->proc_lock);
        __unmap_vcore(p, 0);    /* VC# keep in sync with proc_run_s */
-       __proc_set_state(p, PROC_RUNNABLE_S);
-       schedule_scp(p);
+       __proc_set_state(p, PROC_WAITING);      /* fake a yield */
        spin_unlock(&p->proc_lock);
+       proc_wakeup(p);
 all_out:
        /* we can't return, since we'd write retvals to the old location of the
         * syscall struct (which has been freed and is in the old userspace) (or has
index 3be6a78..50440eb 100644 (file)
@@ -1033,10 +1033,7 @@ void test_ucq(void)
        }
        char *p_envp[] = {"LD_LIBRARY_PATH=/lib", 0};
        struct proc *p = proc_create(program, 0, p_envp);
-       spin_lock(&p->proc_lock);
-       __proc_set_state(p, PROC_RUNNABLE_S);
-       schedule_scp(p);
-       spin_unlock(&p->proc_lock);
+       proc_wakeup(p);
        /* instead of getting rid of the reference created in proc_create, we'll put
         * it in the awaiter */
        waiter->data = p;
index 2cebf99..f2a46e9 100644 (file)
@@ -36,13 +36,13 @@ int main(int argc, char **argv, char **envp)
        #endif
        printf("U: attempting to create and run hello\n");
        p_argv[0] = filename;
-       printf("SPAWN, pid %d, filename %08p\n", getpid(), filename);
+       printf("SPAWN, I'm pid %d, filename %s\n", getpid(), filename);
        child_pid[0] = sys_proc_create(FILENAME, strlen(FILENAME), p_argv, p_envp);
        if (child_pid[0] <= 0)
                printf("Failed to create the child\n");
        else
                if (sys_proc_run(child_pid[0]) < 0)
-                       printf("Failed to run the child\n");
+                       printf("Failed to run the child (pid %d)\n", child_pid[0]);
 
        #if 0
        printf("U: attempting to create and run another hello\n");