Cleans up fork/exec's procinfo/data handling
authorBarret Rhoden <brho@cs.berkeley.edu>
Tue, 13 Sep 2011 18:35:06 +0000 (11:35 -0700)
committerKevin Klues <klueska@cs.berkeley.edu>
Thu, 3 Nov 2011 00:36:06 +0000 (17:36 -0700)
This makes the process fork creates new/fresh by default, and things
needed from procinfo are copied over one at a time.  The old way was
that the procinfo/procdata were identical.

In general, I tend to really dislike fork/exec: lots of hacks and pains
in the ass.  At least it isn't as bad as it used to be (when the proc
creation paths were totally separate).

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

index 4f399ff..c5123d3 100644 (file)
@@ -67,7 +67,6 @@ extern uint32_t LCKD(&idle_lock) num_idlecores;
 
 /* Initialization */
 void proc_init(void);
-void proc_init_procinfo(struct proc *p);
 
 /* Process management: */
 error_t proc_alloc(struct proc **pp, struct proc *parent);
index 99cfc1d..c5f543a 100644 (file)
@@ -42,6 +42,7 @@ typedef struct procinfo {
        size_t max_vcores;
        uint64_t tsc_freq;
        void* heap_bottom;
+       /* for traditional forks, these two need to be memcpy'd over: */
        char* argp[PROCINFO_MAX_ARGP];
        char argbuf[PROCINFO_ARGBUF_SIZE];
        /* glibc relies on stuff above this point.  if you change it, you need to
index 181fc3e..4a3930c 100644 (file)
@@ -83,6 +83,8 @@ WRITES(e->env_pgdir, e->env_cr3, e->procinfo, e->procdata)
                goto env_setup_vm_error_i;
        if (!(e->procdata = get_cont_pages(LOG2_UP(PROCDATA_NUM_PAGES), 0)))
                goto env_setup_vm_error_d;
+       /* Normally we'd 0 the pages here.  We handle it in proc_init_proc*.  Don't
+        * start the process without calling those. */
        for (int i = 0; i < PROCINFO_NUM_PAGES; i++) {
                if (page_insert(e->env_pgdir, kva2page((void*)e->procinfo + i *
                                PGSIZE), (void*SNT)(UINFO + i*PGSIZE), PTE_USER_RO) < 0)
@@ -93,9 +95,6 @@ WRITES(e->env_pgdir, e->env_cr3, e->procinfo, e->procdata)
                                PGSIZE), (void*SNT)(UDATA + i*PGSIZE), PTE_USER_RW) < 0)
                        goto env_setup_vm_error;
        }
-       memset(e->procinfo, 0, sizeof(struct procinfo));
-       memset(e->procdata, 0, sizeof(struct procdata));
-
        /* Finally, set up the Global Shared Data page for all processes.  Can't be
         * trusted, but still very useful at this stage for us.  Consider removing
         * when we have real processes (TODO). 
index 4262ee1..015a36a 100644 (file)
@@ -241,22 +241,26 @@ void proc_init(void)
 }
 
 /* Be sure you init'd the vcore lists before calling this. */
-void proc_init_procinfo(struct proc* p)
+static void proc_init_procinfo(struct proc* p)
 {
-       memset(&p->procinfo->vcoremap, 0, sizeof(p->procinfo->vcoremap));
-       memset(&p->procinfo->pcoremap, 0, sizeof(p->procinfo->pcoremap));
-       p->procinfo->num_vcores = 0;
-       p->procinfo->coremap_seqctr = SEQCTR_INITIALIZER;
-       // TODO: change these too
        p->procinfo->pid = p->pid;
        p->procinfo->ppid = p->ppid;
-       p->procinfo->tsc_freq = system_timing.tsc_freq;
        // TODO: maybe do something smarter here
 #ifdef __CONFIG_DISABLE_SMT__
        p->procinfo->max_vcores = num_cpus >> 1;
 #else
        p->procinfo->max_vcores = MAX(1,num_cpus-num_mgmtcores);
 #endif /* __CONFIG_DISABLE_SMT__ */
+       p->procinfo->tsc_freq = system_timing.tsc_freq;
+       p->procinfo->heap_bottom = (void*)UTEXT;
+       /* 0'ing the arguments.  Some higher function will need to set them */
+       memset(p->procinfo->argp, 0, sizeof(p->procinfo->argp));
+       memset(p->procinfo->argbuf, 0, sizeof(p->procinfo->argbuf));
+       /* 0'ing the vcore/pcore map.  Will link the vcores later. */
+       memset(&p->procinfo->vcoremap, 0, sizeof(p->procinfo->vcoremap));
+       memset(&p->procinfo->pcoremap, 0, sizeof(p->procinfo->pcoremap));
+       p->procinfo->num_vcores = 0;
+       p->procinfo->coremap_seqctr = SEQCTR_INITIALIZER;
        /* For now, we'll go up to the max num_cpus (at runtime).  In the future,
         * there may be cases where we can have more vcores than num_cpus, but for
         * now we'll leave it like this. */
@@ -265,6 +269,11 @@ void proc_init_procinfo(struct proc* p)
        }
 }
 
+static void proc_init_procdata(struct proc *p)
+{
+       memset(p->procdata, 0, sizeof(struct procdata));
+}
+
 /* Allocates and initializes a process, with the given parent.  Currently
  * writes the *p into **pp, and returns 0 on success, < 0 for an error.
  * Errors include:
@@ -301,8 +310,7 @@ error_t proc_alloc(struct proc **pp, struct proc *parent)
        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
-       p->procinfo->heap_bottom = (void*)UTEXT;
-       p->heap_top = (void*)UTEXT;
+       p->heap_top = (void*)UTEXT;     /* heap_bottom set in proc_init_procinfo */
        memset(&p->resources, 0, sizeof(p->resources));
        memset(&p->env_ancillary_state, 0, sizeof(p->env_ancillary_state));
        memset(&p->env_tf, 0, sizeof(p->env_tf));
@@ -312,9 +320,9 @@ error_t proc_alloc(struct proc **pp, struct proc *parent)
        TAILQ_INIT(&p->online_vcs);
        TAILQ_INIT(&p->bulk_preempted_vcs);
        TAILQ_INIT(&p->inactive_vcs);
-       /* Initialize the contents of the e->procinfo structure */
+       /* Init procinfo/procdata.  Procinfo's argp/argb are 0'd */
        proc_init_procinfo(p);
-       /* Initialize the contents of the e->procdata structure */
+       proc_init_procdata(p);
 
        /* Initialize the generic sysevent ring buffer */
        SHARED_RING_INIT(&p->procdata->syseventring);
index 8bfb0a4..9b0295f 100644 (file)
@@ -436,12 +436,16 @@ static ssize_t sys_fork(env_t* e)
                return 0;
        }
 
-       // TODO: (PC) this won't work.  Needs revisiting.
-       // copy procdata and procinfo
-       memcpy(env->procdata,e->procdata,sizeof(struct procdata));
-       memcpy(env->procinfo,e->procinfo,sizeof(struct procinfo));
-       env->procinfo->pid = env->pid;
-       env->procinfo->ppid = env->ppid;
+       /* In general, a forked process should be a fresh process, and we copy over
+        * whatever stuff is needed between procinfo/procdata. */
+       /* Copy over the procinfo argument stuff in case they don't exec */
+       memcpy(env->procinfo->argp, e->procinfo->argp, sizeof(e->procinfo->argp));
+       memcpy(env->procinfo->argbuf, e->procinfo->argbuf,
+              sizeof(e->procinfo->argbuf));
+       #ifdef __i386__
+       /* new guy needs to know about ldt (everything else in procdata is fresh */
+       env->procdata->ldt = e->procdata->ldt;
+       #endif
 
        /* for now, just copy the contents of every present page in the entire
         * address space. */
@@ -517,11 +521,10 @@ static int sys_exec(struct proc *p, char *path, size_t path_l,
                                   sizeof(pi->argbuf)))
                goto mid_error;
        /* This is the point of no return for the process. */
-       /* TODO: issues with this: Need to also assert there are no outstanding
-        * users of the sysrings.  the ldt page will get freed shortly, so that's
-        * okay.  Potentially issues with the nm and vcpd if we were in _M before
-        * and someone is trying to notify. */
-       memset(p->procdata, 0, sizeof(procdata_t));
+       #ifdef __i386__
+       /* clear this, so the new program knows to get an LDT */
+       p->procdata->ldt = 0;
+       #endif
        destroy_vmrs(p);
        close_all_files(&p->open_files, TRUE);
        env_user_mem_free(p, 0, UMAPTOP);