[Mesa-stable] [Mesa-dev] [PATCH 6/7] radeonsi/compute: Stop leaking the input buffer

Emil Velikov emil.l.velikov at gmail.com
Tue Aug 26 13:46:03 PDT 2014


On 08/08/14 15:16, Tom Stellard wrote:
> We were leaking the input buffer used for kernel arguments and since
> we were allocating it using si_upload_const_buffer() we were leaking
> 1 MB per kernel invocation.
> 
> CC: "10.2" <mesa-stable at lists.freedesktop.org>
> ---
>  src/gallium/drivers/radeonsi/si_compute.c | 22 ++++++++++++++--------
>  1 file changed, 14 insertions(+), 8 deletions(-)
> 
> diff --git a/src/gallium/drivers/radeonsi/si_compute.c b/src/gallium/drivers/radeonsi/si_compute.c
> index dff5ddd..01aa0c6 100644
> --- a/src/gallium/drivers/radeonsi/si_compute.c
> +++ b/src/gallium/drivers/radeonsi/si_compute.c
> @@ -48,6 +48,7 @@ struct si_pipe_compute {
>  	struct si_pipe_shader *kernels;
>  	unsigned num_user_sgprs;
>  
> +	struct r600_resource *input_buffer;
>  	struct pipe_resource *global_buffers[MAX_GLOBAL_BUFFERS];
>  
>  	LLVMContextRef llvm_ctx;
> @@ -85,6 +86,9 @@ static void *si_create_compute_state(
>  		LLVMDisposeModule(mod);
>  	}
>  
> +	program->input_buffer =	si_resource_create_custom(sctx->b.b.screen,
> +		PIPE_USAGE_IMMUTABLE, program->input_size);
> +
>  	return program;
>  }
>  
> @@ -167,7 +171,7 @@ static void si_launch_grid(
>  	struct si_context *sctx = (struct si_context*)ctx;
>  	struct si_pipe_compute *program = sctx->cs_shader_state.program;
>  	struct si_pm4_state *pm4 = CALLOC_STRUCT(si_pm4_state);
> -	struct r600_resource *kernel_args_buffer = NULL;
> +	struct r600_resource *input_buffer = program->input_buffer;
>  	unsigned kernel_args_size;
>  	unsigned num_work_size_bytes = 36;
>  	uint32_t kernel_args_offset = 0;
> @@ -199,7 +203,8 @@ static void si_launch_grid(
>  	/* The extra num_work_size_bytes are for work group / work item size information */
>  	kernel_args_size = program->input_size + num_work_size_bytes + 8 /* For scratch va */;
>  
> -	kernel_args = MALLOC(kernel_args_size);
> +	kernel_args = sctx->b.ws->buffer_map(input_buffer->cs_buf,
> +			sctx->b.rings.gfx.cs, PIPE_TRANSFER_WRITE);
>  	for (i = 0; i < 3; i++) {
>  		kernel_args[i] = grid_layout[i];
>  		kernel_args[i + 3] = grid_layout[i] * block_layout[i];
> @@ -236,13 +241,13 @@ static void si_launch_grid(
>  			kernel_args[i]);
>  	}
>  
> -	si_upload_const_buffer(sctx, &kernel_args_buffer, (uint8_t*)kernel_args,
> -					kernel_args_size, &kernel_args_offset);
> -	kernel_args_va = r600_resource_va(ctx->screen,
> -				(struct pipe_resource*)kernel_args_buffer);
> +	sctx->b.ws->buffer_unmap(input_buffer->cs_buf);
> +
> +	kernel_args_va = r600_resource_va(ctx->screen, &input_buffer->b.b);
>  	kernel_args_va += kernel_args_offset;
>  
> -	si_pm4_add_bo(pm4, kernel_args_buffer, RADEON_USAGE_READ, RADEON_PRIO_SHADER_DATA);
> +	si_pm4_add_bo(pm4, input_buffer, RADEON_USAGE_READ,
> +		RADEON_PRIO_SHADER_DATA);
>  
>  	si_pm4_set_reg(pm4, R_00B900_COMPUTE_USER_DATA_0, kernel_args_va);
>  	si_pm4_set_reg(pm4, R_00B900_COMPUTE_USER_DATA_0 + 4, S_008F04_BASE_ADDRESS_HI (kernel_args_va >> 32) | S_008F04_STRIDE(0));
> @@ -374,7 +379,6 @@ static void si_launch_grid(
>  	}
>  #endif
>  
> -	FREE(kernel_args);
>  	si_pm4_free_state(sctx, pm4, ~0);
>  }
>  
> @@ -398,6 +402,8 @@ static void si_delete_compute_state(struct pipe_context *ctx, void* state){
>  	if (program->llvm_ctx){
>  		LLVMContextDispose(program->llvm_ctx);
>  	}
> +	pipe_resource_reference(
> +		(struct pipe_resource **)&program->input_buffer, NULL);
>  
>  	//And then free the program itself.
>  	FREE(program);
> 

Hello Tom,

This patch does not apply cleanly to the 10.2 branch. I have resolved the
conflicts as below. Kindly let me know if I missed anything.

-Emil

commit 70c7ebe5fe1f0f9a49e183857bb8f9d6497ddd7e
Author: Tom Stellard <thomas.stellard at amd.com>
Date:   Fri Aug 8 09:27:34 2014 -0400

    radeonsi/compute: Stop leaking the input buffer

    We were leaking the input buffer used for kernel arguments and since
    we were allocating it using si_upload_const_buffer() we were leaking
    1 MB per kernel invocation.

    CC: "10.2" <mesa-stable at lists.freedesktop.org>
    (cherry picked from commit a15088338ebe544efd90bfa7934cb99521488141)

    Conflicts:
    	src/gallium/drivers/radeonsi/si_compute.c

diff --git a/src/gallium/drivers/radeonsi/si_compute.c
b/src/gallium/drivers/radeonsi/si_compute.c
index b6aa044..d99b188 100644
--- a/src/gallium/drivers/radeonsi/si_compute.c
+++ b/src/gallium/drivers/radeonsi/si_compute.c
@@ -43,6 +43,7 @@ struct si_pipe_compute {
 	struct si_pipe_shader *kernels;
 	unsigned num_user_sgprs;

+	struct r600_resource *input_buffer;
 	struct pipe_resource *global_buffers[MAX_GLOBAL_BUFFERS];

 	LLVMContextRef llvm_ctx;
@@ -80,6 +81,9 @@ static void *si_create_compute_state(
 		LLVMDisposeModule(mod);
 	}

+	program->input_buffer =	si_resource_create_custom(sctx->b.b.screen,
+		PIPE_USAGE_IMMUTABLE, program->input_size);
+
 	return program;
 }

@@ -125,7 +129,7 @@ static void si_launch_grid(
 	struct si_context *sctx = (struct si_context*)ctx;
 	struct si_pipe_compute *program = sctx->cs_shader_state.program;
 	struct si_pm4_state *pm4 = CALLOC_STRUCT(si_pm4_state);
-	struct r600_resource *kernel_args_buffer = NULL;
+	struct r600_resource *input_buffer = program->input_buffer;
 	unsigned kernel_args_size;
 	unsigned num_work_size_bytes = 36;
 	uint32_t kernel_args_offset = 0;
@@ -154,7 +158,9 @@ static void si_launch_grid(

 	/* The extra num_work_size_bytes are for work group / work item size
information */
 	kernel_args_size = program->input_size + num_work_size_bytes;
-	kernel_args = MALLOC(kernel_args_size);
+
+	kernel_args = sctx->b.ws->buffer_map(input_buffer->cs_buf,
+			sctx->b.rings.gfx.cs, PIPE_TRANSFER_WRITE);
 	for (i = 0; i < 3; i++) {
 		kernel_args[i] = grid_layout[i];
 		kernel_args[i + 3] = grid_layout[i] * block_layout[i];
@@ -163,13 +169,13 @@ static void si_launch_grid(

 	memcpy(kernel_args + (num_work_size_bytes / 4), input, program->input_size);

-	si_upload_const_buffer(sctx, &kernel_args_buffer, (uint8_t*)kernel_args,
-					kernel_args_size, &kernel_args_offset);
-	kernel_args_va = r600_resource_va(ctx->screen,
-				(struct pipe_resource*)kernel_args_buffer);
+	sctx->b.ws->buffer_unmap(input_buffer->cs_buf);
+
+	kernel_args_va = input_buffer->gpu_address;
 	kernel_args_va += kernel_args_offset;

-	si_pm4_add_bo(pm4, kernel_args_buffer, RADEON_USAGE_READ,
RADEON_PRIO_SHADER_DATA);
+	si_pm4_add_bo(pm4, input_buffer, RADEON_USAGE_READ,
+		RADEON_PRIO_SHADER_DATA);

 	si_pm4_set_reg(pm4, R_00B900_COMPUTE_USER_DATA_0, kernel_args_va);
 	si_pm4_set_reg(pm4, R_00B900_COMPUTE_USER_DATA_0 + 4,
S_008F04_BASE_ADDRESS_HI (kernel_args_va >> 32) | S_008F04_STRIDE(0));
@@ -288,7 +294,6 @@ static void si_launch_grid(
 	}
 #endif

-	FREE(kernel_args);
 	si_pm4_free_state(sctx, pm4, ~0);
 }

@@ -312,6 +317,8 @@ static void si_delete_compute_state(struct pipe_context
*ctx, void* state){
 	if (program->llvm_ctx){
 		LLVMContextDispose(program->llvm_ctx);
 	}
+	pipe_resource_reference(
+		(struct pipe_resource **)&program->input_buffer, NULL);

 	//And then free the program itself.
 	FREE(program);




More information about the mesa-stable mailing list