From 49a9284f02c563158dbd3b8501458e44a2eef4e3 Mon Sep 17 00:00:00 2001 From: EDiasAlberto Date: Thu, 24 Oct 2024 20:16:40 +0100 Subject: [PATCH 1/8] refactor fixed-point.h to match style standards --- src/threads/fixed-point.h | 53 +++++++++++++++++++++------------------ src/threads/thread.c | 2 +- 2 files changed, 30 insertions(+), 25 deletions(-) diff --git a/src/threads/fixed-point.h b/src/threads/fixed-point.h index c56b3b6..69adb63 100644 --- a/src/threads/fixed-point.h +++ b/src/threads/fixed-point.h @@ -3,37 +3,40 @@ #define FIXED_POINT_H typedef struct -{ - int32_t raw; -} fp32_t; + { + int32_t raw; + } fp32_t; /* Fixed Point Arithmetic bit count constants */ #define NUM_FRAC_BITS 14 #define NUM_INT_BITS (31 - NUM_FRAC_BITS) -#define CONVERSION_CONST (1 << NUM_FRAC_BITS) /* f = 2^q, (2^20) */ +#define CONVERSION_FACTOR (1 << NUM_FRAC_BITS) /* f = 2^q, (2^14) */ /* Fixed Point Arithmetic conversion operations */ /* Converts an integer n to a fixed point number */ inline fp32_t -int_to_fp (int32_t n) +fp_from_int (int32_t n) { - return (fp32_t){ n * CONVERSION_CONST }; + return (fp32_t){ n * CONVERSION_FACTOR }; } -/* Handles conversion of fixed point to integer. First version truncates, second one rounds */ +/* Handles conversion of fixed point to integer, + with truncation */ inline int32_t fp_floor (fp32_t x) { - return x.raw / CONVERSION_CONST; + return x.raw / CONVERSION_FACTOR; } +/* Handles conversion of fixed point to integer, + with rounding */ inline int32_t fp_round (fp32_t x) { if (x.raw >= 0) - return (x.raw + CONVERSION_CONST / 2) / CONVERSION_CONST; + return (x.raw + CONVERSION_FACTOR / 2) / CONVERSION_FACTOR; else - return (x.raw - CONVERSION_CONST / 2) / CONVERSION_CONST; + return (x.raw - CONVERSION_FACTOR / 2) / CONVERSION_FACTOR; } /* Add two fixed points */ @@ -50,32 +53,20 @@ fp_sub (fp32_t x, fp32_t y) return (fp32_t){ x.raw - y.raw }; } -/* Add fixed point to integer */ -inline fp32_t -fp_add_int (fp32_t x, int32_t n) -{ - return (fp32_t){ x.raw + n * CONVERSION_CONST }; -} -/* Subtract integer from fixed point */ -inline fp32_t -fp_sub_int (fp32_t x, int32_t n) -{ - return (fp32_t){ x.raw - n * CONVERSION_CONST }; -} /* Multiple two fixed points */ inline fp32_t fp_mul (fp32_t x, fp32_t y) { - return (fp32_t){ ((int64_t)x.raw) * y.raw / CONVERSION_CONST }; + return (fp32_t){ ((int64_t)x.raw) * y.raw / CONVERSION_FACTOR }; } /* Divide two fixed points */ inline fp32_t fp_div (fp32_t x, fp32_t y) { - return (fp32_t){ ((int64_t)x.raw) * CONVERSION_CONST / y.raw }; + return (fp32_t){ ((int64_t)x.raw) * CONVERSION_FACTOR / y.raw }; } /* Multiply fixed point and integer */ @@ -92,4 +83,18 @@ fp_div_int (fp32_t x, int32_t n) return (fp32_t){ x.raw / n }; } +/* Add fixed point to integer */ +inline fp32_t +fp_add_int (fp32_t x, int32_t n) +{ + return (fp32_t){ x.raw + n * CONVERSION_FACTOR }; +} + +/* Subtract integer from fixed point */ +inline fp32_t +fp_sub_int (fp32_t x, int32_t n) +{ + return (fp32_t){ x.raw - n * CONVERSION_FACTOR }; +} + #endif //FIXED_POINT_H diff --git a/src/threads/thread.c b/src/threads/thread.c index cb60d1b..dd472c1 100644 --- a/src/threads/thread.c +++ b/src/threads/thread.c @@ -163,7 +163,7 @@ thread_tick (void) if (t != idle_thread) ready++; fp32_t old_coeff = fp_div_int (fp_mul_int(load_avg, 59), 60); - fp32_t new_coeff = fp_div_int (int_to_fp (ready), 60); + fp32_t new_coeff = fp_div_int (fp_from_int (ready), 60); load_avg = fp_add (old_coeff, new_coeff); thread_foreach (thread_update_recent_cpu, NULL); From 0b230131f1d33ed9f2942441d9a7cb8294deb2c0 Mon Sep 17 00:00:00 2001 From: EDiasAlberto Date: Thu, 24 Oct 2024 20:17:39 +0100 Subject: [PATCH 2/8] refactor thread struct to keep all changes together --- src/threads/thread.h | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/src/threads/thread.h b/src/threads/thread.h index 8139b44..c7cc364 100644 --- a/src/threads/thread.h +++ b/src/threads/thread.h @@ -104,13 +104,14 @@ struct thread struct list_elem donor_elem; /* List element so that thread can be enlisted in other donors list. */ - /* Shared between thread.c and synch.c. */ - struct list_elem elem; /* List element. */ - /* MLFQS items */ int nice; /* Nice value for this thread */ fp32_t recent_cpu; /* Amount of time this process received */ + /* Shared between thread.c and synch.c. */ + struct list_elem elem; /* List element. */ + + #ifdef USERPROG /* Owned by userprog/process.c. */ uint32_t *pagedir; /* Page directory. */ From 74657bbf9c3fa4bfaa9354accf9ac8b9c88eed2a Mon Sep 17 00:00:00 2001 From: EDiasAlberto Date: Thu, 24 Oct 2024 20:34:44 +0100 Subject: [PATCH 3/8] reorder init_thread for clarity --- src/threads/thread.c | 15 +++++++-------- 1 file changed, 7 insertions(+), 8 deletions(-) diff --git a/src/threads/thread.c b/src/threads/thread.c index dd472c1..e93eeb8 100644 --- a/src/threads/thread.c +++ b/src/threads/thread.c @@ -634,17 +634,16 @@ init_thread (struct thread *t, const char *name, int nice, int priority, t->status = THREAD_BLOCKED; strlcpy (t->name, name, sizeof t->name); t->stack = (uint8_t *) t + PGSIZE; + t->magic = THREAD_MAGIC; + + t->base_priority = priority; + list_init (&t->donors_list); + t->waiting_lock = NULL; t->nice = nice; t->recent_cpu = recent_cpu; - - t->base_priority = priority; - - t->magic = THREAD_MAGIC; - - list_init (&t->donors_list); - t->priority = thread_mlfqs ? calculate_bsd_priority (recent_cpu, nice) : t->base_priority; - t->waiting_lock = NULL; + t->priority = thread_mlfqs ? calculate_bsd_priority (recent_cpu, nice) + : t->base_priority; old_level = intr_disable (); list_push_back (&all_list, &t->allelem); From a6a0c4ad25a0225baf40cce231ccf64f7f91eaed Mon Sep 17 00:00:00 2001 From: EDiasAlberto Date: Thu, 24 Oct 2024 20:47:30 +0100 Subject: [PATCH 4/8] remove empty lines in thread.c --- src/threads/thread.c | 4 ---- 1 file changed, 4 deletions(-) diff --git a/src/threads/thread.c b/src/threads/thread.c index e93eeb8..4afbe68 100644 --- a/src/threads/thread.c +++ b/src/threads/thread.c @@ -291,7 +291,6 @@ thread_unblock (struct thread *t) old_level = intr_disable (); ASSERT (t->status == THREAD_BLOCKED); - /* Insert the thread back into the ready list in priority order. */ list_insert_ordered(&ready_list, &t->elem, priority_more, NULL); @@ -367,10 +366,8 @@ thread_yield (void) if (cur != idle_thread) { - /* Insert the thread back into the ready list in priority order. */ list_insert_ordered(&ready_list, &cur->elem, priority_more, NULL); - } cur->status = THREAD_READY; @@ -402,7 +399,6 @@ bool priority_more (const struct list_elem *a_, const struct list_elem *b_, void *aux UNUSED) { - struct thread *a = list_entry (a_, struct thread, elem); struct thread *b = list_entry (b_, struct thread, elem); From 60acc2e58daa9de37f59c2dfbe121c3efc1f4643 Mon Sep 17 00:00:00 2001 From: EDiasAlberto Date: Thu, 24 Oct 2024 21:46:39 +0100 Subject: [PATCH 5/8] refactor thread_mlfqs checks in synch.c --- src/threads/synch.c | 77 +++++++++++++++++++++----------------------- src/threads/thread.c | 32 ++++++++++-------- 2 files changed, 55 insertions(+), 54 deletions(-) diff --git a/src/threads/synch.c b/src/threads/synch.c index 0ceb746..3fe957c 100644 --- a/src/threads/synch.c +++ b/src/threads/synch.c @@ -305,53 +305,50 @@ lock_release (struct lock *lock) ASSERT (lock != NULL); ASSERT (lock_held_by_current_thread (lock)); - if (!thread_mlfqs) - { - struct thread *current_thread = thread_current (); - struct thread *max_donor = NULL; + struct thread *current_thread = thread_current (); + struct thread *max_donor = NULL; - struct list orphan_list; - list_init (&orphan_list); + struct list orphan_list; + list_init (&orphan_list); - enum intr_level old_level = intr_disable (); - /* Loop through current thread's donors, removing the ones waiting for the - lock being released and keeping track of them (within orphan_list). - Also identifies the highest priority donor thread among them. */ - struct list_elem *tail = list_tail (¤t_thread->donors_list); - struct list_elem *e = list_begin (¤t_thread->donors_list); - while (e != tail) - { - struct thread *donor = list_entry (e, struct thread, donor_elem); - struct list_elem *next = list_next (e); + enum intr_level old_level = intr_disable (); + /* Loop through current thread's donors, removing the ones waiting for the + lock being released and keeping track of them (within orphan_list). + Also identifies the highest priority donor thread among them. */ + struct list_elem *tail = list_tail (¤t_thread->donors_list); + struct list_elem *e = list_begin (¤t_thread->donors_list); + while (e != tail) + { + struct thread *donor = list_entry (e, struct thread, donor_elem); + struct list_elem *next = list_next (e); - /* Excludes donors that aren't waiting for the lock being released, - and tracks the rest. */ - if (donor->waiting_lock == lock) - { - list_remove (e); - list_push_back (&orphan_list, e); + /* Excludes donors that aren't waiting for the lock being released, + and tracks the rest. */ + if (donor->waiting_lock == lock) + { + list_remove (e); + list_push_back (&orphan_list, e); - /* Identify highest priority donor. */ - if (max_donor == NULL || donor->priority > max_donor->priority) - max_donor = donor; - } + /* Identify highest priority donor. */ + if (max_donor == NULL || donor->priority > max_donor->priority) + max_donor = donor; + } - e = next; - } + e = next; + } - /* If there exists a maximum donor thread waiting for this lock to be - released, transfer the remaining orphaned donors to its donor list. */ - if (max_donor != NULL) - { - while (!list_empty (&orphan_list)) - list_push_back (&max_donor->donors_list, list_pop_front (&orphan_list)); - } + /* If there exists a maximum donor thread waiting for this lock to be + released, transfer the remaining orphaned donors to its donor list. */ + if (max_donor != NULL) + { + while (!list_empty (&orphan_list)) + list_push_back (&max_donor->donors_list, list_pop_front (&orphan_list)); + } - intr_set_level (old_level); - /* Removal of donors to this thread may change its effective priority, - so recalculate. */ - thread_recalculate_priority (); - } + intr_set_level (old_level); + /* Removal of donors to this thread may change its effective priority, + so recalculate. */ + thread_recalculate_priority (); lock->holder = NULL; sema_up (&lock->semaphore); diff --git a/src/threads/thread.c b/src/threads/thread.c index 4afbe68..795866e 100644 --- a/src/threads/thread.c +++ b/src/threads/thread.c @@ -157,23 +157,26 @@ thread_tick (void) /* Update system load_avg and all threads recent_cpu every second. */ int64_t ticks = timer_ticks (); - if (thread_mlfqs && (ticks % TIMER_FREQ == 0)) + if (thread_mlfqs) { - size_t ready = threads_ready (); if (t != idle_thread) - ready++; - fp32_t old_coeff = fp_div_int (fp_mul_int(load_avg, 59), 60); - fp32_t new_coeff = fp_div_int (fp_from_int (ready), 60); - load_avg = fp_add (old_coeff, new_coeff); + { + t->recent_cpu = fp_add_int (t->recent_cpu, 1); + } - thread_foreach (thread_update_recent_cpu, NULL); - } + if (ticks % TIMER_FREQ == 0) + { + size_t ready = threads_ready (); + if (t != idle_thread) + ready++; + fp32_t old_coeff = fp_div_int (fp_mul_int(load_avg, 59), 60); + fp32_t new_coeff = fp_div_int (fp_from_int (ready), 60); + load_avg = fp_add (old_coeff, new_coeff); - /* Update current thread's recent_cpu. */ - if (thread_mlfqs && (t != idle_thread)) - { - t->recent_cpu = fp_add_int (t->recent_cpu, 1); - if (ticks % 4 == 0) // recent_cpu was updated, update priority. + thread_foreach (thread_update_recent_cpu, NULL); + } + + if (ticks % TIME_SLICE == 0) // recent_cpu was updated, update priority. t->priority = calculate_bsd_priority (t->recent_cpu, t->nice); } @@ -470,7 +473,8 @@ thread_recalculate_priority (void) { struct thread *t = thread_current (); - ASSERT(!thread_mlfqs) + if (thread_mlfqs) + return; enum intr_level old_level = intr_disable (); t->priority = t->base_priority; From 39f4edd5e685bd736f7585072f911aebf1dee3be Mon Sep 17 00:00:00 2001 From: EDiasAlberto Date: Thu, 24 Oct 2024 22:09:28 +0100 Subject: [PATCH 6/8] remove unnecessary curly brace in thread_tick () --- src/threads/thread.c | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/threads/thread.c b/src/threads/thread.c index 795866e..7d27f1c 100644 --- a/src/threads/thread.c +++ b/src/threads/thread.c @@ -160,9 +160,7 @@ thread_tick (void) if (thread_mlfqs) { if (t != idle_thread) - { t->recent_cpu = fp_add_int (t->recent_cpu, 1); - } if (ticks % TIMER_FREQ == 0) { From 7d196ffc570b4c60c316cf89c3fda9ef7db32176 Mon Sep 17 00:00:00 2001 From: EDiasAlberto Date: Thu, 24 Oct 2024 22:30:38 +0100 Subject: [PATCH 7/8] adjust spacing around brackets to fit styling conventions --- src/threads/thread.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/threads/thread.c b/src/threads/thread.c index 7d27f1c..5ae0a09 100644 --- a/src/threads/thread.c +++ b/src/threads/thread.c @@ -167,7 +167,7 @@ thread_tick (void) size_t ready = threads_ready (); if (t != idle_thread) ready++; - fp32_t old_coeff = fp_div_int (fp_mul_int(load_avg, 59), 60); + fp32_t old_coeff = fp_div_int (fp_mul_int (load_avg, 59), 60); fp32_t new_coeff = fp_div_int (fp_from_int (ready), 60); load_avg = fp_add (old_coeff, new_coeff); @@ -293,7 +293,7 @@ thread_unblock (struct thread *t) ASSERT (t->status == THREAD_BLOCKED); /* Insert the thread back into the ready list in priority order. */ - list_insert_ordered(&ready_list, &t->elem, priority_more, NULL); + list_insert_ordered (&ready_list, &t->elem, priority_more, NULL); t->status = THREAD_READY; intr_set_level (old_level); @@ -368,7 +368,7 @@ thread_yield (void) if (cur != idle_thread) { /* Insert the thread back into the ready list in priority order. */ - list_insert_ordered(&ready_list, &cur->elem, priority_more, NULL); + list_insert_ordered (&ready_list, &cur->elem, priority_more, NULL); } cur->status = THREAD_READY; @@ -726,7 +726,7 @@ static int calculate_bsd_priority (fp32_t recent_cpu, int nice) { - ASSERT(thread_mlfqs); + ASSERT (thread_mlfqs); int priority = PRI_MAX - (fp_round (recent_cpu) / 4) - (nice * 2); if (priority < PRI_MIN) From b88baede647c71a86686aa75af7754dec2645e68 Mon Sep 17 00:00:00 2001 From: EDiasAlberto Date: Thu, 24 Oct 2024 22:45:49 +0100 Subject: [PATCH 8/8] rename recent_cpu update function --- src/threads/thread.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/threads/thread.c b/src/threads/thread.c index 5ae0a09..880737a 100644 --- a/src/threads/thread.c +++ b/src/threads/thread.c @@ -72,7 +72,7 @@ static void init_thread (struct thread *, const char *name, int nice, static bool is_thread (struct thread *) UNUSED; static void *alloc_frame (struct thread *, size_t size); static int calculate_bsd_priority (fp32_t recent_cpu, int nice); -static void thread_update_recent_cpu (struct thread *t, void *aux UNUSED); +static void update_recent_cpu (struct thread *t, void *aux UNUSED); static void schedule (void); void thread_schedule_tail (struct thread *prev); static tid_t allocate_tid (void); @@ -171,7 +171,7 @@ thread_tick (void) fp32_t new_coeff = fp_div_int (fp_from_int (ready), 60); load_avg = fp_add (old_coeff, new_coeff); - thread_foreach (thread_update_recent_cpu, NULL); + thread_foreach (update_recent_cpu, NULL); } if (ticks % TIME_SLICE == 0) // recent_cpu was updated, update priority. @@ -225,8 +225,8 @@ thread_create (const char *name, int priority, return TID_ERROR; /* Initialize thread. */ - struct thread *pt = thread_current (); - init_thread (t, name, pt->nice, priority, pt->recent_cpu); + struct thread *parent_thread = thread_current (); + init_thread (t, name, parent_thread->nice, priority, parent_thread->recent_cpu); tid = t->tid = allocate_tid (); /* Prepare thread for first run by initializing its stack. @@ -454,7 +454,7 @@ thread_get_priority (void) /* Updates recent_cpu for a thread. */ static void -thread_update_recent_cpu (struct thread *t, void *aux UNUSED) +update_recent_cpu (struct thread *t, void *aux UNUSED) { fp32_t curr_recent_cpu = t->recent_cpu; fp32_t dbl_load_avg = fp_mul_int (load_avg, 2);