From a875d5fcb4eec1aeb97b5ab47ae963a9f8f3e5ae Mon Sep 17 00:00:00 2001 From: Themis Demetriades Date: Wed, 23 Oct 2024 13:29:45 +0100 Subject: [PATCH 1/4] Update donate_priority to only attempt to sort position of the donee that isn't waiting for a lock --- src/threads/synch.c | 11 ++++++++--- src/threads/thread.c | 18 +++++++++++++++--- src/threads/thread.h | 2 +- 3 files changed, 24 insertions(+), 7 deletions(-) diff --git a/src/threads/synch.c b/src/threads/synch.c index 1123249..462bda7 100644 --- a/src/threads/synch.c +++ b/src/threads/synch.c @@ -214,12 +214,17 @@ donate_priority (struct thread *donor, struct thread *donee) { /* Also stop propagation of donation once a donee is reached with no donees of its own (sink node in WFG). */ if (donee->waiting_lock == NULL) - donee = NULL; + { + + /* Only the sink node of the WFG isn't waiting for a lock and + could be on the ready list. Thus, as its priority changed, + it must be reinserted into the list. */ + ready_list_reinsert (donee); + donee = NULL; + } else donee = donee->waiting_lock->holder; } - - ready_list_reorder (); } /* Acquires LOCK, sleeping until it becomes available if diff --git a/src/threads/thread.c b/src/threads/thread.c index ff66100..178cdf9 100644 --- a/src/threads/thread.c +++ b/src/threads/thread.c @@ -465,10 +465,22 @@ thread_get_recent_cpu (void) return 0; } +/* Reinsert thread t into the ready list at its correct position + in descending order of priority. Used when this thread's priority + may have changed. */ void -ready_list_reorder (void) +ready_list_reinsert (struct thread *t) { - list_sort (&ready_list, priority_more, NULL); + enum intr_level old_level = intr_disable (); + + /* If the thread isn't ready to run, do nothing. */ + if (t->status == THREAD_READY) + { + list_remove (&t->elem); + list_insert_ordered (&ready_list, &t->elem, priority_more, NULL); + } + + intr_set_level (old_level); } /* Idle thread. Executes when no other thread is ready to run. @@ -558,7 +570,7 @@ init_thread (struct thread *t, const char *name, int priority) t->stack = (uint8_t *) t + PGSIZE; t->base_priority = priority; t->magic = THREAD_MAGIC; - + list_init (&t->donors_list); t->priority = t->base_priority; t->waiting_lock = NULL; diff --git a/src/threads/thread.h b/src/threads/thread.h index de9df61..ffeb2b3 100644 --- a/src/threads/thread.h +++ b/src/threads/thread.h @@ -151,6 +151,6 @@ void thread_set_nice (int); int thread_get_recent_cpu (void); int thread_get_load_avg (void); -void ready_list_reorder (void); +void ready_list_reinsert (struct thread *t); #endif /* threads/thread.h */ -- 2.49.1 From 2cd4da17a4e158a3023790aa3d14f01ffb1bfbc0 Mon Sep 17 00:00:00 2001 From: Themis Demetriades Date: Wed, 23 Oct 2024 13:42:36 +0100 Subject: [PATCH 2/4] Refactor donate_priority to only allow for the current thread to donate its priority --- src/threads/synch.c | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/src/threads/synch.c b/src/threads/synch.c index 462bda7..0649772 100644 --- a/src/threads/synch.c +++ b/src/threads/synch.c @@ -194,11 +194,12 @@ lock_init (struct lock *lock) sema_init (&lock->semaphore, 1); } -/* Allows for the donor to donate its priority to donee, iteratively +/* Current thread donates its priority to donee, iteratively propagating the donation in the case of chains in the wait-for graph. Also keeps track of the donation by updating the donors list. */ static void -donate_priority (struct thread *donor, struct thread *donee) { +donate_priority (struct thread *donee) { + struct thread *donor = thread_current (); list_push_back (&donee->donors_list, &donor->donor_elem); while (donee != NULL) @@ -249,7 +250,7 @@ lock_acquire (struct lock *lock) if (lock->holder != NULL) { t->waiting_lock = lock; - donate_priority (t, lock->holder); + donate_priority (lock->holder); } sema_down (&lock->semaphore); -- 2.49.1 From b0074c80f014ca24763dc2f561bd185f62b6c983 Mon Sep 17 00:00:00 2001 From: Themis Demetriades Date: Wed, 23 Oct 2024 13:51:07 +0100 Subject: [PATCH 3/4] Refactor ready_list_reinsert to require being called with interrupts disabled --- src/threads/synch.c | 2 ++ src/threads/thread.c | 14 ++++++-------- 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/src/threads/synch.c b/src/threads/synch.c index 0649772..7dc6eaa 100644 --- a/src/threads/synch.c +++ b/src/threads/synch.c @@ -220,7 +220,9 @@ donate_priority (struct thread *donee) { /* Only the sink node of the WFG isn't waiting for a lock and could be on the ready list. Thus, as its priority changed, it must be reinserted into the list. */ + enum intr_level old_level = intr_disable (); ready_list_reinsert (donee); + intr_set_level (old_level); donee = NULL; } else diff --git a/src/threads/thread.c b/src/threads/thread.c index 178cdf9..8731b3b 100644 --- a/src/threads/thread.c +++ b/src/threads/thread.c @@ -467,20 +467,18 @@ thread_get_recent_cpu (void) /* Reinsert thread t into the ready list at its correct position in descending order of priority. Used when this thread's priority - may have changed. */ + may have changed. Must be called with interrupts disabled. */ void ready_list_reinsert (struct thread *t) { - enum intr_level old_level = intr_disable (); + ASSERT (intr_get_level () == INTR_OFF); /* If the thread isn't ready to run, do nothing. */ - if (t->status == THREAD_READY) - { - list_remove (&t->elem); - list_insert_ordered (&ready_list, &t->elem, priority_more, NULL); - } + if (t->status != THREAD_READY) + return; - intr_set_level (old_level); + list_remove (&t->elem); + list_insert_ordered (&ready_list, &t->elem, priority_more, NULL); } /* Idle thread. Executes when no other thread is ready to run. -- 2.49.1 From 5f8dea21befb9bf5688383796eb510770ddb2d0b Mon Sep 17 00:00:00 2001 From: Themis Demetriades Date: Wed, 23 Oct 2024 14:10:48 +0100 Subject: [PATCH 4/4] Fix donate_priority to disable interrupts for entire update of possibly-ready donatee's priority --- src/threads/synch.c | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/src/threads/synch.c b/src/threads/synch.c index 7dc6eaa..b99332e 100644 --- a/src/threads/synch.c +++ b/src/threads/synch.c @@ -210,23 +210,25 @@ donate_priority (struct thread *donee) { if (donor->priority <= donee->priority) break; - donee->priority = donor->priority; - /* Also stop propagation of donation once a donee is reached with no donees of its own (sink node in WFG). */ if (donee->waiting_lock == NULL) { - /* Only the sink node of the WFG isn't waiting for a lock and could be on the ready list. Thus, as its priority changed, it must be reinserted into the list. */ enum intr_level old_level = intr_disable (); + donee->priority = donor->priority; ready_list_reinsert (donee); intr_set_level (old_level); + donee = NULL; } else - donee = donee->waiting_lock->holder; + { + donee->priority = donor->priority; + donee = donee->waiting_lock->holder; + } } } -- 2.49.1