From ddcd59fdf8a744cc2b19b1d43e78b300e67320b3 Mon Sep 17 00:00:00 2001 From: Gleb Koval Date: Fri, 8 Nov 2024 02:52:38 +0000 Subject: [PATCH 01/26] Add child and own process result information to struct thread --- src/threads/thread.h | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/src/threads/thread.h b/src/threads/thread.h index 1c05030..cf35b82 100644 --- a/src/threads/thread.h +++ b/src/threads/thread.h @@ -4,6 +4,8 @@ #include #include #include +#include +#include "threads/synch.h" #include "threads/fixed-point.h" /* States in a thread's life cycle. */ @@ -29,6 +31,19 @@ typedef int tid_t; #define NICE_DEFAULT 0 /* Default niceness. */ #define NICE_MAX 20 /* Highest niceness. */ +/* A process result, synchronised between parent and child. */ +struct process_result +{ + tid_t tid; /* The tid of the child process. */ + int exit_status; /* The exit status of the child process. Initially set to + -1, then to 0 when parent dies, or to exit_status when + child dies (whichever happens first). */ + struct lock lock; /* Lock to synchronise access to the exit_status. */ + struct semaphore sema; /* Semaphore to signal the parent that the exit_status + has been set. */ + struct list_elem elem; /* List element for the parent's children list. */ +}; + /* A kernel thread or user process. Each thread structure is stored in its own 4 kB page. The @@ -108,6 +123,11 @@ struct thread int nice; /* Nice value for this thread */ fp32_t recent_cpu; /* Amount of time this process received */ + /* Process wait properties. */ + struct process_result *result; /* Result of the process. */ + struct list child_results; /* List of children's of this thread + process results. */ + /* Shared between thread.c and synch.c. */ struct list_elem elem; /* List element. */ From ec8547aec9f145e5ead34b9f55a78d5ff238df47 Mon Sep 17 00:00:00 2001 From: Gleb Koval Date: Fri, 8 Nov 2024 03:31:48 +0000 Subject: [PATCH 02/26] Implement creation of process results --- src/threads/thread.c | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+) diff --git a/src/threads/thread.c b/src/threads/thread.c index 457f7b9..ee76718 100644 --- a/src/threads/thread.c +++ b/src/threads/thread.c @@ -9,6 +9,7 @@ #include "threads/flags.h" #include "threads/interrupt.h" #include "threads/intr-stubs.h" +#include "threads/malloc.h" #include "threads/palloc.h" #include "threads/switch.h" #include "threads/synch.h" @@ -68,6 +69,7 @@ static void kernel_thread (thread_func *, void *aux); static void idle (void *aux UNUSED); static struct thread *running_thread (void); static struct thread *next_thread_to_run (void); +static void init_process_result (struct thread *t); static void init_thread (struct thread *, const char *name, int nice, int priority, fp32_t recent_cpu); static bool is_thread (struct thread *) UNUSED; @@ -110,6 +112,7 @@ thread_init (void) initial_thread_recent_cpu); initial_thread->status = THREAD_RUNNING; initial_thread->tid = allocate_tid (); + initial_thread->result = NULL; /* Main thread cannot be waited for. */ } /* Starts preemptive thread scheduling by enabling interrupts. @@ -236,6 +239,7 @@ thread_create (const char *name, int priority, struct thread *parent_thread = thread_current (); init_thread (t, name, parent_thread->nice, priority, parent_thread->recent_cpu); tid = t->tid = allocate_tid (); + init_process_result (t); /* Prepare thread for first run by initializing its stack. Do this atomically so intermediate values for the 'stack' @@ -259,6 +263,10 @@ thread_create (const char *name, int priority, intr_set_level (old_level); + /* No need to synchronise child_results since it is only ever accessed by one + thread. */ + list_insert (&parent_thread->child_results, &t->result->elem); + /* Add to run queue. */ thread_unblock (t); @@ -632,6 +640,18 @@ is_thread (struct thread *t) return t != NULL && t->magic == THREAD_MAGIC; } +/* Allocate and initialise a process result for given thread. */ +static void +init_process_result (struct thread *t) +{ + struct process_result *result = malloc (sizeof (struct process_result)); + result->tid = t->tid; + result->exit_status = t->exit_status; + lock_init (&result->lock); + sema_init (&result->sema, 0); + t->result = result; +} + /* Does basic initialization of T as a blocked thread named NAME. */ static void @@ -660,6 +680,7 @@ init_thread (struct thread *t, const char *name, int nice, int priority, t->priority = t->base_priority; t->exit_status = -1; + list_init (&t->child_results); old_level = intr_disable (); list_push_back (&all_list, &t->allelem); From 7349b4e66f3a4542d390a975fe41890d5b242d37 Mon Sep 17 00:00:00 2001 From: Gleb Koval Date: Fri, 8 Nov 2024 03:55:34 +0000 Subject: [PATCH 03/26] Fix typo list_insert -> list_push_back for thread.child_results --- src/threads/thread.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/threads/thread.c b/src/threads/thread.c index ee76718..379894d 100644 --- a/src/threads/thread.c +++ b/src/threads/thread.c @@ -264,8 +264,8 @@ thread_create (const char *name, int priority, intr_set_level (old_level); /* No need to synchronise child_results since it is only ever accessed by one - thread. */ - list_insert (&parent_thread->child_results, &t->result->elem); + thread. By the nature of increasing TIDs, this list is ordered. */ + list_push_back (&parent_thread->child_results, &t->result->elem); /* Add to run queue. */ thread_unblock (t); From fde70dcf5934aa083c7513e47b316024ef3831d2 Mon Sep 17 00:00:00 2001 From: Gleb Koval Date: Fri, 8 Nov 2024 03:56:05 +0000 Subject: [PATCH 04/26] Implement process_wait. --- src/threads/thread.h | 5 ++- src/userprog/process.c | 76 ++++++++++++++++++++++++++++++++++++++---- 2 files changed, 72 insertions(+), 9 deletions(-) diff --git a/src/threads/thread.h b/src/threads/thread.h index cf35b82..6465d53 100644 --- a/src/threads/thread.h +++ b/src/threads/thread.h @@ -36,9 +36,8 @@ struct process_result { tid_t tid; /* The tid of the child process. */ int exit_status; /* The exit status of the child process. Initially set to - -1, then to 0 when parent dies, or to exit_status when - child dies (whichever happens first). */ - struct lock lock; /* Lock to synchronise access to the exit_status. */ + -1, then to exit_status when child dies. */ + struct lock lock; /* Lock to synchronise access to the status and sema. */ struct semaphore sema; /* Semaphore to signal the parent that the exit_status has been set. */ struct list_elem elem; /* List element for the parent's children list. */ diff --git a/src/userprog/process.c b/src/userprog/process.c index 6c8ef2d..5b41a34 100644 --- a/src/userprog/process.c +++ b/src/userprog/process.c @@ -1,6 +1,7 @@ #include "userprog/process.h" #include #include +#include #include #include #include @@ -202,12 +203,34 @@ start_process (void *file_name_) int process_wait (tid_t child_tid UNUSED) { - /* As a temporary wait, waiting will just put the thread to sleep for one - second (TIMER_FREQ = 100 ticks ~ 1 second). */ - /* TODO: Implement process_wait () correctly. Remove the next line. */ - timer_sleep (TIMER_FREQ); - - return 0; /* TODO: Change this too */ + struct process_result *child_result = NULL; + struct list_elem *e; + struct thread *cur = thread_current (); + for (e = list_begin (&cur->child_results); + e != list_end (&cur->child_results); e = list_next (e)) + { + struct process_result *result + = list_entry (e, struct process_result, elem); + if (result->tid == child_tid) + { + child_result = result; + break; + } + /* List is ordered, allowing us to break early. */ + else if (result->tid > child_tid) + break; + } + if (child_result == NULL) + return -1; + /* Wait for child to die. */ + sema_down (&child_result->sema); + /* To prevent waiting for child twice, remove it from the list. + No need to use lock since this is the only thread with access to + the struct process_result now. */ + list_remove (&child_result->elem); + int exit_status = child_result->exit_status; + free (child_result); + return exit_status; } /* Free the current process's resources. */ @@ -219,6 +242,47 @@ process_exit (void) printf ("%s: exit(%d)\n", cur->name, cur->exit_status); + /* Update process result. */ + if (cur->result != NULL) + { + lock_acquire (&cur->result->lock); + cur->result->exit_status = cur->exit_status; + /* Parent has died, child has to free the struct process_result * */ + if (sema_try_down (&cur->result->sema)) + { + lock_release (&cur->result->lock); + free (cur->result); + } + /* Parent is still alive and will be the one to free the + struct process_result *, and may be waiting so call sema_up */ + else + { + sema_up (&cur->result->sema); + lock_release (&cur->result->lock); + } + } + + struct list_elem *e; + for (e = list_begin (&cur->child_results); + e != list_end (&cur->child_results); e = list_next (e)) + { + struct process_result *result + = list_entry (e, struct process_result, elem); + lock_acquire (&result->lock); + /* Child has died (and was not waited for). Free the result. */ + if (sema_try_down (&result->sema)) + { + lock_release (&result->lock); + free (result); + } + /* Child is still alive, signal via sema that parent has died. */ + else + { + sema_up (&result->sema); + lock_release (&result->lock); + } + } + /* Destroy the current process's page directory and switch back to the kernel-only page directory. */ pd = cur->pagedir; From d95894085b0b0d9305561fde7510df262ea21102 Mon Sep 17 00:00:00 2001 From: Gleb Koval Date: Fri, 8 Nov 2024 09:15:22 +0000 Subject: [PATCH 05/26] Implement syscall_exec via process_execute --- src/userprog/syscall.c | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/src/userprog/syscall.c b/src/userprog/syscall.c index 1dbe73b..03e0721 100644 --- a/src/userprog/syscall.c +++ b/src/userprog/syscall.c @@ -110,10 +110,14 @@ syscall_exit (int status) } static pid_t -syscall_exec (const char *cmd_line UNUSED) +syscall_exec (const char *cmd_line) { - //TODO - return 0; + /* To check the end we would need to traverse the null-terminate string, + which is equally unsafe as just leaving process_execute to do it. */ + cmd_line = validate_user_pointer (cmd_line, 1); + if (cmd_line == NULL) + thread_exit (); + process_execute (cmd_line); } static int From 84fe637c7e284dd8dab7e2a7ce036857900e1926 Mon Sep 17 00:00:00 2001 From: Gleb Koval Date: Fri, 8 Nov 2024 09:16:18 +0000 Subject: [PATCH 06/26] Remove process_result lock since it is an invalid solution TODO : synchronise process_result in another way --- src/threads/thread.c | 1 - src/threads/thread.h | 1 - src/userprog/process.c | 22 ++++------------------ 3 files changed, 4 insertions(+), 20 deletions(-) diff --git a/src/threads/thread.c b/src/threads/thread.c index 379894d..aecafdf 100644 --- a/src/threads/thread.c +++ b/src/threads/thread.c @@ -647,7 +647,6 @@ init_process_result (struct thread *t) struct process_result *result = malloc (sizeof (struct process_result)); result->tid = t->tid; result->exit_status = t->exit_status; - lock_init (&result->lock); sema_init (&result->sema, 0); t->result = result; } diff --git a/src/threads/thread.h b/src/threads/thread.h index 6465d53..f6227f5 100644 --- a/src/threads/thread.h +++ b/src/threads/thread.h @@ -37,7 +37,6 @@ struct process_result tid_t tid; /* The tid of the child process. */ int exit_status; /* The exit status of the child process. Initially set to -1, then to exit_status when child dies. */ - struct lock lock; /* Lock to synchronise access to the status and sema. */ struct semaphore sema; /* Semaphore to signal the parent that the exit_status has been set. */ struct list_elem elem; /* List element for the parent's children list. */ diff --git a/src/userprog/process.c b/src/userprog/process.c index 5b41a34..1db334a 100644 --- a/src/userprog/process.c +++ b/src/userprog/process.c @@ -245,21 +245,14 @@ process_exit (void) /* Update process result. */ if (cur->result != NULL) { - lock_acquire (&cur->result->lock); cur->result->exit_status = cur->exit_status; /* Parent has died, child has to free the struct process_result * */ if (sema_try_down (&cur->result->sema)) - { - lock_release (&cur->result->lock); - free (cur->result); - } + free (cur->result); /* Parent is still alive and will be the one to free the struct process_result *, and may be waiting so call sema_up */ else - { - sema_up (&cur->result->sema); - lock_release (&cur->result->lock); - } + sema_up (&cur->result->sema); } struct list_elem *e; @@ -268,19 +261,12 @@ process_exit (void) { struct process_result *result = list_entry (e, struct process_result, elem); - lock_acquire (&result->lock); /* Child has died (and was not waited for). Free the result. */ if (sema_try_down (&result->sema)) - { - lock_release (&result->lock); - free (result); - } + free (result); /* Child is still alive, signal via sema that parent has died. */ else - { - sema_up (&result->sema); - lock_release (&result->lock); - } + sema_up (&result->sema); } /* Destroy the current process's page directory and switch back From 6ed1ccd21ea012ffb37d8c8d47e87b5f90979fc8 Mon Sep 17 00:00:00 2001 From: Gleb Koval Date: Fri, 8 Nov 2024 10:50:10 +0000 Subject: [PATCH 07/26] Fix process_result locking by acquiring in process_wait as well to prevent freeing memory too early --- src/threads/thread.c | 1 + src/threads/thread.h | 7 ++++--- src/userprog/process.c | 27 +++++++++++++++++++++++---- 3 files changed, 28 insertions(+), 7 deletions(-) diff --git a/src/threads/thread.c b/src/threads/thread.c index aecafdf..379894d 100644 --- a/src/threads/thread.c +++ b/src/threads/thread.c @@ -647,6 +647,7 @@ init_process_result (struct thread *t) struct process_result *result = malloc (sizeof (struct process_result)); result->tid = t->tid; result->exit_status = t->exit_status; + lock_init (&result->lock); sema_init (&result->sema, 0); t->result = result; } diff --git a/src/threads/thread.h b/src/threads/thread.h index f6227f5..2947256 100644 --- a/src/threads/thread.h +++ b/src/threads/thread.h @@ -34,9 +34,10 @@ typedef int tid_t; /* A process result, synchronised between parent and child. */ struct process_result { - tid_t tid; /* The tid of the child process. */ - int exit_status; /* The exit status of the child process. Initially set to - -1, then to exit_status when child dies. */ + tid_t tid; /* The tid of the child process. */ + int exit_status; /* The exit status of the child process. Initially set + to -1, then to exit_status when child dies. */ + struct lock lock; /* Lock the exit_status and sema. */ struct semaphore sema; /* Semaphore to signal the parent that the exit_status has been set. */ struct list_elem elem; /* List element for the parent's children list. */ diff --git a/src/userprog/process.c b/src/userprog/process.c index 1db334a..370a216 100644 --- a/src/userprog/process.c +++ b/src/userprog/process.c @@ -224,11 +224,16 @@ process_wait (tid_t child_tid UNUSED) return -1; /* Wait for child to die. */ sema_down (&child_result->sema); + /* We need lock release in process_exit, so we need to acquire (and possibly + wait) for it here to ensure we don't free the lock memory before it is + released in process_exit. */ + lock_acquire (&child_result->lock); /* To prevent waiting for child twice, remove it from the list. No need to use lock since this is the only thread with access to the struct process_result now. */ list_remove (&child_result->elem); int exit_status = child_result->exit_status; + lock_release (&child_result->lock); free (child_result); return exit_status; } @@ -245,14 +250,21 @@ process_exit (void) /* Update process result. */ if (cur->result != NULL) { + lock_acquire (&cur->result->lock); cur->result->exit_status = cur->exit_status; /* Parent has died, child has to free the struct process_result * */ if (sema_try_down (&cur->result->sema)) - free (cur->result); + { + lock_release (&cur->result->lock); + free (cur->result); + } /* Parent is still alive and will be the one to free the struct process_result *, and may be waiting so call sema_up */ else - sema_up (&cur->result->sema); + { + sema_up (&cur->result->sema); + lock_release (&cur->result->lock); + } } struct list_elem *e; @@ -261,12 +273,19 @@ process_exit (void) { struct process_result *result = list_entry (e, struct process_result, elem); + lock_acquire (&result->lock); /* Child has died (and was not waited for). Free the result. */ if (sema_try_down (&result->sema)) - free (result); + { + lock_release (&result->lock); + free (result); + } /* Child is still alive, signal via sema that parent has died. */ else - sema_up (&result->sema); + { + sema_up (&result->sema); + lock_release (&result->lock); + } } /* Destroy the current process's page directory and switch back From 26e38be761a6efe6395cbaace21f0ea8f101f88a Mon Sep 17 00:00:00 2001 From: sBubshait Date: Fri, 8 Nov 2024 14:21:16 +0000 Subject: [PATCH 08/26] Update validate_user_pointer to check if the ptr is mapped to a physical memory address, w/ E --- src/userprog/syscall.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/userprog/syscall.c b/src/userprog/syscall.c index 1dbe73b..506498b 100644 --- a/src/userprog/syscall.c +++ b/src/userprog/syscall.c @@ -5,6 +5,7 @@ #include "threads/interrupt.h" #include "threads/thread.h" #include "userprog/process.h" +#include "userprog/pagedir.h" #include #include @@ -227,8 +228,9 @@ validate_user_pointer (const void *ptr, size_t size) { if (size > 0 && (ptr == NULL || !is_user_vaddr (ptr) || - !is_user_vaddr (ptr + size - 1))) + !is_user_vaddr (ptr + size - 1) || + pagedir_get_page (thread_current()->pagedir, ptr) == NULL)) thread_exit (); - return ptr; + return (void *) ptr; } From a8676f2e099e14c2eae2e9353d31d59891899cb9 Mon Sep 17 00:00:00 2001 From: EDiasAlberto Date: Fri, 8 Nov 2024 14:34:08 +0000 Subject: [PATCH 09/26] Implement syscall for file creation, with relevant locks w/ S. --- src/userprog/syscall.c | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-) diff --git a/src/userprog/syscall.c b/src/userprog/syscall.c index 506498b..04d763f 100644 --- a/src/userprog/syscall.c +++ b/src/userprog/syscall.c @@ -1,14 +1,18 @@ #include "userprog/syscall.h" #include "devices/shutdown.h" #include "devices/input.h" +#include "filesys/filesys.h" #include "threads/vaddr.h" #include "threads/interrupt.h" #include "threads/thread.h" +#include "threads/synch.h" #include "userprog/process.h" #include "userprog/pagedir.h" #include #include +static struct lock filesys_lock; + static void syscall_handler (struct intr_frame *); /* A syscall_function is a function that receives up to 3 arguments, the @@ -68,6 +72,7 @@ void syscall_init (void) { intr_register_int (0x30, 3, INTR_ON, syscall_handler, "syscall"); + lock_init (&filesys_lock); } static void @@ -126,8 +131,13 @@ syscall_wait (pid_t pid) static bool syscall_create (const char *file UNUSED, unsigned initial_size UNUSED) { - //TODO - return 0; + validate_user_pointer (file, 1); + + lock_acquire (&filesys_lock); + bool status = filesys_create (file, initial_size); + lock_release (&filesys_lock); + + return status; } static bool From dca9d8f5a33dd662a00f3beef305c187e271fbad Mon Sep 17 00:00:00 2001 From: EDiasAlberto Date: Fri, 8 Nov 2024 14:35:23 +0000 Subject: [PATCH 10/26] Implement syscall for file removal w/ S. --- src/userprog/syscall.c | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/src/userprog/syscall.c b/src/userprog/syscall.c index 04d763f..cf353cc 100644 --- a/src/userprog/syscall.c +++ b/src/userprog/syscall.c @@ -141,10 +141,15 @@ syscall_create (const char *file UNUSED, unsigned initial_size UNUSED) } static bool -syscall_remove (const char *file UNUSED) +syscall_remove (const char *file) { - //TODO - return 0; + validate_user_pointer (file, 1); + + lock_acquire (&filesys_lock); + bool status = filesys_remove (file); + lock_release (&filesys_lock); + + return status; } static int From b112824a6412760de7637db6f303099fd5ccad07 Mon Sep 17 00:00:00 2001 From: sBubshait Date: Fri, 8 Nov 2024 14:41:27 +0000 Subject: [PATCH 11/26] Implement the exec system call through process_execute, w/ E --- src/userprog/syscall.c | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/src/userprog/syscall.c b/src/userprog/syscall.c index cf353cc..98c9dd4 100644 --- a/src/userprog/syscall.c +++ b/src/userprog/syscall.c @@ -116,10 +116,15 @@ syscall_exit (int status) } static pid_t -syscall_exec (const char *cmd_line UNUSED) +syscall_exec (const char *cmd_line) { - //TODO - return 0; + validate_user_pointer (cmd_line, 1); + + lock_acquire (&filesys_lock); + pid_t pid = process_execute(cmd_line); + lock_release (&filesys_lock); + + return pid; } static int From 5bbe7a03c08b236e20631c0ec05f303338d273c3 Mon Sep 17 00:00:00 2001 From: sBubshait Date: Fri, 8 Nov 2024 15:12:08 +0000 Subject: [PATCH 12/26] Add in syscall hash helper functions for open_file struct: fd_hash and fd_less, w/ E --- src/userprog/syscall.c | 23 +++++++++++++++++++++++ src/userprog/syscall.h | 5 +++++ 2 files changed, 28 insertions(+) diff --git a/src/userprog/syscall.c b/src/userprog/syscall.c index 98c9dd4..555b940 100644 --- a/src/userprog/syscall.c +++ b/src/userprog/syscall.c @@ -13,6 +13,13 @@ static struct lock filesys_lock; +struct open_file + { + int fd; /* File Descriptor / Identifier */ + struct file *file; /* Pointer to the associated file */ + struct hash_elem elem; /* elem for a hash table */ + }; + static void syscall_handler (struct intr_frame *); /* A syscall_function is a function that receives up to 3 arguments, the @@ -239,6 +246,22 @@ syscall_close (int fd UNUSED) //TODO } +unsigned +fd_hash (const struct hash_elem *element, void *aux UNUSED) +{ + return hash_int (hash_entry (element, struct open_file, elem)->fd); +} + +bool +fd_less (const struct hash_elem *a_, const struct hash_elem *b_, + void *aux UNUSED) +{ + struct open_file *a = hash_entry (a_, struct open_file, elem); + struct open_file *b = hash_entry (b_, struct open_file, elem); + + return a->fd < b->fd; +} + /* Validates if a block of memory starting at PTR and of size SIZE bytes is fully contained within user virtual memory. Kills the thread (by calling thread_exit) if the memory is invalid. Otherwise, returns the PTR given. diff --git a/src/userprog/syscall.h b/src/userprog/syscall.h index 702a6c7..9563059 100644 --- a/src/userprog/syscall.h +++ b/src/userprog/syscall.h @@ -1,8 +1,13 @@ #ifndef USERPROG_SYSCALL_H #define USERPROG_SYSCALL_H +#include + typedef int pid_t; void syscall_init (void); +unsigned fd_hash (const struct hash_elem *element, void *aux); +bool fd_less (const struct hash_elem *a, const struct hash_elem *b, void *aux); + #endif /* userprog/syscall.h */ From 5bd94894e051ff421f2badd26ae0ebddd47f2f95 Mon Sep 17 00:00:00 2001 From: sBubshait Date: Fri, 8 Nov 2024 15:13:04 +0000 Subject: [PATCH 13/26] Update thread structure to add a hash table of open files and initialise it, w/ E --- src/threads/thread.c | 3 +++ src/threads/thread.h | 2 ++ 2 files changed, 5 insertions(+) diff --git a/src/threads/thread.c b/src/threads/thread.c index 457f7b9..04380dd 100644 --- a/src/threads/thread.c +++ b/src/threads/thread.c @@ -1,5 +1,6 @@ #include "threads/thread.h" #include +#include #include #include #include @@ -15,6 +16,7 @@ #include "threads/vaddr.h" #ifdef USERPROG #include "userprog/process.h" +#include "userprog/syscall.h" #endif /* Random value for struct thread's `magic' member. @@ -660,6 +662,7 @@ init_thread (struct thread *t, const char *name, int nice, int priority, t->priority = t->base_priority; t->exit_status = -1; + hash_init (&t->open_files, fd_hash, fd_less, NULL); old_level = intr_disable (); list_push_back (&all_list, &t->allelem); diff --git a/src/threads/thread.h b/src/threads/thread.h index 1c05030..d17b4d7 100644 --- a/src/threads/thread.h +++ b/src/threads/thread.h @@ -2,6 +2,7 @@ #define THREADS_THREAD_H #include +#include #include #include #include "threads/fixed-point.h" @@ -116,6 +117,7 @@ struct thread #ifdef USERPROG /* Owned by userprog/process.c. */ uint32_t *pagedir; /* Page directory. */ + struct hash open_files; /* Hash Table of FD -> Struct File */ #endif /* Owned by thread.c. */ From 92e93b80608a45359aa2735b15549d3791a5a92e Mon Sep 17 00:00:00 2001 From: EDiasAlberto Date: Fri, 8 Nov 2024 15:33:47 +0000 Subject: [PATCH 14/26] Implement syscall for file opening and refactor open_files initialisation in thread.c w/ S. --- src/threads/thread.c | 2 +- src/userprog/syscall.c | 29 ++++++++++++++++++++++++----- src/userprog/syscall.h | 2 ++ 3 files changed, 27 insertions(+), 6 deletions(-) diff --git a/src/threads/thread.c b/src/threads/thread.c index 04380dd..037f309 100644 --- a/src/threads/thread.c +++ b/src/threads/thread.c @@ -238,6 +238,7 @@ thread_create (const char *name, int priority, struct thread *parent_thread = thread_current (); init_thread (t, name, parent_thread->nice, priority, parent_thread->recent_cpu); tid = t->tid = allocate_tid (); + hash_init (&t->open_files, fd_hash, fd_less, NULL); /* Prepare thread for first run by initializing its stack. Do this atomically so intermediate values for the 'stack' @@ -662,7 +663,6 @@ init_thread (struct thread *t, const char *name, int nice, int priority, t->priority = t->base_priority; t->exit_status = -1; - hash_init (&t->open_files, fd_hash, fd_less, NULL); old_level = intr_disable (); list_push_back (&all_list, &t->allelem); diff --git a/src/userprog/syscall.c b/src/userprog/syscall.c index 555b940..f89e2f9 100644 --- a/src/userprog/syscall.c +++ b/src/userprog/syscall.c @@ -4,6 +4,7 @@ #include "filesys/filesys.h" #include "threads/vaddr.h" #include "threads/interrupt.h" +#include "threads/malloc.h" #include "threads/thread.h" #include "threads/synch.h" #include "userprog/process.h" @@ -12,11 +13,12 @@ #include static struct lock filesys_lock; +static unsigned fd_counter = MIN_USER_FD; struct open_file { - int fd; /* File Descriptor / Identifier */ - struct file *file; /* Pointer to the associated file */ + int fd; /* File Descriptor / Identifier */ + struct file *file; /* Pointer to the associated file */ struct hash_elem elem; /* elem for a hash table */ }; @@ -165,10 +167,27 @@ syscall_remove (const char *file) } static int -syscall_open (const char *file UNUSED) +syscall_open (const char *file) { - //TODO - return 0; + validate_user_pointer (file, 1); + + lock_acquire (&filesys_lock); + struct file *ptr = filesys_open (file); + lock_release (&filesys_lock); + if (ptr == NULL) + return -1; + + struct open_file *file_info + = (struct open_file*) malloc (sizeof (struct open_file)); + if (file_info == NULL) + return -1; + + file_info->fd = fd_counter++; + file_info->file = ptr; + + hash_insert (&thread_current ()->open_files, &file_info->elem); + + return file_info->fd; } static int diff --git a/src/userprog/syscall.h b/src/userprog/syscall.h index 9563059..0f9288b 100644 --- a/src/userprog/syscall.h +++ b/src/userprog/syscall.h @@ -3,6 +3,8 @@ #include +#define MIN_USER_FD 2 + typedef int pid_t; void syscall_init (void); From 5424276603219adf560c4748713ccf5377775f34 Mon Sep 17 00:00:00 2001 From: sBubshait Date: Fri, 8 Nov 2024 15:50:48 +0000 Subject: [PATCH 15/26] Add a helper function to get a file from its descriptor (FD), w/ E --- src/userprog/syscall.c | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/src/userprog/syscall.c b/src/userprog/syscall.c index f89e2f9..ee91f43 100644 --- a/src/userprog/syscall.c +++ b/src/userprog/syscall.c @@ -281,6 +281,22 @@ fd_less (const struct hash_elem *a_, const struct hash_elem *b_, return a->fd < b->fd; } +/* Gets a file from its descriptor (FD number). If there is no file with the fd + FD it returns NULL. */ +static struct file * +fd_get_file (int fd) +{ + /* We have to set up a fake open_file in order to be able to search the hash + table. See hash.h. */ + struct open_file *fake_file_info; + fake_file_info->fd = fd; + + struct file *file + = hash_find (thread_current ()->open_files, fake_file_info->elem); + + return file; +} + /* Validates if a block of memory starting at PTR and of size SIZE bytes is fully contained within user virtual memory. Kills the thread (by calling thread_exit) if the memory is invalid. Otherwise, returns the PTR given. From a80084e9077abe7a936872a526be0c1eea725bf5 Mon Sep 17 00:00:00 2001 From: sBubshait Date: Fri, 8 Nov 2024 15:54:28 +0000 Subject: [PATCH 16/26] Fix Bug in fd_get_file declaration use open_file instead of file, w/ E --- src/userprog/syscall.c | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/src/userprog/syscall.c b/src/userprog/syscall.c index ee91f43..faeac2d 100644 --- a/src/userprog/syscall.c +++ b/src/userprog/syscall.c @@ -44,6 +44,7 @@ static void syscall_seek (int fd, unsigned position); static unsigned syscall_tell (int fd); static void syscall_close (int fd); +static struct open_file *fd_get_file (int fd); static void *validate_user_pointer (const void *ptr, size_t size); /* A struct defining a syscall_function pointer along with its arity. */ @@ -283,7 +284,7 @@ fd_less (const struct hash_elem *a_, const struct hash_elem *b_, /* Gets a file from its descriptor (FD number). If there is no file with the fd FD it returns NULL. */ -static struct file * +static struct open_file * fd_get_file (int fd) { /* We have to set up a fake open_file in order to be able to search the hash @@ -291,10 +292,10 @@ fd_get_file (int fd) struct open_file *fake_file_info; fake_file_info->fd = fd; - struct file *file - = hash_find (thread_current ()->open_files, fake_file_info->elem); + struct hash_elem *e + = hash_find (&thread_current ()->open_files, &fake_file_info->elem); - return file; + return hash_entry (e, struct open_file, elem); } /* Validates if a block of memory starting at PTR and of size SIZE bytes is From 75bd3fbde07969666254b9c70a921455bcd1c45e Mon Sep 17 00:00:00 2001 From: EDiasAlberto Date: Fri, 8 Nov 2024 16:02:51 +0000 Subject: [PATCH 17/26] Implement syscall for close() and fix typing bug in fd_get_file w/ S. --- src/userprog/syscall.c | 20 +++++++++++++++----- 1 file changed, 15 insertions(+), 5 deletions(-) diff --git a/src/userprog/syscall.c b/src/userprog/syscall.c index faeac2d..57a5018 100644 --- a/src/userprog/syscall.c +++ b/src/userprog/syscall.c @@ -1,6 +1,7 @@ #include "userprog/syscall.h" #include "devices/shutdown.h" #include "devices/input.h" +#include "filesys/file.h" #include "filesys/filesys.h" #include "threads/vaddr.h" #include "threads/interrupt.h" @@ -261,9 +262,18 @@ syscall_tell (int fd UNUSED) } static void -syscall_close (int fd UNUSED) +syscall_close (int fd) { - //TODO + struct open_file *file_info = fd_get_file (fd); + if (file_info != NULL) + { + hash_delete (&thread_current ()->open_files, &file_info->elem); + + lock_acquire (&filesys_lock); + file_close (file_info->file); + lock_release (&filesys_lock); + free (file_info); + } } unsigned @@ -289,11 +299,11 @@ fd_get_file (int fd) { /* We have to set up a fake open_file in order to be able to search the hash table. See hash.h. */ - struct open_file *fake_file_info; - fake_file_info->fd = fd; + struct open_file fake_file_info; + fake_file_info.fd = fd; struct hash_elem *e - = hash_find (&thread_current ()->open_files, &fake_file_info->elem); + = hash_find (&thread_current ()->open_files, &fake_file_info.elem); return hash_entry (e, struct open_file, elem); } From 3cfbe198e08695002e53392d2f4ab02cacaf5113 Mon Sep 17 00:00:00 2001 From: EDiasAlberto Date: Fri, 8 Nov 2024 16:10:46 +0000 Subject: [PATCH 18/26] Implement syscall for seek w/ S. --- src/userprog/syscall.c | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/src/userprog/syscall.c b/src/userprog/syscall.c index 57a5018..102458f 100644 --- a/src/userprog/syscall.c +++ b/src/userprog/syscall.c @@ -249,9 +249,15 @@ syscall_write (int fd, const void *buffer, unsigned size) } static void -syscall_seek (int fd UNUSED, unsigned position UNUSED) +syscall_seek (int fd, unsigned position) { - //TODO + struct open_file *file_info = fd_get_file (fd); + if (file_info != NULL) + { + lock_acquire (&filesys_lock); + file_seek (file_info->file, position); + lock_release (&filesys_lock); + } } static unsigned From 2a1cc3c3613e9d344ddf7e915de338bebbe11e88 Mon Sep 17 00:00:00 2001 From: sBubshait Date: Fri, 8 Nov 2024 16:14:41 +0000 Subject: [PATCH 19/26] Implement filesize and tell system calls, w/ E --- src/userprog/syscall.c | 26 ++++++++++++++++++++------ 1 file changed, 20 insertions(+), 6 deletions(-) diff --git a/src/userprog/syscall.c b/src/userprog/syscall.c index 102458f..f0aaf47 100644 --- a/src/userprog/syscall.c +++ b/src/userprog/syscall.c @@ -193,10 +193,17 @@ syscall_open (const char *file) } static int -syscall_filesize (int fd UNUSED) +syscall_filesize (int fd) { - //TODO - return 0; + struct open_file *file_info = fd_get_file (fd); + if (file_info == NULL) + return -1; + + lock_acquire (&filesys_lock); + int bytes = file_length (file_info->file); + lock_release (&filesys_lock); + + return bytes; } static int @@ -261,10 +268,17 @@ syscall_seek (int fd, unsigned position) } static unsigned -syscall_tell (int fd UNUSED) +syscall_tell (int fd) { - //TODO - return 0; + struct open_file *file_info = fd_get_file (fd); + if (file_info == NULL) + return 0; + + lock_acquire (&filesys_lock); + unsigned pos = file_tell (file_info->file); + lock_release (&filesys_lock); + + return pos; } static void From 18694d7b629bdb13ced060f78ce897ac614c6ab0 Mon Sep 17 00:00:00 2001 From: EDiasAlberto Date: Fri, 8 Nov 2024 16:25:49 +0000 Subject: [PATCH 20/26] Implement file reading syscall and fix fd validation w/ S. --- src/userprog/syscall.c | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/src/userprog/syscall.c b/src/userprog/syscall.c index f0aaf47..9a42214 100644 --- a/src/userprog/syscall.c +++ b/src/userprog/syscall.c @@ -211,7 +211,7 @@ syscall_read (int fd, void *buffer, unsigned size) { /* Only console (fd = 0) or other files, not including STDOUT, (fd > 1) are allowed. */ - if (fd < 0 && fd != STDOUT_FILENO) + if (fd < 0 || fd == STDOUT_FILENO) return -1; validate_user_pointer (buffer, size); @@ -228,7 +228,14 @@ syscall_read (int fd, void *buffer, unsigned size) else { /* Reading from a file. */ - return 0; // TODO: Implement Write to Files + struct open_file *file_info = fd_get_file (fd); + if (file_info == NULL) + return -1; + + lock_acquire (&filesys_lock); + int bytes_written = file_read (file_info->file, buffer, size); + lock_release (&filesys_lock); + return bytes_written; } } From 8912ef466027bc1b665874383046bec487fffc0d Mon Sep 17 00:00:00 2001 From: sBubshait Date: Fri, 8 Nov 2024 16:26:40 +0000 Subject: [PATCH 21/26] Implement writing to file system files in the write system call, w/ E --- src/userprog/syscall.c | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/src/userprog/syscall.c b/src/userprog/syscall.c index 9a42214..4bd8732 100644 --- a/src/userprog/syscall.c +++ b/src/userprog/syscall.c @@ -258,7 +258,15 @@ syscall_write (int fd, const void *buffer, unsigned size) else { /* Writing to a file. */ - return 0; // TODO: Implement Write to Files + struct open_file *file_info = fd_get_file (fd); + if (file_info == NULL) + return 0; + + lock_acquire (&filesys_lock); + int bytes = file_write (file_info->file, buffer, size); + lock_release (&filesys_lock); + + return bytes; } } From e40794e672668ea20f68b8c892a22758a3392f1c Mon Sep 17 00:00:00 2001 From: sBubshait Date: Fri, 8 Nov 2024 16:48:19 +0000 Subject: [PATCH 22/26] Fix Bug in fd_get_file: In case fd not found, then returns NULL, w/ E --- src/userprog/syscall.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/userprog/syscall.c b/src/userprog/syscall.c index 4bd8732..96c099d 100644 --- a/src/userprog/syscall.c +++ b/src/userprog/syscall.c @@ -340,6 +340,9 @@ fd_get_file (int fd) struct hash_elem *e = hash_find (&thread_current ()->open_files, &fake_file_info.elem); + if (e == NULL) + return NULL; + return hash_entry (e, struct open_file, elem); } From 115c650c55256b92290c6ae8ce84f7bf0bd610cd Mon Sep 17 00:00:00 2001 From: sBubshait Date: Fri, 8 Nov 2024 19:10:42 +0000 Subject: [PATCH 23/26] Fix Bug in thread initialisation: only init hash if USERPROG is defined --- src/threads/thread.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/threads/thread.c b/src/threads/thread.c index 037f309..1877dff 100644 --- a/src/threads/thread.c +++ b/src/threads/thread.c @@ -238,7 +238,10 @@ thread_create (const char *name, int priority, struct thread *parent_thread = thread_current (); init_thread (t, name, parent_thread->nice, priority, parent_thread->recent_cpu); tid = t->tid = allocate_tid (); - hash_init (&t->open_files, fd_hash, fd_less, NULL); + + #ifdef USERPROG + hash_init (&t->open_files, fd_hash, fd_less, NULL); + #endif /* Prepare thread for first run by initializing its stack. Do this atomically so intermediate values for the 'stack' From 7778e05aa42e7fa8c6f0e2aee1924f3c2df64ec8 Mon Sep 17 00:00:00 2001 From: Gleb Koval Date: Sat, 9 Nov 2024 11:06:36 +0000 Subject: [PATCH 24/26] Fix deadlock by release of lock and semaphore in the wrong order --- src/userprog/process.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/userprog/process.c b/src/userprog/process.c index 370a216..3943b21 100644 --- a/src/userprog/process.c +++ b/src/userprog/process.c @@ -262,8 +262,8 @@ process_exit (void) struct process_result *, and may be waiting so call sema_up */ else { - sema_up (&cur->result->sema); lock_release (&cur->result->lock); + sema_up (&cur->result->sema); } } @@ -283,8 +283,8 @@ process_exit (void) /* Child is still alive, signal via sema that parent has died. */ else { - sema_up (&result->sema); lock_release (&result->lock); + sema_up (&result->sema); } } From 1bfd73b2020b7beb4c91c67fb35c8c3a1847b57b Mon Sep 17 00:00:00 2001 From: EDiasAlberto Date: Sun, 10 Nov 2024 01:32:48 +0000 Subject: [PATCH 25/26] Comment syscall functions and handlers --- src/userprog/syscall.c | 54 ++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 54 insertions(+) diff --git a/src/userprog/syscall.c b/src/userprog/syscall.c index 96c099d..ccb02f3 100644 --- a/src/userprog/syscall.c +++ b/src/userprog/syscall.c @@ -79,6 +79,9 @@ static const syscall_arguments syscall_lookup[] = static const int LOOKUP_SIZE = sizeof (syscall_lookup) / sizeof (syscall_arguments); + +/* Initialises the syscall handling system, as well as a global lock to + synchronise all file access between processes. */ void syscall_init (void) { @@ -86,6 +89,9 @@ syscall_init (void) lock_init (&filesys_lock); } +/* Function that takes a interrupt frame containing a syscall and its args. + Validates the arguments and pointers before calling the relevant + high-level system call function, storing its output (if any) in f->eax */ static void syscall_handler (struct intr_frame *f) { @@ -111,6 +117,8 @@ syscall_handler (struct intr_frame *f) f->eax = syscall.function (args[0], args[1], args[2]); } +/* Called upon a "halt" syscall, resulting in a complete shutdown of the + process, via shutdown_power_off (); */ static void syscall_halt (void) { @@ -126,6 +134,10 @@ syscall_exit (int status) thread_exit (); } +/* Executes a given command with the relevant args, by calling process_execute. + Acquires the filesystem lock as process_execute accesses the file system. + Returns PID for the process that is running the CMD_LINE + */ static pid_t syscall_exec (const char *cmd_line) { @@ -138,12 +150,17 @@ syscall_exec (const char *cmd_line) return pid; } +/* Handles the syscall of wait. Effectively a wrapper for process_wait as the + necessary validation and such all happens in process_wait anyway. */ static int syscall_wait (pid_t pid) { return process_wait (pid); } +/* Handles the syscall for file creation. First validates the user file + pointer. Acquires the file system lock to prevent synchronisation issues, + and then uses FILESYS_CREATE to create the file, returning the same status */ static bool syscall_create (const char *file UNUSED, unsigned initial_size UNUSED) { @@ -156,6 +173,9 @@ syscall_create (const char *file UNUSED, unsigned initial_size UNUSED) return status; } +/* Handles the syscall for file removal. First validates the user file pointer. + Acquires the file system lock to prevent synchronisation issues, and then + uses FILESYS_REMOVE to remove the file, returning the same success status */ static bool syscall_remove (const char *file) { @@ -168,6 +188,10 @@ syscall_remove (const char *file) return status; } +/* Handles the syscall for opening a file connection. First, validates the file + pointer. Then it acquires a lock for the file system, in order to open the + connection without synchronisation issues. It then maps a new fd to this file + in the hash table before returning the fd. */ static int syscall_open (const char *file) { @@ -179,19 +203,27 @@ syscall_open (const char *file) if (ptr == NULL) return -1; + /* Allocate space for a struct representing a mapping from an FD to a struct + file. */ struct open_file *file_info = (struct open_file*) malloc (sizeof (struct open_file)); if (file_info == NULL) return -1; + /* Populate the above struct, with a unique FD and the current open file */ file_info->fd = fd_counter++; file_info->file = ptr; + /* Add the new FD->file mapping to the hashtable for the current thread */ hash_insert (&thread_current ()->open_files, &file_info->elem); + /* Return the new FD */ return file_info->fd; } +/* Handles the syscall for getting a file's size. Converts a provided FD into + the asssociated file struct. Acquire the lock for the filesystem and use + FILE_LENGTH to calculate the length for return. */ static int syscall_filesize (int fd) { @@ -206,6 +238,10 @@ syscall_filesize (int fd) return bytes; } +/* Handles the syscall for reading SIZE bytes from a file referenced by FD. + If the FD references the console, use input_getc (), otherwise convert the + FD to its associated file struct, acquire the filesystem lock, read up to + SIZE bytes and then return the number of bytes read.*/ static int syscall_read (int fd, void *buffer, unsigned size) { @@ -239,6 +275,10 @@ syscall_read (int fd, void *buffer, unsigned size) } } +/* Handles the syscall for writing SIZE bytes to a file referenced by FD. + If the FD references the console, use put_buf (), otherwise convert the + FD to its associated file struct, acquire the filesystem lock, write up to + SIZE bytes and then return the number of bytes written.*/ static int syscall_write (int fd, const void *buffer, unsigned size) { @@ -270,6 +310,10 @@ syscall_write (int fd, const void *buffer, unsigned size) } } +/* Handles the syscall for seeking to POSITION bytes in a file referenced by + FD. Converts the FD to its associated file struct, acquires the filesystem + lock and then uses file_seek to adjust the cursor to a specific position in + the file.*/ static void syscall_seek (int fd, unsigned position) { @@ -282,6 +326,9 @@ syscall_seek (int fd, unsigned position) } } +/* Handles the syscall for returning the next byte in a file referenced by + FD. Converts the FD to its associated file struct, acquires the filesystem + lock and then uses file_tell to read the next byte.*/ static unsigned syscall_tell (int fd) { @@ -296,6 +343,9 @@ syscall_tell (int fd) return pos; } +/* Handles the syscall for closing a connection to a file. Converts the FD to + its associated file struct. If it exists, it removes it from the hash table, + acquires the filesystem lock, and uses file_close to close the connection.*/ static void syscall_close (int fd) { @@ -311,12 +361,16 @@ syscall_close (int fd) } } +/* Hashing function needed for the open_file table. Returns a hash for an entry, + based on its FD. */ unsigned fd_hash (const struct hash_elem *element, void *aux UNUSED) { return hash_int (hash_entry (element, struct open_file, elem)->fd); } +/* Comparator function for the open_file table. Compares two entries based on + the FDs. */ bool fd_less (const struct hash_elem *a_, const struct hash_elem *b_, void *aux UNUSED) From 98a581840666de1fd45810ac1268301f9e2cd105 Mon Sep 17 00:00:00 2001 From: EDiasAlberto Date: Mon, 11 Nov 2024 17:55:24 +0000 Subject: [PATCH 26/26] add file_deny_write and file_allow_write to process creation and exiting to make executable read-only --- src/threads/thread.h | 1 + src/userprog/process.c | 5 +++++ 2 files changed, 6 insertions(+) diff --git a/src/threads/thread.h b/src/threads/thread.h index 6c65dcb..33a5485 100644 --- a/src/threads/thread.h +++ b/src/threads/thread.h @@ -127,6 +127,7 @@ struct thread struct process_result *result; /* Result of the process. */ struct list child_results; /* List of children's of this thread process results. */ + struct file *exec_file; /* Thread's currently running file */ /* Shared between thread.c and synch.c. */ struct list_elem elem; /* List element. */ diff --git a/src/userprog/process.c b/src/userprog/process.c index 3943b21..e11eff9 100644 --- a/src/userprog/process.c +++ b/src/userprog/process.c @@ -181,6 +181,10 @@ start_process (void *file_name_) if (!success) thread_exit (); + struct file *exec_file = filesys_open (file_name); + thread_current ()->exec_file = exec_file; + file_deny_write (exec_file); + /* Start the user process by simulating a return from an interrupt, implemented by intr_exit (in threads/intr-stubs.S). Because intr_exit takes all of its @@ -246,6 +250,7 @@ process_exit (void) uint32_t *pd; printf ("%s: exit(%d)\n", cur->name, cur->exit_status); + file_close (cur->exec_file); /* Update process result. */ if (cur->result != NULL)