- Fix removed chunks locking: removed chunks actually never have to do with the handle any more, we shouldn't have to spinlock any header lock for them.

- in particular, in starpu_data_invalidate, one has to record that the interface is not allocated any more.
- BTW, add a size parameter to _starpu_request_mem_chunk_removal, so the caller can provide it even especially when the handle is already freed, and thus _starpu_request_mem_chunk_removal shouldn't be able to call _starpu_data_get_size itself.

src/datawizard/filters.c

@@ -273,6 +273,7 @@ void starpu_data_unpartition(starpu_data_handle_t root_handle, unsigned gatherin
 {
 	unsigned child;
 	unsigned node;
+	unsigned sizes[root_handle->nchildren];
 
 	_starpu_spin_lock(&root_handle->header_lock);
 
@@ -287,6 +288,8 @@ void starpu_data_unpartition(starpu_data_handle_t root_handle, unsigned gatherin
 		if (child_handle->nchildren > 0)
 			starpu_data_unpartition(child_handle, gathering_node);
 
+		sizes[child] = _starpu_data_get_size(child_handle);
+
 		/* If this is a multiformat handle, we must convert the data now */
 #ifdef STARPU_DEVEL
 #warning TODO: _starpu_fetch_data_on_node should be doing it
@@ -362,9 +365,7 @@ void starpu_data_unpartition(starpu_data_handle_t root_handle, unsigned gatherin
 #ifdef STARPU_DEVEL
 #warning FIXME!! this needs access to the child interface, which was freed above!
 #endif
-				_starpu_spin_lock(&root_handle->header_lock);
-				_starpu_request_mem_chunk_removal(child_handle, node, 1);
-				_starpu_spin_unlock(&root_handle->header_lock);
+				_starpu_request_mem_chunk_removal(child_handle, node, sizes[child]);
 			}
 		}
 
@@ -375,7 +376,7 @@ void starpu_data_unpartition(starpu_data_handle_t root_handle, unsigned gatherin
 
 		if (!isvalid && root_handle->per_node[node].allocated && root_handle->per_node[node].automatically_allocated)
 			/* free the data copy in a lazy fashion */
-			_starpu_request_mem_chunk_removal(root_handle, node, 1);
+			_starpu_request_mem_chunk_removal(root_handle, node, _starpu_data_get_size(root_handle));
 
 		/* if there was no invalid copy, the node still has a valid copy */
 		still_valid[node] = isvalid;

src/datawizard/interfaces/data_interface.c

@@ -555,10 +555,11 @@ static void _starpu_data_unregister(starpu_data_handle_t handle, unsigned cohere
 
 	/* Destroy the data now */
 	unsigned node;
+	size_t size = _starpu_data_get_size(handle);
 	for (node = 0; node < STARPU_MAXNODES; node++)
 	{
 		/* free the data copy in a lazy fashion */
-		_starpu_request_mem_chunk_removal(handle, node, 1);
+		_starpu_request_mem_chunk_removal(handle, node, size);
 	}
 
 	_starpu_data_free_interfaces(handle);
@@ -595,6 +596,7 @@ void starpu_data_unregister_submit(starpu_data_handle_t handle)
 static void _starpu_data_invalidate(void *data)
 {
 	starpu_data_handle_t handle = data;
+	size_t size = _starpu_data_get_size(handle);
 	_starpu_spin_lock(&handle->header_lock);
 
 	unsigned node;
@@ -605,7 +607,9 @@ static void _starpu_data_invalidate(void *data)
 		if (local->allocated && local->automatically_allocated)
 		{
 			/* free the data copy in a lazy fashion */
-			_starpu_request_mem_chunk_removal(handle, node, 0);
+			_starpu_request_mem_chunk_removal(handle, node, size);
+			local->allocated = 0;
+			local->automatically_allocated = 0;
 		}
 
 		local->state = STARPU_INVALID;

src/datawizard/memalloc.c

@@ -694,18 +694,14 @@ static void register_mem_chunk(struct _starpu_data_replicate *replicate, unsigne
 /* This function is called when the handle is destroyed (eg. when calling
  * unregister or unpartition). It puts all the memchunks that refer to the
  * specified handle into the cache.
- * handle_deleted specifies whether the handle is deleted or not (and thus we
- * need to update it)
  */
-void _starpu_request_mem_chunk_removal(starpu_data_handle_t handle, unsigned node, int handle_deleted)
+void _starpu_request_mem_chunk_removal(starpu_data_handle_t handle, unsigned node, size_t size)
 {
 	_starpu_spin_checklocked(&handle->header_lock);
 
 	_STARPU_PTHREAD_RWLOCK_WRLOCK(&mc_rwlock[node]);
 
-	size_t size = _starpu_data_get_size(handle);
-
-	/* TODO: expensive, handle should its own list of chunks? */
+	/* TODO: expensive, handle should have its own list of chunks? */
 	/* iterate over the list of memory chunks and remove the entry */
 	struct _starpu_mem_chunk *mc, *next_mc;
 	for (mc = _starpu_mem_chunk_list_begin(mc_list[node]);
@@ -717,10 +713,13 @@ void _starpu_request_mem_chunk_removal(starpu_data_handle_t handle, unsigned nod
 		if (mc->data == handle)
 		{
 			/* we found the data */
+
+			/* Record the allocated size, so that later in memory
+			 * reclaiming we can estimate how much memory we free
+			 * by freeing this.  */
 			mc->size = size;
-			if (handle_deleted)
-				/* This memchunk doesn't have to do with the data any more. */
-				mc->data = NULL;
+			/* This memchunk doesn't have to do with the data any more. */
+			mc->data = NULL;
 
 			/* remove it from the main list */
 			_starpu_mem_chunk_list_erase(mc_list[node], mc);

src/datawizard/memalloc.h

@@ -62,7 +62,7 @@ LIST_TYPE(_starpu_mem_chunk_lru,
 
 void _starpu_init_mem_chunk_lists(void);
 void _starpu_deinit_mem_chunk_lists(void);
-void _starpu_request_mem_chunk_removal(starpu_data_handle_t handle, unsigned node, int handle_deleted);
+void _starpu_request_mem_chunk_removal(starpu_data_handle_t handle, unsigned node, size_t size);
 int _starpu_allocate_memory_on_node(starpu_data_handle_t handle, struct _starpu_data_replicate *replicate, unsigned is_prefetch);
 size_t _starpu_free_all_automatically_allocated_buffers(unsigned node);
 void _starpu_memchunk_recently_used(struct _starpu_mem_chunk *mc, unsigned node);