feat: initial implementation of shared pages with proper destruction

src/threads/thread.c

@@ -134,8 +134,6 @@ thread_start (void)
                   t))
     PANIC ("Failed to initialise child results table for main thread.");
 
-  lock_init (&shared_files_lock);
-
   /* Create the idle thread. */
   struct semaphore idle_started;
   sema_init (&idle_started, 0);

src/userprog/pagedir.c

@@ -50,7 +50,8 @@ pagedir_destroy (uint32_t *pd)
               {
                 void *page = pte_get_page (*pte);
                 frame_owner_remove (page, thread_current ());
-                frame_free (page);
+                // frame_free (page);
+                palloc_free_page (page);
               }
             page_cleanup_swap (pte);
 #else

src/vm/frame.c

@@ -5,6 +5,7 @@
 
 #include "frame.h"
 #include "page.h"
+#include "filesys/file.h"
 #include "threads/malloc.h"
 #include "threads/vaddr.h"
 #include "userprog/pagedir.h"
@@ -106,24 +107,48 @@ frame_alloc (enum palloc_flags flags, void *upage, struct thread *owner)
          been modified *before* eviction begins to prevent the owner of the
          victim page from accessing/modifying it mid-eviction. */
       struct list_elem *e;
+      struct file *file = NULL;
       for (e = list_begin (&victim->owners); e != list_end (&victim->owners);
            e = list_next (e))
         {
           struct frame_owner *frame_owner
               = list_entry (e, struct frame_owner, elem);
+          file = frame_owner->thread->exec_file;
           pagedir_clear_page (frame_owner->thread->pagedir, victim->upage);
         }
 
+      /* If file is found then it must be the same for all owners, and might
+         have a single shared page entry. */
+      struct shared_page_entry *shared_page = NULL;
+      if (file != NULL)
+        {
+          lock_acquire (&shared_files_lock);
+          shared_page = shared_page_get (file, victim->upage);
+          ASSERT (shared_page == NULL || shared_page->frame != NULL);
+          if (shared_page == NULL)
+            lock_release (&shared_files_lock);
+        }
+
       // TODO: Lock PTE of victim page for victim process.
 
       size_t swap_slot = swap_out (victim->frame);
-      for (e = list_begin (&victim->owners); e != list_end (&victim->owners);
-           e = list_next (e))
+
+      /* If frame had a shared page, unsign it, and set the swap slot.
+         Otherwise, set the swap slot in the pagedir of the owners threads. */
+      if (shared_page != NULL)
         {
-          struct frame_owner *frame_owner
-              = list_entry (e, struct frame_owner, elem);
-          page_set_swap (frame_owner->thread, victim->upage, swap_slot);
+          shared_page->frame = NULL;
+          shared_page->swap_slot = swap_slot;
+          lock_release (&shared_files_lock);
         }
+      else
+        for (e = list_begin (&victim->owners); e != list_end (&victim->owners);
+             e = list_next (e))
+          {
+            struct frame_owner *frame_owner
+                = list_entry (e, struct frame_owner, elem);
+            page_set_swap (frame_owner->thread, victim->upage, swap_slot);
+          }
 
       /* If zero flag is set, zero out the victim page. */
       if (flags & PAL_ZERO)

src/vm/page.c

@@ -1,5 +1,6 @@
 #include "page.h"
 #include <string.h>
+#include <stdio.h>
 #include "devices/swap.h"
 #include "filesys/file.h"
 #include "filesys/filesys.h"
@@ -15,7 +16,6 @@
 #define ADDR_START_BIT 12
 
 struct hash shared_files;
-struct lock shared_files_lock;
 
 static unsigned page_hash (const struct hash_elem *e, void *aux UNUSED);
 static bool page_less (const struct hash_elem *a_, const struct hash_elem *b_,
@@ -24,8 +24,6 @@ static struct shared_file_entry *shared_file_insert (struct file *file);
 static struct shared_page_entry *shared_page_insert (struct file *file,
                                                      void *upage, void *frame);
 static struct shared_file_entry *shared_file_get (struct file *file);
-static struct shared_page_entry *shared_page_get (struct file *file,
-                                                  void *upage);
 static unsigned shared_file_hash (const struct hash_elem *e, void *aux UNUSED);
 static bool shared_file_less (const struct hash_elem *a_,
                               const struct hash_elem *b_, void *aux UNUSED);
@@ -114,6 +112,7 @@ page_get (void *upage)
 bool
 page_load (struct page_entry *page)
 {
+  struct thread *t = thread_current ();
   /* If the page is read-only, we want to check if it is a shared page already
      loaded into memory. If it is, we can just map the page to the frame. */
   if (!page->writable)
@@ -125,6 +124,13 @@ page_load (struct page_entry *page)
       /* Mark page as shared and install the shared frame. */
       if (shared_page != NULL)
         {
+          if (shared_page->frame == NULL)
+            {
+              void *frame = frame_alloc (PAL_USER, page->upage, t);
+              if (frame == NULL)
+                return false;
+              shared_page->frame = frame;
+            }
           lock_release (&shared_files_lock);
           if (!install_page (page->upage, shared_page->frame, false))
             return false;
@@ -132,13 +138,10 @@ page_load (struct page_entry *page)
           return true;
         }
     }
-
   /* 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
      panics as this should not happen if eviction is working correctly. */
-  struct thread *t = thread_current ();
-  void *frame
-      = frame_alloc (PAL_USER, page->upage, t); // TODO : PAL_USER or 0???
+  void *frame = frame_alloc (PAL_USER, page->upage, t);
   if (frame == NULL)
     PANIC ("Could not allocate a frame to load page into memory.");
 
@@ -195,6 +198,7 @@ page_cleanup (struct hash_elem *e, void *aux UNUSED)
   uint32_t *pd = t->pagedir;
   if (pd != NULL && page->shared)
     {
+      frame_owner_remove (pagedir_get_page (pd, page->upage), t);
       pagedir_clear_page (pd, page->upage);
       page_unset_swap (t, page->upage);
     }
@@ -254,7 +258,11 @@ shared_page_cleanup (struct hash_elem *e, void *aux UNUSED)
 {
   struct shared_page_entry *shared_page
       = hash_entry (e, struct shared_page_entry, elem);
-  // frame_free (shared_page->frame);
+  if (shared_page->frame == NULL)
+    swap_drop (shared_page->swap_slot);
+  else
+    /* Note: ref_count <= 0, so it is guaranteed that the frame is unused. */
+    frame_free (shared_page->frame);
   free (shared_page);
 }
 
@@ -316,7 +324,7 @@ shared_file_get (struct file *file)
 
 /* Gets a shared_page_entry from the shared_pages table using the file and upage
    of the page. Returns NULL if no such page_entry exists in the hash map.*/
-static struct shared_page_entry *
+struct shared_page_entry *
 shared_page_get (struct file *file, void *upage)
 {
   /* Search first for the file within the shared_pages structure */

src/vm/page.h

@@ -5,6 +5,8 @@
 #include "threads/synch.h"
 #include "filesys/off_t.h"
 
+struct lock shared_files_lock;
+
 enum page_type {
   PAGE_EXECUTABLE,
   PAGE_EMPTY
@@ -36,6 +38,7 @@ struct shared_file_entry {
 struct shared_page_entry {
   void *upage;
   void *frame;
+  size_t swap_slot;
 
   struct hash_elem elem;
 };
@@ -55,5 +58,6 @@ bool page_cleanup_swap (uint32_t *pte);
 void shared_files_init ();
 bool use_shared_file (struct file *file);
 bool unuse_shared_file (struct file *file);
+struct shared_page_entry *shared_page_get (struct file *file, void *upage);
 
 #endif /* vm/frame.h */