[bug report] drm/i915: Initial selftests for exercising eviction

[bug report] drm/i915: Initial selftests for exercising eviction

[Dan Carpenter]

Hello Chris Wilson,

The patch f40a7b7558ef: "drm/i915: Initial selftests for exercising
eviction" from Feb 13, 2017, leads to the following static checker
warning:

	drivers/gpu/drm/i915/selftests/i915_gem_evict.c:193 igt_overcommit()
	warn: passing a valid pointer to 'PTR_ERR'

drivers/gpu/drm/i915/selftests/i915_gem_evict.c
    167 static int igt_overcommit(void *arg)
    168 {
    169 	struct drm_i915_private *i915 = arg;
    170 	struct drm_i915_gem_object *obj;
    171 	struct i915_vma *vma;
    172 	LIST_HEAD(objects);
    173 	int err;
    174 
    175 	/* Fill the GGTT with pinned objects and then try to pin one more.
    176 	 * We expect it to fail.
    177 	 */
    178 
    179 	err = populate_ggtt(i915, &objects);
    180 	if (err)
    181 		goto cleanup;
    182 
    183 	obj = i915_gem_object_create_internal(i915, I915_GTT_PAGE_SIZE);
    184 	if (IS_ERR(obj)) {
    185 		err = PTR_ERR(obj);
    186 		goto cleanup;
    187 	}
    188 
    189 	quirk_add(obj, &objects);
    190 
    191 	vma = i915_gem_object_ggtt_pin(obj, NULL, 0, 0, 0);
    192 	if (!IS_ERR(vma) || PTR_ERR(vma) != -ENOSPC) {
                    ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
This condition doesn't make sense.  It's equivalent to:

		if (PTR_ERR(vma) != -ENOSPC) {

Maybe what was intended was:

		if (IS_ERR(vma) && PTR_ERR(vma) != -ENOSPC) {

I don't know.

--> 193 		pr_err("Failed to evict+insert, i915_gem_object_ggtt_pin returned err=%d\n", (int)PTR_ERR(vma));
    194 		err = -EINVAL;
    195 		goto cleanup;
    196 	}
    197 
    198 cleanup:
    199 	cleanup_objects(i915, &objects);
    200 	return err;
    201 }

regards,
dan carpenter
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [bug report] drm/i915: Initial selftests for exercising eviction

[Chris Wilson]

Quoting Dan Carpenter (2019-11-14 06:54:37)
> Hello Chris Wilson,
> 
> The patch f40a7b7558ef: "drm/i915: Initial selftests for exercising
> eviction" from Feb 13, 2017, leads to the following static checker
> warning:
> 
>         drivers/gpu/drm/i915/selftests/i915_gem_evict.c:202 igt_overcommit()
>         warn: passing zero to 'PTR_ERR'
> 
> drivers/gpu/drm/i915/selftests/i915_gem_evict.c
>    190                  goto cleanup;
>    191  
>    192          obj = i915_gem_object_create_internal(gt->i915, I915_GTT_PAGE_SIZE);
>    193          if (IS_ERR(obj)) {
>    194                  err = PTR_ERR(obj);
>    195                  goto cleanup;
>    196          }
>    197  
>    198          quirk_add(obj, &objects);
>    199  
>    200          vma = i915_gem_object_ggtt_pin(obj, NULL, 0, 0, 0);
>    201          if (!IS_ERR(vma) || PTR_ERR(vma) != -ENOSPC) {
>                     ^^^^^^^^^^^^
> Is this reversed?

No, the setup is supposed to force the pin() to fail with ENOSPC.

if (vma != ERR_PTR(-ENOSPC))

would be a clearer way to write it.
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[bug report] drm/i915: Initial selftests for exercising eviction

[Dan Carpenter]

Hello Chris Wilson,

The patch f40a7b7558ef: "drm/i915: Initial selftests for exercising
eviction" from Feb 13, 2017, leads to the following static checker
warning:

	drivers/gpu/drm/i915/selftests/i915_gem_evict.c:202 igt_overcommit()
	warn: passing zero to 'PTR_ERR'

drivers/gpu/drm/i915/selftests/i915_gem_evict.c
   190                  goto cleanup;
   191  
   192          obj = i915_gem_object_create_internal(gt->i915, I915_GTT_PAGE_SIZE);
   193          if (IS_ERR(obj)) {
   194                  err = PTR_ERR(obj);
   195                  goto cleanup;
   196          }
   197  
   198          quirk_add(obj, &objects);
   199  
   200          vma = i915_gem_object_ggtt_pin(obj, NULL, 0, 0, 0);
   201          if (!IS_ERR(vma) || PTR_ERR(vma) != -ENOSPC) {
                    ^^^^^^^^^^^^
Is this reversed?

   202                  pr_err("Failed to evict+insert, i915_gem_object_ggtt_pin returned err=%d\n", (int)PTR_ERR(vma));
                                                                                                     ^^^^^^^^^^^^^^^^^

   203                  err = -EINVAL;
   204                  goto cleanup;
   205          }
   206  
   207  cleanup:
   208          cleanup_objects(ggtt, &objects);
   209          return err;
   210  }


regards,
dan carpenter
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx