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

["Christian König" <christian.koenig@amd.com>]

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.khatri@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;