Parent processes track children
authorBarret Rhoden <brho@cs.berkeley.edu>
Mon, 22 Oct 2012 22:43:48 +0000 (15:43 -0700)
committerBarret Rhoden <brho@cs.berkeley.edu>
Tue, 23 Oct 2012 01:25:33 +0000 (18:25 -0700)
Instead of relying on just the child having the ppid number.  Plus,
there's a little more info in the monitor commands (ps, procinfo), and
trywait was cleaned up a bit.

If the comment about why children don't keep pointers to their parents
doesn't make sense, look over the kref Documentation and think about how
we could ever safely disown a child (like if a parent terminates first).

kern/include/env.h
kern/include/process.h
kern/src/process.c
kern/src/syscall.c

index 71920d3..546e55b 100644 (file)
 #include <vfs.h>
 #include <schedule.h>
 
-/* List def for the three vcore lists */
 TAILQ_HEAD(vcore_tailq, vcore);
+/* 'struct proc_list' declared in sched.h (not ideal...) */
 
 // TODO: clean this up.
 struct proc {
-       TAILQ_ENTRY(proc) proc_arsc_link NOINIT; // Free list link pointers for the arsc list
+       TAILQ_ENTRY(proc) proc_arsc_link;
+       TAILQ_ENTRY(proc) sibling_link;
        spinlock_t proc_lock;
        trapframe_t env_tf;                                             // Saved registers
        ancillary_state_t env_ancillary_state;  // State saved when descheduled
        pid_t pid;
-       pid_t ppid;                 // Parent's PID
-       pid_t exitcode;                         // exit() param or main() return value
+       /* Tempting to add a struct proc *parent, but we'd need to protect the use
+        * of that reference from concurrent parent-death (letting init inherit
+        * children, etc), which is basically what we do when we do pid2proc.  If we
+        * do add *parent, it'll be a weak ref, and you'll need to lock the child to
+        * kref_get or to remove the pointer. */
+       pid_t ppid;                                     /* parent's pid, not a reference */
+       struct proc_list children;      /* protected by the proc lock for now */
+       int exitcode;                           /* exit() param or main() return value */
        struct semaphore state_change;
        uint32_t state;                         // Status of the process
        struct kref p_kref;             /* Refcnt */
index 227b955..7e3a984 100644 (file)
@@ -72,6 +72,7 @@ void proc_run_s(struct proc *p);
 void __proc_run_m(struct proc *p);
 void proc_restartcore(void);
 void proc_destroy(struct proc *p);
+void proc_disown_child(struct proc *parent, struct proc *child);
 int proc_change_to_m(struct proc *p);
 void __proc_save_context_s(struct proc *p, struct trapframe *tf);
 void proc_yield(struct proc *SAFE p, bool being_nice);
index 46f1082..2d6b302 100644 (file)
@@ -250,8 +250,16 @@ error_t proc_alloc(struct proc **pp, struct proc *parent)
        /* Set the basic status variables. */
        spinlock_init(&p->proc_lock);
        p->exitcode = 1337;     /* so we can see processes killed by the kernel */
+       if (parent) {
+               p->ppid = parent->pid;
+               spin_lock(&parent->proc_lock);
+               TAILQ_INSERT_TAIL(&parent->children, p, sibling_link);
+               spin_unlock(&parent->proc_lock);
+       } else {
+               p->ppid = 0;
+       }
+       TAILQ_INIT(&p->children);
        init_sem(&p->state_change, 0);
-       p->ppid = parent ? parent->pid : 0;
        p->state = PROC_CREATED; /* shouldn't go through state machine for init */
        p->env_flags = 0;
        p->env_entry = 0; // cheating.  this really gets set later
@@ -750,6 +758,20 @@ void proc_destroy(struct proc *p)
                kthread_runnable(sleeper);
 }
 
+/* Called when a parent is done with its child, and no longer wants to track the
+ * child, nor to allow the child to track it. */
+void proc_disown_child(struct proc *parent, struct proc *child)
+{
+       /* lock protects from concurrent inserts / removals from the list */
+       spin_lock(&parent->proc_lock);
+       TAILQ_REMOVE(&parent->children, child, sibling_link);
+       /* After this, the child won't be able to get more refs to us, but it may
+        * still have some references in running code. */
+       child->ppid = 0;
+       spin_unlock(&parent->proc_lock);
+       proc_decref(child);     /* ref that was keeping the child alive after dying */
+}
+
 /* Turns *p into an MCP.  Needs to be called from a local syscall of a RUNNING_S
  * process.  Returns 0 if it succeeded, an error code otherwise. */
 int proc_change_to_m(struct proc *p)
@@ -1978,10 +2000,10 @@ void print_allpids(void)
        {
                struct proc *p = (struct proc*)item;
                assert(p);
-               printk("%8d %s\n", p->pid, procstate2str(p->state));
+               printk("%8d %-10s %6d\n", p->pid, procstate2str(p->state), p->ppid);
        }
-       printk("PID      STATE    \n");
-       printk("------------------\n");
+       printk("     PID STATE      Parent    \n");
+       printk("------------------------------\n");
        spin_lock(&pid_hash_lock);
        hash_for_each(pid_hash, print_proc_state);
        spin_unlock(&pid_hash_lock);
@@ -1990,7 +2012,7 @@ void print_allpids(void)
 void print_proc_info(pid_t pid)
 {
        int j = 0;
-       struct proc *p = pid2proc(pid);
+       struct proc *child, *p = pid2proc(pid);
        struct vcore *vc_i;
        if (!p) {
                printk("Bad PID.\n");
@@ -2030,9 +2052,9 @@ void print_proc_info(pid_t pid)
                               file_name(files->fd_array[i].fd_file));
                }
        spin_unlock(&files->lock);
-       /* No one cares, and it clutters the terminal */
-       //printk("Vcore 0's Last Trapframe:\n");
-       //print_trapframe(&p->env_tf);
+       printk("Children: (PID (struct proc *))\n");
+       TAILQ_FOREACH(child, &p->children, sibling_link)
+               printk("\t%d (%08p)\n", child->pid, child);
        /* no locking / unlocking or refcnting */
        // spin_unlock(&p->proc_lock);
        proc_decref(p);
index 6ed8691..6a7d11a 100644 (file)
@@ -563,7 +563,11 @@ all_out:
        smp_idle();                             /* will reenable interrupts */
 }
 
-static ssize_t sys_trywait(env_t* e, pid_t pid, int* status)
+/* Note: we only allow waiting on children (no such thing as threads, for
+ * instance).  Right now we only allow waiting on termination (not signals),
+ * and we don't have a way for parents to disown their children (such as
+ * ignoring SIGCHLD, see man 2 waitpid's Notes). */
+static int sys_trywait(struct proc *parent, pid_t pid, int *status)
 {
        /* TODO:
         * - WAIT should handle stop and start via signal too
@@ -571,53 +575,38 @@ static ssize_t sys_trywait(env_t* e, pid_t pid, int* status)
         * - should have an option for WNOHANG, and a bunch of other things.
         * - think about what functions we want to work with MCPS
         *   */
-       struct proc* p = pid2proc(pid);
-
-       // TODO: this syscall is racy, so we only support for single-core procs
-       if(e->state != PROC_RUNNING_S)
-               return -1;
-
-       // TODO: need to use errno properly.  sadly, ROS error codes conflict..
-
-       if(p)
-       {
-               ssize_t ret;
-
-               if(current->pid == p->ppid)
-               {
-                       /* Block til there is some activity */
-                       if (!(p->state == PROC_DYING)) {
-                               sleep_on(&p->state_change);
-                       }
-                       if(p->state == PROC_DYING)
-                       {
-                               memcpy_to_user(e,status,&p->exitcode,sizeof(int));
-                               printd("[PID %d] waited for PID %d (code %d)\n",
-                                      e->pid,p->pid,p->exitcode);
-                               ret = 0;
-                       }
-                       else // not dead yet
-                       {
-                               warn("Should not have reached here.");
-                               set_errno(ESUCCESS);
-                               ret = -1;
-                       }
-               }
-               else // not a child of the calling process
-               {
-                       set_errno(EPERM);
-                       ret = -1;
-               }
+       struct proc* child = pid2proc(pid);
+       int ret = -1;
+       int ret_status;
 
-               // if the wait succeeded, decref twice
-               if (ret == 0)
-                       proc_decref(p);
-               proc_decref(p);
-               return ret;
+       if (!child) {
+               set_errno(ECHILD);      /* ECHILD also used for no proc */
+               goto out;
        }
-
-       set_errno(EPERM);
-       return -1;
+       if (!(parent->pid == child->ppid)) {
+               set_errno(ECHILD);
+               goto out_decref;
+       }
+       /* Block til there is some activity (DYING for now) */
+       if (!(child->state == PROC_DYING)) {
+               sleep_on(&child->state_change);
+               cpu_relax();
+       }
+       assert(child->state == PROC_DYING);
+       ret_status = child->exitcode;
+       /* wait succeeded - need to clean up the proc. */
+       proc_disown_child(parent, child);
+       /* fall through */
+out_success:
+       /* ignoring the retval here - don't care if they have a bad addr. */
+       memcpy_to_user(parent, status, &ret_status, sizeof(ret_status));
+       printd("[PID %d] waited for PID %d (code %d)\n", parent->pid,
+              pid, ret_status);
+       ret = 0;
+out_decref:
+       proc_decref(child);
+out:
+       return ret;
 }
 
 /************** Memory Management Syscalls **************/