[2/2] radv/winsys: use alloca() when creating the buffer object list

Submitted by Samuel Pitoiset on July 12, 2018, 2:26 p.m.

Details

Message ID 20180712142635.25379-2-samuel.pitoiset@gmail.com
State Rejected
Headers show
Series "Series without cover letter" ( rev: 1 ) in Mesa

Not browsing as part of any series.

Commit Message

Samuel Pitoiset July 12, 2018, 2:26 p.m.
Signed-off-by: Samuel Pitoiset <samuel.pitoiset@gmail.com>
---
 src/amd/vulkan/winsys/amdgpu/radv_amdgpu_cs.c | 16 +++-------------
 1 file changed, 3 insertions(+), 13 deletions(-)

Patch hide | download patch | download mbox

diff --git a/src/amd/vulkan/winsys/amdgpu/radv_amdgpu_cs.c b/src/amd/vulkan/winsys/amdgpu/radv_amdgpu_cs.c
index c94c0f339f..e7ad9d7f6b 100644
--- a/src/amd/vulkan/winsys/amdgpu/radv_amdgpu_cs.c
+++ b/src/amd/vulkan/winsys/amdgpu/radv_amdgpu_cs.c
@@ -605,11 +605,7 @@  static int radv_amdgpu_create_bo_list(struct radv_amdgpu_winsys *ws,
 
 		pthread_mutex_lock(&ws->global_bo_list_lock);
 
-		handles = malloc(sizeof(handles[0]) * ws->num_buffers);
-		if (!handles) {
-			pthread_mutex_unlock(&ws->global_bo_list_lock);
-			return -ENOMEM;
-		}
+		handles = alloca(sizeof(handles[0]) * ws->num_buffers);
 
 		LIST_FOR_EACH_ENTRY(bo, &ws->global_bo_list, global_list_item) {
 			assert(num < ws->num_buffers);
@@ -619,7 +615,7 @@  static int radv_amdgpu_create_bo_list(struct radv_amdgpu_winsys *ws,
 		r = amdgpu_bo_list_create(ws->dev, ws->num_buffers,
 					  handles, NULL,
 					  bo_list);
-		free(handles);
+
 		pthread_mutex_unlock(&ws->global_bo_list_lock);
 	} else if (count == 1 && !num_extra_bo && !extra_cs && !radv_bo_list &&
 	           !radv_amdgpu_cs(cs_array[0])->num_virtual_buffers) {
@@ -652,11 +648,7 @@  static int radv_amdgpu_create_bo_list(struct radv_amdgpu_winsys *ws,
 			*bo_list = 0;
 			return 0;
 		}
-		amdgpu_bo_handle *handles = malloc(sizeof(amdgpu_bo_handle) * total_buffer_count);
-		if (!handles) {
-			free(handles);
-			return -ENOMEM;
-		}
+		amdgpu_bo_handle *handles = alloca(sizeof(amdgpu_bo_handle) * total_buffer_count);
 
 		for (unsigned i = 0; i < num_extra_bo; i++) {
 			handles[i] = extra_bo_array[i]->bo;
@@ -735,8 +727,6 @@  static int radv_amdgpu_create_bo_list(struct radv_amdgpu_winsys *ws,
 		} else {
 			*bo_list = 0;
 		}
-
-		free(handles);
 	}
 
 	return r;

Comments

Not really a fan of this one, we can literally have thousands of
entries which seems largish when considering the stacks for heavily
threaded stuff are often much smaller.

On Thu, Jul 12, 2018 at 4:26 PM, Samuel Pitoiset
<samuel.pitoiset@gmail.com> wrote:
> Signed-off-by: Samuel Pitoiset <samuel.pitoiset@gmail.com>
> ---
>  src/amd/vulkan/winsys/amdgpu/radv_amdgpu_cs.c | 16 +++-------------
>  1 file changed, 3 insertions(+), 13 deletions(-)
>
> diff --git a/src/amd/vulkan/winsys/amdgpu/radv_amdgpu_cs.c b/src/amd/vulkan/winsys/amdgpu/radv_amdgpu_cs.c
> index c94c0f339f..e7ad9d7f6b 100644
> --- a/src/amd/vulkan/winsys/amdgpu/radv_amdgpu_cs.c
> +++ b/src/amd/vulkan/winsys/amdgpu/radv_amdgpu_cs.c
> @@ -605,11 +605,7 @@ static int radv_amdgpu_create_bo_list(struct radv_amdgpu_winsys *ws,
>
>                 pthread_mutex_lock(&ws->global_bo_list_lock);
>
> -               handles = malloc(sizeof(handles[0]) * ws->num_buffers);
> -               if (!handles) {
> -                       pthread_mutex_unlock(&ws->global_bo_list_lock);
> -                       return -ENOMEM;
> -               }
> +               handles = alloca(sizeof(handles[0]) * ws->num_buffers);
>
>                 LIST_FOR_EACH_ENTRY(bo, &ws->global_bo_list, global_list_item) {
>                         assert(num < ws->num_buffers);
> @@ -619,7 +615,7 @@ static int radv_amdgpu_create_bo_list(struct radv_amdgpu_winsys *ws,
>                 r = amdgpu_bo_list_create(ws->dev, ws->num_buffers,
>                                           handles, NULL,
>                                           bo_list);
> -               free(handles);
> +
>                 pthread_mutex_unlock(&ws->global_bo_list_lock);
>         } else if (count == 1 && !num_extra_bo && !extra_cs && !radv_bo_list &&
>                    !radv_amdgpu_cs(cs_array[0])->num_virtual_buffers) {
> @@ -652,11 +648,7 @@ static int radv_amdgpu_create_bo_list(struct radv_amdgpu_winsys *ws,
>                         *bo_list = 0;
>                         return 0;
>                 }
> -               amdgpu_bo_handle *handles = malloc(sizeof(amdgpu_bo_handle) * total_buffer_count);
> -               if (!handles) {
> -                       free(handles);
> -                       return -ENOMEM;
> -               }
> +               amdgpu_bo_handle *handles = alloca(sizeof(amdgpu_bo_handle) * total_buffer_count);
>
>                 for (unsigned i = 0; i < num_extra_bo; i++) {
>                         handles[i] = extra_bo_array[i]->bo;
> @@ -735,8 +727,6 @@ static int radv_amdgpu_create_bo_list(struct radv_amdgpu_winsys *ws,
>                 } else {
>                         *bo_list = 0;
>                 }
> -
> -               free(handles);
>         }
>
>         return r;
> --
> 2.18.0
>
> _______________________________________________
> mesa-dev mailing list
> mesa-dev@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
On 07/12/2018 04:29 PM, Bas Nieuwenhuizen wrote:
> Not really a fan of this one, we can literally have thousands of
> entries which seems largish when considering the stacks for heavily
> threaded stuff are often much smaller.

Hmm yeah, you are probably right. What about the first one?

> 
> On Thu, Jul 12, 2018 at 4:26 PM, Samuel Pitoiset
> <samuel.pitoiset@gmail.com> wrote:
>> Signed-off-by: Samuel Pitoiset <samuel.pitoiset@gmail.com>
>> ---
>>   src/amd/vulkan/winsys/amdgpu/radv_amdgpu_cs.c | 16 +++-------------
>>   1 file changed, 3 insertions(+), 13 deletions(-)
>>
>> diff --git a/src/amd/vulkan/winsys/amdgpu/radv_amdgpu_cs.c b/src/amd/vulkan/winsys/amdgpu/radv_amdgpu_cs.c
>> index c94c0f339f..e7ad9d7f6b 100644
>> --- a/src/amd/vulkan/winsys/amdgpu/radv_amdgpu_cs.c
>> +++ b/src/amd/vulkan/winsys/amdgpu/radv_amdgpu_cs.c
>> @@ -605,11 +605,7 @@ static int radv_amdgpu_create_bo_list(struct radv_amdgpu_winsys *ws,
>>
>>                  pthread_mutex_lock(&ws->global_bo_list_lock);
>>
>> -               handles = malloc(sizeof(handles[0]) * ws->num_buffers);
>> -               if (!handles) {
>> -                       pthread_mutex_unlock(&ws->global_bo_list_lock);
>> -                       return -ENOMEM;
>> -               }
>> +               handles = alloca(sizeof(handles[0]) * ws->num_buffers);
>>
>>                  LIST_FOR_EACH_ENTRY(bo, &ws->global_bo_list, global_list_item) {
>>                          assert(num < ws->num_buffers);
>> @@ -619,7 +615,7 @@ static int radv_amdgpu_create_bo_list(struct radv_amdgpu_winsys *ws,
>>                  r = amdgpu_bo_list_create(ws->dev, ws->num_buffers,
>>                                            handles, NULL,
>>                                            bo_list);
>> -               free(handles);
>> +
>>                  pthread_mutex_unlock(&ws->global_bo_list_lock);
>>          } else if (count == 1 && !num_extra_bo && !extra_cs && !radv_bo_list &&
>>                     !radv_amdgpu_cs(cs_array[0])->num_virtual_buffers) {
>> @@ -652,11 +648,7 @@ static int radv_amdgpu_create_bo_list(struct radv_amdgpu_winsys *ws,
>>                          *bo_list = 0;
>>                          return 0;
>>                  }
>> -               amdgpu_bo_handle *handles = malloc(sizeof(amdgpu_bo_handle) * total_buffer_count);
>> -               if (!handles) {
>> -                       free(handles);
>> -                       return -ENOMEM;
>> -               }
>> +               amdgpu_bo_handle *handles = alloca(sizeof(amdgpu_bo_handle) * total_buffer_count);
>>
>>                  for (unsigned i = 0; i < num_extra_bo; i++) {
>>                          handles[i] = extra_bo_array[i]->bo;
>> @@ -735,8 +727,6 @@ static int radv_amdgpu_create_bo_list(struct radv_amdgpu_winsys *ws,
>>                  } else {
>>                          *bo_list = 0;
>>                  }
>> -
>> -               free(handles);
>>          }
>>
>>          return r;
>> --
>> 2.18.0
>>
>> _______________________________________________
>> mesa-dev mailing list
>> mesa-dev@lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/mesa-dev