diff mbox series

drm/amdgpu: Remove hidden double memset from amdgpu_vm_pt_clear()

Message ID 20240813140835.82748-1-tursulin@igalia.com (mailing list archive)
State New, archived
Headers show
Series drm/amdgpu: Remove hidden double memset from amdgpu_vm_pt_clear() | expand

Commit Message

Tvrtko Ursulin Aug. 13, 2024, 2:08 p.m. UTC 
From: Tvrtko Ursulin <tvrtko.ursulin@igalia.com>

When CONFIG_INIT_STACK_ALL_ZERO is set and so -ftrivial-auto-var-init=zero
compiler option active, compiler fails to notice that later in
amdgpu_vm_pt_clear() there  is a second memset to clear the same on stack
struct amdgpu_vm_update_params.

If we replace this memset with an explicit automatic variable initializer,
compiler can then see it and avoid clearing this struct twice.

Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@igalia.com>
---
This is perhaps a bit questionable, regardless of how annoying it is to
know there is this double memset.
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_vm_pt.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

Comments

Tvrtko Ursulin Aug. 13, 2024, 2:10 p.m. UTC | #1 
On 13/08/2024 15:08, Tvrtko Ursulin wrote:
> From: Tvrtko Ursulin <tvrtko.ursulin@igalia.com>
> 
> When CONFIG_INIT_STACK_ALL_ZERO is set and so -ftrivial-auto-var-init=zero
> compiler option active, compiler fails to notice that later in
> amdgpu_vm_pt_clear() there  is a second memset to clear the same on stack
> struct amdgpu_vm_update_params.
> 
> If we replace this memset with an explicit automatic variable initializer,
> compiler can then see it and avoid clearing this struct twice.
> 
> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@igalia.com>
> ---
> This is perhaps a bit questionable, regardless of how annoying it is to
> know there is this double memset.
> ---
>   drivers/gpu/drm/amd/amdgpu/amdgpu_vm_pt.c | 3 +--
>   1 file changed, 1 insertion(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm_pt.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm_pt.c
> index e39d6e7643bf..ecdc8fffe941 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm_pt.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm_pt.c
> @@ -361,7 +361,7 @@ int amdgpu_vm_pt_clear(struct amdgpu_device *adev, struct amdgpu_vm *vm,
>   {
>   	unsigned int level = adev->vm_manager.root_level;
>   	struct ttm_operation_ctx ctx = { true, false };
> -	struct amdgpu_vm_update_params params;
> +	struct amdgpu_vm_update_params params = { };
>   	struct amdgpu_bo *ancestor = &vmbo->bo;
>   	unsigned int entries;
>   	struct amdgpu_bo *bo = &vmbo->bo;
> @@ -398,7 +398,6 @@ int amdgpu_vm_pt_clear(struct amdgpu_device *adev, struct amdgpu_vm *vm,
>   	if (r)
>   		goto exit;
>   
> -	memset(&params, 0, sizeof(params));
>   	params.adev = adev;
>   	params.vm = vm;
>   	params.immediate = immediate;

Or even move all above three into the automatic initializer since all 
are the function input arguments.

Regards,

Tvrtko
diff mbox series

Patch

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm_pt.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm_pt.c
index e39d6e7643bf..ecdc8fffe941 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm_pt.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm_pt.c
@@ -361,7 +361,7 @@  int amdgpu_vm_pt_clear(struct amdgpu_device *adev, struct amdgpu_vm *vm,
 {
 	unsigned int level = adev->vm_manager.root_level;
 	struct ttm_operation_ctx ctx = { true, false };
-	struct amdgpu_vm_update_params params;
+	struct amdgpu_vm_update_params params = { };
 	struct amdgpu_bo *ancestor = &vmbo->bo;
 	unsigned int entries;
 	struct amdgpu_bo *bo = &vmbo->bo;
@@ -398,7 +398,6 @@  int amdgpu_vm_pt_clear(struct amdgpu_device *adev, struct amdgpu_vm *vm,
 	if (r)
 		goto exit;
 
-	memset(&params, 0, sizeof(params));
 	params.adev = adev;
 	params.vm = vm;
 	params.immediate = immediate;