diff mbox

MARS: Remove api argument params structs

Message ID 48C972DF.4080409@am.sony.com
State New
Delegated to: Yuji Mano
Headers show

Commit Message

Yuji Mano Sept. 11, 2008, 7:34 p.m. UTC 
This removes the mars_params and mars_task_params structure previously passed
into the mars context and mars task APIs.

Parameter arguments are now passed in directly.

Signed-off-by: Yuji Mano <yuji.mano@am.sony.com>

Comments

Kazunori Asayama Sept. 12, 2008, 8:28 a.m. UTC | #1 
Yuji Mano wrote:
> This removes the mars_params and mars_task_params structure previously passed
> into the mars context and mars task APIs.
> 
> Parameter arguments are now passed in directly.
> 
> Signed-off-by: Yuji Mano <yuji.mano@am.sony.com>
> 
> ---
>  include/host/mars/mars_context.h |   23 +++-----------------
>  include/host/mars/mars_task.h    |   32 +++++++---------------------
>  src/host/lib/mars_context.c      |   44 +++++++++++++++------------------------
>  src/host/lib/mars_task.c         |   34 +++++++++++++-----------------
>  4 files changed, 46 insertions(+), 87 deletions(-)
> 

(snip)

> -int mars_initialize(struct mars_context *mars, struct mars_params *params)
> +int mars_initialize(struct mars_context *mars, uint32_t num_mpus)
>  {
>  	MARS_CHECK_RET(mars, MARS_ERROR_NULL);
> +	MARS_CHECK_RET(num_mpus_max() > 0, MARS_ERROR_INTERNAL);
> +	MARS_CHECK_RET(num_mpus_max() >= num_mpus, MARS_ERROR_PARAMS);

The num_mpus_max() is called repeatedly in this function. Instead, I
think the return value should be cached. (The spe_cpu_info_get() checks
sysfs, so it's not lightweight.)
diff mbox

Patch

--- a/include/host/mars/mars_context.h
+++ b/include/host/mars/mars_context.h
@@ -44,32 +44,17 @@ 
  * \brief [host] MARS Context API
  */
 
+#include <stdint.h>
 #include <pthread.h>
 
 /**
  * \ingroup group_mars_context
- * \brief MARS params structure
- *
- * This structure is initialized by the user and passed into
- * \ref mars_initialize for MARS context initialization.
- */
-struct mars_params {
-	/** number of mpus utilized by each MARS context
-	 * [between 0 and maximum available physical MPUs]
-	 */
-	unsigned int num_mpus;
-};
-
-/**
- * \ingroup group_mars_context
  * \brief MARS context structure
  *
  * An instance of this structure must be created and initialized before
  * using any of the MARS API.
  */
 struct mars_context {
-	/* parameters for the MARS context */
-	struct mars_params *params;
 	/* parameters for the MARS kernel */
 	struct mars_kernel_params *kernel_params;
 	/* process queue where process requests are added */
@@ -77,7 +62,7 @@  struct mars_context {
 	/* array of mpu context threads */
 	pthread_t *mpu_context_threads;
 	/* num of mpu context threads */
-	unsigned int mpu_context_count;
+	uint32_t mpu_context_count;
 };
 
 #if defined(__cplusplus)
@@ -98,7 +83,7 @@  extern "C" {
  * each MARS context will suffer the large over head of MPU context switches.
  *
  * \param[out] ctx		- address of context instance to initialize
- * \param[in] params		- pointer to MARS parameters structure
+ * \param[in] num_mpus		- number of mpus utilized by MARS context
  * \return
  *	MARS_SUCCESS		- successfully initialized MARS context
  * \n	MARS_ERROR_NULL		- null pointer specified
@@ -106,7 +91,7 @@  extern "C" {
  * \n	MARS_ERROR_MEMORY	- not enough memory
  * \n	MARS_ERROR_INTERNAL	- some internal error occurred
  */
-int mars_initialize(struct mars_context *ctx, struct mars_params *params);
+int mars_initialize(struct mars_context *ctx, uint32_t num_mpus);
 
 /**
  * \ingroup group_mars_context
--- a/include/host/mars/mars_task.h
+++ b/include/host/mars/mars_task.h
@@ -55,24 +55,6 @@  extern "C" {
 #include "mars/mars_task_signal.h"
 #include "mars/mars_task_types.h"
 
-struct mars_context;
-struct mars_exec_context;
-
-/**
- * \ingroup group_mars_task
- * \brief MARS task params structure
- *
- * This structure is initialized by the user and passed into
- * \ref mars_task_initialize for MARS task initialization.
- */
-struct mars_task_params {
-	/** name of task */
-	const char *name;
-	/** address of MPU program elf image */
-	const void *elf_image;
-	/** size of context save area allocated for task */
-	uint32_t context_save_size;
-};
 
 /**
  * \ingroup group_mars_task
@@ -87,8 +69,7 @@  struct mars_task_params {
  * The task is in the finished state upon creation and may be finalized by
  * \ref mars_task_finalize without ever being scheduled for execution.
  *
- * When initializing a MARS task, you must specify its parameters through the
- * \ref mars_task_params structure:
+ * When initializing a MARS task, you must specify its parameters:
  * \n
  * - \e name: The name is optional, but if specified to other than NULL its
  *	length must not exceed \ref MARS_TASK_NAME_LEN_MAX.
@@ -107,7 +88,9 @@  struct mars_task_params {
  *
  * \param[in] mars		- pointer to initialized MARS context
  * \param[out] id		- pointer to task id to be initialized
- * \param[in] params		- pointer to task params of this task
+ * \param[in] name		- name of task
+ * \param[in] elf_image		- address of MPU program elf image
+ * \param[in] context_save_size - size of context save area allocated for task
  * \return
  *	MARS_SUCCESS		- successfully initialized MARS task
  * \n	MARS_ERROR_NULL		- null pointer specified
@@ -115,8 +98,11 @@  struct mars_task_params {
  * \n	MARS_ERROR_MEMORY	- not enough memory
  * \n	MARS_ERROR_LIMIT	- task queue is currently full
  */
-int mars_task_initialize(struct mars_context *mars, struct mars_task_id *id,
-			struct mars_task_params *params);
+int mars_task_initialize(struct mars_context *mars,
+			struct mars_task_id *id,
+			const char *name,
+			const void *elf_image,
+			uint32_t context_save_size);
 
 /**
  * \ingroup group_mars_task
--- a/src/host/lib/mars_context.c
+++ b/src/host/lib/mars_context.c
@@ -48,9 +48,14 @@ 
 
 extern struct spe_program_handle mars_kernel_entry;
 
-static int num_mpus_max(void)
+static uint32_t num_mpus_max(void)
 {
-	return spe_cpu_info_get(SPE_COUNT_PHYSICAL_SPES, -1);
+	int ret;
+
+	ret = spe_cpu_info_get(SPE_COUNT_PHYSICAL_SPES, -1);
+	MARS_CHECK_RET(ret >= 0, 0);
+
+	return ret;
 }
 
 static void *mpu_context_thread(void *arg)
@@ -77,7 +82,7 @@  static void *mpu_context_thread(void *ar
 	return (void *)MARS_SUCCESS;
 }
 
-static int create_mpu_contexts(struct mars_context *mars, int num_mpu_contexts)
+static int create_mpu_contexts(struct mars_context *mars, uint32_t num_mpus)
 {
 	MARS_ASSERT(mars);
 
@@ -85,7 +90,7 @@  static int create_mpu_contexts(struct ma
 	unsigned int i;
 
 	/* create threads for each mpu context */
-	for (i = 0; i < num_mpu_contexts; i++) {
+	for (i = 0; i < num_mpus; i++) {
 		struct mars_kernel_params *params = &mars->kernel_params[i];
 		params->id = i;
 		params->mars_context_ea =
@@ -120,39 +125,27 @@  static int destroy_mpu_contexts(struct m
 	return MARS_SUCCESS;
 }
 
-int mars_initialize(struct mars_context *mars, struct mars_params *params)
+int mars_initialize(struct mars_context *mars, uint32_t num_mpus)
 {
 	MARS_CHECK_RET(mars, MARS_ERROR_NULL);
+	MARS_CHECK_RET(num_mpus_max() > 0, MARS_ERROR_INTERNAL);
+	MARS_CHECK_RET(num_mpus_max() >= num_mpus, MARS_ERROR_PARAMS);
 
 	MARS_PRINT("Initialize MARS Context (%p)\n", mars);
 
 	int ret;
-	unsigned int num_mpu_contexts;
 
 	/* zero context */
 	memset(mars, 0, sizeof(struct mars_context));
 
-	/* allocate and initialize params */
-	if (params) {
-		MARS_CHECK_RET(params->num_mpus <= num_mpus_max(),
-				MARS_ERROR_PARAMS);
-
-		mars->params = (struct mars_params *)
-			malloc(sizeof(struct mars_params));
-		MARS_CHECK_RET(mars->params, MARS_ERROR_MEMORY);
-		memcpy(mars->params, params, sizeof(struct mars_params));
-
-		num_mpu_contexts = mars->params->num_mpus;
-	} else {
-		mars->params = NULL;
-
-		num_mpu_contexts = num_mpus_max();
-	}
+	/* num mpus specified is default max */
+	if (!num_mpus)
+		num_mpus = num_mpus_max();
 
 	/* allocate kernel params */
 	mars->kernel_params = (struct mars_kernel_params *)
 		memalign(MARS_KERNEL_PARAMS_ALIGN,
-			sizeof(struct mars_kernel_params) * num_mpu_contexts);
+			sizeof(struct mars_kernel_params) * num_mpus);
 	MARS_CHECK_CLEANUP_RET(mars->kernel_params, mars_finalize(mars),
 				MARS_ERROR_MEMORY);
 
@@ -169,12 +162,12 @@  int mars_initialize(struct mars_context 
 
 	/* allocate mpu context thread array */
 	mars->mpu_context_threads = (pthread_t *)
-		malloc(sizeof(pthread_t) * num_mpu_contexts);
+		malloc(sizeof(pthread_t) * num_mpus);
 	MARS_CHECK_CLEANUP_RET(mars->mpu_context_threads, mars_finalize(mars),
 				MARS_ERROR_MEMORY);
 
 	/* create contexts */
-	ret = create_mpu_contexts(mars, num_mpu_contexts);
+	ret = create_mpu_contexts(mars, num_mpus);
 	MARS_CHECK_CLEANUP_RET(ret == MARS_SUCCESS, mars_finalize(mars), ret);
 
 	return MARS_SUCCESS;
@@ -199,7 +192,6 @@  int mars_finalize(struct mars_context *m
 	}
 
 	/* free allocated memory */
-	free(mars->params);
 	free(mars->kernel_params);
 	free(mars->workload_queue);
 	free(mars->mpu_context_threads);
--- a/src/host/lib/mars_task.c
+++ b/src/host/lib/mars_task.c
@@ -45,17 +45,23 @@ 
 #include "mars/mars_error.h"
 #include "mars/mars_debug.h"
 
-int mars_task_initialize(struct mars_context *mars, struct mars_task_id *id,
-			struct mars_task_params *params)
+int mars_task_initialize(struct mars_context *mars,
+			struct mars_task_id *id,
+			const char *name,
+			const void *elf_image,
+			uint32_t context_save_size)
 {
 	MARS_CHECK_RET(mars, MARS_ERROR_NULL);
 	MARS_CHECK_RET(id, MARS_ERROR_NULL);
-	MARS_CHECK_RET(params, MARS_ERROR_NULL);
-	MARS_CHECK_RET(params->elf_image, MARS_ERROR_PARAMS);
+	MARS_CHECK_RET(elf_image, MARS_ERROR_NULL);
+	MARS_CHECK_RET(!name || strlen(name) < MARS_TASK_NAME_LEN_MAX,
+			MARS_ERROR_PARAMS);
+	MARS_CHECK_RET(context_save_size <= MARS_TASK_CONTEXT_SAVE_SIZE_MAX,
+			MARS_ERROR_PARAMS);
 
 	int ret;
 	struct mars_task_context *task;
-	Elf32_Ehdr *ehdr = (Elf32_Ehdr *)params->elf_image;
+	Elf32_Ehdr *ehdr = (Elf32_Ehdr *)elf_image;
 	Elf32_Phdr *phdr = (Elf32_Phdr *)((void *)ehdr + ehdr->e_phoff);
 
 	//MARS_CHECK_RET(ehdr->e_phnum == 1, MARS_ERROR_FORMAT);
@@ -67,11 +73,8 @@  int mars_task_initialize(struct mars_con
 	id->mars_context_ea = (uint64_t)(uintptr_t)mars;
 
 	/* copy the task name into task id */
-	if (params->name) {
-		MARS_CHECK_RET(strlen(params->name) < MARS_TASK_NAME_LEN_MAX,
-				MARS_ERROR_PARAMS);
-		strcpy((char *)&id->name, params->name);
-	}
+	if (name)
+		strcpy((char *)&id->name, name);
 
 	/* begin process to add the task to the workload queue */
 	ret = workload_queue_add_begin(mars->workload_queue, &id->workload_id,
@@ -90,15 +93,8 @@  int mars_task_initialize(struct mars_con
 	task->entry = ehdr->e_entry;
 
 	/* allocate the task context area if specified */
-	if (params->context_save_size) {
-		MARS_CHECK_CLEANUP_RET(params->context_save_size <=
-			MARS_TASK_CONTEXT_SAVE_SIZE_MAX,
-			workload_queue_add_cancel(mars->workload_queue,
-				id->workload_id),
-			MARS_ERROR_PARAMS);
-
-		task->context_save_size =
-			params->context_save_size;
+	if (context_save_size) {
+		task->context_save_size = context_save_size;
 		task->context_save_area = (uint64_t)(uintptr_t)
 			memalign(MARS_TASK_CONTEXT_SAVE_ALIGN,
 				task->context_save_size);