Cleaned up issues with PAGE_UNMAPPED and friends
authorBarret Rhoden <brho@cs.berkeley.edu>
Thu, 8 Jul 2010 00:07:18 +0000 (17:07 -0700)
committerKevin Klues <klueska@cs.berkeley.edu>
Thu, 3 Nov 2011 00:35:48 +0000 (17:35 -0700)
Removed get_free_va_range(), which is now done with VMRs, cleaned up
cases of PAGED_OUT pages (mostly made it panic since we don't swap
properly), and took care of a couple other details related to the
meaning of UNMAPPED, PAGED_OUT, and PRESENT.

kern/arch/i686/pmap.c
kern/arch/sparc/pmap.c
kern/include/mm.h
kern/include/pmap.h
kern/src/env.c
kern/src/mm.c
kern/src/pmap.c
kern/src/syscall.c

index 0f711d3..2f96baa 100644 (file)
@@ -638,28 +638,6 @@ int get_va_perms(pde_t *pgdir, const void *SNT va)
        return the_pte & the_pde & (PTE_U | PTE_W | PTE_P);
 }
 
-void *get_free_va_range(pde_t *pgdir, uintptr_t addr, size_t len)
-{
-       addr = ROUNDUP(MAX(addr,BRK_END),PGSIZE);
-       len = ROUNDUP(len,PGSIZE);
-
-       for(char* a = (char*)addr; a < (char*)USTACKBOT; a += PGSIZE)
-       {
-               for(char* b = a; b < a+len; b += PGSIZE)
-               {
-                       pte_t* pte = pgdir_walk(pgdir,b,0);
-                       if(pte && !PAGE_UNMAPPED(*pte))
-                       {
-                               a = b;
-                               break;
-                       }
-                       if(b+PGSIZE == a+len)
-                               return a;
-               }
-       }
-       return NULL;
-}
-
 /* Flushes a TLB, including global pages.  We should always have the CR4_PGE
  * flag set, but just in case, we'll check.  Toggling this bit flushes the TLB.
  */
index 2e65987..bf7d22a 100644 (file)
@@ -121,28 +121,6 @@ int get_va_perms(pde_t *pgdir, const void *SNT va)
        return pte == NULL ? 0 : (*pte & (PTE_ACC | PTE_PTE));
 }
 
-void *get_free_va_range(pde_t *pgdir, uintptr_t addr, size_t len)
-{
-       addr = ROUNDUP(MAX(addr,BRK_END),PGSIZE);
-       len = ROUNDUP(len,PGSIZE);
-
-       for(char* a = (char*)addr; a < (char*)USTACKBOT; a += PGSIZE)
-       {
-               for(char* b = a; b < a+len; b += PGSIZE)
-               {
-                       pte_t* pte = pgdir_walk(pgdir,b,0);
-                       if(pte && !PAGE_UNMAPPED(*pte))
-                       {
-                               a = b;
-                               break;
-                       }
-                       if(b+PGSIZE == a+len)
-                               return a;
-               }
-       }
-       return NULL;
-}
-
 void
 page_check(void)
 {
index 4aec314..d4425d3 100644 (file)
 #include <sys/queue.h>
 #include <slab.h>
 
-/* Memory region for a process, consisting of linear(virtual) addresses.  This
- * is what the kernel allocates a process, and the physical mapping can be done
- * lazily (or not).  This way, if a page is swapped out, and the PTE says it
- * isn't present, we still have a way to account for how the whole region ought
- * to be dealt with.
- * Some things are per-region:
- * - probably something with shared memory
- * - mmaping files: we can have a logical connection to something other than
- *   anonymous memory
- * - on a fault, was this memory supposed to be there? (swap, lazy, etc), or is
- *   the region free?
- * Others are per-page:
- * - was this page supposed to be protected somehow(guard)? could be per-region
- * - where is this page in the swap?
- * If we try to store this info in the PTE, we only have 31 bits, and it's more
- * arch dependent.  Handling jumbos is a pain.  And it's replicated across all
- * pages for a coarse granularity things.  And we can't add things easily.
- *
- * so a process has a (sorted) list of these for it's VA space, hanging off it's
- * struct proc.  or off it's mm?
- * - we don't share an mm between processes anymore (tasks/threads)
- *   - though we share most everything with vpm.
- *   - want to be able to do all the same things with vpm as with regular mem
- *     (file back mmap, etc)
- *   - contexts or whatever share lots of mem things, like accounting, limits,
- *   overall process stuff, the rest of the page tables.
- *     - so there should be some overall mm, and probably directly in the
- *     struct proc (or just one other struct directly embedded, not a pointer
- *     to one where a bunch of processes use it)
- *             - if we embed, mm.h doesn't need to know about process.h
- *   so an mm can have a bunch of "address spaces" - or at least different
- *   contexts
- *
- * how does this change or where does this belong with virtual private memory?
- * will also affect get_free_va_range
- *     - also, do we want a separate brk per?  or just support mmap on private mem?
- */
 struct file;
 struct proc;                                                           /* preprocessor games */
 
@@ -61,7 +24,11 @@ struct mm {
        spinlock_t mm_lock;
 };
 
-/* Basic structure defining a region of a process's virtual memory */
+/* Basic structure defining a region of a process's virtual memory.  Note we
+ * don't refcnt these.  Either they are in the TAILQ/tree, or they should be
+ * freed.  There should be no other references floating around.  We still need
+ * to sort out how we share memory and how we'll do private memory with these
+ * VMRs. */
 struct vm_region {
        TAILQ_ENTRY(vm_region)          vm_link;
        struct proc                                     *vm_proc;       /* owning process, for now */
index 8e056aa..63128b0 100644 (file)
@@ -100,9 +100,6 @@ memcpy_to_user(env_t* env, void*DANGEROUS va,
 /* Arch specific implementations for these */
 pte_t *pgdir_walk(pde_t *COUNT(NPDENTRIES) pgdir, const void *SNT va, int create);
 int get_va_perms(pde_t *COUNT(NPDENTRIES) pgdir, const void *SNT va);
-// TODO: should this be per process, per mm, per pgdir, etc?
-// - can't ask this question without knowing the "context"
-void *get_free_va_range(pde_t *pgdir, uintptr_t addr, size_t len);
 
 static inline page_t *SAFE ppn2page(size_t ppn)
 {
index a13ecef..e7a9a0f 100644 (file)
@@ -281,6 +281,7 @@ void env_user_mem_free(env_t* e, void* start, size_t len)
                } else {
                        assert(PAGE_PAGED_OUT(*pte));
                        /* TODO: (SWAP) deal with this */
+                       panic("Swapping not supported!");
                        *pte = 0;
                }
                return 0;
index 758103e..58bbb55 100644 (file)
@@ -440,6 +440,7 @@ int __do_munmap(struct proc *p, uintptr_t addr, size_t len)
                                 * 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;
                        }
                }
@@ -490,16 +491,19 @@ int __handle_page_fault(struct proc* p, uintptr_t va, int prot)
                return -EFAULT;
        /* find offending PTE (prob don't read this in).  This might alloc an
         * intermediate page table page. */
-       pte_t* ppte = pgdir_walk(p->env_pgdir, (void*)va, 1);
-       if (!ppte)
+       pte_t *pte = pgdir_walk(p->env_pgdir, (void*)va, 1);
+       if (!pte)
                return -ENOMEM;
-       pte_t pte = *ppte;
-       assert(PAGE_UNMAPPED(pte));                     /* should be munmapped already */
        /* a spurious, valid PF is possible due to a legit race: the page might have
         * been faulted in by another core already (and raced on the memory lock),
         * in which case we should just return. */
-       if (PAGE_PRESENT(pte))
+       if (PAGE_PRESENT(*pte)) {
                return 0;
+       } else if (PAGE_PAGED_OUT(*pte)) {
+               /* TODO: (SWAP) bring in the paged out frame. (BLK) */
+               panic("Swapping not supported!");
+               return 0;
+       }
        /* allocate a page; maybe zero-fill it */
        bool zerofill = (vmr->vm_file == NULL);
        page_t *a_page;
@@ -529,6 +533,6 @@ int __handle_page_fault(struct proc* p, uintptr_t va, int prot)
        int pte_perm = (vmr->vm_perm & PROT_WRITE) ? PTE_USER_RW :
                       (vmr->vm_perm & (PROT_READ|PROT_EXEC)) ? PTE_USER_RO : 0;
        page_incref(a_page);
-       *ppte = PTE(page2ppn(a_page), PTE_P | pte_perm);
+       *pte = PTE(page2ppn(a_page), PTE_P | pte_perm);
        return 0;
 }
index 60cceff..7d88581 100644 (file)
@@ -117,6 +117,7 @@ int page_insert(pde_t *pgdir, page_t *pp, void *va, int perm)
        // and we don't want to page_remove (which could free pp) and then 
        // continue as if pp wasn't freed.  moral = up the ref asap
        page_incref(pp);
+       /* Careful, page remove handles the cases where the page is PAGED_OUT. */
        if (!PAGE_UNMAPPED(*pte))
                page_remove(pgdir, va);
        *pte = PTE(page2ppn(pp), PTE_P | perm);
@@ -124,6 +125,8 @@ int page_insert(pde_t *pgdir, page_t *pp, void *va, int perm)
 }
 
 /**
+ * DEPRECATED - this conflicts with VM regions.
+ *
  * @brief Map the physical page 'pp' at the first virtual address that is free 
  * in the range 'vab' to 'vae' in page directory 'pgdir'.
  *
@@ -213,10 +216,9 @@ page_t *page_lookup(pde_t *pgdir, void *va, pte_t **pte_store)
  * @param pgdir the page directory from with the page sholuld be removed
  * @param va    the virtual address at which the page we are trying to 
  *              remove is mapped
- * TODO: consider deprecating this, esp given issues with TLB management.  might
- * want to have the caller need to manage the TLB.  Also note it is used in
- * env_user_mem_free, minus the walk.
- */
+ * TODO: consider deprecating this, or at least changing how it works with TLBs.
+ * Might want to have the caller need to manage the TLB.  Also note it is used
+ * in env_user_mem_free, minus the walk. */
 void page_remove(pde_t *pgdir, void *va)
 {
        pte_t *pte;
@@ -227,14 +229,16 @@ void page_remove(pde_t *pgdir, void *va)
                return;
 
        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. */
+               /* TODO: (TLB) need to do a shootdown, inval sucks.  And might want to
+                * manage the TLB / free pages differently. (like by the caller).
+                * Careful about the proc/memory lock here. */
                page = ppn2page(PTE2PPN(*pte));
                *pte = 0;
-               page_decref(page);
                tlb_invalidate(pgdir, va);
+               page_decref(page);
        } else if (PAGE_PAGED_OUT(*pte)) {
                /* TODO: (SWAP) need to free this from the swap */
+               panic("Swapping not supported!");
                *pte = 0;
        }
 }
@@ -243,7 +247,7 @@ void page_remove(pde_t *pgdir, void *va)
  * @brief Invalidate a TLB entry, but only if the page tables being
  * edited are the ones currently in use by the processor.
  *
- * TODO: Need to sort this for cross core lovin'
+ * TODO: (TLB) Need to sort this for cross core lovin'
  *
  * @param pgdir the page directory assocaited with the tlb entry 
  *              we are trying to invalidate
index 457ec0b..36a8a09 100644 (file)
@@ -359,6 +359,7 @@ static ssize_t sys_fork(env_t* e)
                        /* 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;
@@ -536,6 +537,8 @@ static ssize_t sys_shared_page_alloc(env_t* p1,
                                      int p1_flags, int p2_flags
                                     )
 {
+       /* When we remove/change this, also get rid of page_insert_in_range() */
+       printk("[kernel] the current shared page alloc is deprecated.\n");
        //if (!VALID_USER_PERMS(p1_flags)) return -EPERM;
        //if (!VALID_USER_PERMS(p2_flags)) return -EPERM;