Re: [Intel-gfx] [PATCH] drm/i915/gem: Use local pointer ttm for __i915_ttm_move

["Thomas Hellström (Intel)" <thomas_os@shipmail.org>]

Hi, Jasmine,

On 12/3/21 22:49, Jasmine Newsome wrote:
> To avoid confusion with deferencing possible null pointer bo->ttm,
> replace pointer bo->ttm with local pointer ttm in i915_ttm_move
> as ttm has checks for null before getting passed to __i915_ttm_move

It's OK to use the local variable ttm here because it has previously 
been assigned from bo->ttm.

However, if a static analyzer gets confused by that and the fact that a 
NULL pointer might be passed to __i915_ttm_move, (which will not 
dereference it), then that's a static analyzer false positive.

So the commit message needs to be rewritten saying something like the 
code is getting a bit more readable if the local variable is used since 
it was previously assigned from bo->ttm, that's ok.

Also please add your Signed-off-by: To the patch.

Thanks,

Thomas




> ---
>   drivers/gpu/drm/i915/gem/i915_gem_ttm_move.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/i915/gem/i915_gem_ttm_move.c b/drivers/gpu/drm/i915/gem/i915_gem_ttm_move.c
> index 80df9f592407..56b6573b5c93 100644
> --- a/drivers/gpu/drm/i915/gem/i915_gem_ttm_move.c
> +++ b/drivers/gpu/drm/i915/gem/i915_gem_ttm_move.c
> @@ -763,7 +763,7 @@ int i915_ttm_move(struct ttm_buffer_object *bo, bool evict,
>   			return PTR_ERR(dep);
>   		}
>   
> -		migration_fence = __i915_ttm_move(bo, clear, dst_mem, bo->ttm,
> +		migration_fence = __i915_ttm_move(bo, clear, dst_mem, ttm,
>   						  dst_rsgt, true, dep);
>   		dma_fence_put(dep);
>   	}