From d03e2530461487c6e365b02e8768f0cc21dfa8a4 Mon Sep 17 00:00:00 2001 From: Themis Demetriades Date: Thu, 5 Dec 2024 16:51:15 +0000 Subject: [PATCH] feat: implement synchronisation to protecting access to PTEs of SPTs during eviction --- src/userprog/exception.c | 40 +++++++++++++++------ src/userprog/process.c | 4 +-- src/userprog/syscall.c | 5 ++- src/vm/frame.c | 13 ++----- src/vm/page.c | 77 +++++++++++++++++++++++++++++++++------- src/vm/page.h | 20 +++++++---- 6 files changed, 114 insertions(+), 45 deletions(-) diff --git a/src/userprog/exception.c b/src/userprog/exception.c index aae3690..c4bfa97 100644 --- a/src/userprog/exception.c +++ b/src/userprog/exception.c @@ -2,6 +2,7 @@ #include #include #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 (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,25 @@ 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; - } + hash_delete (&thread_current ()->pages, &page->elem); + lock_release (&page->lock); + page_cleanup (&page->elem, NULL); - /* 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 +290,14 @@ 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); + if (success && page->type == PAGE_FILE) + { + hash_delete (&thread_current ()->pages, &page->elem); + page_cleanup (&page->elem, NULL); + } break; default: return false; diff --git a/src/userprog/process.c b/src/userprog/process.c index 9c46b0e..ca99aee 100644 --- a/src/userprog/process.c +++ b/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. */ diff --git a/src/userprog/syscall.c b/src/userprog/syscall.c index 44397ad..44a8505 100644 --- a/src/userprog/syscall.c +++ b/src/userprog/syscall.c @@ -440,8 +440,8 @@ 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, - PAGE_FILE) == NULL) + if (page_insert_file (file, ofs, addr + ofs, read_bytes, zero_bytes, true, + PAGE_MMAP) == NULL) return MMAP_FAILURE; } @@ -450,7 +450,6 @@ syscall_mmap (int fd, void *addr) if (mmap == NULL) return MMAP_FAILURE; - return mmap->mapping; } diff --git a/src/vm/frame.c b/src/vm/frame.c index 98339f8..2c57d53 100644 --- a/src/vm/frame.c +++ b/src/vm/frame.c @@ -9,7 +9,6 @@ #include "threads/vaddr.h" #include "userprog/pagedir.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'.*/ @@ -76,7 +75,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); @@ -93,15 +92,7 @@ frame_alloc (enum palloc_flags flags, void *upage, struct thread *owner) 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); - - // TODO: Lock PTE of victim page for victim process. - - size_t swap_slot = swap_out (victim->frame); - page_set_swap (victim->owner, victim->upage, swap_slot); + page_insert_swapped (victim->upage, victim->frame, victim->owner); /* If zero flag is set, zero out the victim page. */ if (flags & PAL_ZERO) diff --git a/src/vm/page.c b/src/vm/page.c index 37d95e5..d1f9f64 100644 --- a/src/vm/page.c +++ b/src/vm/page.c @@ -1,10 +1,12 @@ #include "page.h" +#include #include #include #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,65 @@ 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); + +/* 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 = malloc(sizeof (struct page_entry)); + if (page == NULL) + return NULL; + + page->upage = upage; + lock_init (&page->lock); + + /* 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); + ASSERT (pte != NULL); + + page_flag_swap (pte, true); + lock_acquire (&page->lock); + struct hash_elem *elem = hash_insert (&owner->pages, &page->elem); + ASSERT(elem == NULL); + + 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; hash_insert (&thread_current ()->pages, &page->elem); return page; @@ -71,7 +116,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 +159,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 +195,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 +205,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); } diff --git a/src/vm/page.h b/src/vm/page.h index 73c8d34..893317d 100644 --- a/src/vm/page.h +++ b/src/vm/page.h @@ -2,10 +2,12 @@ #define VM_PAGE_H #include "threads/thread.h" +#include "threads/synch.h" #include "filesys/off_t.h" enum page_type { PAGE_FILE, + PAGE_MMAP, PAGE_EMPTY }; @@ -13,6 +15,11 @@ 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 +33,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_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 (void *upage); -bool page_load (struct page_entry *page, bool writable); +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 */