[Intel-gfx] [bug report] drm/i915/ttm: add tt shmem backend

[Intel-gfx] [bug report] drm/i915/ttm: add tt shmem backend

[Dan Carpenter]

Hello Matthew Auld,

The patch 7ae034590cea: "drm/i915/ttm: add tt shmem backend" from Oct
18, 2021, leads to the following Smatch static checker warning:

	drivers/gpu/drm/i915/gem/i915_gem_ttm.c:335 i915_ttm_eviction_valuable()
	warn: signedness bug returning '(-16)'

drivers/gpu/drm/i915/gem/i915_gem_ttm.c
    324 static bool i915_ttm_eviction_valuable(struct ttm_buffer_object *bo,
               ^^^^

    325                                        const struct ttm_place *place)
    326 {
    327         struct drm_i915_gem_object *obj = i915_ttm_to_gem(bo);
    328 
    329         /*
    330          * EXTERNAL objects should never be swapped out by TTM, instead we need
    331          * to handle that ourselves. TTM will already skip such objects for us,
    332          * but we would like to avoid grabbing locks for no good reason.
    333          */
    334         if (bo->ttm && bo->ttm->page_flags & TTM_TT_FLAG_EXTERNAL)
--> 335                 return -EBUSY;
                        ^^^^^^^^^^^^^^
This should be return false probably.

    336 
    337         /* Will do for now. Our pinned objects are still on TTM's LRU lists */
    338         return i915_gem_object_evictable(obj);
    339 }

regards,
dan carpenter

Re: [Intel-gfx] [bug report] drm/i915/ttm: add tt shmem backend

[Matthew Auld]

On 18/11/2021 15:01, Dan Carpenter wrote:
> Hello Matthew Auld,
> 
> The patch 7ae034590cea: "drm/i915/ttm: add tt shmem backend" from Oct
> 18, 2021, leads to the following Smatch static checker warning:
> 
> 	drivers/gpu/drm/i915/gem/i915_gem_ttm.c:335 i915_ttm_eviction_valuable()
> 	warn: signedness bug returning '(-16)'
> 
> drivers/gpu/drm/i915/gem/i915_gem_ttm.c
>      324 static bool i915_ttm_eviction_valuable(struct ttm_buffer_object *bo,
>                 ^^^^
> 
>      325                                        const struct ttm_place *place)
>      326 {
>      327         struct drm_i915_gem_object *obj = i915_ttm_to_gem(bo);
>      328
>      329         /*
>      330          * EXTERNAL objects should never be swapped out by TTM, instead we need
>      331          * to handle that ourselves. TTM will already skip such objects for us,
>      332          * but we would like to avoid grabbing locks for no good reason.
>      333          */
>      334         if (bo->ttm && bo->ttm->page_flags & TTM_TT_FLAG_EXTERNAL)
> --> 335                 return -EBUSY;
>                          ^^^^^^^^^^^^^^
> This should be return false probably.

Oh, indeed. Did you also want to send a patch for that? Thanks.

> 
>      336
>      337         /* Will do for now. Our pinned objects are still on TTM's LRU lists */
>      338         return i915_gem_object_evictable(obj);
>      339 }
> 
> regards,
> dan carpenter
> 

Re: [Intel-gfx] [bug report] drm/i915/ttm: add tt shmem backend

[Dan Carpenter]

On Thu, Nov 18, 2021 at 03:15:31PM +0000, Matthew Auld wrote:
> On 18/11/2021 15:01, Dan Carpenter wrote:
> > Hello Matthew Auld,
> > 
> > The patch 7ae034590cea: "drm/i915/ttm: add tt shmem backend" from Oct
> > 18, 2021, leads to the following Smatch static checker warning:
> > 
> > 	drivers/gpu/drm/i915/gem/i915_gem_ttm.c:335 i915_ttm_eviction_valuable()
> > 	warn: signedness bug returning '(-16)'
> > 
> > drivers/gpu/drm/i915/gem/i915_gem_ttm.c
> >      324 static bool i915_ttm_eviction_valuable(struct ttm_buffer_object *bo,
> >                 ^^^^
> > 
> >      325                                        const struct ttm_place *place)
> >      326 {
> >      327         struct drm_i915_gem_object *obj = i915_ttm_to_gem(bo);
> >      328
> >      329         /*
> >      330          * EXTERNAL objects should never be swapped out by TTM, instead we need
> >      331          * to handle that ourselves. TTM will already skip such objects for us,
> >      332          * but we would like to avoid grabbing locks for no good reason.
> >      333          */
> >      334         if (bo->ttm && bo->ttm->page_flags & TTM_TT_FLAG_EXTERNAL)
> > --> 335                 return -EBUSY;
> >                          ^^^^^^^^^^^^^^
> > This should be return false probably.
> 
> Oh, indeed. Did you also want to send a patch for that? Thanks.
> 

Sure.  Will do.

regards,
dan carpenter