Browse Source

Make a copy of the interface to the memchunk only when the latter gets detached from the data, and thus the interface code will not work on it. Drop the copy when the memchunk gets reattached. This allows interfaces to modify pointers in the interface in unpack, notably

Samuel Thibault 11 years ago
parent
commit
ea5d8f596b

+ 2 - 0
ChangeLog

@@ -87,6 +87,8 @@ Changes:
   * StarPU-MPI: Fix for being able to receive data with the same tag
   * StarPU-MPI: Fix for being able to receive data with the same tag
     from several nodes (see mpi/tests/gather.c)
     from several nodes (see mpi/tests/gather.c)
   * StarPU-MPI: Fix overzealous allocation of memory.
   * StarPU-MPI: Fix overzealous allocation of memory.
+  * Interfaces: Allow interface implementation to change pointers at will, in
+    unpack notably.
 
 
 Small changes:
 Small changes:
   * Rename function starpu_trace_user_event() as
   * Rename function starpu_trace_user_event() as

+ 1 - 1
src/datawizard/filters.c

@@ -341,7 +341,6 @@ void starpu_data_unpartition(starpu_data_handle_t root_handle, unsigned gatherin
 		_starpu_spin_lock(&child_handle->header_lock);
 		_starpu_spin_lock(&child_handle->header_lock);
 
 
 		_starpu_data_unregister_ram_pointer(child_handle);
 		_starpu_data_unregister_ram_pointer(child_handle);
-		_starpu_data_free_interfaces(child_handle);
 
 
 		for (worker = 0; worker < nworkers; worker++)
 		for (worker = 0; worker < nworkers; worker++)
 		{
 		{
@@ -424,6 +423,7 @@ void starpu_data_unpartition(starpu_data_handle_t root_handle, unsigned gatherin
 	for (child = 0; child < root_handle->nchildren; child++)
 	for (child = 0; child < root_handle->nchildren; child++)
 	{
 	{
 		starpu_data_handle_t child_handle = starpu_data_get_child(root_handle, child);
 		starpu_data_handle_t child_handle = starpu_data_get_child(root_handle, child);
+		_starpu_data_free_interfaces(child_handle);
 		_starpu_spin_unlock(&child_handle->header_lock);
 		_starpu_spin_unlock(&child_handle->header_lock);
 		_starpu_spin_destroy(&child_handle->header_lock);
 		_starpu_spin_destroy(&child_handle->header_lock);
 
 

+ 1 - 1
src/datawizard/interfaces/data_interface.c

@@ -754,7 +754,6 @@ static void _starpu_data_unregister(starpu_data_handle_t handle, unsigned cohere
 	size_t size = _starpu_data_get_size(handle);
 	size_t size = _starpu_data_get_size(handle);
 
 
 	_starpu_data_unregister_ram_pointer(handle);
 	_starpu_data_unregister_ram_pointer(handle);
-	_starpu_data_free_interfaces(handle);
 
 
 	/* Destroy the data now */
 	/* Destroy the data now */
 	unsigned node;
 	unsigned node;
@@ -774,6 +773,7 @@ static void _starpu_data_unregister(starpu_data_handle_t handle, unsigned cohere
 		if (local->allocated && local->automatically_allocated)
 		if (local->allocated && local->automatically_allocated)
 			_starpu_request_mem_chunk_removal(handle, local, starpu_worker_get_memory_node(worker), size);
 			_starpu_request_mem_chunk_removal(handle, local, starpu_worker_get_memory_node(worker), size);
 	}
 	}
+	_starpu_data_free_interfaces(handle);
 
 
 	_starpu_memory_stats_free(handle);
 	_starpu_memory_stats_free(handle);
 	_starpu_data_requester_list_delete(handle->req_list);
 	_starpu_data_requester_list_delete(handle->req_list);

+ 29 - 10
src/datawizard/memalloc.c

@@ -259,6 +259,8 @@ static size_t free_memory_on_node(struct _starpu_mem_chunk *mc, unsigned node)
 	if (mc->automatically_allocated &&
 	if (mc->automatically_allocated &&
 		(!handle || replicate->refcnt == 0))
 		(!handle || replicate->refcnt == 0))
 	{
 	{
+		void *interface;
+
 		if (handle)
 		if (handle)
 			STARPU_ASSERT(replicate->allocated);
 			STARPU_ASSERT(replicate->allocated);
 
 
@@ -273,8 +275,14 @@ static size_t free_memory_on_node(struct _starpu_mem_chunk *mc, unsigned node)
 		}
 		}
 #endif
 #endif
 
 
+		if (handle)
+			interface = replicate->data_interface;
+		else
+			interface = mc->chunk_interface;
+		STARPU_ASSERT(interface);
+
 		_STARPU_TRACE_START_FREE(node, mc->size);
 		_STARPU_TRACE_START_FREE(node, mc->size);
-		mc->ops->free_data_on_node(mc->chunk_interface, node);
+		mc->ops->free_data_on_node(interface, node);
 		_STARPU_TRACE_END_FREE(node);
 		_STARPU_TRACE_END_FREE(node);
 
 
 		if (handle)
 		if (handle)
@@ -310,7 +318,6 @@ static size_t do_free_mem_chunk(struct _starpu_mem_chunk *mc, unsigned node)
 	/* remove the mem_chunk from the list */
 	/* remove the mem_chunk from the list */
 	_starpu_mem_chunk_list_erase(mc_list[node], mc);
 	_starpu_mem_chunk_list_erase(mc_list[node], mc);
 
 
-	free(mc->chunk_interface);
 	_starpu_mem_chunk_delete(mc);
 	_starpu_mem_chunk_delete(mc);
 
 
 	return size;
 	return size;
@@ -352,7 +359,7 @@ static size_t try_to_free_mem_chunk(struct _starpu_mem_chunk *mc, unsigned node)
 
 
 		if (mc->replicate->refcnt == 0)
 		if (mc->replicate->refcnt == 0)
 		{
 		{
-			/* Note taht there is no need to transfer any data or
+			/* Note that there is no need to transfer any data or
 			 * to update the status in terms of MSI protocol
 			 * to update the status in terms of MSI protocol
 			 * because this memchunk is associated to a replicate
 			 * because this memchunk is associated to a replicate
 			 * in "relaxed coherency" mode. */
 			 * in "relaxed coherency" mode. */
@@ -410,6 +417,8 @@ static size_t try_to_free_mem_chunk(struct _starpu_mem_chunk *mc, unsigned node)
  * therefore not in the cache. */
  * therefore not in the cache. */
 static void reuse_mem_chunk(unsigned node, struct _starpu_data_replicate *new_replicate, struct _starpu_mem_chunk *mc, unsigned is_already_in_mc_list)
 static void reuse_mem_chunk(unsigned node, struct _starpu_data_replicate *new_replicate, struct _starpu_mem_chunk *mc, unsigned is_already_in_mc_list)
 {
 {
+	void *interface;
+
 	/* we found an appropriate mem chunk: so we get it out
 	/* we found an appropriate mem chunk: so we get it out
 	 * of the "to free" list, and reassign it to the new
 	 * of the "to free" list, and reassign it to the new
 	 * piece of data */
 	 * piece of data */
@@ -420,15 +429,24 @@ static void reuse_mem_chunk(unsigned node, struct _starpu_data_replicate *new_re
 		old_replicate->allocated = 0;
 		old_replicate->allocated = 0;
 		old_replicate->automatically_allocated = 0;
 		old_replicate->automatically_allocated = 0;
 		old_replicate->initialized = 0;
 		old_replicate->initialized = 0;
+		interface = old_replicate->data_interface;
 	}
 	}
+	else
+		interface = mc->chunk_interface;
 
 
 	new_replicate->allocated = 1;
 	new_replicate->allocated = 1;
 	new_replicate->automatically_allocated = 1;
 	new_replicate->automatically_allocated = 1;
 	new_replicate->initialized = 0;
 	new_replicate->initialized = 0;
 
 
 	STARPU_ASSERT(new_replicate->data_interface);
 	STARPU_ASSERT(new_replicate->data_interface);
-	STARPU_ASSERT(mc->chunk_interface);
-	memcpy(new_replicate->data_interface, mc->chunk_interface, mc->size_interface);
+	STARPU_ASSERT(interface);
+	memcpy(new_replicate->data_interface, interface, mc->size_interface);
+
+	if (!old_replicate)
+	{
+		free(mc->chunk_interface);
+		mc->chunk_interface = NULL;
+	}
 
 
 	mc->data = new_replicate->handle;
 	mc->data = new_replicate->handle;
 	/* mc->ops, mc->footprint and mc->interface should be
 	/* mc->ops, mc->footprint and mc->interface should be
@@ -721,12 +739,8 @@ static struct _starpu_mem_chunk *_starpu_memchunk_init(struct _starpu_data_repli
 	mc->relaxed_coherency = replicate->relaxed_coherency;
 	mc->relaxed_coherency = replicate->relaxed_coherency;
 	mc->replicate = replicate;
 	mc->replicate = replicate;
 	mc->replicate->mc = mc;
 	mc->replicate->mc = mc;
-
-	/* Save a copy of the interface */
-	mc->chunk_interface = malloc(interface_size);
+	mc->chunk_interface = NULL;
 	mc->size_interface = interface_size;
 	mc->size_interface = interface_size;
-	STARPU_ASSERT(mc->chunk_interface);
-	memcpy(mc->chunk_interface, replicate->data_interface, interface_size);
 
 
 	return mc;
 	return mc;
 }
 }
@@ -765,6 +779,11 @@ void _starpu_request_mem_chunk_removal(starpu_data_handle_t handle, struct _star
 	 * by freeing this.  */
 	 * by freeing this.  */
 	mc->size = size;
 	mc->size = size;
 
 
+	/* Also keep the interface parameters and pointers, for later reuse
+	 * while detached, or freed */
+	mc->chunk_interface = malloc(mc->size_interface);
+	memcpy(mc->chunk_interface, replicate->data_interface, mc->size_interface);
+
 	/* This memchunk doesn't have to do with the data any more. */
 	/* This memchunk doesn't have to do with the data any more. */
 	replicate->mc = NULL;
 	replicate->mc = NULL;
 	mc->replicate = NULL;
 	mc->replicate = NULL;

+ 8 - 6
src/datawizard/memalloc.h

@@ -1,6 +1,6 @@
 /* StarPU --- Runtime system for heterogeneous multicore architectures.
 /* StarPU --- Runtime system for heterogeneous multicore architectures.
  *
  *
- * Copyright (C) 2009, 2010, 2012-2013  Université de Bordeaux 1
+ * Copyright (C) 2009-2010, 2012-2014  Université de Bordeaux 1
  * Copyright (C) 2010, 2011, 2012, 2013  Centre National de la Recherche Scientifique
  * Copyright (C) 2010, 2011, 2012, 2013  Centre National de la Recherche Scientifique
  *
  *
  * StarPU is free software; you can redistribute it and/or modify
  * StarPU is free software; you can redistribute it and/or modify
@@ -33,11 +33,13 @@ LIST_TYPE(_starpu_mem_chunk,
 
 
 	uint32_t footprint;
 	uint32_t footprint;
 
 
-	/* The footprint of the data is not sufficient to determine whether two
-	 * pieces of data have the same layout (there could be collision in the
-	 * hash function ...) so we still keep a copy of the actual layout (ie.
-	 * the data interface) to stay on the safe side. We make a copy of
-	 * because when a data is deleted, the memory chunk remains.
+	/*
+	 * When re-using a memchunk, the footprint of the data is not
+	 * sufficient to determine whether two pieces of data have the same
+	 * layout (there could be collision in the hash function ...) so we
+	 * still keep a copy of the actual layout (ie. the data interface) to
+	 * stay on the safe side while the memchunk is detached from an actual
+	 * data.
 	 */
 	 */
 	struct starpu_data_interface_ops *ops;
 	struct starpu_data_interface_ops *ops;
 	void *chunk_interface;
 	void *chunk_interface;