Merge pull request #49 from phanikishoreg/refactor-sandbox_alloc

feat: Improved Sandbox Alloc

runtime/include/arch/x86_64/context.h

@@ -120,6 +120,9 @@ arch_mcontext_save(struct arch_context *ctx, mcontext_t *mc)
 static inline int
 arch_context_switch(struct arch_context *current, struct arch_context *next)
 {
+	/* Assumption: Software Interrupts are disabled by caller */
+	assert(software_interrupt_is_disabled);
+
 	/* if both current and next are NULL, there is no state change */
 	assert(current != NULL || next != NULL);
 

runtime/include/global_request_scheduler.h

@@ -14,8 +14,7 @@ struct global_request_scheduler_config {
 };
 
 
-void global_request_scheduler_initialize(struct global_request_scheduler_config *config);
-
+void                    global_request_scheduler_initialize(struct global_request_scheduler_config *config);
 struct sandbox_request *global_request_scheduler_add(struct sandbox_request *);
 int                     global_request_scheduler_remove(struct sandbox_request **);
 uint64_t                global_request_scheduler_peek();

runtime/include/module.h

@@ -2,6 +2,8 @@
 
 #include <uv.h>
 
+#include "panic.h"
+#include "software_interrupt.h"
 #include "types.h"
 
 struct module {
@@ -127,12 +129,21 @@ module_initialize_memory(struct module *module)
 /**
  * Validate module, defined as having a non-NULL dynamical library handle and entry function pointer
  * @param module - module to validate
- * @return true if valid. false if invalid
  */
-static inline bool
-module_is_valid(struct module *module)
+static inline void
+module_validate(struct module *module)
 {
-	return (module && module->dynamic_library_handle && module->main);
+	/* Assumption: Software Interrupts are disabled by caller */
+	assert(!software_interrupt_is_enabled());
+
+	if (!module) {
+		panic("%lu | module %p | module is unexpectedly NULL\n", pthread_self(), module);
+	} else if (!module->dynamic_library_handle) {
+		panic("%lu | module %p | module->dynamic_library_handle is unexpectedly NULL\n", pthread_self(),
+		      module);
+	} else if (!module->main) {
+		panic("%lu | module %p | module->main is unexpectedly NULL\n", pthread_self(), module);
+	}
 }
 
 /**

runtime/include/panic.h

@@ -1,5 +1,6 @@
 #pragma once
 
+#include <assert.h>
 #include <stdio.h>
 #include <stdarg.h>
 #include <stdlib.h>

runtime/include/sandbox.h

@@ -97,7 +97,6 @@ void            sandbox_free(struct sandbox *sandbox);
 void            sandbox_main(struct sandbox *sandbox);
 int             sandbox_parse_http_request(struct sandbox *sandbox, size_t length);
 
-
 /**
  * Given a sandbox, returns the module that sandbox is executing
  * @param sandbox the sandbox whose module we want

runtime/include/software_interrupt.h

@@ -1,6 +1,7 @@
 #pragma once
 
 #include <assert.h>
+#include <panic.h>
 #include <signal.h>
 #include <stdbool.h>
 #include <stdio.h>
@@ -31,7 +32,9 @@ software_interrupt_disable(void)
 static inline void
 software_interrupt_enable(void)
 {
-	if (__sync_bool_compare_and_swap(&software_interrupt_is_disabled, 1, 0) == false) assert(0);
+	if (__sync_bool_compare_and_swap(&software_interrupt_is_disabled, 1, 0) == false) {
+		panic("Recursive call to software_interrupt_enable\n");
+	}
 }
 
 /**

runtime/src/global_request_scheduler.c

@@ -33,6 +33,7 @@ static struct global_request_scheduler_config global_request_scheduler = { .add_
 void
 global_request_scheduler_initialize(struct global_request_scheduler_config *config)
 {
+	assert(config != NULL);
 	memcpy(&global_request_scheduler, config, sizeof(struct global_request_scheduler_config));
 }
 
@@ -44,6 +45,7 @@ global_request_scheduler_initialize(struct global_request_scheduler_config *conf
 struct sandbox_request *
 global_request_scheduler_add(struct sandbox_request *sandbox_request)
 {
+	assert(sandbox_request != NULL);
 	return global_request_scheduler.add_fn(sandbox_request);
 }
 
@@ -55,6 +57,7 @@ global_request_scheduler_add(struct sandbox_request *sandbox_request)
 int
 global_request_scheduler_remove(struct sandbox_request **removed_sandbox)
 {
+	assert(removed_sandbox != NULL);
 	return global_request_scheduler.remove_fn(removed_sandbox);
 }
 

runtime/src/local_runqueue_list.c

@@ -43,22 +43,27 @@ local_runqueue_list_remove_and_return()
 struct sandbox *
 local_runqueue_list_get_next()
 {
+	struct sandbox_request *sandbox_request;
+
 	// If our local runqueue is empty, try to pull and allocate a sandbox request from the global request scheduler
 	if (local_runqueue_is_empty()) {
-		struct sandbox_request *sandbox_request;
-
-		int return_code = global_request_scheduler_remove(&sandbox_request);
-		if (return_code != 0) return NULL;
+		if (global_request_scheduler_remove(&sandbox_request) < 0) goto err;
 
-		/* TODO: sandbox_allocate should free sandbox_request on success */
-		/* TODO: sandbox_allocate should return RC so we can readd sandbox_request to global_request_scheduler
-		 * if needed */
 		struct sandbox *sandbox = sandbox_allocate(sandbox_request);
-		assert(sandbox);
-		free(sandbox_request);
+		if (!sandbox) goto sandbox_allocate_err;
+
 		sandbox->state = SANDBOX_RUNNABLE;
 		local_runqueue_add(sandbox);
+
+	done:
 		return sandbox;
+	sandbox_allocate_err:
+		debuglog("local_runqueue_list_get_next failed to allocate sandbox, returning request to global request "
+		         "scheduler\n");
+		global_request_scheduler_add(sandbox_request);
+	err:
+		sandbox = NULL;
+		goto done;
 	}
 
 	/* Execute Round Robin Scheduling Logic */
@@ -84,7 +89,8 @@ local_runqueue_list_append(struct sandbox *sandbox_to_append)
 }
 
 /**
- * Conditionally checks to see if current sandbox should be preempted FIFO doesn't preempt, so just return.
+ * Conditionally checks to see if current sandbox should be preempted.
+ * FIFO doesn't preempt, so just return.
  */
 void
 local_runqueue_list_preempt(ucontext_t *user_context)

runtime/src/local_runqueue_minheap.c

@@ -6,6 +6,7 @@
 #include "local_runqueue_minheap.h"
 #include "panic.h"
 #include "priority_queue.h"
+#include "software_interrupt.h"
 
 
 __thread static struct priority_queue local_runqueue_minheap;
@@ -70,8 +71,12 @@ local_runqueue_minheap_delete(struct sandbox *sandbox)
 struct sandbox *
 local_runqueue_minheap_get_next()
 {
-	struct sandbox *sandbox    = NULL;
-	int             sandbox_rc = local_runqueue_minheap_remove(&sandbox);
+	/* Assumption: Software Interrupts are disabed by caller */
+	assert(!software_interrupt_is_enabled());
+
+	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 */
@@ -82,20 +87,27 @@ local_runqueue_minheap_get_next()
 		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 */
-		struct sandbox_request *sandbox_request;
-		int                     sandbox_request_rc = global_request_scheduler_remove(&sandbox_request);
-		if (sandbox_request_rc != 0) return NULL;
+		if (global_request_scheduler_remove(&sandbox_request) < 0) goto err;
 
+		/* Try to allocate a sandbox, returning the request on failure */
 		sandbox = sandbox_allocate(sandbox_request);
-		assert(sandbox);
-		free(sandbox_request);
+		if (!sandbox) goto sandbox_allocate_err;
+
 		sandbox->state = SANDBOX_RUNNABLE;
 		local_runqueue_minheap_add(sandbox);
 	} 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:
+	debuglog("local_runqueue_minheap_get_next failed to allocating sandbox. Readding request to global "
+	         "request scheduler\n");
+	global_request_scheduler_add(sandbox_request);
+err:
+	sandbox = NULL;
+	goto done;
 }
 
 
@@ -109,7 +121,8 @@ local_runqueue_minheap_preempt(ucontext_t *user_context)
 {
 	assert(user_context != NULL);
 
-	software_interrupt_disable(); /* no nesting! */
+	/* Prevent nested preemption */
+	software_interrupt_disable();
 
 	struct sandbox *current_sandbox = current_sandbox_get();
 
@@ -126,37 +139,27 @@ local_runqueue_minheap_preempt(ucontext_t *user_context)
 	uint64_t local_deadline                   = priority_queue_peek(&local_runqueue_minheap);
 	uint64_t global_deadline                  = global_request_scheduler_peek();
 
-	/* Our local deadline should only be ULONG_MAX if our local runqueue is empty */
-	if (local_deadline == ULONG_MAX) { assert(local_runqueue_minheap.first_free == 1); };
-
-	/*
-	 * If we're able to get a sandbox request with a tighter deadline, preempt the current context and run it
-	 *
-	 * TODO: Factor quantum and/or sandbox allocation time into decision
-	 * Something like global_request_scheduler_peek() - software_interrupt_interval_duration_in_cycles;
-	 */
-
+	/* If we're able to get a sandbox request with a tighter deadline, preempt the current context and run it */
+	struct sandbox_request *sandbox_request;
 	if (global_deadline < local_deadline) {
 		debuglog("Thread %lu | Sandbox %lu | Had deadline of %lu. Trying to preempt for request with %lu\n",
 		         pthread_self(), current_sandbox->allocation_timestamp, local_deadline, global_deadline);
 
-		struct sandbox_request *sandbox_request;
-		int                     return_code = global_request_scheduler_remove(&sandbox_request);
+		int return_code = global_request_scheduler_remove(&sandbox_request);
 
-		// If we were unable to get a sandbox_request, exit
+		/* If we were unable to get a sandbox_request, exit */
 		if (return_code != 0) goto done;
 
 		debuglog("Thread %lu Preempted %lu for %lu\n", pthread_self(), local_deadline,
 		         sandbox_request->absolute_deadline);
+
 		/* Allocate the request */
 		struct sandbox *next_sandbox = sandbox_allocate(sandbox_request);
-		assert(next_sandbox);
-		free(sandbox_request);
-		next_sandbox->state = SANDBOX_RUNNABLE;
+		if (!next_sandbox) goto err_sandbox_allocate;
 
-		/* Add it to the runqueue */
+		/* Set as runnable and add it to the runqueue */
+		next_sandbox->state = SANDBOX_RUNNABLE;
 		local_runqueue_add(next_sandbox);
-		debuglog("[%p: %s]\n", sandbox, sandbox->module->name);
 
 		/* Save the context of the currently executing sandbox before switching from it */
 		arch_mcontext_save(&current_sandbox->ctxt, &user_context->uc_mcontext);
@@ -174,6 +177,14 @@ local_runqueue_minheap_preempt(ucontext_t *user_context)
 	}
 done:
 	if (should_enable_software_interrupt) software_interrupt_enable();
+	return;
+err_sandbox_allocate:
+	assert(sandbox_request);
+	debuglog("local_runqueue_minheap_preempt failed to allocate sandbox, returning request to global request "
+	         "scheduler\n");
+	global_request_scheduler_add(sandbox_request);
+err:
+	goto done;
 }
 
 

runtime/src/sandbox.c

@@ -380,19 +380,27 @@ err_stack_allocation_failed:
 	return -1;
 }
 
+/**
+ * Allocates a new sandbox from a sandbox request
+ * Frees the sandbox request on success
+ * @param sandbox_request request being allocated
+ * @returns sandbox * on success, NULL on error
+ */
 struct sandbox *
 sandbox_allocate(struct sandbox_request *sandbox_request)
 {
+	/* Assumption: Caller has disabled software interrupts */
+	assert(!software_interrupt_is_enabled());
+
+	/* Validate Arguments */
 	assert(sandbox_request != NULL);
-	assert(sandbox_request->module != NULL);
-	assert(module_is_valid(sandbox_request->module));
+	module_validate(sandbox_request->module);
 
+	struct sandbox *sandbox;
 	char *          error_message = "";
-	int             rc;
-	struct sandbox *sandbox = NULL;
 
 	/* Allocate Sandbox control structures, buffers, and linear memory in a 4GB address space */
-	sandbox = (struct sandbox *)sandbox_allocate_memory(sandbox_request->module);
+	sandbox = sandbox_allocate_memory(sandbox_request->module);
 	if (!sandbox) {
 		error_message = "failed to allocate sandbox heap and linear memory";
 		goto err_memory_allocation_failed;
@@ -402,8 +410,7 @@ sandbox_allocate(struct sandbox_request *sandbox_request)
 	sandbox->state = SANDBOX_INITIALIZING;
 
 	/* Allocate the Stack */
-	rc = sandbox_allocate_stack(sandbox);
-	if (rc != 0) {
+	if (sandbox_allocate_stack(sandbox) == -1) {
 		error_message = "failed to allocate sandbox heap and linear memory";
 		goto err_stack_allocation_failed;
 	}
@@ -429,6 +436,7 @@ sandbox_allocate(struct sandbox_request *sandbox_request)
 	/* Initialize Parsec control structures (used by Completion Queue) */
 	ps_list_init_d(sandbox);
 
+	free(sandbox_request);
 done:
 	return sandbox;
 err_stack_allocation_failed: