From acc768e17790936d8235964b11adc7fbda5cbce9 Mon Sep 17 00:00:00 2001 From: sBubshait Date: Wed, 4 Dec 2024 13:00:16 +0000 Subject: [PATCH 01/17] Add mmap module in vm defining mmap_entry structure and some helper functions --- src/Makefile.build | 1 + src/vm/mmap.c | 1 + src/vm/mmap.h | 18 ++++++++++++++++++ 3 files changed, 20 insertions(+) create mode 100644 src/vm/mmap.c create mode 100644 src/vm/mmap.h diff --git a/src/Makefile.build b/src/Makefile.build index 7778f57..fea3a67 100644 --- a/src/Makefile.build +++ b/src/Makefile.build @@ -64,6 +64,7 @@ userprog_SRC += userprog/tss.c # TSS management. # Virtual memory code. vm_SRC += vm/frame.c # Frame table manager. vm_SRC += vm/page.c # Page table manager. +vm_SRC += vm/mmap.c # Memory-mapped files. vm_SRC += devices/swap.c # Swap block manager. # Filesystem code. diff --git a/src/vm/mmap.c b/src/vm/mmap.c new file mode 100644 index 0000000..328f37b --- /dev/null +++ b/src/vm/mmap.c @@ -0,0 +1 @@ +#include "mmap.h" \ No newline at end of file diff --git a/src/vm/mmap.h b/src/vm/mmap.h new file mode 100644 index 0000000..8fd64ee --- /dev/null +++ b/src/vm/mmap.h @@ -0,0 +1,18 @@ +#ifndef VM_MMAP_H +#define VM_MMAP_H + +#include + +/* A mapping identifier type. */ +typedef unsigned mapid_t; + +/* A structure to represent a memory mapped file. */ +struct mmap_entry { + mapid_t mapping; /* The mapping identifier of the mapped file. */ + struct file *file; /* A pointer to the file that is being mapped. */ + void *upage; /* The start address of the file data in the user VM. */ + + struct hash_elem elem; /* An elem for the hash table. */ +}; + +#endif /* vm/mmap.h */ From 85aabd86cd1f0f9654d152019daec39ad1b799cd Mon Sep 17 00:00:00 2001 From: sBubshait Date: Wed, 4 Dec 2024 14:55:28 +0000 Subject: [PATCH 02/17] Update gitlab ci file to include mmap tests in the automated testing pipeline --- .gitlab-ci.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml index 768269b..ce5bf22 100644 --- a/.gitlab-ci.yml +++ b/.gitlab-ci.yml @@ -37,4 +37,4 @@ test_vm: extends: .pintos_tests variables: DIR: vm - IGNORE: (tests/vm/pt-grow-stack|tests/vm/pt-grow-pusha|tests/vm/pt-big-stk-obj|tests/vm/pt-overflowstk|tests/vm/pt-write-code2|tests/vm/pt-grow-stk-sc|tests/vm/page-linear|tests/vm/page-parallel|tests/vm/page-merge-seq|tests/vm/page-merge-par|tests/vm/page-merge-stk|tests/vm/page-merge-mm|tests/vm/mmap-read|tests/vm/mmap-close|tests/vm/mmap-overlap|tests/vm/mmap-twice|tests/vm/mmap-write|tests/vm/mmap-exit|tests/vm/mmap-shuffle|tests/vm/mmap-clean|tests/vm/mmap-inherit|tests/vm/mmap-misalign|tests/vm/mmap-null|tests/vm/mmap-over-code|tests/vm/mmap-over-data|tests/vm/mmap-over-stk|tests/vm/mmap-remove) + IGNORE: (tests/vm/pt-grow-stack|tests/vm/pt-grow-pusha|tests/vm/pt-big-stk-obj|tests/vm/pt-overflowstk|tests/vm/pt-write-code2|tests/vm/pt-grow-stk-sc|tests/vm/page-linear|tests/vm/page-parallel|tests/vm/page-merge-seq|tests/vm/page-merge-par|tests/vm/page-merge-stk|tests/vm/page-merge-mm) From b3042b5aa66ebf342076b78ef140a8a6fc0e5601 Mon Sep 17 00:00:00 2001 From: sBubshait Date: Wed, 4 Dec 2024 14:56:52 +0000 Subject: [PATCH 03/17] Update thread structure to add mmap files table and a counter for mappings of the thread --- src/threads/thread.h | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/threads/thread.h b/src/threads/thread.h index bdfad35..2904022 100644 --- a/src/threads/thread.h +++ b/src/threads/thread.h @@ -137,6 +137,10 @@ struct thread struct hash pages; /* Table of open user pages. */ + /* Memory mapped files for user virtual memory. */ + struct hash mmap_files; /* List of memory mapped files. */ + unsigned int mmap_counter; /* Counter for memory mapped files. */ + #ifdef USERPROG /* Owned by userprog/process.c. */ uint32_t *pagedir; /* Page directory. */ From 1ce09a49a11315d4159e47e30448b8c662f37724 Mon Sep 17 00:00:00 2001 From: sBubshait Date: Wed, 4 Dec 2024 15:08:43 +0000 Subject: [PATCH 04/17] Add helper functions to initialise the memory-mapped files table and counter --- src/threads/thread.c | 3 +++ src/vm/mmap.c | 36 +++++++++++++++++++++++++++++++++++- src/vm/mmap.h | 3 +++ 3 files changed, 41 insertions(+), 1 deletion(-) diff --git a/src/threads/thread.c b/src/threads/thread.c index c2944cc..cda9f49 100644 --- a/src/threads/thread.c +++ b/src/threads/thread.c @@ -16,6 +16,7 @@ #include "threads/synch.h" #include "threads/vaddr.h" #include "vm/page.h" +#include "vm/mmap.h" #ifdef USERPROG #include "userprog/process.h" #include "userprog/syscall.h" @@ -274,6 +275,8 @@ thread_create (const char *name, int priority, } #endif + mmap_init (); + /* Prepare thread for first run by initializing its stack. Do this atomically so intermediate values for the 'stack' member cannot be observed. */ diff --git a/src/vm/mmap.c b/src/vm/mmap.c index 328f37b..48bbf39 100644 --- a/src/vm/mmap.c +++ b/src/vm/mmap.c @@ -1 +1,35 @@ -#include "mmap.h" \ No newline at end of file +#include "mmap.h" +#include "threads/malloc.h" + +static unsigned mmap_hash (const struct hash_elem *e, void *aux); +static bool mmap_less (const struct hash_elem *a_, const struct hash_elem *b_, + void *aux); + +/* Initializes the mmap table for the current thread. Sets the counter to 0. */ +bool +mmap_init (void) +{ + struct thread *t = thread_current (); + t->mmap_counter = 0; + return hash_init (&t->mmap_files, mmap_hash, mmap_less, NULL); +} + +/* A hash function for the mmap table. Returns a hash for an entry, based on its + mapping. */ +static unsigned +mmap_hash (const struct hash_elem *e, void *aux UNUSED) +{ + return hash_entry (e, struct mmap_entry, elem)->mapping; +} + +/* A comparator function for the mmap table. Compares two entries based on their + mappings. */ +static bool +mmap_less (const struct hash_elem *a_, const struct hash_elem *b_, + void *aux UNUSED) +{ + const struct mmap_entry *a = hash_entry (a_, struct mmap_entry, elem); + const struct mmap_entry *b = hash_entry (b_, struct mmap_entry, elem); + + return a->mapping < b->mapping; +} diff --git a/src/vm/mmap.h b/src/vm/mmap.h index 8fd64ee..30bf76a 100644 --- a/src/vm/mmap.h +++ b/src/vm/mmap.h @@ -2,6 +2,7 @@ #define VM_MMAP_H #include +#include "threads/thread.h" /* A mapping identifier type. */ typedef unsigned mapid_t; @@ -15,4 +16,6 @@ struct mmap_entry { struct hash_elem elem; /* An elem for the hash table. */ }; +bool mmap_init (void); + #endif /* vm/mmap.h */ From a2f46f3b722e93c59aa428074f8cc2dfea9a2929 Mon Sep 17 00:00:00 2001 From: sBubshait Date: Wed, 4 Dec 2024 15:14:02 +0000 Subject: [PATCH 05/17] Add a mmap destroy function to cleanup all mmap hash table entries upon thread exit --- src/threads/thread.c | 2 ++ src/vm/mmap.c | 18 ++++++++++++++++++ src/vm/mmap.h | 2 ++ 3 files changed, 22 insertions(+) diff --git a/src/threads/thread.c b/src/threads/thread.c index cda9f49..0073037 100644 --- a/src/threads/thread.c +++ b/src/threads/thread.c @@ -397,6 +397,8 @@ thread_exit (void) process_exit (); #endif + mmap_destroy (); + /* Remove thread from all threads list, set our status to dying, and schedule another process. That process will destroy us when it calls thread_schedule_tail(). */ diff --git a/src/vm/mmap.c b/src/vm/mmap.c index 48bbf39..50a0cdf 100644 --- a/src/vm/mmap.c +++ b/src/vm/mmap.c @@ -33,3 +33,21 @@ mmap_less (const struct hash_elem *a_, const struct hash_elem *b_, return a->mapping < b->mapping; } + +/* Cleans up the mmap table for the current thread. Frees the memory allocated + for the mmap entry. */ +static void +mmap_cleanup (struct hash_elem *e, void *aux UNUSED) +{ + struct mmap_entry *mmap = hash_entry (e, struct mmap_entry, elem); + file_close (mmap->file); + free (mmap); +} + +/* Destroys the mmap table for the current thread. Frees all the memory + allocated for the mmap entries. */ +void +mmap_destroy (void) +{ + hash_destroy (&thread_current ()->mmap_files, mmap_cleanup); +} \ No newline at end of file diff --git a/src/vm/mmap.h b/src/vm/mmap.h index 30bf76a..de71ada 100644 --- a/src/vm/mmap.h +++ b/src/vm/mmap.h @@ -3,6 +3,7 @@ #include #include "threads/thread.h" +#include "filesys/file.h" /* A mapping identifier type. */ typedef unsigned mapid_t; @@ -17,5 +18,6 @@ struct mmap_entry { }; bool mmap_init (void); +void mmap_destroy (void); #endif /* vm/mmap.h */ From 6e838aa06a86d72a6788dca165119abddaeeb40f Mon Sep 17 00:00:00 2001 From: sBubshait Date: Wed, 4 Dec 2024 15:24:11 +0000 Subject: [PATCH 06/17] Fix Bug in thread.c: Only initialise and destroy mmap files table if VM is defined --- src/threads/thread.c | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/src/threads/thread.c b/src/threads/thread.c index 0073037..214a458 100644 --- a/src/threads/thread.c +++ b/src/threads/thread.c @@ -15,11 +15,13 @@ #include "threads/switch.h" #include "threads/synch.h" #include "threads/vaddr.h" -#include "vm/page.h" -#include "vm/mmap.h" #ifdef USERPROG #include "userprog/process.h" #include "userprog/syscall.h" +#include "vm/page.h" +#endif +#ifdef VM +#include "vm/mmap.h" #endif /* Random value for struct thread's `magic' member. @@ -275,7 +277,9 @@ thread_create (const char *name, int priority, } #endif +#ifdef VM mmap_init (); +#endif /* Prepare thread for first run by initializing its stack. Do this atomically so intermediate values for the 'stack' @@ -397,7 +401,9 @@ thread_exit (void) process_exit (); #endif +#ifdef VM mmap_destroy (); +#endif /* Remove thread from all threads list, set our status to dying, and schedule another process. That process will destroy us From 6b0f708d8f6e997dad234109a18f57f99a444108 Mon Sep 17 00:00:00 2001 From: sBubshait Date: Wed, 4 Dec 2024 15:26:00 +0000 Subject: [PATCH 07/17] Update mmap to add an insert helper function to allocate and add new mmap entries to the hash table --- src/vm/mmap.c | 37 +++++++++++++++++++++++++++++-------- src/vm/mmap.h | 1 + 2 files changed, 30 insertions(+), 8 deletions(-) diff --git a/src/vm/mmap.c b/src/vm/mmap.c index 50a0cdf..1f78040 100644 --- a/src/vm/mmap.c +++ b/src/vm/mmap.c @@ -4,6 +4,7 @@ static unsigned mmap_hash (const struct hash_elem *e, void *aux); static bool mmap_less (const struct hash_elem *a_, const struct hash_elem *b_, void *aux); +static void mmap_cleanup(struct hash_elem *e, void *aux); /* Initializes the mmap table for the current thread. Sets the counter to 0. */ bool @@ -14,6 +15,34 @@ mmap_init (void) return hash_init (&t->mmap_files, mmap_hash, mmap_less, NULL); } +/* Inserts a new mmap entry into the mmap table for the current thread. Upage + is the start address of the file data in the user VM. */ +struct mmap_entry * +mmap_insert (struct file *file, void *upage) +{ + if (file == NULL || upage == NULL) + return NULL; + + struct mmap_entry *mmap = malloc (sizeof (struct mmap_entry)); + if (mmap == NULL) + return NULL; + + mmap->mapping = thread_current ()->mmap_counter++; + mmap->file = file; + mmap->upage = upage; + + hash_insert (&thread_current ()->mmap_files, &mmap->elem); + return mmap; +} + +/* Destroys the mmap table for the current thread. Frees all the memory + allocated for the mmap entries. */ +void +mmap_destroy (void) +{ + hash_destroy (&thread_current ()->mmap_files, mmap_cleanup); +} + /* A hash function for the mmap table. Returns a hash for an entry, based on its mapping. */ static unsigned @@ -43,11 +72,3 @@ mmap_cleanup (struct hash_elem *e, void *aux UNUSED) file_close (mmap->file); free (mmap); } - -/* Destroys the mmap table for the current thread. Frees all the memory - allocated for the mmap entries. */ -void -mmap_destroy (void) -{ - hash_destroy (&thread_current ()->mmap_files, mmap_cleanup); -} \ No newline at end of file diff --git a/src/vm/mmap.h b/src/vm/mmap.h index de71ada..c4a6208 100644 --- a/src/vm/mmap.h +++ b/src/vm/mmap.h @@ -18,6 +18,7 @@ struct mmap_entry { }; bool mmap_init (void); +struct mmap_entry *mmap_insert (struct file *file, void *upage); void mmap_destroy (void); #endif /* vm/mmap.h */ From 67f16cb2a6ef59a86a588e5fbad1c88282408993 Mon Sep 17 00:00:00 2001 From: sBubshait Date: Wed, 4 Dec 2024 15:31:53 +0000 Subject: [PATCH 08/17] Update syscall.c to allow mmap and unmap system calls through helper handler functions for each --- src/userprog/syscall.c | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/src/userprog/syscall.c b/src/userprog/syscall.c index c6d4259..e85f194 100644 --- a/src/userprog/syscall.c +++ b/src/userprog/syscall.c @@ -11,11 +11,13 @@ #include "threads/synch.h" #include "userprog/process.h" #include "userprog/pagedir.h" +#include "vm/mmap.h" #include #include #define MAX_SYSCALL_ARGS 3 #define EXIT_FAILURE -1 +#define MMAP_FAILURE -1 struct open_file { @@ -45,6 +47,8 @@ static int syscall_write (int fd, const void *buffer, unsigned size); static void syscall_seek (int fd, unsigned position); static unsigned syscall_tell (int fd); static void syscall_close (int fd); +static mapid_t syscall_mmap (int fd, void *addr); +static void syscall_munmap (mapid_t mapping); static struct open_file *fd_get_file (int fd); static void validate_user_pointer (const void *start, size_t size, bool write); @@ -74,6 +78,8 @@ static const struct syscall_arguments syscall_lookup[] = [SYS_SEEK] = {(syscall_function) syscall_seek, 2}, [SYS_TELL] = {(syscall_function) syscall_tell, 1}, [SYS_CLOSE] = {(syscall_function) syscall_close, 1}, + [SYS_MMAP] = {(syscall_function) syscall_mmap, 2}, + [SYS_MUNMAP] = {(syscall_function) syscall_munmap, 1} }; /* The number of syscall functions (i.e, number of elements) within the @@ -394,6 +400,20 @@ syscall_close (int fd) } } +/* Handles the syscall for memory mapping a file. */ +static mapid_t +syscall_mmap (int fd, void *addr) +{ + return MMAP_FAILURE; +} + +/* Handles the syscall for unmapping a memory mapped file. */ +static void +syscall_munmap (mapid_t mapping UNUSED) +{ + +} + /* Hashing function needed for the open_file table. Returns a hash for an entry, based on its FD. */ unsigned From 72fa0c1bbbb6f7d2367c9132a3cdada153a9c1b1 Mon Sep 17 00:00:00 2001 From: sBubshait Date: Wed, 4 Dec 2024 17:41:14 +0000 Subject: [PATCH 09/17] Fix Bug: Initialise the mmap table for the newly created thread rather than the current thread --- src/threads/thread.c | 2 +- src/vm/mmap.c | 6 +++--- src/vm/mmap.h | 2 +- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/src/threads/thread.c b/src/threads/thread.c index 214a458..e95a40a 100644 --- a/src/threads/thread.c +++ b/src/threads/thread.c @@ -278,7 +278,7 @@ thread_create (const char *name, int priority, #endif #ifdef VM - mmap_init (); + mmap_init (t); #endif /* Prepare thread for first run by initializing its stack. diff --git a/src/vm/mmap.c b/src/vm/mmap.c index 1f78040..3d6bd79 100644 --- a/src/vm/mmap.c +++ b/src/vm/mmap.c @@ -6,11 +6,11 @@ static bool mmap_less (const struct hash_elem *a_, const struct hash_elem *b_, void *aux); static void mmap_cleanup(struct hash_elem *e, void *aux); -/* Initializes the mmap table for the current thread. Sets the counter to 0. */ +/* Initializes the mmap table for the given thread, setting the mmap counter to + 0 and initializing the hash table. */ bool -mmap_init (void) +mmap_init (struct thread *t) { - struct thread *t = thread_current (); t->mmap_counter = 0; return hash_init (&t->mmap_files, mmap_hash, mmap_less, NULL); } diff --git a/src/vm/mmap.h b/src/vm/mmap.h index c4a6208..a64210a 100644 --- a/src/vm/mmap.h +++ b/src/vm/mmap.h @@ -17,7 +17,7 @@ struct mmap_entry { struct hash_elem elem; /* An elem for the hash table. */ }; -bool mmap_init (void); +bool mmap_init (struct thread *t); struct mmap_entry *mmap_insert (struct file *file, void *upage); void mmap_destroy (void); From ad6e4b4059165ee841ea9105729e7de49d1ce7b1 Mon Sep 17 00:00:00 2001 From: sBubshait Date: Wed, 4 Dec 2024 17:42:53 +0000 Subject: [PATCH 10/17] Implement syscall_mmap to validate and then map all file data into a user address in memory --- src/userprog/syscall.c | 52 +++++++++++++++++++++++++++++++++++++++++- 1 file changed, 51 insertions(+), 1 deletion(-) diff --git a/src/userprog/syscall.c b/src/userprog/syscall.c index e85f194..7e94048 100644 --- a/src/userprog/syscall.c +++ b/src/userprog/syscall.c @@ -11,6 +11,7 @@ #include "threads/synch.h" #include "userprog/process.h" #include "userprog/pagedir.h" +#include "vm/page.h" #include "vm/mmap.h" #include #include @@ -404,7 +405,56 @@ syscall_close (int fd) static mapid_t syscall_mmap (int fd, void *addr) { - return MMAP_FAILURE; + /* Ensure the FD is for a file in the filesystem (not STDIN or STDOUT). */ + if (fd == STDOUT_FILENO || fd == STDIN_FILENO) + return MMAP_FAILURE; + + /* Validate that there is a file associated with the given FD. */ + struct open_file *file_info = fd_get_file (fd); + if (file_info == NULL) + return MMAP_FAILURE; + + /* Ensure that the address is page-aligned and it's neither NULL nor zero. */ + if (addr == 0 || addr == NULL || pg_ofs (addr) != 0) + return MMAP_FAILURE; + + /* Reopen the file to obtain a separate and independent reference to the file + for the mapping. */ + struct file *file = file_reopen (file_info->file); + if (file == NULL) + return MMAP_FAILURE; + + /* Get the size of the file. Mmap fails if the file is empty. */ + off_t file_size = file_length (file); + if (file_size == 0) + return MMAP_FAILURE; + + /* Check and ensure that there is enough space in the user virtual memory to + hold the entire file. */ + for (off_t ofs = 0; ofs < file_size; ofs += PGSIZE) + { + if (page_get (addr + ofs) != NULL) + return MMAP_FAILURE; + } + + /* Map the file data into the user virtual memory starting from addr. */ + for (off_t ofs = 0; ofs < file_size; ofs += PGSIZE) + { + 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_EXECUTABLE) == NULL) + return MMAP_FAILURE; + } + + /* Create a new mapping for the file. */ + struct mmap_entry *mmap = mmap_insert (file, addr); + if (mmap == NULL) + return MMAP_FAILURE; + + + return mmap->mapping; } /* Handles the syscall for unmapping a memory mapped file. */ From 941e1e067aee7496029048a29e02677abea5696b Mon Sep 17 00:00:00 2001 From: sBubshait Date: Wed, 4 Dec 2024 17:51:30 +0000 Subject: [PATCH 11/17] Update SPT page entry to change type from EXECUTABLE to PAGE_FILE to capture mmaps in addition to executables --- src/userprog/exception.c | 2 +- src/userprog/process.c | 2 +- src/userprog/syscall.c | 8 +++++--- src/vm/page.c | 18 ------------------ src/vm/page.h | 2 +- 5 files changed, 8 insertions(+), 24 deletions(-) diff --git a/src/userprog/exception.c b/src/userprog/exception.c index 1e3ccc0..5e1d82e 100644 --- a/src/userprog/exception.c +++ b/src/userprog/exception.c @@ -194,7 +194,7 @@ try_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_EXECUTABLE: + case PAGE_FILE: success = page_load (page, page->writable); break; default: diff --git a/src/userprog/process.c b/src/userprog/process.c index f4a7439..8d649e9 100644 --- a/src/userprog/process.c +++ b/src/userprog/process.c @@ -706,7 +706,7 @@ load_segment (struct file *file, off_t ofs, uint8_t *upage, /* 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) + writable, PAGE_FILE) == NULL) return false; /* Advance. */ diff --git a/src/userprog/syscall.c b/src/userprog/syscall.c index 7e94048..16e345f 100644 --- a/src/userprog/syscall.c +++ b/src/userprog/syscall.c @@ -444,7 +444,7 @@ syscall_mmap (int fd, void *addr) off_t zero_bytes = PGSIZE - read_bytes; if (page_insert (file, ofs, addr + ofs, read_bytes, zero_bytes, true, - PAGE_EXECUTABLE) == NULL) + PAGE_FILE) == NULL) return MMAP_FAILURE; } @@ -457,9 +457,11 @@ syscall_mmap (int fd, void *addr) return mmap->mapping; } -/* Handles the syscall for unmapping a memory mapped file. */ +/* Handles the syscall for unmapping a memory mapped file. + + Pre: mapping is a valid mapping identifier returned by mmap syscall.*/ static void -syscall_munmap (mapid_t mapping UNUSED) +syscall_munmap (mapid_t mapping) { } diff --git a/src/vm/page.c b/src/vm/page.c index 010bd9d..18f718e 100644 --- a/src/vm/page.c +++ b/src/vm/page.c @@ -107,21 +107,3 @@ 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 have a present bit of 0 and stores the specified swap slot value in the - entry for later retrieval from disk. */ -void -page_set_swap (struct thread *owner, void *upage, size_t swap_slot) -{ - -} - -/* 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. */ -size_t -page_get_swap (struct thread *owner, void *upage) -{ - -} \ No newline at end of file diff --git a/src/vm/page.h b/src/vm/page.h index 481f219..f4a4aba 100644 --- a/src/vm/page.h +++ b/src/vm/page.h @@ -5,7 +5,7 @@ #include "filesys/off_t.h" enum page_type { - PAGE_EXECUTABLE, + PAGE_FILE, PAGE_EMPTY }; From 857cae3578974f0176c7e9b1bcc3edc001f16473 Mon Sep 17 00:00:00 2001 From: sBubshait Date: Wed, 4 Dec 2024 18:08:05 +0000 Subject: [PATCH 12/17] Update mmap to add a get helper function to find a mmap entry from its mapping --- src/vm/mmap.c | 15 +++++++++++++++ src/vm/mmap.h | 1 + 2 files changed, 16 insertions(+) diff --git a/src/vm/mmap.c b/src/vm/mmap.c index 3d6bd79..528c597 100644 --- a/src/vm/mmap.c +++ b/src/vm/mmap.c @@ -15,6 +15,21 @@ mmap_init (struct thread *t) return hash_init (&t->mmap_files, mmap_hash, mmap_less, NULL); } +struct mmap_entry * +mmap_get (mapid_t mapping) +{ + struct mmap_entry fake_mmap_entry; + fake_mmap_entry.mapping = mapping; + + struct hash_elem *e + = hash_find (&thread_current ()->mmap_files, &fake_mmap_entry.elem); + + if (e == NULL) + return NULL; + + return hash_entry (e, struct mmap_entry, elem); +} + /* Inserts a new mmap entry into the mmap table for the current thread. Upage is the start address of the file data in the user VM. */ struct mmap_entry * diff --git a/src/vm/mmap.h b/src/vm/mmap.h index a64210a..5f7143b 100644 --- a/src/vm/mmap.h +++ b/src/vm/mmap.h @@ -18,6 +18,7 @@ struct mmap_entry { }; bool mmap_init (struct thread *t); +struct mmap_entry *mmap_get (mapid_t mapping); struct mmap_entry *mmap_insert (struct file *file, void *upage); void mmap_destroy (void); From 02b79d19342502ccf8cc6b9f73f9314a7ca0a2d9 Mon Sep 17 00:00:00 2001 From: sBubshait Date: Wed, 4 Dec 2024 18:13:07 +0000 Subject: [PATCH 13/17] Update mmap to add temporarily page_set_swap until swap is implemented --- src/vm/mmap.c | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/src/vm/mmap.c b/src/vm/mmap.c index 528c597..864cfcd 100644 --- a/src/vm/mmap.c +++ b/src/vm/mmap.c @@ -87,3 +87,21 @@ mmap_cleanup (struct hash_elem *e, void *aux UNUSED) file_close (mmap->file); free (mmap); } + +/* Updates the 'owner' thread's page table entry for virtual address 'upage' + to have a present bit of 0 and stores the specified swap slot value in the + entry for later retrieval from disk. */ +void +page_set_swap (struct thread *owner, void *upage, size_t swap_slot) +{ + +} + +/* 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. */ +size_t +page_get_swap (struct thread *owner, void *upage) +{ + return 0; +} From ecbb4e74a552640f4f29afeee715e2f882fbd067 Mon Sep 17 00:00:00 2001 From: sBubshait Date: Wed, 4 Dec 2024 19:07:28 +0000 Subject: [PATCH 14/17] Implement the unmap system call, writing back to the file if a page is dirty before removing from SPT --- src/userprog/syscall.c | 33 ++++++++++++++++++++++++++++++++- src/vm/mmap.c | 42 ++++++++++++++++++++++++++++++++++++++++++ src/vm/mmap.h | 1 + 3 files changed, 75 insertions(+), 1 deletion(-) diff --git a/src/userprog/syscall.c b/src/userprog/syscall.c index 16e345f..2a32cd3 100644 --- a/src/userprog/syscall.c +++ b/src/userprog/syscall.c @@ -459,11 +459,42 @@ syscall_mmap (int fd, void *addr) /* Handles the syscall for unmapping a memory mapped file. - Pre: mapping is a valid mapping identifier returned by mmap syscall.*/ + Pre: mapping is a valid mapping identifier returned by mmap syscall. */ static void syscall_munmap (mapid_t mapping) { + /* Get the mmap entry from the mapping identifier. */ + struct mmap_entry *mmap = mmap_get (mapping); + /* Free all the pages associated with the mapping, writing back to the file + if necessary. */ + off_t length = file_length (mmap->file); + for (off_t ofs = 0; ofs < length; ofs += PGSIZE) + { + void *upage = mmap->upage + ofs; + + /* Get the SPT page entry for this page. */ + struct page_entry *page = page_get(upage); + if (page == NULL) + continue; + + /* Write the page back to the file if it is dirty. */ + if (pagedir_is_dirty (thread_current ()->pagedir, upage)) + { + lock_acquire (&filesys_lock); + file_write_at (mmap->file, upage, page->read_bytes, ofs); + lock_release (&filesys_lock); + } + + /* Remove the page from the supplemental page table. */ + hash_delete (&thread_current ()->pages, &page->elem); + } + + /* Remove the mapping from the mmap table. Close the file and free the mmap + entry. */ + hash_delete (&thread_current ()->mmap_files, &mmap->elem); + file_close (mmap->file); + free(mmap); } /* Hashing function needed for the open_file table. Returns a hash for an entry, diff --git a/src/vm/mmap.c b/src/vm/mmap.c index 864cfcd..b3b16db 100644 --- a/src/vm/mmap.c +++ b/src/vm/mmap.c @@ -1,5 +1,9 @@ #include "mmap.h" +#include "page.h" +#include "threads/vaddr.h" #include "threads/malloc.h" +#include "userprog/syscall.h" +#include "userprog/pagedir.h" static unsigned mmap_hash (const struct hash_elem *e, void *aux); static bool mmap_less (const struct hash_elem *a_, const struct hash_elem *b_, @@ -50,6 +54,44 @@ mmap_insert (struct file *file, void *upage) return mmap; } +/* Unmaps the given mmap entry from the current thread's mmap table. */ +void +mmap_unmap (struct mmap_entry *mmap) +{ + if (mmap == NULL) + return; + + /* Free all the pages associated with the mapping, writing back to the file + if necessary. */ + off_t length = file_length (mmap->file); + for (off_t ofs = 0; ofs < length; ofs += PGSIZE) + { + void *upage = mmap->upage + ofs; + + /* Get the SPT page entry for this page. */ + struct page_entry *page = page_get(upage); + if (page == NULL) + continue; + + /* Write the page back to the file if it is dirty. */ + if (pagedir_is_dirty (thread_current ()->pagedir, upage)) + { + lock_acquire (&filesys_lock); + file_write_at (mmap->file, upage, page->read_bytes, ofs); + lock_release (&filesys_lock); + } + + /* Remove the page from the supplemental page table. */ + hash_delete (&thread_current ()->pages, &page->elem); + } + + /* Remove the mapping from the mmap table. Close the file and free the mmap + entry. */ + hash_delete (&thread_current ()->mmap_files, &mmap->elem); + file_close (mmap->file); + free(mmap); +} + /* Destroys the mmap table for the current thread. Frees all the memory allocated for the mmap entries. */ void diff --git a/src/vm/mmap.h b/src/vm/mmap.h index 5f7143b..593e070 100644 --- a/src/vm/mmap.h +++ b/src/vm/mmap.h @@ -20,6 +20,7 @@ struct mmap_entry { bool mmap_init (struct thread *t); struct mmap_entry *mmap_get (mapid_t mapping); struct mmap_entry *mmap_insert (struct file *file, void *upage); +void mmap_unmap (struct mmap_entry *mmap); void mmap_destroy (void); #endif /* vm/mmap.h */ From 806d6bc19ea54535430b532beffdaa8776175f40 Mon Sep 17 00:00:00 2001 From: sBubshait Date: Wed, 4 Dec 2024 21:59:38 +0000 Subject: [PATCH 15/17] Refactor: Move destroying mmap data into process_exit instead of thread --- src/threads/thread.c | 4 ---- src/userprog/process.c | 4 ++++ 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/threads/thread.c b/src/threads/thread.c index e95a40a..ceb53df 100644 --- a/src/threads/thread.c +++ b/src/threads/thread.c @@ -401,10 +401,6 @@ thread_exit (void) process_exit (); #endif -#ifdef VM - mmap_destroy (); -#endif - /* Remove thread from all threads list, set our status to dying, and schedule another process. That process will destroy us when it calls thread_schedule_tail(). */ diff --git a/src/userprog/process.c b/src/userprog/process.c index 8d649e9..4e61fe6 100644 --- a/src/userprog/process.c +++ b/src/userprog/process.c @@ -25,6 +25,7 @@ #include "threads/synch.h" #include "devices/timer.h" #include "vm/page.h" +#include "vm/mmap.h" #ifdef VM #include "vm/frame.h" #endif @@ -363,6 +364,9 @@ process_exit (void) struct thread *cur = thread_current (); uint32_t *pd; + /* Unmap all memory mapped files */ + mmap_destroy (); + /* Clean up all open files */ hash_destroy (&cur->open_files, fd_cleanup); hash_destroy (&cur->pages, page_cleanup); From 26a2d40325c7eea257f2e057cefb52a204f01ae6 Mon Sep 17 00:00:00 2001 From: sBubshait Date: Wed, 4 Dec 2024 22:00:59 +0000 Subject: [PATCH 16/17] Implement implicitly unmapping all mmapped files when a process exits. Refactor to reduce duplication --- src/userprog/syscall.c | 33 +++++---------------------------- src/vm/mmap.c | 13 +++++-------- src/vm/mmap.h | 1 + src/vm/page.c | 1 - 4 files changed, 11 insertions(+), 37 deletions(-) diff --git a/src/userprog/syscall.c b/src/userprog/syscall.c index 2a32cd3..2078c9f 100644 --- a/src/userprog/syscall.c +++ b/src/userprog/syscall.c @@ -466,35 +466,12 @@ syscall_munmap (mapid_t mapping) /* Get the mmap entry from the mapping identifier. */ struct mmap_entry *mmap = mmap_get (mapping); - /* Free all the pages associated with the mapping, writing back to the file - if necessary. */ - off_t length = file_length (mmap->file); - for (off_t ofs = 0; ofs < length; ofs += PGSIZE) - { - void *upage = mmap->upage + ofs; - - /* Get the SPT page entry for this page. */ - struct page_entry *page = page_get(upage); - if (page == NULL) - continue; - - /* Write the page back to the file if it is dirty. */ - if (pagedir_is_dirty (thread_current ()->pagedir, upage)) - { - lock_acquire (&filesys_lock); - file_write_at (mmap->file, upage, page->read_bytes, ofs); - lock_release (&filesys_lock); - } - - /* Remove the page from the supplemental page table. */ - hash_delete (&thread_current ()->pages, &page->elem); - } - - /* Remove the mapping from the mmap table. Close the file and free the mmap - entry. */ + /* Delete the mmap entry from the hash table. */ hash_delete (&thread_current ()->mmap_files, &mmap->elem); - file_close (mmap->file); - free(mmap); + + /* Unmap the mmap entry: free the pages and write back to the file if + necessary. NOTE. freeing and cleaning up is also handled by mmap_unmap. */ + mmap_unmap (mmap); } /* Hashing function needed for the open_file table. Returns a hash for an entry, diff --git a/src/vm/mmap.c b/src/vm/mmap.c index b3b16db..c34991d 100644 --- a/src/vm/mmap.c +++ b/src/vm/mmap.c @@ -4,6 +4,7 @@ #include "threads/malloc.h" #include "userprog/syscall.h" #include "userprog/pagedir.h" +#include static unsigned mmap_hash (const struct hash_elem *e, void *aux); static bool mmap_less (const struct hash_elem *a_, const struct hash_elem *b_, @@ -85,11 +86,8 @@ mmap_unmap (struct mmap_entry *mmap) hash_delete (&thread_current ()->pages, &page->elem); } - /* Remove the mapping from the mmap table. Close the file and free the mmap - entry. */ - hash_delete (&thread_current ()->mmap_files, &mmap->elem); file_close (mmap->file); - free(mmap); + free (mmap); } /* Destroys the mmap table for the current thread. Frees all the memory @@ -120,14 +118,13 @@ mmap_less (const struct hash_elem *a_, const struct hash_elem *b_, return a->mapping < b->mapping; } -/* Cleans up the mmap table for the current thread. Frees the memory allocated - for the mmap entry. */ +/* Cleans up the mmap table for the current thread. Implicitly unmaps the mmap + entry, freeing pages and writing back to the file if necessary. */ static void mmap_cleanup (struct hash_elem *e, void *aux UNUSED) { struct mmap_entry *mmap = hash_entry (e, struct mmap_entry, elem); - file_close (mmap->file); - free (mmap); + mmap_unmap (mmap); } /* Updates the 'owner' thread's page table entry for virtual address 'upage' diff --git a/src/vm/mmap.h b/src/vm/mmap.h index 593e070..cba21fc 100644 --- a/src/vm/mmap.h +++ b/src/vm/mmap.h @@ -21,6 +21,7 @@ bool mmap_init (struct thread *t); struct mmap_entry *mmap_get (mapid_t mapping); struct mmap_entry *mmap_insert (struct file *file, void *upage); void mmap_unmap (struct mmap_entry *mmap); +void mmap_umap_all (void); void mmap_destroy (void); #endif /* vm/mmap.h */ diff --git a/src/vm/page.c b/src/vm/page.c index 18f718e..c75fdcc 100644 --- a/src/vm/page.c +++ b/src/vm/page.c @@ -59,7 +59,6 @@ page_get (void *upage) struct hash_elem *e = hash_find (&thread_current ()->pages, &fake_page_entry.elem); - if (e == NULL) return NULL; From 61f6374006ad35f52e627f8041dc2f9b51e83661 Mon Sep 17 00:00:00 2001 From: sBubshait Date: Wed, 4 Dec 2024 22:06:09 +0000 Subject: [PATCH 17/17] Update gitlab CI to only run the tests associated with this feature (mmapped-files). --- .gitlab-ci.yml | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml index ce5bf22..a291160 100644 --- a/.gitlab-ci.yml +++ b/.gitlab-ci.yml @@ -32,9 +32,10 @@ test_userprog: extends: .pintos_tests variables: DIR: userprog + IGNORE: (tests/userprog/no-vm/multi-oom) test_vm: extends: .pintos_tests variables: DIR: vm - IGNORE: (tests/vm/pt-grow-stack|tests/vm/pt-grow-pusha|tests/vm/pt-big-stk-obj|tests/vm/pt-overflowstk|tests/vm/pt-write-code2|tests/vm/pt-grow-stk-sc|tests/vm/page-linear|tests/vm/page-parallel|tests/vm/page-merge-seq|tests/vm/page-merge-par|tests/vm/page-merge-stk|tests/vm/page-merge-mm) + IGNORE: (tests/vm/pt-grow-stack|tests/vm/pt-grow-pusha|tests/vm/pt-big-stk-obj|tests/vm/pt-overflowstk|tests/vm/pt-write-code2|tests/vm/pt-grow-stk-sc|tests/vm/page-linear|tests/vm/page-parallel|tests/vm/page-merge-seq|tests/vm/page-merge-par|tests/vm/page-merge-stk|tests/vm/page-merge-mm|tests/vm/mmap-over-stk)