diff --git a/src/userprog/exception.c b/src/userprog/exception.c index c2e973c..3996776 100644 --- a/src/userprog/exception.c +++ b/src/userprog/exception.c @@ -13,8 +13,7 @@ static long long page_fault_cnt; static void kill (struct intr_frame *); static void page_fault (struct intr_frame *); -static bool try_fetch_page (void *upage, bool not_present, bool write, - bool user); +static bool try_fetch_page (void *upage, bool write); /* Registers handlers for interrupts that can be caused by user programs. @@ -150,9 +149,15 @@ page_fault (struct intr_frame *f) write = (f->error_code & PF_W) != 0; user = (f->error_code & PF_U) != 0; + /* If the fault address is in a user page that is not present, then it might + just need to be lazily loaded. So, we check our SPT to see if the page + is expected to have data loaded in memory. */ void *upage = pg_round_down (fault_addr); - if (try_fetch_page (upage, not_present, write, user)) - return; + if (not_present && is_user_vaddr (upage) && upage != NULL) + { + if (try_fetch_page (upage, write)) + return; + } /* To implement virtual memory, delete the rest of the function body, and replace it with code that brings in the page to @@ -166,11 +171,10 @@ page_fault (struct intr_frame *f) } static bool -try_fetch_page (void *upage, bool not_present, bool write, bool user) +try_fetch_page (void *upage, bool write) { - if (!not_present || !is_user_vaddr (upage) || upage == NULL) - return false; - + /* 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; @@ -182,6 +186,7 @@ try_fetch_page (void *upage, bool not_present, bool write, bool user) return true; } + /* Load the page into memory based on the type of data it is expecting. */ switch (page->type) { case PAGE_EXECUTABLE: return page_load (page, write); diff --git a/src/userprog/process.c b/src/userprog/process.c index d0e6bdf..f4a7439 100644 --- a/src/userprog/process.c +++ b/src/userprog/process.c @@ -623,7 +623,6 @@ load (const char *file_name, void (**eip) (void), void **esp) done: /* We arrive here whether the load is successful or not. */ - file_close (file); lock_release (&filesys_lock); return success; } @@ -691,14 +690,13 @@ validate_segment (const struct Elf32_Phdr *phdr, struct file *file) or disk read error occurs. */ static bool load_segment (struct file *file, off_t ofs, uint8_t *upage, - uint32_t read_bytes, uint32_t zero_bytes, bool writable) + uint32_t read_bytes, uint32_t zero_bytes, bool writable) { ASSERT ((read_bytes + zero_bytes) % PGSIZE == 0); ASSERT (pg_ofs (upage) == 0); ASSERT (ofs % PGSIZE == 0); - file_seek (file, ofs); - while (read_bytes > 0 || zero_bytes > 0) + while (read_bytes > 0 || zero_bytes > 0) { /* Calculate how to fill this page. We will read PAGE_READ_BYTES bytes from FILE @@ -706,15 +704,15 @@ load_segment (struct file *file, off_t ofs, uint8_t *upage, size_t page_read_bytes = read_bytes < PGSIZE ? read_bytes : PGSIZE; size_t page_zero_bytes = PGSIZE - page_read_bytes; - /* Add the page to the SPT */ + /* 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_EXECUTABLE) == NULL) return false; /* Advance. */ - ofs += page_read_bytes; read_bytes -= page_read_bytes; zero_bytes -= page_zero_bytes; + ofs += PGSIZE; upage += PGSIZE; } return true; diff --git a/src/vm/page.c b/src/vm/page.c index 5a4e1e0..010bd9d 100644 --- a/src/vm/page.c +++ b/src/vm/page.c @@ -69,30 +69,34 @@ page_get (void *upage) bool page_load (struct page_entry *page, bool writable) { - void *frame = frame_alloc (PAL_USER, page->upage, thread_current()); + /* 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 ()); if (frame == NULL) - return false; // TODO: Try to evict a page instead. + PANIC ("Could not allocate a frame to load page into memory."); + /* Map the page to the frame. */ if (!install_page (page->upage, frame, writable)) - { - frame_free (frame); - return false; - } - - printf ("Hi! 3\n"); + { + frame_free (frame); + return false; + } + /* Move the file pointer to the correct location in the file. Then, read the + data from the file into the frame. Checks that we were able to read the + expected number of bytes. */ file_seek (page->file, page->offset); - int read = -1; - if ((read = file_read (page->file, frame, page->read_bytes)) != (int) page->read_bytes) + if (file_read (page->file, frame, page->read_bytes) != (int) page->read_bytes) { - printf ("%p, %p, %d %d\n", page->file, frame, read, page->offset); frame_free (frame); return false; } - printf ("Hi! 4\n"); - + /* Zero out the remaining bytes in the frame. */ memset (frame + page->read_bytes, 0, page->zero_bytes); + + /* Mark the page as loaded successfully. */ return true; }