fix: free non-shared pages that are in swap + utility functions for frame owners.

src/userprog/pagedir.c

@@ -5,6 +5,10 @@
 #include "threads/init.h"
 #include "threads/pte.h"
 #include "threads/palloc.h"
+#ifdef VM
+#include "vm/frame.h"
+#include "vm/page.h"
+#endif
 
 static uint32_t *active_pd (void);
 
@@ -39,8 +43,16 @@ pagedir_destroy (uint32_t *pd)
         uint32_t *pte;
         
         for (pte = pt; pte < pt + PGSIZE / sizeof *pte; pte++)
-          if (*pte & PTE_P) 
-            palloc_free_page (pte_get_page (*pte));
+          {
+#ifdef VM
+            if (*pte & PTE_P)
+              frame_free (pte_get_page (*pte));
+            page_cleanup_swap (pte);
+#else
+            if (*pte & PTE_P)
+              palloc_free_page (pte_get_page (*pte));
+#endif
+          }
         palloc_free_page (pt);
       }
   palloc_free_page (pd);

src/userprog/process.c

@@ -826,6 +826,7 @@ static void
 free_usr_kpage (void *kpage)
 {
 #ifdef VM
+  frame_owner_remove (kpage, thread_current ());
   frame_free (kpage);
 #else
   palloc_free_page (kpage);

src/vm/frame.c

@@ -55,6 +55,8 @@ struct frame_metadata
 hash_hash_func frame_metadata_hash;
 hash_less_func frame_metadata_less;
 
+static struct frame_metadata *frame_metadata_find (void *frame);
+
 static struct list_elem *lru_next (struct list_elem *e);
 static struct list_elem *lru_prev (struct list_elem *e);
 static struct frame_metadata *get_victim (void);
@@ -120,8 +122,7 @@ frame_alloc (enum palloc_flags flags, void *upage, struct thread *owner)
         {
           struct frame_owner *frame_owner
               = list_entry (e, struct frame_owner, elem);
-          page_set_swap (frame_owner->thread->pagedir, victim->upage,
-                         swap_slot);
+          page_set_swap (frame_owner->thread, victim->upage, swap_slot);
         }
 
       /* If zero flag is set, zero out the victim page. */
@@ -172,23 +173,63 @@ frame_alloc (enum palloc_flags flags, void *upage, struct thread *owner)
   return frame_metadata->frame;
 }
 
+/* Add a thread to a frame's frame_metadata owners list. */
+bool
+frame_owner_insert (void *frame, struct thread *owner)
+{
+  struct frame_metadata *frame_metadata = frame_metadata_find (frame);
+  if (frame_metadata == NULL)
+    return false;
+
+  struct frame_owner *frame_owner = malloc (sizeof (struct frame_owner));
+  if (frame_owner == NULL)
+    return false;
+  frame_owner->thread = owner;
+  list_push_back (&frame_metadata->owners, &frame_owner->elem);
+  return true;
+}
+
+/* Remove and deallocate a frame owner from the frame_metadata owners list. */
+void
+frame_owner_remove (void *frame, struct thread *owner)
+{
+  struct frame_metadata *frame_metadata = frame_metadata_find (frame);
+  if (frame_metadata == NULL)
+    PANIC ("Attempted to remove an owner from a frame at kernel "
+           "address %p, but this address is not allocated!\n",
+           frame);
+
+  struct list_elem *oe;
+  for (oe = list_begin (&frame_metadata->owners);
+       oe != list_end (&frame_metadata->owners);)
+    {
+      struct frame_owner *frame_owner
+          = list_entry (oe, struct frame_owner, elem);
+      oe = list_next (oe);
+      if (frame_owner->thread == owner)
+        {
+          list_remove (oe);
+          free (frame_owner);
+          return;
+        }
+    }
+  NOT_REACHED ();
+}
+
 /* Attempt to deallocate a frame for a user process by removing it from the
    frame table as well as lru_list, and freeing the underlying page
    memory & metadata struct. Panics if the frame isn't active in memory. */
 void
 frame_free (void *frame)
   {
-    struct frame_metadata key_metadata;
-    key_metadata.frame = frame;
+    struct frame_metadata *frame_metadata = frame_metadata_find (frame);
+    if (frame_metadata == NULL)
+      PANIC ("Attempted to free a frame at kernel "
+             "address %p, but this address is not allocated!\n",
+             frame);
 
-    struct hash_elem *e =
-      hash_delete (&frame_table, &key_metadata.hash_elem);
-    if (e == NULL) PANIC ("Attempted to free a frame at kernel address %p, "
-                          "but this address is not allocated!\n", frame);
+    ASSERT (list_empty (&frame_metadata->owners));
 
-    struct frame_metadata *frame_metadata =
-      hash_entry (e, struct frame_metadata, hash_elem);
-    
     lock_acquire (&lru_lock);
     list_remove (&frame_metadata->list_elem);
 
@@ -208,6 +249,19 @@ frame_free (void *frame)
     palloc_free_page (frame);
   }
 
+/* Find a frame_metadata entry in the frame table. */
+static struct frame_metadata *
+frame_metadata_find (void *frame)
+{
+  struct frame_metadata key_metadata;
+  key_metadata.frame = frame;
+
+  struct hash_elem *e = hash_find (&frame_table, &key_metadata.hash_elem);
+  if (e == NULL)
+    return NULL;
+  return hash_entry (e, struct frame_metadata, hash_elem);
+}
+
 /* TODO: Account for page aliases when checking accessed bit. */
 /* A pre-condition for calling this function is that the calling thread
    owns lru_lock and that lru_list is non-empty. */

src/vm/frame.h

@@ -8,4 +8,7 @@ void frame_init (void);
 void *frame_alloc (enum palloc_flags, void *, struct thread *);
 void frame_free (void *frame);
 
+bool frame_owner_insert (void *frame, struct thread *owner);
+void frame_owner_remove (void *frame, struct thread *owner);
+
 #endif /* vm/frame.h */

src/vm/page.c

@@ -1,5 +1,6 @@
 #include "page.h"
 #include <string.h>
+#include "devices/swap.h"
 #include "filesys/file.h"
 #include "filesys/filesys.h"
 #include "threads/malloc.h"
@@ -29,6 +30,7 @@ static unsigned shared_page_hash (const struct hash_elem *e, void *aux UNUSED);
 static bool shared_page_less (const struct hash_elem *a_,
                               const struct hash_elem *b_, void *aux UNUSED);
 static void shared_page_cleanup (struct hash_elem *e, void *aux UNUSED);
+static void page_unset_swap (struct thread *owner, void *upage);
 
 /* Initialise a thread's supplemental pages table. */
 bool
@@ -131,7 +133,9 @@ page_load (struct page_entry *page)
   /* 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. */
-  void *frame = frame_alloc (PAL_USER, page->upage, thread_current ()); // TODO : PAL_USER or 0???
+  struct thread *t = thread_current ();
+  void *frame
+      = frame_alloc (PAL_USER, page->upage, t); // TODO : PAL_USER or 0???
   if (frame == NULL)
     PANIC ("Could not allocate a frame to load page into memory.");
 
@@ -170,6 +174,7 @@ page_load (struct page_entry *page)
 fail:
   if (!page->writable)
     lock_release (&shared_files_lock);
+  frame_owner_remove (frame, t);
   frame_free (frame);
   return false;
 }
@@ -179,12 +184,17 @@ fail:
 void
 page_cleanup (struct hash_elem *e, void *aux UNUSED)
 {
+  struct thread *t = thread_current ();
   struct page_entry *page = hash_entry (e, struct page_entry, elem);
 
-  /* If page is shared then mark it as not present to avoid it being freed. */
-  uint32_t *pd = thread_current ()->pagedir;
+  /* If page is shared then mark it as not present and not in swap, to avoid
+   * being freed. */
+  uint32_t *pd = t->pagedir;
   if (pd != NULL && page->shared)
-    pagedir_clear_page (pd, page->upage);
+    {
+      pagedir_clear_page (pd, page->upage);
+      page_unset_swap (t, page->upage);
+    }
   free (page);
 }
 
@@ -389,6 +399,19 @@ page_set_swap (struct thread *owner, void *upage, size_t swap_slot)
   invalidate_pagedir (owner->pagedir);
 }
 
+/* Updates the page table entry for virtual address 'upage' to flag the page as
+   NOT being stored in swap, and clears the excess bits. */
+static void
+page_unset_swap (struct thread *owner, void *upage)
+{
+  uint32_t *pte = lookup_page (owner->pagedir, upage, false);
+
+  *pte &= ~(1 << SWAP_FLAG_BIT);
+  *pte &= ~PTE_ADDR;
+
+  invalidate_pagedir (owner->pagedir);
+}
+
 /* Returns true iff the page with user address 'upage' owned by 'owner' 
    is flagged to be in the swap disk via the owner's page table. */
 bool
@@ -413,3 +436,16 @@ page_get_swap (struct thread *owner, void *upage)
    /* Masks the address bits and returns truncated value. */
    return ((*pte & PTE_ADDR) >> ADDR_START_BIT);
 }
+
+/* If the swap bit is set for a page table entry, drop the swap. */
+bool
+page_cleanup_swap (uint32_t *pte)
+{
+  if ((*pte & (1 << SWAP_FLAG_BIT)) != 0)
+    {
+      size_t swap_slot = ((*pte & PTE_ADDR) >> ADDR_START_BIT);
+      swap_drop (swap_slot);
+      return true;
+    }
+  return false;
+}

src/vm/page.h

@@ -53,6 +53,7 @@ void page_cleanup (struct hash_elem *e, void *aux UNUSED);
 void page_set_swap (struct thread *, void *, size_t);
 bool page_in_swap (struct thread *, void *);
 size_t page_get_swap (struct thread *, void *);
+bool page_cleanup_swap (uint32_t *pte);
 
 void shared_files_init ();
 bool use_shared_file (struct file *file);