[Intel-gfx] [PATCH] drm/i915/gem: Check function return in live_nop_switch

[Intel-gfx] [PATCH] drm/i915/gem: Check function return in live_nop_switch

[Oak Zeng]

Fail this test earlier if i915_request_await_dma_fence
fails.

Signed-off-by: Oak Zeng <oak.zeng@intel.com>
---
 drivers/gpu/drm/i915/gem/selftests/i915_gem_context.c | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/gem/selftests/i915_gem_context.c b/drivers/gpu/drm/i915/gem/selftests/i915_gem_context.c
index b32f7fed2d9c..c0b85e861014 100644
--- a/drivers/gpu/drm/i915/gem/selftests/i915_gem_context.c
+++ b/drivers/gpu/drm/i915/gem/selftests/i915_gem_context.c
@@ -82,7 +82,14 @@ static int live_nop_switch(void *arg)
 				goto out_file;
 			}
 			if (rq) {
-				i915_request_await_dma_fence(this, &rq->fence);
+				if (i915_request_await_dma_fence(this,
+					&rq->fence)) {
+					pr_err("Failed to populate %ld contexts\n", n);
+					intel_gt_set_wedged(to_gt(i915));
+					i915_request_put(rq);
+					err = -EIO;
+					goto out_file;
+				}
 				i915_request_put(rq);
 			}
 			rq = i915_request_get(this);
-- 
2.25.1

[Intel-gfx] ✗ Fi.CI.BUILD: failure for drm/i915/gem: Check function return in live_nop_switch

[Patchwork]

== Series Details ==

Series: drm/i915/gem: Check function return in live_nop_switch
URL   : https://patchwork.freedesktop.org/series/95584/
State : failure

== Summary ==

CALL    scripts/checksyscalls.sh
  CALL    scripts/atomic/check-atomics.sh
  DESCEND objtool
  CHK     include/generated/compile.h
  CC [M]  drivers/gpu/drm/i915/gem/i915_gem_context.o
In file included from drivers/gpu/drm/i915/gem/i915_gem_context.c:2283:
drivers/gpu/drm/i915/gem/selftests/i915_gem_context.c: In function ‘live_nop_switch’:
drivers/gpu/drm/i915/gem/selftests/i915_gem_context.c:88:26: error: implicit declaration of function ‘to_gt’; did you mean ‘uc_to_gt’? [-Werror=implicit-function-declaration]
      intel_gt_set_wedged(to_gt(i915));
                          ^~~~~
                          uc_to_gt
drivers/gpu/drm/i915/gem/selftests/i915_gem_context.c:88:26: error: passing argument 1 of ‘intel_gt_set_wedged’ makes pointer from integer without a cast [-Werror=int-conversion]
      intel_gt_set_wedged(to_gt(i915));
                          ^~~~~~~~~~~
In file included from ./drivers/gpu/drm/i915/gt/intel_gtt.h:28,
                 from ./drivers/gpu/drm/i915/gt/gen6_ppgtt.h:9,
                 from drivers/gpu/drm/i915/gem/i915_gem_context.c:72:
./drivers/gpu/drm/i915/gt/intel_reset.h:44:43: note: expected ‘struct intel_gt *’ but argument is of type ‘int’
 void intel_gt_set_wedged(struct intel_gt *gt);
                          ~~~~~~~~~~~~~~~~~^~
cc1: all warnings being treated as errors
scripts/Makefile.build:277: recipe for target 'drivers/gpu/drm/i915/gem/i915_gem_context.o' failed
make[4]: *** [drivers/gpu/drm/i915/gem/i915_gem_context.o] Error 1
scripts/Makefile.build:540: recipe for target 'drivers/gpu/drm/i915' failed
make[3]: *** [drivers/gpu/drm/i915] Error 2
scripts/Makefile.build:540: recipe for target 'drivers/gpu/drm' failed
make[2]: *** [drivers/gpu/drm] Error 2
scripts/Makefile.build:540: recipe for target 'drivers/gpu' failed
make[1]: *** [drivers/gpu] Error 2
Makefile:1868: recipe for target 'drivers' failed
make: *** [drivers] Error 2

Re: [Intel-gfx] [PATCH] drm/i915/gem: Check function return in live_nop_switch

[kernel test robot]

[-- Attachment #1: Type: text/plain, Size: 7512 bytes --]

Hi Oak,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on drm-intel/for-linux-next]
[also build test ERROR on drm-tip/drm-tip v5.15-rc4 next-20211007]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/Oak-Zeng/drm-i915-gem-Check-function-return-in-live_nop_switch/20211008-055247
base:   git://anongit.freedesktop.org/drm-intel for-linux-next
config: x86_64-randconfig-a016-20211004 (attached as .config)
compiler: gcc-9 (Debian 9.3.0-22) 9.3.0
reproduce (this is a W=1 build):
        # https://github.com/0day-ci/linux/commit/2d7a26805f7bf806f78efb6542a16f67ed8bc58c
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Oak-Zeng/drm-i915-gem-Check-function-return-in-live_nop_switch/20211008-055247
        git checkout 2d7a26805f7bf806f78efb6542a16f67ed8bc58c
        # save the attached .config to linux build tree
        make W=1 ARCH=x86_64 

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All error/warnings (new ones prefixed by >>):

   In file included from drivers/gpu/drm/i915/gem/i915_gem_context.c:2279:
   drivers/gpu/drm/i915/gem/selftests/i915_gem_context.c: In function 'live_nop_switch':
>> drivers/gpu/drm/i915/gem/selftests/i915_gem_context.c:94:26: error: implicit declaration of function 'to_gt'; did you mean 'uc_to_gt'? [-Werror=implicit-function-declaration]
      94 |      intel_gt_set_wedged(to_gt(i915));
         |                          ^~~~~
         |                          uc_to_gt
>> drivers/gpu/drm/i915/gem/selftests/i915_gem_context.c:94:26: warning: passing argument 1 of 'intel_gt_set_wedged' makes pointer from integer without a cast [-Wint-conversion]
      94 |      intel_gt_set_wedged(to_gt(i915));
         |                          ^~~~~~~~~~~
         |                          |
         |                          int
   In file included from drivers/gpu/drm/i915/gt/intel_gtt.h:28,
                    from drivers/gpu/drm/i915/gt/gen6_ppgtt.h:9,
                    from drivers/gpu/drm/i915/gem/i915_gem_context.c:72:
   drivers/gpu/drm/i915/gt/intel_reset.h:44:43: note: expected 'struct intel_gt *' but argument is of type 'int'
      44 | void intel_gt_set_wedged(struct intel_gt *gt);
         |                          ~~~~~~~~~~~~~~~~~^~
   cc1: some warnings being treated as errors


vim +94 drivers/gpu/drm/i915/gem/selftests/i915_gem_context.c

    35	
    36	static int live_nop_switch(void *arg)
    37	{
    38		const unsigned int nctx = 1024;
    39		struct drm_i915_private *i915 = arg;
    40		struct intel_engine_cs *engine;
    41		struct i915_gem_context **ctx;
    42		struct igt_live_test t;
    43		struct file *file;
    44		unsigned long n;
    45		int err = -ENODEV;
    46	
    47		/*
    48		 * Create as many contexts as we can feasibly get away with
    49		 * and check we can switch between them rapidly.
    50		 *
    51		 * Serves as very simple stress test for submission and HW switching
    52		 * between contexts.
    53		 */
    54	
    55		if (!DRIVER_CAPS(i915)->has_logical_contexts)
    56			return 0;
    57	
    58		file = mock_file(i915);
    59		if (IS_ERR(file))
    60			return PTR_ERR(file);
    61	
    62		ctx = kcalloc(nctx, sizeof(*ctx), GFP_KERNEL);
    63		if (!ctx) {
    64			err = -ENOMEM;
    65			goto out_file;
    66		}
    67	
    68		for (n = 0; n < nctx; n++) {
    69			ctx[n] = live_context(i915, file);
    70			if (IS_ERR(ctx[n])) {
    71				err = PTR_ERR(ctx[n]);
    72				goto out_file;
    73			}
    74		}
    75	
    76		for_each_uabi_engine(engine, i915) {
    77			struct i915_request *rq = NULL;
    78			unsigned long end_time, prime;
    79			ktime_t times[2] = {};
    80	
    81			times[0] = ktime_get_raw();
    82			for (n = 0; n < nctx; n++) {
    83				struct i915_request *this;
    84	
    85				this = igt_request_alloc(ctx[n], engine);
    86				if (IS_ERR(this)) {
    87					err = PTR_ERR(this);
    88					goto out_file;
    89				}
    90				if (rq) {
    91					if (i915_request_await_dma_fence(this,
    92						&rq->fence)) {
    93						pr_err("Failed to populate %ld contexts\n", n);
  > 94						intel_gt_set_wedged(to_gt(i915));
    95						i915_request_put(rq);
    96						err = -EIO;
    97						goto out_file;
    98					}
    99					i915_request_put(rq);
   100				}
   101				rq = i915_request_get(this);
   102				i915_request_add(this);
   103			}
   104			if (i915_request_wait(rq, 0, HZ / 5) < 0) {
   105				pr_err("Failed to populated %d contexts\n", nctx);
   106				intel_gt_set_wedged(&i915->gt);
   107				i915_request_put(rq);
   108				err = -EIO;
   109				goto out_file;
   110			}
   111			i915_request_put(rq);
   112	
   113			times[1] = ktime_get_raw();
   114	
   115			pr_info("Populated %d contexts on %s in %lluns\n",
   116				nctx, engine->name, ktime_to_ns(times[1] - times[0]));
   117	
   118			err = igt_live_test_begin(&t, i915, __func__, engine->name);
   119			if (err)
   120				goto out_file;
   121	
   122			end_time = jiffies + i915_selftest.timeout_jiffies;
   123			for_each_prime_number_from(prime, 2, 8192) {
   124				times[1] = ktime_get_raw();
   125	
   126				rq = NULL;
   127				for (n = 0; n < prime; n++) {
   128					struct i915_request *this;
   129	
   130					this = igt_request_alloc(ctx[n % nctx], engine);
   131					if (IS_ERR(this)) {
   132						err = PTR_ERR(this);
   133						goto out_file;
   134					}
   135	
   136					if (rq) { /* Force submission order */
   137						i915_request_await_dma_fence(this, &rq->fence);
   138						i915_request_put(rq);
   139					}
   140	
   141					/*
   142					 * This space is left intentionally blank.
   143					 *
   144					 * We do not actually want to perform any
   145					 * action with this request, we just want
   146					 * to measure the latency in allocation
   147					 * and submission of our breadcrumbs -
   148					 * ensuring that the bare request is sufficient
   149					 * for the system to work (i.e. proper HEAD
   150					 * tracking of the rings, interrupt handling,
   151					 * etc). It also gives us the lowest bounds
   152					 * for latency.
   153					 */
   154	
   155					rq = i915_request_get(this);
   156					i915_request_add(this);
   157				}
   158				GEM_BUG_ON(!rq);
   159				if (i915_request_wait(rq, 0, HZ / 5) < 0) {
   160					pr_err("Switching between %ld contexts timed out\n",
   161					       prime);
   162					intel_gt_set_wedged(&i915->gt);
   163					i915_request_put(rq);
   164					break;
   165				}
   166				i915_request_put(rq);
   167	
   168				times[1] = ktime_sub(ktime_get_raw(), times[1]);
   169				if (prime == 2)
   170					times[0] = times[1];
   171	
   172				if (__igt_timeout(end_time, NULL))
   173					break;
   174			}
   175	
   176			err = igt_live_test_end(&t);
   177			if (err)
   178				goto out_file;
   179	
   180			pr_info("Switch latencies on %s: 1 = %lluns, %lu = %lluns\n",
   181				engine->name,
   182				ktime_to_ns(times[0]),
   183				prime - 1, div64_u64(ktime_to_ns(times[1]), prime - 1));
   184		}
   185	
   186	out_file:
   187		fput(file);
   188		return err;
   189	}
   190	

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 40941 bytes --]

Re: [Intel-gfx] [PATCH] drm/i915/gem: Check function return in live_nop_switch

[kernel test robot]

[-- Attachment #1: Type: text/plain, Size: 7723 bytes --]

Hi Oak,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on drm-intel/for-linux-next]
[also build test ERROR on drm-tip/drm-tip v5.15-rc4 next-20211007]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/Oak-Zeng/drm-i915-gem-Check-function-return-in-live_nop_switch/20211008-055247
base:   git://anongit.freedesktop.org/drm-intel for-linux-next
config: x86_64-randconfig-a016-20211004 (attached as .config)
compiler: gcc-9 (Debian 9.3.0-22) 9.3.0
reproduce (this is a W=1 build):
        # https://github.com/0day-ci/linux/commit/2d7a26805f7bf806f78efb6542a16f67ed8bc58c
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Oak-Zeng/drm-i915-gem-Check-function-return-in-live_nop_switch/20211008-055247
        git checkout 2d7a26805f7bf806f78efb6542a16f67ed8bc58c
        # save the attached .config to linux build tree
        make W=1 ARCH=x86_64 

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All error/warnings (new ones prefixed by >>):

   In file included from drivers/gpu/drm/i915/gem/i915_gem_context.c:2279:
   drivers/gpu/drm/i915/gem/selftests/i915_gem_context.c: In function 'live_nop_switch':
>> drivers/gpu/drm/i915/gem/selftests/i915_gem_context.c:94:26: error: implicit declaration of function 'to_gt'; did you mean 'uc_to_gt'? [-Werror=implicit-function-declaration]
      94 |      intel_gt_set_wedged(to_gt(i915));
         |                          ^~~~~
         |                          uc_to_gt
>> drivers/gpu/drm/i915/gem/selftests/i915_gem_context.c:94:26: warning: passing argument 1 of 'intel_gt_set_wedged' makes pointer from integer without a cast [-Wint-conversion]
      94 |      intel_gt_set_wedged(to_gt(i915));
         |                          ^~~~~~~~~~~
         |                          |
         |                          int
   In file included from drivers/gpu/drm/i915/gt/intel_gtt.h:28,
                    from drivers/gpu/drm/i915/gt/gen6_ppgtt.h:9,
                    from drivers/gpu/drm/i915/gem/i915_gem_context.c:72:
   drivers/gpu/drm/i915/gt/intel_reset.h:44:43: note: expected 'struct intel_gt *' but argument is of type 'int'
      44 | void intel_gt_set_wedged(struct intel_gt *gt);
         |                          ~~~~~~~~~~~~~~~~~^~
   cc1: some warnings being treated as errors


vim +94 drivers/gpu/drm/i915/gem/selftests/i915_gem_context.c

    35	
    36	static int live_nop_switch(void *arg)
    37	{
    38		const unsigned int nctx = 1024;
    39		struct drm_i915_private *i915 = arg;
    40		struct intel_engine_cs *engine;
    41		struct i915_gem_context **ctx;
    42		struct igt_live_test t;
    43		struct file *file;
    44		unsigned long n;
    45		int err = -ENODEV;
    46	
    47		/*
    48		 * Create as many contexts as we can feasibly get away with
    49		 * and check we can switch between them rapidly.
    50		 *
    51		 * Serves as very simple stress test for submission and HW switching
    52		 * between contexts.
    53		 */
    54	
    55		if (!DRIVER_CAPS(i915)->has_logical_contexts)
    56			return 0;
    57	
    58		file = mock_file(i915);
    59		if (IS_ERR(file))
    60			return PTR_ERR(file);
    61	
    62		ctx = kcalloc(nctx, sizeof(*ctx), GFP_KERNEL);
    63		if (!ctx) {
    64			err = -ENOMEM;
    65			goto out_file;
    66		}
    67	
    68		for (n = 0; n < nctx; n++) {
    69			ctx[n] = live_context(i915, file);
    70			if (IS_ERR(ctx[n])) {
    71				err = PTR_ERR(ctx[n]);
    72				goto out_file;
    73			}
    74		}
    75	
    76		for_each_uabi_engine(engine, i915) {
    77			struct i915_request *rq = NULL;
    78			unsigned long end_time, prime;
    79			ktime_t times[2] = {};
    80	
    81			times[0] = ktime_get_raw();
    82			for (n = 0; n < nctx; n++) {
    83				struct i915_request *this;
    84	
    85				this = igt_request_alloc(ctx[n], engine);
    86				if (IS_ERR(this)) {
    87					err = PTR_ERR(this);
    88					goto out_file;
    89				}
    90				if (rq) {
    91					if (i915_request_await_dma_fence(this,
    92						&rq->fence)) {
    93						pr_err("Failed to populate %ld contexts\n", n);
  > 94						intel_gt_set_wedged(to_gt(i915));
    95						i915_request_put(rq);
    96						err = -EIO;
    97						goto out_file;
    98					}
    99					i915_request_put(rq);
   100				}
   101				rq = i915_request_get(this);
   102				i915_request_add(this);
   103			}
   104			if (i915_request_wait(rq, 0, HZ / 5) < 0) {
   105				pr_err("Failed to populated %d contexts\n", nctx);
   106				intel_gt_set_wedged(&i915->gt);
   107				i915_request_put(rq);
   108				err = -EIO;
   109				goto out_file;
   110			}
   111			i915_request_put(rq);
   112	
   113			times[1] = ktime_get_raw();
   114	
   115			pr_info("Populated %d contexts on %s in %lluns\n",
   116				nctx, engine->name, ktime_to_ns(times[1] - times[0]));
   117	
   118			err = igt_live_test_begin(&t, i915, __func__, engine->name);
   119			if (err)
   120				goto out_file;
   121	
   122			end_time = jiffies + i915_selftest.timeout_jiffies;
   123			for_each_prime_number_from(prime, 2, 8192) {
   124				times[1] = ktime_get_raw();
   125	
   126				rq = NULL;
   127				for (n = 0; n < prime; n++) {
   128					struct i915_request *this;
   129	
   130					this = igt_request_alloc(ctx[n % nctx], engine);
   131					if (IS_ERR(this)) {
   132						err = PTR_ERR(this);
   133						goto out_file;
   134					}
   135	
   136					if (rq) { /* Force submission order */
   137						i915_request_await_dma_fence(this, &rq->fence);
   138						i915_request_put(rq);
   139					}
   140	
   141					/*
   142					 * This space is left intentionally blank.
   143					 *
   144					 * We do not actually want to perform any
   145					 * action with this request, we just want
   146					 * to measure the latency in allocation
   147					 * and submission of our breadcrumbs -
   148					 * ensuring that the bare request is sufficient
   149					 * for the system to work (i.e. proper HEAD
   150					 * tracking of the rings, interrupt handling,
   151					 * etc). It also gives us the lowest bounds
   152					 * for latency.
   153					 */
   154	
   155					rq = i915_request_get(this);
   156					i915_request_add(this);
   157				}
   158				GEM_BUG_ON(!rq);
   159				if (i915_request_wait(rq, 0, HZ / 5) < 0) {
   160					pr_err("Switching between %ld contexts timed out\n",
   161					       prime);
   162					intel_gt_set_wedged(&i915->gt);
   163					i915_request_put(rq);
   164					break;
   165				}
   166				i915_request_put(rq);
   167	
   168				times[1] = ktime_sub(ktime_get_raw(), times[1]);
   169				if (prime == 2)
   170					times[0] = times[1];
   171	
   172				if (__igt_timeout(end_time, NULL))
   173					break;
   174			}
   175	
   176			err = igt_live_test_end(&t);
   177			if (err)
   178				goto out_file;
   179	
   180			pr_info("Switch latencies on %s: 1 = %lluns, %lu = %lluns\n",
   181				engine->name,
   182				ktime_to_ns(times[0]),
   183				prime - 1, div64_u64(ktime_to_ns(times[1]), prime - 1));
   184		}
   185	
   186	out_file:
   187		fput(file);
   188		return err;
   189	}
   190	

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all(a)lists.01.org

[-- Attachment #2: config.gz --]
[-- Type: application/gzip, Size: 40941 bytes --]

Re: [Intel-gfx] [PATCH] drm/i915/gem: Check function return in live_nop_switch

[Tvrtko Ursulin]

On 07/10/2021 23:55, Oak Zeng wrote:
> Fail this test earlier if i915_request_await_dma_fence
> fails.

Why only this instance and not the other one in the same function?

> Signed-off-by: Oak Zeng <oak.zeng@intel.com>
> ---
>   drivers/gpu/drm/i915/gem/selftests/i915_gem_context.c | 9 ++++++++-
>   1 file changed, 8 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/i915/gem/selftests/i915_gem_context.c b/drivers/gpu/drm/i915/gem/selftests/i915_gem_context.c
> index b32f7fed2d9c..c0b85e861014 100644
> --- a/drivers/gpu/drm/i915/gem/selftests/i915_gem_context.c
> +++ b/drivers/gpu/drm/i915/gem/selftests/i915_gem_context.c
> @@ -82,7 +82,14 @@ static int live_nop_switch(void *arg)
>   				goto out_file;
>   			}
>   			if (rq) {
> -				i915_request_await_dma_fence(this, &rq->fence);
> +				if (i915_request_await_dma_fence(this,
> +					&rq->fence)) {

err = i915_request_await_dma_fence...
if (err)

So you get the correct error and don't need to assign one below.

> +					pr_err("Failed to populate %ld contexts\n", n);

Might as well unique error message.

> +					intel_gt_set_wedged(to_gt(i915));

Check your base tree.

> +					i915_request_put(rq);
> +					err = -EIO;
> +					goto out_file;
> +				}
>   				i915_request_put(rq);
>   			}
>   			rq = i915_request_get(this);
> 

Regards,

Tvrtko