Page map interface and munmap changes
authorBarret Rhoden <brho@cs.berkeley.edu>
Mon, 13 Jan 2014 18:24:12 +0000 (10:24 -0800)
committerBarret Rhoden <brho@cs.berkeley.edu>
Fri, 17 Jan 2014 22:57:12 +0000 (14:57 -0800)
The PM interface was slimmed a bit.  We still will need to change the
usages of pm_put_page for mmap a little.

munmap is a bit more efficient, and the VMRs are explicitly unmapped.
We'll need to do this instead of just blindly decreffing an entire
address space, since we need to handle PM pages differently.

kern/include/mm.h
kern/include/page_alloc.h
kern/include/pagemap.h
kern/src/blockdev.c
kern/src/mm.c
kern/src/pagemap.c
kern/src/process.c
kern/src/syscall.c
kern/src/vfs.c

index cf1affa..b8dd752 100644 (file)
@@ -58,7 +58,7 @@ void destroy_vmr(struct vm_region *vmr);
 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 unmap_and_destroy_vmrs(struct proc *p);
 int duplicate_vmrs(struct proc *p, struct proc *new_p);
 void print_vmrs(struct proc *p);
 
index b0a7e4c..bda74ce 100644 (file)
@@ -32,6 +32,7 @@ typedef LIST_ENTRY(page) page_list_entry_t;
 #define PG_UPTODATE            0x002   /* page map, filled with file data */
 #define PG_DIRTY               0x004   /* page map, data is dirty */
 #define PG_BUFFER              0x008   /* is a buffer page, has BHs */
+#define PG_PAGEMAP             0x010   /* belongs to a page map */
 
 /* TODO: this struct is not protected from concurrent operations in some
  * functions.  If you want to lock on it, use the spinlock in the semaphore.
index e302bf3..bbbed5e 100644 (file)
@@ -55,9 +55,7 @@ struct page_map_operations {
 
 /* Page cache functions */
 void pm_init(struct page_map *pm, struct page_map_operations *op, void *host);
-struct page *pm_find_page(struct page_map *pm, unsigned long index);
-int pm_insert_page(struct page_map *pm, unsigned long index, struct page *page);
-int pm_remove_page(struct page_map *pm, struct page *page);
 int pm_load_page(struct page_map *pm, unsigned long index, struct page **pp);
+void pm_put_page(struct page *page);
 
 #endif /* ROS_KERN_PAGEMAP_H */
index 8e4f3b5..1f12299 100644 (file)
@@ -276,7 +276,7 @@ void bdev_dirty_buffer(struct buffer_head *bh)
  * reclaiming will be in page sized chunks from the page cache. */
 void bdev_put_buffer(struct buffer_head *bh)
 {
-       page_decref(bh->bh_page);
+       pm_put_page(bh->bh_page);
 }
 
 /* Block device page map ops: */
index 52cbc2a..3fba9bc 100644 (file)
@@ -25,6 +25,8 @@
 #include <vfs.h>
 #include <smp.h>
 
+static int __vmr_free_pgs(struct proc *p, pte_t *pte, void *va, void *arg);
+
 struct kmem_cache *vmr_kcache;
 
 void vmr_init(void)
@@ -234,13 +236,16 @@ void isolate_vmrs(struct proc *p, uintptr_t va, size_t len)
                split_vmr(vmr, va + len);
 }
 
-/* Destroys all vmrs of a process - important for when files are mmap()d and
- * probably later when we share memory regions */
-void destroy_vmrs(struct proc *p)
+void unmap_and_destroy_vmrs(struct proc *p)
 {
-       struct vm_region *vm_i;
-       TAILQ_FOREACH(vm_i, &p->vm_regions, vm_link)
-               destroy_vmr(vm_i);
+       struct vm_region *vmr_i, *vmr_temp;
+       /* need the safe style, since destroy_vmr modifies the list */
+       TAILQ_FOREACH_SAFE(vmr_i, &p->vm_regions, vm_link, vmr_temp) {
+               /* note this CB sets the PTE = 0, regardless of if it was P or not */
+               env_user_mem_walk(p, (void*)vmr_i->vm_base,
+                                 vmr_i->vm_end - vmr_i->vm_base, __vmr_free_pgs, 0);
+               destroy_vmr(vmr_i);
+       }
 }
 
 /* Helper: copies the contents of pages from p to new p.  For pages that aren't
@@ -288,37 +293,6 @@ static int copy_pages(struct proc *p, struct proc *new_p, uintptr_t va_start,
        }
        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 %p 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
@@ -648,43 +622,74 @@ int munmap(struct proc *p, uintptr_t addr, size_t len)
        return ret;
 }
 
+static int __munmap_mark_not_present(struct proc *p, pte_t *pte, void *va,
+                                     void *arg)
+{
+       bool *shootdown_needed = (bool*)arg;
+       struct page *page;
+       /* could put in some checks here for !P and also !0 */
+       if (!PAGE_PRESENT(*pte))        /* unmapped (== 0) *ptes are also not PTE_P */
+               return 0;
+       page = ppn2page(PTE2PPN(*pte));
+       *pte &= ~PTE_P;
+       *shootdown_needed = TRUE;
+       return 0;
+}
+
+/* If our page is actually in the PM, we don't do anything.  All a page map
+ * really needs is for our VMR to no longer track it (vmr being in the pm's
+ * list) and to not point at its pages (mark it 0, dude).
+ *
+ * But private mappings mess with that a bit.  Luckily, we can tell by looking
+ * at a page whether the specific page is in the PM or not.  If it isn't, we
+ * still need to free our "VMR local" copy.
+ *
+ * For pages in a PM, we're racing with PM removers.  Both of us sync with the
+ * mm lock, so once we hold the lock, it's a matter of whether or not the PTE is
+ * 0 or not.  If it isn't, then we're still okay to look at the page.  Consider
+ * the PTE a weak ref on the page.  So long as you hold the mm lock, you can
+ * look at the PTE and know the page isn't being freed. */
+static int __vmr_free_pgs(struct proc *p, pte_t *pte, void *va, void *arg)
+{
+       struct page *page;
+       if (!*pte)
+               return 0;
+       page = ppn2page(PTE2PPN(*pte));
+       *pte = 0;
+       if (!(atomic_read(&page->pg_flags) & PG_PAGEMAP))
+               page_decref(page);
+       return 0;
+}
+
 int __do_munmap(struct proc *p, uintptr_t addr, size_t len)
 {
-       struct vm_region *vmr, *next_vmr;
+       struct vm_region *vmr, *next_vmr, *first_vmr;
        pte_t *pte;
        bool shootdown_needed = FALSE;
 
        /* TODO: this will be a bit slow, since we end up doing three linear
         * searches (two in isolate, one in find_first). */
        isolate_vmrs(p, addr, len);
-       vmr = find_first_vmr(p, addr);
+       first_vmr = find_first_vmr(p, addr);
+       vmr = first_vmr;
        while (vmr && vmr->vm_base < addr + len) {
-               for (uintptr_t va = vmr->vm_base; va < vmr->vm_end; va += PGSIZE) { 
-                       pte = pgdir_walk(p->env_pgdir, (void*)va, 0);
-                       if (!pte)
-                               continue;
-                       if (PAGE_PRESENT(*pte)) {
-                               /* TODO: (TLB) race here, where the page can be given out before
-                                * the shootdown happened.  Need to put it on a temp list. */
-                               page_t *page = ppn2page(PTE2PPN(*pte));
-                               *pte = 0;
-                               page_decref(page);
-                               shootdown_needed = TRUE;
-                       } else if (PAGE_PAGED_OUT(*pte)) {
-                               /* TODO: (SWAP) mark free in the swapfile or whatever.  For now,
-                                * PAGED_OUT is also being used to mean "hasn't been mapped
-                                * yet".  Note we now allow PAGE_UNMAPPED, unlike older
-                                * versions of mmap(). */
-                               panic("Swapping not supported!");
-                               *pte = 0;
-                       }
-               }
+               env_user_mem_walk(p, (void*)vmr->vm_base, vmr->vm_end - vmr->vm_base,
+                                 __munmap_mark_not_present, &shootdown_needed);
+               vmr = TAILQ_NEXT(vmr, vm_link);
+       }
+       /* we haven't freed the pages yet; still using the PTEs to store the them.
+        * There should be no races with inserts/faults, since we still hold the mm
+        * lock since the previous CB. */
+       if (shootdown_needed)
+               proc_tlbshootdown(p, addr, addr + len);
+       vmr = first_vmr;
+       while (vmr && vmr->vm_base < addr + len) {
+               env_user_mem_walk(p, (void*)vmr->vm_base, vmr->vm_end - vmr->vm_base,
+                                     __vmr_free_pgs, 0);
                next_vmr = TAILQ_NEXT(vmr, vm_link);
                destroy_vmr(vmr);
                vmr = next_vmr;
        }
-       if (shootdown_needed)
-               proc_tlbshootdown(p, addr, addr + len);
        return 0;
 }
 
index 7db0905..c984911 100644 (file)
@@ -26,7 +26,7 @@ void pm_init(struct page_map *pm, struct page_map_operations *op, void *host)
 
 /* Looks up the index'th page in the page map, returning an incref'd reference,
  * or 0 if it was not in the map. */
-struct page *pm_find_page(struct page_map *pm, unsigned long index)
+static struct page *pm_find_page(struct page_map *pm, unsigned long index)
 {
        spin_lock(&pm->pm_tree_lock);
        struct page *page = (struct page*)radix_lookup(&pm->pm_tree, index);
@@ -40,14 +40,16 @@ struct page *pm_find_page(struct page_map *pm, unsigned long index)
  * error code if there was one already (EEXIST) or we ran out of memory
  * (ENOMEM).  On success, this will preemptively lock the page, and will also
  * store a reference to the page in the pm. */
-int pm_insert_page(struct page_map *pm, unsigned long index, struct page *page)
+static int pm_insert_page(struct page_map *pm, unsigned long index,
+                          struct page *page)
 {
        int error = 0;
        spin_lock(&pm->pm_tree_lock);
        error = radix_insert(&pm->pm_tree, index, page);
        if (!error) {
                page_incref(page);
-               atomic_or(&page->pg_flags, PG_LOCKED | PG_BUFFER);
+               /* setting PG_BUF since we know it'll be used for IO later... */
+               atomic_or(&page->pg_flags, PG_LOCKED | PG_BUFFER | PG_PAGEMAP);
                page->pg_sem.nr_signals = 0;            /* ensure others will block */
                page->pg_mapping = pm;
                page->pg_index = index;
@@ -57,25 +59,9 @@ int pm_insert_page(struct page_map *pm, unsigned long index, struct page *page)
        return error;
 }
 
-/* Removes the page, including its reference.  Not sure yet what interface we
- * want to this (pm and index or page), and this has never been used.  There are
- * also issues with when you want to call this, since a page in the cache may be
- * mmap'd by someone else. */
-int pm_remove_page(struct page_map *pm, struct page *page)
+void pm_put_page(struct page *page)
 {
-       void *retval;
-       warn("pm_remove_page() hasn't been thought through or tested.");
-       /* TODO: check for dirty pages, don't let them be removed right away.  Need
-        * to schedule them for writeback, and then remove them later (callback).
-        * Also, need to be careful - anyone holding a reference to a page can dirty
-        * it concurrently. */
-       spin_lock(&pm->pm_tree_lock);
-       retval = radix_delete(&pm->pm_tree, page->pg_index);
-       spin_unlock(&pm->pm_tree_lock);
-       assert(retval == (void*)page);
        page_decref(page);
-       pm->pm_num_pages--;
-       return 0;
 }
 
 /* Makes sure the index'th page of the mapped object is loaded in the page cache
@@ -143,6 +129,6 @@ int pm_load_page(struct page_map *pm, unsigned long index, struct page **pp)
        assert(!error);
        /* Unlock, since we're done with the page and it is up to date */
        unlock_page(page);
-       assert(page->pg_flags & PG_UPTODATE);
+       assert(atomic_read(&page->pg_flags) & PG_UPTODATE);
        return 0;
 }
index 5404f92..ccac1cd 100644 (file)
@@ -352,6 +352,12 @@ struct proc *proc_create(struct file *prog, char **argv, char **envp)
        return p;
 }
 
+static int __cb_assert_no_pg(struct proc *p, pte_t *pte, void *va, void *arg)
+{
+       assert(!*pte);
+       return 0;
+}
+
 /* This is called by kref_put(), once the last reference to the process is
  * gone.  Don't call this otherwise (it will panic).  It will clean up the
  * address space and deallocate any other used memory. */
@@ -368,7 +374,8 @@ static void __proc_free(struct kref *kref)
        /* close plan9 dot and slash and free fgrp fd and fgrp */
        kref_put(&p->fs_env.root->d_kref);
        kref_put(&p->fs_env.pwd->d_kref);
-       destroy_vmrs(p);
+       /* now we'll finally decref files for the file-backed vmrs */
+       unmap_and_destroy_vmrs(p);
        frontend_proc_free(p);  /* TODO: please remove me one day */
        /* Free any colors allocated to this process */
        if (p->cache_colors_map != global_cache_colors_map) {
@@ -382,9 +389,11 @@ static void __proc_free(struct kref *kref)
                panic("Proc not in the pid table in %s", __FUNCTION__);
        spin_unlock(&pid_hash_lock);
        put_free_pid(p->pid);
-       /* Flush all mapped pages in the user portion of the address space */
-       env_user_mem_free(p, 0, UVPT);
-       /* These need to be free again, since they were allocated with a refcnt. */
+       /* all memory below UMAPTOP should have been freed via the VMRs.  the stuff
+        * above is the global page and procinfo/procdata */
+       env_user_mem_free(p, (void*)UMAPTOP, UVPT - UMAPTOP); /* 3rd arg = len... */
+       env_user_mem_walk(p, 0, UMAPTOP, __cb_assert_no_pg, 0);
+       /* These need to be freed again, since they were allocated with a refcnt. */
        free_cont_pages(p->procinfo, LOG2_UP(PROCINFO_NUM_PAGES));
        free_cont_pages(p->procdata, LOG2_UP(PROCDATA_NUM_PAGES));
 
@@ -773,8 +782,11 @@ void proc_destroy(struct proc *p)
         * a parent sleeping on our pipe, that parent won't wake up to decref until
         * the pipe closes.  And if the parent doesnt decref, we don't free.
         * alternatively, we could send a SIGCHILD to the parent, but that would
-        * require parent's to never ignore that signal (or risk never reaping) */
-       //close_9ns_files(p);
+        * require parent's to never ignore that signal (or risk never reaping).
+        *
+        * Also note that any mmap'd files will still be mmapped.  You can close the
+        * file after mmapping, with no effect. */
+       //close_9ns_files(p, FALSE);
        close_all_files(&p->open_files, FALSE);
        /* Tell the ksched about our death, and which cores we freed up */
        __sched_proc_destroy(p, pc_arr, nr_cores_revoked);
index d9bd39b..40a29aa 100644 (file)
@@ -578,7 +578,7 @@ static int sys_exec(struct proc *p, char *path, size_t path_l,
        #endif
        /* When we destroy our memory regions, accessing cur_sysc would PF */
        pcpui->cur_kthread->sysc = 0;
-       destroy_vmrs(p);
+       unmap_and_destroy_vmrs(p);
        close_all_files(&p->open_files, TRUE);
        env_user_mem_free(p, 0, UMAPTOP);
        if (load_elf(p, program)) {
index 188b773..38928d7 100644 (file)
@@ -1168,7 +1168,7 @@ ssize_t generic_file_read(struct file *file, char *buf, size_t count,
                }
                buf += copy_amt;
                page_off = 0;
-               page_decref(page);      /* it's still in the cache, we just don't need it */
+               pm_put_page(page);      /* it's still in the cache, we just don't need it */
        }
        assert(buf == buf_end);
        *offset += count;
@@ -1218,7 +1218,8 @@ ssize_t generic_file_write(struct file *file, const char *buf, size_t count,
                }
                buf += copy_amt;
                page_off = 0;
-               page_decref(page);      /* it's still in the cache, we just don't need it */
+               atomic_or(&page->pg_flags, PG_DIRTY);
+               pm_put_page(page);      /* it's still in the cache, we just don't need it */
        }
        assert(buf == buf_end);
        *offset += count;