Browse Source

We now check whether the following blocking calls are called from a callback or
a codelet. If so, we return a -EDEADLK, otherwise 0 is returned.
- starpu_notify_data_modification
- starpu_prefetch_data_on_node
- starpu_sync_data_with_mem
A test was added to make sure this works correctly.

** TODO: update documentation **

Cédric Augonnet 15 years ago
parent
commit
02119279fc

+ 3 - 3
include/starpu-data.h

@@ -43,15 +43,15 @@ void starpu_delete_data(struct starpu_data_state_t *state);
 
 void starpu_advise_if_data_is_important(struct starpu_data_state_t *state, unsigned is_important);
 
-void starpu_sync_data_with_mem(struct starpu_data_state_t *state);
-void starpu_notify_data_modification(struct starpu_data_state_t *state, uint32_t modifying_node);
+int starpu_sync_data_with_mem(struct starpu_data_state_t *state);
+int starpu_notify_data_modification(struct starpu_data_state_t *state, uint32_t modifying_node);
 
 void starpu_malloc_pinned_if_possible(void **A, size_t dim);
 void starpu_free_pinned_if_possible(void *A);
 
 int starpu_request_data_allocation(struct starpu_data_state_t *state, uint32_t node);
 
-void starpu_prefetch_data_on_node(struct starpu_data_state_t *state, unsigned node, unsigned async);
+int starpu_prefetch_data_on_node(struct starpu_data_state_t *state, unsigned node, unsigned async);
 
 unsigned starpu_get_worker_memory_node(unsigned workerid);
 

+ 44 - 0
src/core/errorcheck.c

@@ -0,0 +1,44 @@
+/*
+ * StarPU
+ * Copyright (C) INRIA 2008-2009 (see AUTHORS file)
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU Lesser General Public License as published by
+ * the Free Software Foundation; either version 2.1 of the License, or (at
+ * your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful, but
+ * WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.
+ *
+ * See the GNU Lesser General Public License in COPYING.LGPL for more details.
+ */
+
+#include <core/errorcheck.h>
+#include <core/workers.h>
+
+void set_local_worker_status(worker_status st)
+{
+	struct worker_s *worker = get_local_worker_key();
+	STARPU_ASSERT(worker);
+
+	worker->status = st;
+}
+
+worker_status get_local_worker_status(void)
+{
+	struct worker_s *worker = get_local_worker_key();
+	if (STARPU_UNLIKELY(!worker))
+		return STATUS_INVALID;
+
+	return worker->status;
+}
+
+/* It is forbidden to call blocking operations with Callback and during the
+ * execution of a task. */
+unsigned worker_may_perform_blocking_calls(void)
+{
+	worker_status st = get_local_worker_status();
+
+	return ( !(st == STATUS_CALLBACK) && !(st == STATUS_EXECUTING));
+}

+ 41 - 0
src/core/errorcheck.h

@@ -0,0 +1,41 @@
+/*
+ * StarPU
+ * Copyright (C) INRIA 2008-2009 (see AUTHORS file)
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU Lesser General Public License as published by
+ * the Free Software Foundation; either version 2.1 of the License, or (at
+ * your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful, but
+ * WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.
+ *
+ * See the GNU Lesser General Public License in COPYING.LGPL for more details.
+ */
+
+#ifndef __ERRORCHECK_H__
+#define __ERRORCHECK_H__
+
+#include <starpu.h>
+
+typedef enum {
+	/* invalid status (for instance if we request the status of some thread
+	 * that is not controlled by StarPU */
+	STATUS_INVALID,
+	/* everything that does not fit the other status */
+	STATUS_UNKNOWN,
+	/* during the initialization */
+	STATUS_INITIALIZING,
+	/* during the execution of a codelet */
+	STATUS_EXECUTING,
+	/* during the execution of the callback */
+	STATUS_CALLBACK
+} worker_status;
+
+void set_local_worker_status(worker_status st);
+worker_status get_local_worker_status(void);
+
+unsigned worker_may_perform_blocking_calls(void);
+
+#endif // __ERRORCHECK_H__

+ 20 - 5
src/datawizard/user_interactions.c

@@ -65,8 +65,12 @@ static inline void _starpu_sync_data_with_mem_continuation(void *arg)
 	pthread_mutex_unlock(&statenode->lock);
 }
 
-void starpu_sync_data_with_mem(data_state *state)
+int starpu_sync_data_with_mem(data_state *state)
 {
+	/* it is forbidden to call this function from a callback or a codelet */
+	if (STARPU_UNLIKELY(!worker_may_perform_blocking_calls()))
+		return -EDEADLK;
+
 	struct state_and_node statenode =
 	{
 		.state = state,
@@ -91,6 +95,8 @@ void starpu_sync_data_with_mem(data_state *state)
 			pthread_cond_wait(&statenode.cond, &statenode.lock);
 		pthread_mutex_unlock(&statenode.lock);
 	}
+
+	return 0;
 }
 
 static inline void do_notify_data_modification(data_state *state, uint32_t modifying_node)
@@ -120,9 +126,12 @@ static inline void _notify_data_modification_continuation(void *arg)
 }
 
 /* in case the application did modify the data ... invalidate all other copies  */
-void starpu_notify_data_modification(data_state *state, uint32_t modifying_node)
+int starpu_notify_data_modification(data_state *state, uint32_t modifying_node)
 {
-	/* this may block .. XXX */
+	/* It is forbidden to call this function from a callback or a codelet */
+	if (STARPU_UNLIKELY(!worker_may_perform_blocking_calls()))
+		return -EDEADLK;
+
 	struct state_and_node statenode =
 	{
 		.state = state,
@@ -146,6 +155,8 @@ void starpu_notify_data_modification(data_state *state, uint32_t modifying_node)
 
 	/* remove the "lock"/reference */
 	notify_data_dependencies(state);
+
+	return 0;
 }
 
 static void _prefetch_data_on_node(void *arg)
@@ -164,9 +175,12 @@ static void _prefetch_data_on_node(void *arg)
 
 }
 
-void starpu_prefetch_data_on_node(data_state *state, unsigned node, unsigned async)
+int starpu_prefetch_data_on_node(data_state *state, unsigned node, unsigned async)
 {
-	/* this may block .. XXX */
+	/* it is forbidden to call this function from a callback or a codelet */
+	if (STARPU_UNLIKELY(!worker_may_perform_blocking_calls()))
+		return -EDEADLK;
+
 	struct state_and_node statenode =
 	{
 		.state = state,
@@ -193,4 +207,5 @@ void starpu_prefetch_data_on_node(data_state *state, unsigned node, unsigned asy
 		pthread_mutex_unlock(&statenode.lock);
 	}
 
+	return 0;
 }

+ 4 - 0
tests/Makefile.am

@@ -84,6 +84,7 @@ check_PROGRAMS += 				\
 	datawizard/dining_philosophers		\
 	datawizard/readers_and_writers		\
 	errorcheck/starpu_init_noworker		\
+	errorcheck/invalid_blocking_calls	\
 	helper/cublas_init			\
 	helper/pinned_memory			\
 	helper/execute_on_all			\
@@ -139,6 +140,9 @@ datawizard_readers_and_writers_SOURCES = 	\
 errorcheck_starpu_init_noworker_SOURCES =	\
 	errorcheck/starpu_init_noworker.c
 
+errorcheck_invalid_blocking_calls_SOURCES =	\
+	errorcheck/invalid_blocking_calls.c
+
 helper_cublas_init_SOURCES =			\
 	helper/cublas_init.c
 

+ 86 - 0
tests/errorcheck/invalid_blocking_calls.c

@@ -0,0 +1,86 @@
+/*
+ * StarPU
+ * Copyright (C) INRIA 2008-2009 (see AUTHORS file)
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU Lesser General Public License as published by
+ * the Free Software Foundation; either version 2.1 of the License, or (at
+ * your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful, but
+ * WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.
+ *
+ * See the GNU Lesser General Public License in COPYING.LGPL for more details.
+ */
+
+#include <starpu.h>
+
+static starpu_data_handle handle;
+static unsigned data = 42;
+
+static void wrong_func(starpu_data_interface_t *descr, void *arg)
+{
+	/* try to fetch data in the RAM while we are in a codelet, such a
+	 * blocking call is forbidden */
+	int ret = starpu_sync_data_with_mem(handle);
+	if (ret != -EDEADLK)
+		exit(-1);
+}
+
+static starpu_codelet wrong_codelet = 
+{
+	.where = CORE|CUDA,
+	.core_func = wrong_func,
+	.cuda_func = wrong_func,
+	.model = NULL,
+	.nbuffers = 0
+};
+
+static void wrong_callback(void *arg)
+{
+	int ret = starpu_sync_data_with_mem(handle);
+	if (ret != -EDEADLK)
+		exit(-1);
+}
+
+int main(int argc, char **argv)
+{
+	int ret;
+
+	starpu_init(NULL);
+
+	/* register a piece of data */
+	starpu_register_vector_data(&handle, 0, (uintptr_t)&data,
+						1, sizeof(unsigned));
+
+	struct starpu_task *task = starpu_task_create();
+
+	task->cl = &wrong_codelet;
+
+	task->buffers[0].handle = handle;
+	task->buffers[0].mode = STARPU_RW;
+
+	task->callback_func = wrong_callback;
+
+	task->synchronous = 1;
+	ret = starpu_submit_task(task);
+	if (ret == -ENODEV)
+		goto enodev;
+
+	/* This call is valid as it is done by the application outside a
+	 * callback */
+	ret = starpu_sync_data_with_mem(handle);
+	if (ret)
+		return -1;
+
+	starpu_shutdown();
+
+	return 0;
+
+enodev:
+	fprintf(stderr, "WARNING: No one can execute this task\n");
+	/* yes, we do not perform the computation but we did detect that no one
+	 * could perform the kernel, so this is not an error from StarPU */
+	return 0;
+}