From f963828db7faafde893e58d60d5c34139d1a1a85 Mon Sep 17 00:00:00 2001 From: Sean McBride Date: Wed, 29 Jul 2020 15:07:36 -0400 Subject: [PATCH] chore: general pq cleanup --- runtime/include/priority_queue.h | 1 + runtime/src/global_request_scheduler.c | 2 +- runtime/src/global_request_scheduler_deque.c | 12 +---- .../src/global_request_scheduler_minheap.c | 4 +- runtime/src/local_runqueue_minheap.c | 31 +++++------- runtime/src/priority_queue.c | 49 ++++++++++++++++--- 6 files changed, 60 insertions(+), 39 deletions(-) diff --git a/runtime/include/priority_queue.h b/runtime/include/priority_queue.h index 2cc4ef2..534196a 100644 --- a/runtime/include/priority_queue.h +++ b/runtime/include/priority_queue.h @@ -41,5 +41,6 @@ int priority_queue_dequeue(struct priority_queue *self, void **dequeued_ele int priority_queue_length(struct priority_queue *self); uint64_t priority_queue_peek(struct priority_queue *self); int priority_queue_delete(struct priority_queue *self, void *value); +int priority_queue_top(struct priority_queue *self, void **dequeued_element); #endif /* PRIORITY_QUEUE_H */ diff --git a/runtime/src/global_request_scheduler.c b/runtime/src/global_request_scheduler.c index b83fea4..b0b95e9 100644 --- a/runtime/src/global_request_scheduler.c +++ b/runtime/src/global_request_scheduler.c @@ -52,7 +52,7 @@ global_request_scheduler_add(struct sandbox_request *sandbox_request) /** * Removes a sandbox request according to the scheduling policy of the variant * @param removed_sandbox where to write the adddress of the removed sandbox - * @returns 0 if successful, -1 if empty, -2 if unable to take lock or perform atomic operation + * @returns 0 if successfully returned a sandbox request, -ENOENT if empty, -EAGAIN if atomic operation unsuccessful */ int global_request_scheduler_remove(struct sandbox_request **removed_sandbox) diff --git a/runtime/src/global_request_scheduler_deque.c b/runtime/src/global_request_scheduler_deque.c index b202bc2..ec718fc 100644 --- a/runtime/src/global_request_scheduler_deque.c +++ b/runtime/src/global_request_scheduler_deque.c @@ -28,20 +28,12 @@ global_request_scheduler_deque_add(void *sandbox_request_raw) * * Relevant Read: https://www.dre.vanderbilt.edu/~schmidt/PDF/work-stealing-dequeue.pdf * - * @returns 0 if successfully returned a sandbox request, -1 if empty, -2 if atomic instruction unsuccessful + * @returns 0 if successfully returned a sandbox request, -ENOENT if empty, -EAGAIN if atomic instruction unsuccessful */ static int global_request_scheduler_deque_remove(struct sandbox_request **removed_sandbox_request) { - int return_code; - return_code = deque_steal_sandbox(global_request_scheduler_deque, removed_sandbox_request); - /* The Deque uses different return codes other than 0, so map here */ - if (return_code == -2) { - return_code = -1; - } else if (return_code == -11) { - return_code = -2; - } - return return_code; + return deque_steal_sandbox(global_request_scheduler_deque, removed_sandbox_request); } void diff --git a/runtime/src/global_request_scheduler_minheap.c b/runtime/src/global_request_scheduler_minheap.c index c8cc628..6a96979 100644 --- a/runtime/src/global_request_scheduler_minheap.c +++ b/runtime/src/global_request_scheduler_minheap.c @@ -22,13 +22,13 @@ global_request_scheduler_minheap_add(void *sandbox_request) int return_code = priority_queue_enqueue(&global_request_scheduler_minheap, sandbox_request); /* TODO: Propagate -1 to caller */ - if (return_code == -1) panic("Request Queue is full\n"); + if (return_code == -ENOSPC) panic("Request Queue is full\n"); return sandbox_request; } /** * @param pointer to the pointer that we want to set to the address of the removed sandbox request - * @returns 0 if successful, -1 if empty, -2 if unable to take lock or perform atomic operation + * @returns 0 if successful, -ENOENT if empty, -EAGAIN if unable to take lock */ int global_request_scheduler_minheap_remove(struct sandbox_request **removed_sandbox_request) diff --git a/runtime/src/local_runqueue_minheap.c b/runtime/src/local_runqueue_minheap.c index 2c7e336..fdd767d 100644 --- a/runtime/src/local_runqueue_minheap.c +++ b/runtime/src/local_runqueue_minheap.c @@ -9,7 +9,6 @@ #include "priority_queue.h" #include "software_interrupt.h" - __thread static struct priority_queue local_runqueue_minheap; /** @@ -34,20 +33,22 @@ local_runqueue_minheap_add(struct sandbox *sandbox) int return_code = priority_queue_enqueue(&local_runqueue_minheap, sandbox); /* TODO: propagate RC to caller */ - if (return_code == -1) panic("Thread Runqueue is full!\n"); + if (return_code == -ENOSPC) panic("Thread Runqueue is full!\n"); } /** * Removes the highest priority sandbox from the run queue * @param pointer to test to address of removed sandbox if successful - * @returns 0 if successful, -1 if empty, -2 if unable to take lock + * @returns RC 0 if successfully set dequeued_element, -ENOENT if empty */ static int local_runqueue_minheap_remove(struct sandbox **to_remove) { assert(!software_interrupt_is_enabled()); + int rc = priority_queue_dequeue(&local_runqueue_minheap, (void **)to_remove); + if (rc == -EAGAIN) panic("Worker unexpectedly unable to take lock on own runqueue\n"); - return priority_queue_dequeue(&local_runqueue_minheap, (void **)to_remove); + return rc; } /** @@ -82,18 +83,13 @@ local_runqueue_minheap_get_next() struct sandbox * sandbox = NULL; struct sandbox_request *sandbox_request; - int sandbox_rc = local_runqueue_minheap_remove(&sandbox); - - if (sandbox_rc == 0) { - /* Sandbox ready pulled from local runqueue */ + int sandbox_rc = priority_queue_top(&local_runqueue_minheap, (void **)&sandbox); - /* TODO: We remove and immediately re-add sandboxes. This seems like extra work. Consider an API to get - * the min without removing it - */ - local_runqueue_minheap_add(sandbox); - } else if (sandbox_rc == -1) { - /* local runqueue was empty, try to pull a sandbox request and return NULL if we're unable to get one */ - if (global_request_scheduler_remove(&sandbox_request) < 0) goto err; + if (sandbox_rc == -EAGAIN) { + panic("Worker unexpectedly unable to take lock on own runqueue\n"); + } else if (sandbox_rc == -ENOENT) { + /* local runqueue empty, try to pull a sandbox request */ + if (global_request_scheduler_remove(&sandbox_request) < 0) goto done; /* Try to allocate a sandbox, returning the request on failure */ sandbox = sandbox_allocate(sandbox_request); @@ -101,11 +97,8 @@ local_runqueue_minheap_get_next() assert(sandbox->state == SANDBOX_INITIALIZED); sandbox_set_as_runnable(sandbox, SANDBOX_INITIALIZED); - - } else if (sandbox_rc == -2) { - /* Unable to take lock, so just return NULL and try later */ - assert(sandbox == NULL); } + done: return sandbox; sandbox_allocate_err: diff --git a/runtime/src/priority_queue.c b/runtime/src/priority_queue.c index d04179e..6565a43 100644 --- a/runtime/src/priority_queue.c +++ b/runtime/src/priority_queue.c @@ -1,4 +1,5 @@ #include +#include #include #include #include @@ -15,7 +16,7 @@ * Adds a value to the end of the binary heap * @param self the priority queue * @param new_item the value we are adding - * @return 0 on success. -1 when priority queue is full + * @return 0 on success. -ENOSPC when priority queue is full */ static inline int priority_queue_append(struct priority_queue *self, void *new_item) @@ -23,7 +24,7 @@ priority_queue_append(struct priority_queue *self, void *new_item) assert(self != NULL); assert(ck_spinlock_fas_locked(&self->lock)); - if (self->first_free >= MAX) return -1; + if (self->first_free >= MAX) return -ENOSPC; self->items[self->first_free++] = new_item; return 0; @@ -165,7 +166,7 @@ priority_queue_length(struct priority_queue *self) /** * @param self - the priority queue we want to add to * @param value - the value we want to add - * @returns 0 on success. -1 on full. + * @returns 0 on success. -ENOSPC on full. */ int priority_queue_enqueue(struct priority_queue *self, void *value) @@ -173,7 +174,7 @@ priority_queue_enqueue(struct priority_queue *self, void *value) assert(self != NULL); ck_spinlock_fas_lock(&self->lock); - if (priority_queue_append(self, value) == -1) return -1; + if (priority_queue_append(self, value) == -ENOSPC) return -ENOSPC; /* If this is the first element we add, update the highest priority */ if (self->first_free == 2) { @@ -213,7 +214,7 @@ priority_queue_delete(struct priority_queue *self, void *value) /** * @param self - the priority queue we want to add to * @param dequeued_element a pointer to set to the dequeued element - * @returns RC 0 if successfully set dequeued_element, -1 if empty, -2 if unable to take lock + * @returns RC 0 if successfully set dequeued_element, -ENOENT if empty, -EAGAIN if unable to take lock */ int priority_queue_dequeue(struct priority_queue *self, void **dequeued_element) @@ -225,12 +226,12 @@ priority_queue_dequeue(struct priority_queue *self, void **dequeued_element) int return_code; if (ck_spinlock_fas_trylock(&self->lock) == false) { - return_code = -2; + return_code = -EAGAIN; goto done; }; if (priority_queue_is_empty_locked(self)) { - return_code = -1; + return_code = -ENOENT; goto release_lock; } @@ -254,6 +255,40 @@ done: return return_code; } +/** + * Returns the top of the priority queue without removing it + * @param self - the priority queue we want to add to + * @param dequeued_element a pointer to set to the top element + * @returns RC 0 if successfully set dequeued_element, -ENOENT if empty, -EAGAIN if unable to take lock + */ +int +priority_queue_top(struct priority_queue *self, void **dequeued_element) +{ + assert(self != NULL); + assert(dequeued_element != NULL); + assert(self->get_priority_fn != NULL); + + int return_code; + + if (ck_spinlock_fas_trylock(&self->lock) == false) { + return_code = -EAGAIN; + goto done; + }; + + if (priority_queue_is_empty_locked(self)) { + return_code = -ENOENT; + goto release_lock; + } + + *dequeued_element = self->items[1]; + return_code = 0; + +release_lock: + ck_spinlock_fas_unlock(&self->lock); +done: + return return_code; +} + /** * Peek at the priority of the highest priority task without having to take the lock * Because this is a min-heap PQ, the highest priority is the lowest 64-bit integer