Re: [Patch v2 2/2] drm/amdgpu: move variable declaration to top of amdgpu_cs_parser_bos

[Christian König]

On 24.09.25 12:01, Sunil Khatri wrote:
> In function amdgpu_cs_parser_bos, declare the variables in the beginning
> of the function and not during the initialization.
> 
> Signed-off-by: Sunil Khatri <sunil.kha...@amd.com>
> ---
>  drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c | 11 +++++------
>  1 file changed, 5 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c 
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
> index 31eea1c7dac3..a9bdc368c981 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
> @@ -854,6 +854,9 @@ static int amdgpu_cs_parser_bos(struct amdgpu_cs_parser 
> *p,
>       struct amdgpu_vm *vm = &fpriv->vm;
>       struct amdgpu_bo_list_entry *e;
>       struct drm_gem_object *obj;
> +     struct amdgpu_bo *bo;
> +     struct mm_struct *usermm;
> +     bool userpage_invalidated;
>       unsigned long index;
>       unsigned int i;
>       int r;
> @@ -882,14 +885,12 @@ static int amdgpu_cs_parser_bos(struct amdgpu_cs_parser 
> *p,
>        * amdgpu_ttm_backend_bind() to flush and invalidate new pages
>        */
>       amdgpu_bo_list_for_each_userptr_entry(e, p->bo_list) {
> -             bool userpage_invalidated = false;
> -             struct amdgpu_bo *bo = e->bo;
> +             userpage_invalidated = false;
> +             bo = e->bo;

Those can stay as they are. (Ok there should be an empty line between the 
declaration of variables and code).

>               e->range = kzalloc(sizeof(*e->range), GFP_KERNEL);
>               if (unlikely(!e->range))
>                       return -ENOMEM;
>  
> -             int i;
> -

But that here looks really fishy. It basically means that we have an "int it" 
declared in this block which overrides the "unsigned int i" declared.

Please make a patch just removing this "int it" here and feel free to just add 
my rb and push to amd-staging-drm-next.

I'm really wondering how the heck that happened?

Thanks,
Christian.


>               r = amdgpu_ttm_tt_get_user_pages(bo, e->range);
>               if (r)
>                       goto out_free_user_pages;
> @@ -930,8 +931,6 @@ static int amdgpu_cs_parser_bos(struct amdgpu_cs_parser 
> *p,
>       }
>  
>       amdgpu_bo_list_for_each_userptr_entry(e, p->bo_list) {
> -             struct mm_struct *usermm;
> -
>               usermm = amdgpu_ttm_tt_get_usermm(e->bo->tbo.ttm);
>               if (usermm && usermm != current->mm) {
>                       r = -EPERM;