Uses kref for struct page
authorBarret Rhoden <brho@cs.berkeley.edu>
Thu, 23 Sep 2010 21:47:45 +0000 (14:47 -0700)
committerKevin Klues <klueska@cs.berkeley.edu>
Thu, 3 Nov 2011 00:35:54 +0000 (17:35 -0700)
Some ghettoness, due to the way upage_alloc works.

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

index 03ee4dc..9d29eaf 100644 (file)
@@ -38,15 +38,14 @@ void page_alloc_init()
 {
        // First Bootstrap the page alloc process
        static bool RO bootstrapped = FALSE;
-       if(!bootstrapped) {
+       if (!bootstrapped) {
                bootstrapped = SINIT(TRUE);
                page_alloc_bootstrap();
        }
 
        // Then, initialize the array required to manage the colored page free list
-       for(int i=0; i<llc_cache->num_colors; i++) {
+       for (int i = 0; i < llc_cache->num_colors; i++)
                LIST_INIT(&(colored_page_free_list[i]));
-       }
 
        //  Then, mark the pages already in use by the kernel. 
        //  1) Mark page 0 as in use.
@@ -61,12 +60,13 @@ void page_alloc_init()
        extern char (SNT RO end)[];
        physaddr_t physaddr_after_kernel = PADDR(PTRROUNDUP(boot_freemem, PGSIZE));
 
-       atomic_set(&pages[0].pg_refcnt, 1);
+       page_setref(&pages[0], 1);
        // alloc the second page, since we will need it later to init the other cores
        // probably need to be smarter about what page we use (make this dynamic) TODO
-       atomic_set(&pages[1].pg_refcnt, 1);
+       page_setref(&pages[1], 1);
        for (i = 2; i < LA2PPN(IOPHYSMEM); i++) {
-               pages[i].pg_refcnt = 0;
+               /* this ought to be unnecessary */
+               page_setref(&pages[i], 0);
                LIST_INSERT_HEAD(
                   &(colored_page_free_list[get_page_color(page2ppn(&pages[i]), 
                                                               llc_cache)]),
@@ -74,14 +74,12 @@ void page_alloc_init()
                   pg_link
                );
        }
-       for (i = LA2PPN(IOPHYSMEM); i < LA2PPN(EXTPHYSMEM); i++) {
-               atomic_set(&pages[i].pg_refcnt, 1);
-       }
-       for (i = LA2PPN(EXTPHYSMEM); i < LA2PPN(physaddr_after_kernel); i++) {
-               atomic_set(&pages[i].pg_refcnt, 1);
-       }
+       for (i = LA2PPN(IOPHYSMEM); i < LA2PPN(EXTPHYSMEM); i++)
+               page_setref(&pages[i], 1);
+       for (i = LA2PPN(EXTPHYSMEM); i < LA2PPN(physaddr_after_kernel); i++)
+               page_setref(&pages[i], 1);
        for (i = LA2PPN(physaddr_after_kernel); i < LA2PPN(maxaddrpa); i++) {
-               atomic_set(&pages[i].pg_refcnt, 0);
+               page_setref(&pages[i], 0);
                LIST_INSERT_HEAD(
                   &(colored_page_free_list[get_page_color(page2ppn(&pages[i]), 
                                                               llc_cache)]),
@@ -91,9 +89,8 @@ void page_alloc_init()
        }
        // this block out all memory above maxaddrpa.  will need another mechanism
        // to allocate and map these into the kernel address space
-       for (i = LA2PPN(maxaddrpa); i < npages; i++) {
-               atomic_set(&pages[i].pg_refcnt, 1);
-       }
+       for (i = LA2PPN(maxaddrpa); i < npages; i++)
+               page_setref(&pages[i], 1);
        printk("Page alloc init successful\n");
 }
 
index 6509d35..902764c 100644 (file)
@@ -700,13 +700,13 @@ page_check(void)
        }
        assert(PTE_ADDR(boot_pgdir[0]) == page2pa(pp0));
        assert(check_va2pa(boot_pgdir, 0x0) == page2pa(pp1));
-       assert(atomic_read(&pp1->pg_refcnt) == 1);
-       assert(atomic_read(&pp0->pg_refcnt) == 1);
+       assert(kref_refcnt(&pp1->pg_kref) == 1);
+       assert(kref_refcnt(&pp0->pg_kref) == 1);
 
        // should be able to map pp2 at PGSIZE because pp0 is already allocated for page table
        assert(page_insert(boot_pgdir, pp2, (void*SNT) PGSIZE, 0) == 0);
        assert(check_va2pa(boot_pgdir, PGSIZE) == page2pa(pp2));
-       assert(atomic_read(&pp2->pg_refcnt) == 1);
+       assert(kref_refcnt(&pp2->pg_kref) == 1);
 
        // Make sure that pgdir_walk returns a pointer to the pte and
        // not the table or some other garbage
@@ -721,7 +721,7 @@ page_check(void)
        // should be able to map pp2 at PGSIZE because it's already there
        assert(page_insert(boot_pgdir, pp2, (void*SNT) PGSIZE, PTE_U) == 0);
        assert(check_va2pa(boot_pgdir, PGSIZE) == page2pa(pp2));
-       assert(atomic_read(&pp2->pg_refcnt) == 1);
+       assert(kref_refcnt(&pp2->pg_kref) == 1);
 
        // Make sure that we actually changed the permission on pp2 when we re-mapped it
        {
@@ -743,8 +743,8 @@ page_check(void)
        assert(check_va2pa(boot_pgdir, 0) == page2pa(pp1));
        assert(check_va2pa(boot_pgdir, PGSIZE) == page2pa(pp1));
        // ... and ref counts should reflect this
-       assert(atomic_read(&pp1->pg_refcnt) == 2);
-       assert(atomic_read(&pp2->pg_refcnt) == 0);
+       assert(kref_refcnt(&pp1->pg_kref) == 2);
+       assert(kref_refcnt(&pp2->pg_kref) == 0);
 
        // pp2 should be returned by page_alloc
        assert(kpage_alloc(&pp) == 0 && pp == pp2);
@@ -754,15 +754,15 @@ page_check(void)
        page_remove(boot_pgdir, 0x0);
        assert(check_va2pa(boot_pgdir, 0x0) == ~0);
        assert(check_va2pa(boot_pgdir, PGSIZE) == page2pa(pp1));
-       assert(atomic_read(&pp1->pg_refcnt) == 1);
-       assert(atomic_read(&pp2->pg_refcnt) == 0);
+       assert(kref_refcnt(&pp1->pg_kref) == 1);
+       assert(kref_refcnt(&pp2->pg_kref) == 0);
 
        // unmapping pp1 at PGSIZE should free it
        page_remove(boot_pgdir, (void*SNT) PGSIZE);
        assert(check_va2pa(boot_pgdir, 0x0) == ~0);
        assert(check_va2pa(boot_pgdir, PGSIZE) == ~0);
-       assert(atomic_read(&pp1->pg_refcnt) == 0);
-       assert(atomic_read(&pp2->pg_refcnt) == 0);
+       assert(kref_refcnt(&pp1->pg_kref) == 0);
+       assert(kref_refcnt(&pp2->pg_kref) == 0);
 
        // so it should be returned by page_alloc
        assert(kpage_alloc(&pp) == 0 && pp == pp1);
@@ -774,8 +774,8 @@ page_check(void)
        // forcibly take pp0 back
        assert(PTE_ADDR(boot_pgdir[0]) == page2pa(pp0));
        boot_pgdir[0] = 0;
-       assert(atomic_read(&pp0->pg_refcnt) == 1);
-       atomic_set(&pp0->pg_refcnt, 0);
+       assert(kref_refcnt(&pp0->pg_kref) == 1);
+       page_setref(pp0, 0);
 
        // Catch invalid pointer addition in pgdir_walk - i.e. pgdir + PDX(va)
        {
@@ -789,7 +789,7 @@ page_check(void)
 
          // Clean up again
          boot_pgdir[PDX(va)] = 0;
-         atomic_set(&pp0->pg_refcnt, 0);
+         page_setref(pp0, 0);
        }
 
        // give free list back
index 0af4641..843a782 100644 (file)
@@ -64,12 +64,12 @@ void page_alloc_init()
 
        // mark [0, physaddr_after_kernel) as in-use
        for(i = 0; i < LA2PPN(physaddr_after_kernel); i++)
-               atomic_init(&pages[i].pg_refcnt, 1);
+               page_setref(&pages[i], 1);
 
        // mark [physaddr_after_kernel, maxaddrpa) as free
        for(i = LA2PPN(physaddr_after_kernel); i < LA2PPN(maxaddrpa); i++)
        {
-               pages[i].pg_refcnt = 0;
+               page_setref(&pages[i], 0);
                 LIST_INSERT_HEAD(
                    &(colored_page_free_list[get_page_color(i,llc_cache)]),
                    &pages[i],
@@ -79,5 +79,5 @@ void page_alloc_init()
 
        // mark [maxaddrpa, ...) as in-use (as they are invalid)
        for(i = LA2PPN(maxaddrpa); i < npages; i++)
-               atomic_init(&pages[i].pg_refcnt, 1);
+               page_setref(&pages[i], 1);
 }
index 0e9448c..74b1ee5 100644 (file)
@@ -14,6 +14,7 @@
 #include <arch/mmu.h>
 #include <colored_page_alloc.h>
 #include <process.h>
+#include <kref.h>
 
 struct page_map;               /* preprocessor games */
 
@@ -34,7 +35,7 @@ typedef LIST_ENTRY(page) page_list_entry_t;
  * reference counting and atomic operations. */
 struct page {
        LIST_ENTRY(page)                        pg_link;        /* membership in various lists */
-       atomic_t                                        pg_refcnt;
+       struct kref                                     pg_kref;
        unsigned int                            pg_flags;
        struct page_map                         *pg_mapping;
        unsigned long                           pg_index;
index a43b5b0..a36189c 100644 (file)
@@ -65,7 +65,7 @@ void  vm_init(void);
 
 void   page_init(void);
 void   page_check(void);
-int        page_insert(pde_t *COUNT(NPDENTRIES) pgdir, page_t *pp, void *SNT va, int perm);
+int        page_insert(pde_t *pgdir, struct page *page, void *SNT va, int perm);
 void*COUNT(PGSIZE) page_insert_in_range(pde_t *COUNT(NPDENTRIES) pgdir, page_t *pp, 
                              void *SNT vab, void *SNT vae, int perm);
 void   page_remove(pde_t *COUNT(NPDENTRIES) pgdir, void *SNT va);
index 02bf991..f59e175 100644 (file)
@@ -108,7 +108,7 @@ WRITES(e->env_pgdir, e->env_cr3, e->procinfo, e->procdata)
                // Up it, so it never goes away.  One per user, plus one from page_alloc
                // This is necessary, since it's in the per-process range of memory that
                // gets freed during page_free.
-               page_incref(shared_page);
+               __kref_get(&shared_page->pg_kref, 1);
        }
 
        // Inserted into every process's address space at UGDATA
index 5e66db1..d072a6e 100644 (file)
@@ -603,7 +603,9 @@ int __handle_page_fault(struct proc *p, uintptr_t va, int prot)
         * separately (file, no file) */
        int pte_prot = (vmr->vm_prot & PROT_WRITE) ? PTE_USER_RW :
                       (vmr->vm_prot & (PROT_READ|PROT_EXEC)) ? PTE_USER_RO : 0;
-       page_incref(a_page);    /* incref, since we manually insert in the pgdir */
+       /* Need to incref, since we manually insert in the pgdir.  Internal version,
+        * to go along with the upage_alloc from above. */
+       __kref_get(&a_page->pg_kref, 1);
        *pte = PTE(page2ppn(a_page), PTE_P | pte_prot);
        return 0;
 }
index c2a45b1..3f59e1a 100644 (file)
@@ -67,8 +67,10 @@ static void __page_clear(page_t *SAFE page)
        /* Allocate a page from that color */                                   \
        if(i < (base_color+range)) {                                            \
                *page = LIST_FIRST(&colored_page_free_list[i]);                     \
-               LIST_REMOVE(*page, pg_link);                                      \
+               LIST_REMOVE(*page, pg_link);                                        \
                __page_clear(*page);                                                \
+               /* Note the 0 initial value, due to how user pages are refcnt'd */  \
+               page_setref((*page), 0);                                            \
                return i;                                                           \
        }                                                                       \
        return -ENOMEM;
@@ -102,12 +104,14 @@ static ssize_t __colored_page_alloc(uint8_t* map, page_t** page,
 static error_t __page_alloc_specific(page_t** page, size_t ppn)
 {
        page_t* sp_page = ppn2page(ppn);
-       if (atomic_read(&sp_page->pg_refcnt) != 0)
+       if (!page_is_free(ppn))
                return -ENOMEM;
        *page = sp_page;
        LIST_REMOVE(*page, pg_link);
-
        __page_clear(*page);
+       /* Note the 0 initial value, due to how user pages are refcnt'd.  If you
+        * have a page, you need to kref_get it before you *use* it. */
+       page_setref(*page, 0);
        return 0;
 }
 
@@ -129,8 +133,7 @@ error_t upage_alloc(struct proc* p, page_t** page, int zero)
                                             page, p->next_cache_color);
        spin_unlock_irqsave(&colored_page_free_list_lock);
 
-       if(ret >= 0)
-       {
+       if (ret >= 0) {
                if(zero)
                        memset(page2kva(*page),0,PGSIZE);
                p->next_cache_color = (ret + 1) & (llc_cache->num_colors-1);
@@ -143,13 +146,14 @@ error_t kpage_alloc(page_t** page)
 {
        ssize_t ret;
        spin_lock_irqsave(&colored_page_free_list_lock);
-       if((ret = __page_alloc_from_color_range(page, global_next_color, 
+       if ((ret = __page_alloc_from_color_range(page, global_next_color, 
                                    llc_cache->num_colors - global_next_color)) < 0)
                ret = __page_alloc_from_color_range(page, 0, global_next_color);
 
-       if(ret >= 0) {
+       if (ret >= 0) {
                global_next_color = ret;        
-               page_incref(*page);
+               /* this is the "it's okay if it was 0" kref */
+               __kref_get(&(*page)->pg_kref, 1);
                ret = ESUCCESS;
        }
        spin_unlock_irqsave(&colored_page_free_list_lock);
@@ -195,7 +199,7 @@ void *get_cont_pages(size_t order, int flags)
        for(int i=0; i<npages; i++) {
                page_t* page;
                __page_alloc_specific(&page, first+i);
-               page_incref(page); 
+               __kref_get(&page->pg_kref, 1);
        }
        spin_unlock_irqsave(&colored_page_free_list_lock);
        return ppn2kva(first);
@@ -243,11 +247,13 @@ error_t kpage_alloc_specific(page_t** page, size_t ppn)
        return 0;
 }
 
-/*
- * Return a page to the free list.
- * (This function should only be called when pp->pg_refcnt reaches 0.)
- * You must hold the page_free list lock before calling this.
- */
+/* Returns a page to the free list.
+ *
+ * This function should only be called when the refcnt reaches 0, meaning from
+ * page_release().
+ *
+ * You must hold the page_free list lock before calling this, which is
+ * accomplished via the page_decref locking hacks. */
 static error_t __page_free(page_t* page) 
 {
        __page_clear(page);
@@ -270,14 +276,12 @@ error_t page_free(page_t *SAFE page)
        return retval;
 }
 
-/*
- * Check if a page with the given physical page # is free
- */
+/* Check if a page with the given physical page # is free. */
 int page_is_free(size_t ppn) {
        page_t* page = ppn2page(ppn);
-       if (atomic_read(&page->pg_refcnt) == 0)
-               return TRUE;
-       return FALSE;
+       if (kref_refcnt(&page->pg_kref))
+               return FALSE;
+       return TRUE;
 }
 
 /*
@@ -288,16 +292,13 @@ void page_incref(page_t *page)
        __page_incref(page);
 }
 
-/* TODO: (REF) poor refcnting */
 void __page_incref(page_t *page)
 {
-       atomic_inc(&page->pg_refcnt);
+       kref_get(&page->pg_kref, 1);
 }
 
-/*
- * Decrement the reference count on a page,
- * freeing it if there are no more refs.
- */
+/* Decrement the reference count on a page, freeing it if there are no more
+ * refs. */
 void page_decref(page_t *page)
 {
        spin_lock_irqsave(&colored_page_free_list_lock);
@@ -305,29 +306,26 @@ void page_decref(page_t *page)
        spin_unlock_irqsave(&colored_page_free_list_lock);
 }
 
-/*
- * Decrement the reference count on a page,
- * freeing it if there are no more refs.
- *
- * TODO: (REF) this is insufficient protection (poor use of atomics, etc).
- */
+/* Decrement the reference count on a page, freeing it if there are no more
+ * refs. */
 static void __page_decref(page_t *page)
 {
-       if (atomic_read(&page->pg_refcnt) == 0) {
-               panic("Trying to Free already freed page: %d...\n", page2ppn(page));
-               return;
-       }
-       atomic_dec(&page->pg_refcnt);
-       if (atomic_read(&page->pg_refcnt) == 0)
-               __page_free(page);
+       kref_put(&page->pg_kref);
 }
 
-/*
- * Set the reference count on a page to a specific value
- */
+/* Kref release function. */
+static void page_release(struct kref *kref)
+{
+       struct page *page = container_of(kref, struct page, pg_kref);
+       __page_free(page);
+}
+
+/* Helper when initializing a page - just to prevent the proliferation of
+ * page_release references (and because this function is sitting around in the
+ * code).  Sets the reference count on a page to a specific value, usually 1. */
 void page_setref(page_t *page, size_t val)
 {
-       atomic_set(&page->pg_refcnt, val);
+       kref_init(&page->pg_kref, page_release, val); 
 }
 
 /*
@@ -335,7 +333,7 @@ void page_setref(page_t *page, size_t val)
  */
 size_t page_getref(page_t *page)
 {
-       return atomic_read(&page->pg_refcnt);
+       return kref_refcnt(&page->pg_kref);
 }
 
 /* Attempts to get a lock on the page for IO operations.  If it is already
index e202fc7..139116b 100644 (file)
@@ -99,19 +99,23 @@ void page_init(void)
  *                   into which the page should be inserted
  *
  */
-int page_insert(pde_t *pgdir, page_t *pp, void *va, int perm) 
+int page_insert(pde_t *pgdir, struct page *page, void *va, int perm) 
 {
        pte_t* pte = pgdir_walk(pgdir, va, 1);
        if (!pte)
                return -ENOMEM;
-       // need to up the ref count in case pp is already mapped at va
-       // 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);
+       /* Two things here:  First, we need to up the ref count of the page we want
+        * to insert in case it is already mapped at va.  In that case we don't want
+        * page_remove to ultimately free it, and then for us to continue as if pp
+        * wasn't freed. (moral = up the ref asap)
+        * Secondly, we need to use the __kref_get() since it is possible that pp
+        * has a refcnt of 0 - which is what happens when you get a fresh page from
+        * the free list (for now). */
+       __kref_get(&page->pg_kref, 1);
        /* 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);
+       *pte = PTE(page2ppn(page), PTE_P | perm);
        return 0;
 }