Merge branch 'vm/mmap-write-back-on-eviction' into 'vm/virtual-memory/themis-synch'

Write back mmap file pages to file upon eviction

See merge request lab2425_autumn/pintos_22!59

src/userprog/exception.c

@@ -2,6 +2,7 @@
 #include <inttypes.h>
 #include <stdio.h>
 #include "stdbool.h"
+#include "threads/synch.h"
 #include "userprog/gdt.h"
 #include "userprog/pagedir.h"
 #include "userprog/process.h"
@@ -249,6 +250,12 @@ grow_stack (void *upage)
 bool
 fetch_page (void *upage, bool write)
 {
+  /* Check if the page is in the supplemental page table. That is, it is a page
+     that is expected to be in memory. */
+  struct page_entry *page = page_get (thread_current (), upage);
+  if (page == NULL)
+    return false;
+
   /* Check if the non-present user page is in the swap partition.
    If so, swap it back into main memory, updating the PTE for
    the faulted virtual address to point to the newly allocated
@@ -256,20 +263,23 @@ fetch_page (void *upage, bool write)
   struct thread *t = thread_current ();
   if (page_in_swap (t, upage))
   {
-    size_t swap_slot = page_get_swap (t, upage);
+    /* NOTE: This code should be refactored and moved into helper functions
+      within 'page.c'.*/
     void *kpage = frame_alloc (0, upage, t);
+    lock_acquire (&page->lock);
+
+    size_t swap_slot = page_get_swap (t, upage);
     swap_in (kpage, swap_slot);
 
-    bool writeable = pagedir_is_writable (t->pagedir, upage);
-    if (pagedir_set_page (t->pagedir, upage, kpage, writeable))
-      return true;
-  }
+    lock_release (&page->lock);
 
-  /* Check if the page is in the supplemental page table. That is, it is a page
-     that is expected to be in memory. */
-  struct page_entry *page = page_get (upage);
-  if (page == NULL)
-    return false;
+    bool writeable = pagedir_is_writable (t->pagedir, upage);
+
+    /* TODO: When this returns false we should quit the page fault,
+      but currently we continue and check the stack conditions in the
+      page fault handler. */
+    return pagedir_set_page (t->pagedir, upage, kpage, writeable);
+  }
 
   /* An attempt to write to a non-writeable should fail. */
   if (write && !page->writable)
@@ -278,8 +288,9 @@ fetch_page (void *upage, bool write)
   /* Load the page into memory based on the type of data it is expecting. */
   bool success = false;
   switch (page->type) {
+    case PAGE_MMAP:
     case PAGE_FILE:
-      success = page_load (page, page->writable);
+      success = page_load_file (page, page->writable);
       break;
     default:
       return false;

src/userprog/process.c

@@ -714,8 +714,8 @@ load_segment (struct file *file, off_t ofs, uint8_t *upage,
       size_t page_zero_bytes = PGSIZE - page_read_bytes;
 
       /* Add the page metadata to the SPT to be lazy loaded later on */
-      if (page_insert (file, ofs, upage, page_read_bytes, page_zero_bytes,
-                       writable, PAGE_FILE) == NULL)
+      if (page_insert_file (file, ofs, upage, page_read_bytes, page_zero_bytes,
+                            writable, PAGE_FILE) == NULL)
         return false;
 
       /* Advance. */

src/userprog/syscall.c

@@ -462,7 +462,7 @@ syscall_mmap (int fd, void *addr)
      hold the entire file. */
   for (off_t ofs = 0; ofs < file_size; ofs += PGSIZE)
     {
-      if (page_get (addr + ofs) != NULL)
+      if (page_get (thread_current (), addr + ofs) != NULL)
         return MMAP_FAILURE;
     }
 
@@ -472,7 +472,7 @@ syscall_mmap (int fd, void *addr)
       off_t read_bytes = file_size - ofs < PGSIZE ? file_size - ofs : PGSIZE;
       off_t zero_bytes = PGSIZE - read_bytes;
 
-      if (page_insert (file, ofs, addr + ofs, read_bytes, zero_bytes, true,
+      if (page_insert_file (file, ofs, addr + ofs, read_bytes, zero_bytes, true,
                        PAGE_FILE) == NULL)
         return MMAP_FAILURE;
     }

src/vm/frame.c

@@ -2,14 +2,14 @@
 #include <hash.h>
 #include <list.h>
 #include <string.h>
-
 #include "frame.h"
 #include "page.h"
+#include "filesys/file.h"
 #include "threads/malloc.h"
 #include "threads/vaddr.h"
 #include "userprog/pagedir.h"
+#include "userprog/syscall.h"
 #include "threads/synch.h"
-#include "devices/swap.h"
 
 /* Hash table that maps every active frame's kernel virtual address
    to its corresponding 'frame_metadata'.*/
@@ -79,7 +79,7 @@ frame_alloc (enum palloc_flags flags, void *upage, struct thread *owner)
 {
   struct frame_metadata *frame_metadata;
   flags |= PAL_USER;
-  
+
   lock_acquire (&lru_lock);
   void *frame = palloc_get_page (flags);
 
@@ -95,16 +95,25 @@ frame_alloc (enum palloc_flags flags, void *upage, struct thread *owner)
       struct frame_metadata *victim = get_victim ();
       ASSERT (victim != NULL); /* get_victim () should never return null. */
 
-      /* 2. Swap out victim into disk. */
-      /* Mark page as 'not present' and flag the page directory as having
-         been modified *before* eviction begins to prevent the owner of the
-         victim page from accessing/modifying it mid-eviction. */
-      pagedir_clear_page (victim->owner->pagedir, victim->upage);
+      /* 2. Handle victim page writing based on its type. */
+      struct page_entry *victim_page = page_get (victim->upage);
+      if (victim_page != NULL && victim_page->type == PAGE_MMAP)
+        {
+          /* If it was a memory-mapped file page, we just write it back
+             to the file if it was dirty. */
+          if (pagedir_is_dirty(owner->pagedir, victim->upage))
+            {
+              lock_acquire (&filesys_lock);
+              file_write_at (victim_page->file, victim->upage,
+                             victim_page->read_bytes, victim_page->offset);
+              lock_release (&filesys_lock);
+            }
 
-      // TODO: Lock PTE of victim page for victim process.
+        }
+      else
+        /* Otherwise, insert the page into swap. */
+        page_insert_swapped (victim->upage, victim->frame, victim->owner);
 
-      size_t swap_slot = swap_out (victim->frame);
-      page_set_swap (victim->owner, victim->upage, swap_slot);
 
       /* If zero flag is set, zero out the victim page. */
       if (flags & PAL_ZERO)

src/vm/mmap.c

@@ -1,5 +1,6 @@
 #include "mmap.h"
 #include "page.h"
+#include "threads/thread.h"
 #include "threads/vaddr.h"
 #include "threads/malloc.h"
 #include "userprog/syscall.h"
@@ -70,7 +71,7 @@ mmap_unmap (struct mmap_entry *mmap)
     void *upage = mmap->upage + ofs;
 
     /* Get the SPT page entry for this page. */
-    struct page_entry *page = page_get(upage);
+    struct page_entry *page = page_get(thread_current (), upage);
     if (page == NULL)
       continue;
 

src/vm/page.c

@@ -1,10 +1,12 @@
 #include "page.h"
+#include <stdint.h>
 #include <string.h>
 #include <stdio.h>
 #include "filesys/file.h"
 #include "threads/pte.h"
 #include "threads/malloc.h"
 #include "threads/palloc.h"
+#include "devices/swap.h"
 #include "userprog/process.h"
 #include "userprog/pagedir.h"
 #include "vm/frame.h"
@@ -33,22 +35,66 @@ page_less (const struct hash_elem *a_, const struct hash_elem *b_,
   return a->upage < b->upage;
 }
 
-/* Allocate and insert a new page entry into the thread's page table. */
+static void page_flag_swap (uint32_t *pte, bool set);
+static void page_set_swap (struct thread *owner, uint32_t *pte,
+                           size_t swap_slot);
+
+// TODO: Deal with NULL malloc returns
+/* Swap out 'owner' process's 'upage' stored at 'kpage'. Then, allocate and
+   insert a new page entry into the user process thread's SPT representing
+   this swapped out page. */
 struct page_entry *
-page_insert (struct file *file, off_t ofs, void *upage, uint32_t read_bytes,
-             uint32_t zero_bytes, bool writable, enum page_type type)
+page_insert_swapped (void *upage, void *kpage, struct thread *owner)
+{
+  /* 1. Initialize swapped page entry. */
+  struct page_entry *page = page_get (thread_current (), upage);
+  if (page == NULL)
+    {
+      page = malloc (sizeof (struct page_entry));
+      if (page == NULL)
+        return NULL;
+      page->upage = upage;
+      lock_init (&page->lock);
+      hash_insert (&owner->pages, &page->elem);
+    }
+
+  /* Mark page as 'swapped' and flag the page directory as having
+     been modified *before* eviction begins to prevent the owner of the
+     victim page from accessing/modifying it mid-eviction. */
+  /* TODO: We need to stop the process from destroying pagedir mid-eviction,
+    as this could render the page table entry invalid. */
+  uint32_t *pte = lookup_page (owner->pagedir, upage, false);
+
+  page_flag_swap (pte, true);
+  lock_acquire (&page->lock);
+  pagedir_clear_page (owner->pagedir, upage);
+
+  size_t swap_slot = swap_out (kpage);
+  page_set_swap (owner, pte, swap_slot);
+
+  lock_release (&page->lock);
+  return page;
+}
+
+/* Allocate and insert a new page entry into the user process thread's
+   SPT representing a file page. */
+struct page_entry *
+page_insert_file (struct file *file, off_t ofs, void *upage,
+                  uint32_t read_bytes, uint32_t zero_bytes, bool writable,
+                  enum page_type type)
 {
   struct page_entry *page = malloc(sizeof (struct page_entry));
   if (page == NULL)
     return NULL;
 
+  page->type = type;
   page->file = file;
   page->offset = ofs;
   page->upage = upage;
   page->read_bytes = read_bytes;
   page->zero_bytes = zero_bytes;
   page->writable = writable;
-  page->type = type;
+  lock_init (&page->lock);
 
   hash_insert (&thread_current ()->pages, &page->elem);
   return page;
@@ -57,13 +103,13 @@ page_insert (struct file *file, off_t ofs, void *upage, uint32_t read_bytes,
 /* Gets a page_entry from the starting address of the page. Returns NULL if no
    such page_entry exists in the hash map.*/
 struct page_entry *
-page_get (void *upage)
+page_get (struct thread *thread, void *upage)
 {
   struct page_entry fake_page_entry;
   fake_page_entry.upage = upage;
 
   struct hash_elem *e
-    = hash_find (&thread_current ()->pages, &fake_page_entry.elem);
+    = hash_find (&thread->pages, &fake_page_entry.elem);
   if (e == NULL)
       return NULL;
 
@@ -71,7 +117,7 @@ page_get (void *upage)
 }
 
 bool
-page_load (struct page_entry *page, bool writable)
+page_load_file (struct page_entry *page, bool writable)
 {
   /* Allocate a frame for the page. If a frame allocation fails, then
      frame_alloc should try to evict a page. If it is still NULL, the OS
@@ -114,14 +160,21 @@ page_cleanup (struct hash_elem *e, void *aux UNUSED)
   free (hash_entry (e, struct page_entry, elem));
 }
 
-/* Updates the 'owner' thread's page table entry for virtual address 'upage'
-   to flag the page as being stored in swap, and stores the specified swap slot
-   value in the entry at the address bits for later retrieval from disk. */
+/* Flags the provided page table entry as representing a swapped out page. */
 void
-page_set_swap (struct thread *owner, void *upage, size_t swap_slot)
-{
-  uint32_t *pte = lookup_page (owner->pagedir, upage, false);
+page_flag_swap (uint32_t *pte, bool set)
+  {
+    if (set)
+      *pte |= (1 << SWAP_FLAG_BIT);
+    else
+      *pte &= ~(1 << SWAP_FLAG_BIT);
+  }
 
+/* Sets the address bits of the page table entry to the provided swap slot
+   value. To be used for later retrieval of the swap slot when page faulting. */
+static void
+page_set_swap (struct thread *owner, uint32_t *pte, size_t swap_slot)
+{
   /* Store the provided swap slot in the address bits of the page table
      entry, truncating excess bits. */
   *pte |= (1 << SWAP_FLAG_BIT);
@@ -143,7 +196,7 @@ page_in_swap (struct thread *owner, void *upage)
 
 /* Given that the page with user address 'upage' owned by 'owner' is flagged
    to be in the swap disk via the owner's page table, returns its stored
-   swap slot. Otherwise panics the kernel. */
+   swap slot and marks the PTE as not being in swap. */
 size_t
 page_get_swap (struct thread *owner, void *upage)
 {
@@ -153,5 +206,6 @@ page_get_swap (struct thread *owner, void *upage)
    ASSERT ((*pte & PTE_P) == 0);
 
    /* Masks the address bits and returns truncated value. */
+   page_flag_swap (pte, false);
    return ((*pte & PTE_ADDR) >> ADDR_START_BIT);
 }

src/vm/page.h

@@ -2,17 +2,23 @@
 #define VM_PAGE_H
 
 #include "threads/thread.h"
+#include "threads/synch.h"
 #include "filesys/off_t.h"
 
 enum page_type {
   PAGE_FILE,
-  PAGE_EMPTY
+  PAGE_MMAP
 };
 
 struct page_entry {
   enum page_type type; /* Type of Data that should go into the page */
   void *upage;         /* Start Address of the User Page (Key of hash table). */
 
+  /* Data for swapped pages */
+  struct lock lock;    /* Enforces mutual exclusion in accessing the page
+                          referenced by the entry between its owning process
+                          and any thread performing page eviction. */
+
   /* File Data */
   struct file *file;   /* Pointer to the file for executables. */
   off_t offset;        /* Offset of the page content within the file. */
@@ -26,15 +32,16 @@ struct page_entry {
 unsigned page_hash (const struct hash_elem *e, void *aux);
 bool page_less (const struct hash_elem *a_, const struct hash_elem *b_,
                 void *aux);
-struct page_entry *page_insert (struct file *file, off_t ofs, void *upage,
-                                uint32_t read_bytes, uint32_t zero_bytes,
-                                bool writable, enum page_type type);
-struct page_entry *page_get (void *upage);
-bool page_load (struct page_entry *page, bool writable);
+struct page_entry *page_insert_swapped (void *upage, void* kpage,
+                                        struct thread *owner);
+struct page_entry *page_insert_file (struct file *file, off_t ofs, void *upage,
+                                     uint32_t read_bytes, uint32_t zero_bytes,
+                                     bool writable, enum page_type);
+struct page_entry *page_get (struct thread *thread, void *upage);
+bool page_load_file (struct page_entry *page, bool writable);
 void page_cleanup (struct hash_elem *e, void *aux);
 
-void page_set_swap (struct thread *, void *, size_t);
 bool page_in_swap (struct thread *, void *);
-size_t page_get_swap (struct thread *, void *);
+size_t page_get_swap (struct thread *owner, void *upage);
 
 #endif /* vm/frame.h */