Preserves mmaps of MAP_SHARED files across fork()
authorBarret Rhoden <brho@cs.berkeley.edu>
Fri, 3 Feb 2012 23:44:09 +0000 (15:44 -0800)
committerBarret Rhoden <brho@cs.berkeley.edu>
Fri, 3 Feb 2012 23:44:09 +0000 (15:44 -0800)
mm.c's copy_pages() uses env_user_mem_walk(), which is probably faster
than a pgdir walk for each PTE, esp on 64 bit architectures.  Still, I
kept around code that can do the copy_page manually, since I'm not a
huge fan of the mem walk - I'll probably change its interface when the
need arises.

kern/include/mm.h
kern/src/mm.c
kern/src/syscall.c
tests/test_mmap_ipc.c

index e5debb3..550eafa 100644 (file)
@@ -59,7 +59,7 @@ struct vm_region *find_vmr(struct proc *p, uintptr_t va);
 struct vm_region *find_first_vmr(struct proc *p, uintptr_t va);
 void isolate_vmrs(struct proc *p, uintptr_t va, size_t len);
 void destroy_vmrs(struct proc *p);
-void duplicate_vmrs(struct proc *p, struct proc *new_p);
+int duplicate_vmrs(struct proc *p, struct proc *new_p);
 void print_vmrs(struct proc *p);
 
 /* mmap() related functions.  These manipulate VMRs and change the hardware page
index 7d59b50..cb81adf 100644 (file)
@@ -243,19 +243,94 @@ void destroy_vmrs(struct proc *p)
                destroy_vmr(vm_i);
 }
 
-/* This will make new_p have the same VMRs as p, though it does nothing to
- * ensure the physical pages or whatever are shared/mapped/copied/whatever.
+/* Helper: copies the contents of pages from p to new p.  For pages that aren't
+ * present, once we support swapping or CoW, we can do something more
+ * intelligent.  0 on success, -ERROR on failure. */
+static int copy_pages(struct proc *p, struct proc *new_p, uintptr_t va_start,
+                      uintptr_t va_end)
+{
+       /* Sanity checks.  If these fail, we had a screwed up VMR.
+        * Check for: alignment, wraparound, or userspace addresses */
+       if ((PGOFF(va_start)) ||
+           (PGOFF(va_end)) ||
+           (va_end < va_start) ||      /* now, start > UMAPTOP -> end > UMAPTOP */
+           (va_end > UMAPTOP)) {
+               warn("VMR mapping is probably screwed up (%08p - %08p)", va_start,
+                    va_end);
+               return -EINVAL;
+       }
+       int copy_page(struct proc *p, pte_t *pte, void *va, void *arg) {
+               struct proc *new_p = (struct proc*)arg;
+               struct page *pp;
+               if (PAGE_PRESENT(*pte)) {
+                       /* TODO: check for jumbos */
+                       if (upage_alloc(new_p, &pp, 0))
+                               return -ENOMEM;
+                       if (page_insert(new_p->env_pgdir, pp, va, *pte & PTE_PERM)) {
+                               page_decref(pp);
+                               return -ENOMEM;
+                       }
+                       pagecopy(page2kva(pp), ppn2kva(PTE2PPN(*pte)));
+                       page_decref(pp);
+               } else if (PAGE_PAGED_OUT(*pte)) {
+                       /* TODO: (SWAP) will need to either make a copy or CoW/refcnt the
+                        * backend store.  For now, this PTE will be the same as the
+                        * original PTE */
+                       panic("Swapping not supported!");
+               } else {
+                       panic("Weird PTE %08p in %s!", *pte, __FUNCTION__);
+               }
+               return 0;
+       }
+       return env_user_mem_walk(p, (void*)va_start, va_end - va_start, &copy_page,
+                                new_p);
+       /* here's how to do it without the env_user_mem_walk */
+#if 0
+       pte_t *old_pte;
+       struct page *pp;
+       /* For each page, check the old PTE and copy present pages over */
+       for (uintptr_t va_i = va_start; va_i < va_end; va_i += PGSIZE) {
+               old_pte = pgdir_walk(p->env_pgdir, (void*)va_i, 0);
+               if (!old_pte)
+                       continue;
+               if (PAGE_PRESENT(*old_pte)) {
+                       /* TODO: check for jumbos */
+                       if (upage_alloc(new_p, &pp, 0))
+                               return -ENOMEM;
+                       if (page_insert(new_p->env_pgdir, pp, (void*)va_i,
+                                       *old_pte & PTE_PERM)) {
+                               page_decref(pp);
+                               return -ENOMEM;
+                       }
+                       pagecopy(page2kva(pp), ppn2kva(PTE2PPN(*old_pte)));
+                       page_decref(pp);
+               } else if (PAGE_PAGED_OUT(*old_pte)) {
+                       /* TODO: (SWAP) will need to either make a copy or CoW/refcnt the
+                        * backend store.  For now, this PTE will be the same as the
+                        * original PTE */
+                       panic("Swapping not supported!");
+               } else {
+                       panic("Weird PTE %08p in %s!", *old_pte, __FUNCTION__);
+               }
+       }
+       return 0;
+#endif
+}
+
+/* This will make new_p have the same VMRs as p, and it will make sure all
+ * physical pages are copied over, with the exception of MAP_SHARED files.
  * This is used by fork().
  *
  * Note that if you are working on a VMR that is a file, you'll want to be
  * careful about how it is mapped (SHARED, PRIVATE, etc). */
-void duplicate_vmrs(struct proc *p, struct proc *new_p)
+int duplicate_vmrs(struct proc *p, struct proc *new_p)
 {
+       int ret = 0;
        struct vm_region *vmr, *vm_i;
        TAILQ_FOREACH(vm_i, &p->vm_regions, vm_link) {
                vmr = kmem_cache_alloc(vmr_kcache, 0);
                if (!vmr)
-                       panic("EOM!");
+                       return -ENOMEM;
                vmr->vm_proc = new_p;
                vmr->vm_base = vm_i->vm_base;
                vmr->vm_end = vm_i->vm_end;
@@ -265,8 +340,15 @@ void duplicate_vmrs(struct proc *p, struct proc *new_p)
                        kref_get(&vm_i->vm_file->f_kref, 1);
                vmr->vm_file = vm_i->vm_file;
                vmr->vm_foff = vm_i->vm_foff;
+               if (!vmr->vm_file || vmr->vm_flags & MAP_PRIVATE) {
+                       assert(!(vmr->vm_flags & MAP_SHARED));
+                       /* Copy over the memory from one VMR to the other */
+                       if ((ret = copy_pages(p, new_p, vmr->vm_base, vmr->vm_end)))
+                               return ret;
+               }
                TAILQ_INSERT_TAIL(&new_p->vm_regions, vmr, vm_link);
        }
+       return 0;
 }
 
 void print_vmrs(struct proc *p)
index dc03d7f..ef1ec78 100644 (file)
@@ -424,36 +424,13 @@ static ssize_t sys_fork(env_t* e)
                if(GET_BITMASK_BIT(e->cache_colors_map,i))
                        cache_color_alloc(llc_cache, env->cache_colors_map);
 
-       duplicate_vmrs(e, env);
-
-       int copy_page(env_t* e, pte_t* pte, void* va, void* arg)
-       {
-               env_t* env = (env_t*)arg;
-
-               if(PAGE_PRESENT(*pte))
-               {
-                       page_t* pp;
-                       if(upage_alloc(env,&pp,0))
-                               return -1;
-                       if(page_insert(env->env_pgdir,pp,va,*pte & PTE_PERM))
-                       {
-                               page_decref(pp);
-                               return -1;
-                       }
-                       pagecopy(page2kva(pp),ppn2kva(PTE2PPN(*pte)));
-                       page_decref(pp);
-               } else {
-                       assert(PAGE_PAGED_OUT(*pte));
-                       /* TODO: (SWAP) will need to either make a copy or CoW/refcnt the
-                        * backend store.  For now, this PTE will be the same as the
-                        * original PTE */
-                       panic("Swapping not supported!");
-                       pte_t* newpte = pgdir_walk(env->env_pgdir,va,1);
-                       if(!newpte)
-                               return -1;
-                       *newpte = *pte;
-               }
-               return 0;
+       /* Make the new process have the same VMRs as the older.  This will copy the
+        * contents of non MAP_SHARED pages to the new VMRs. */
+       if (duplicate_vmrs(e, env)) {
+               proc_destroy(env);      /* this is prob what you want, not decref by 2 */
+               proc_decref(env);
+               set_errno(ENOMEM);
+               return -1;
        }
 
        /* In general, a forked process should be a fresh process, and we copy over
@@ -467,14 +444,6 @@ static ssize_t sys_fork(env_t* e)
        env->procdata->ldt = e->procdata->ldt;
        #endif
 
-       /* for now, just copy the contents of every present page in the entire
-        * address space. */
-       if (env_user_mem_walk(e, 0, UMAPTOP, &copy_page, env)) {
-               proc_destroy(env);      /* this is prob what you want, not decref by 2 */
-               proc_decref(env);
-               set_errno(ENOMEM);
-               return -1;
-       }
        clone_files(&e->open_files, &env->open_files);
        __proc_ready(env);
        __proc_set_state(env, PROC_RUNNABLE_S);
index e6fc3e2..bff83fb 100644 (file)
@@ -23,25 +23,18 @@ int main(void)
        first[0] = 3;
        printf("the first number after initialization is %d at %08p\n", first[0],
               first);
-       munmap(first, 4096);
        if ((pid = fork()) < 0) {
                perror("fork error");
                exit(1);
        }
        if (pid == 0) {
-               first = (int*)mmap(0, 4096, PROT_READ | PROT_WRITE, MAP_SHARED, pFile,
-                                  0);
                /* delay here, to avoid the race a bit */
                udelay(1000000);
                printf("After fork in the parent, the first number is %d\n", first[0]);
                first[0] = 99;
                printf("Pid 0 sees value %d at mmapped address %08p\n", first[0],
                       first);
-       }
-       else
-       {
-               first = (int*)mmap(0, 4096, PROT_READ | PROT_WRITE, MAP_SHARED, pFile,
-                                  0);
+       } else {
                printf("After fork in the child, the first number is %d\n", first[0]);
                first[0] = 11;
                printf("Child pid %d sees value %d at mmapped address %08p\n", pid,