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

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

[Jasmine Newsome]

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
---
 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);
 	}
-- 
2.25.1

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

[Thomas Hellström (Intel)]

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);
>   	}

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

[Jasmine Newsome]

Using local pointer ttm as argument in  __i915_ttm_move
instead of bo->ttm, as local pointer was previously assigned
to bo->ttm in function. This will make code a bit more readable.

Signed-off-by: Jasmine Newsome <jasmine.newsome@intel.com>
---
 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);
 	}
-- 
2.25.1

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

[Jasmine Newsome]

Using local pointer ttm as argument in  __i915_ttm_move
instead of bo->ttm, as local pointer was previously assigned
to bo->ttm in function. This will make code a bit more readable.

Signed-off-by: Jasmine Newsome <jasmine.newsome@intel.com>
---
 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);
 	}
-- 
2.25.1

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

[Hellstrom, Thomas]

On Wed, 2021-12-08 at 16:05 -0800, Jasmine Newsome wrote:
> Using local pointer ttm as argument in  __i915_ttm_move
> instead of bo->ttm, as local pointer was previously assigned
> to bo->ttm in function. This will make code a bit more readable.
> 
> Signed-off-by: Jasmine Newsome <jasmine.newsome@intel.com>

Reviewed-by: Thomas Hellström <thomas.hellstrom@linux.intel.com>


> ---
>  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);
>         }

----------------------------------------------------------------------
Intel Sweden AB
Registered Office: Isafjordsgatan 30B, 164 40 Kista, Stockholm, Sweden
Registration Number: 556189-6027

This e-mail and any attachments may contain confidential material for
the sole use of the intended recipient(s). Any review or distribution
by others is strictly prohibited. If you are not the intended
recipient, please contact the sender and delete all copies.

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

[Jasmine Newsome]

Using local pointer ttm as argument in  __i915_ttm_move instead of bo->ttm,
as local pointer was previously assigned to bo->ttm in function.
This will make code a bit more readable.

Reviewed-by: Thomas Hellström <thomas.hellstrom@linux.intel.com>
Signed-off-by: Jasmine Newsome <jasmine.newsome@intel.com>
---
 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 ee9612a3ee5e..8653855d808b 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_ttm_move.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_ttm_move.c
@@ -525,7 +525,7 @@ int i915_ttm_move(struct ttm_buffer_object *bo, bool evict,
 			return ret;
 		}
 
-		migration_fence = __i915_ttm_move(bo, ctx, clear, dst_mem, bo->ttm,
+		migration_fence = __i915_ttm_move(bo, ctx, clear, dst_mem, ttm,
 						  dst_rsgt, true, &deps);
 		i915_deps_fini(&deps);
 	}
-- 
2.25.1