From c0ff53beffa05db22b933217217b4f60258d7575 Mon Sep 17 00:00:00 2001 From: Gleb Koval Date: Wed, 4 Dec 2024 12:27:39 +0000 Subject: [PATCH] feat: proper cleanup for shared pages --- src/userprog/process.c | 6 ++- src/vm/page.c | 95 +++++++++++++++++++++++++++++++++++------- src/vm/page.h | 2 + 3 files changed, 85 insertions(+), 18 deletions(-) diff --git a/src/userprog/process.c b/src/userprog/process.c index f4a7439..205d0f2 100644 --- a/src/userprog/process.c +++ b/src/userprog/process.c @@ -172,8 +172,9 @@ start_process (void *proc_start_data) to store the command that executed the process. */ if (data->success) { - data->success = - process_init_stack (data->cmd_saveptr, &if_.esp, data->file_name); + data->success = use_shared_file (exec_file) + && process_init_stack (data->cmd_saveptr, &if_.esp, + data->file_name); } /* Signal that the process has finished attempting to load. */ @@ -366,6 +367,7 @@ process_exit (void) /* Clean up all open files */ hash_destroy (&cur->open_files, fd_cleanup); hash_destroy (&cur->pages, page_cleanup); + unuse_shared_file (cur->exec_file); /* Close the executable file, implicitly allowing it to be written to. */ if (cur->exec_file != NULL) diff --git a/src/vm/page.c b/src/vm/page.c index cf5f1f7..1e205f0 100644 --- a/src/vm/page.c +++ b/src/vm/page.c @@ -12,14 +12,19 @@ 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_, void *aux UNUSED); +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); 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); /* Initialise a thread's supplemental pages table. */ bool @@ -180,7 +185,7 @@ page_cleanup (struct hash_elem *e, void *aux UNUSED) } /* Initialise the shared files and table and lock. */ -bool +void shared_files_init () { lock_init (&shared_files_lock); @@ -188,16 +193,64 @@ shared_files_init () PANIC ("Failed to initialise shared_files table."); } +bool +use_shared_file (struct file *file) +{ + lock_acquire (&shared_files_lock); + struct shared_file_entry *shared_file = shared_file_get (file); + if (shared_file == NULL) + { + shared_file = shared_file_insert (file); + if (shared_file == NULL) + { + lock_release (&shared_files_lock); + return false; + } + } + shared_file->ref_count++; + lock_release (&shared_files_lock); + return true; +} + +bool +unuse_shared_file (struct file *file) +{ + lock_acquire (&shared_files_lock); + struct shared_file_entry *shared_file = shared_file_get (file); + if (shared_file == NULL) + { + lock_release (&shared_files_lock); + return false; + } + shared_file->ref_count--; + if (shared_file->ref_count <= 0) + { + hash_destroy (&shared_file->pages, shared_page_cleanup); + hash_delete (&shared_files, &shared_file->elem); + free (shared_file); + } + lock_release (&shared_files_lock); +} + +static void +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); + free (shared_page); +} + /* Hashing function needed for the shared_file table. Returns a hash for an entry based on its file pointer. */ -unsigned +static unsigned shared_file_hash (const struct hash_elem *e, void *aux UNUSED) { return file_hash (hash_entry (e, struct shared_file_entry, elem)->file); } /* Less function needed for the shared_file table. */ -bool +static bool shared_file_less (const struct hash_elem *a_, const struct hash_elem *b_, void *aux UNUSED) { @@ -246,7 +299,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.*/ -struct shared_page_entry * +static struct shared_page_entry * shared_page_get (struct file *file, void *upage) { /* Search first for the file within the shared_pages structure */ @@ -266,6 +319,27 @@ shared_page_get (struct file *file, void *upage) return hash_entry (e, struct shared_page_entry, elem); } +static struct shared_file_entry * +shared_file_insert (struct file *file) +{ + struct shared_file_entry *shared_file + = malloc (sizeof (struct shared_file_entry)); + if (shared_file == NULL) + return NULL; + + shared_file->file = file; + shared_file->ref_count = 0; + if (!hash_init (&shared_file->pages, shared_page_hash, shared_page_less, + NULL)) + { + free (shared_file); + return NULL; + } + + hash_insert (&shared_files, &shared_file->elem); + return shared_file; +} + static struct shared_page_entry * shared_page_insert (struct file *file, void *upage, void *frame) { @@ -280,23 +354,12 @@ shared_page_insert (struct file *file, void *upage, void *frame) /* If shared file doesn't exist in table, also create it. */ if (shared_file == NULL) { - shared_file = malloc (sizeof (struct shared_file_entry)); + shared_file = shared_file_insert (file); if (shared_file == NULL) { free (shared_page); return NULL; } - - shared_file->file = file; - shared_file->ref_count = 0; - if (!hash_init (&shared_file->pages, shared_page_hash, shared_page_less, NULL)) - { - free (shared_page); - free (shared_file); - return NULL; - } - - hash_insert (&shared_files, &shared_file->elem); } shared_page->upage = upage; diff --git a/src/vm/page.h b/src/vm/page.h index c6fb36c..b23703d 100644 --- a/src/vm/page.h +++ b/src/vm/page.h @@ -52,6 +52,8 @@ bool page_load (struct page_entry *page); void page_cleanup (struct hash_elem *e, void *aux UNUSED); void shared_files_init (); +bool use_shared_file (struct file *file); +bool unuse_shared_file (struct file *file); void page_set_swap (struct thread *, void *, size_t); size_t page_get_swap (struct thread *, void *);