Igt-dev Archive on lore.kernel.org
 help / color / mirror / Atom feed
* [igt-dev] [PATCH i-g-t] lib/igt_dummyload: Drop ahnd from igt_spin_t
@ 2022-02-24 22:02 Ashutosh Dixit
  2022-02-24 23:09 ` [igt-dev] ✗ Fi.CI.BAT: failure for " Patchwork
  2022-02-25 11:29 ` [igt-dev] [PATCH i-g-t] " Zbigniew Kempczyński
  0 siblings, 2 replies; 5+ messages in thread
From: Ashutosh Dixit @ 2022-02-24 22:02 UTC (permalink / raw)
  To: igt-dev; +Cc: jasmine.newsome

In 4d9396e67930 we have started storing the opts with which the spin was
created as part of igt_spin_t. The ahnd stored as part of igt_spin_t is
therefore redundant. We can get ahnd from opts.ahnd.

Cc: Zbigniew Kempczynski <zbigniew.kempczynski@intel.com>
Cc: Jasmine Newsome <jasmine.newsome@intel.com>
Signed-off-by: Ashutosh Dixit <ashutosh.dixit@intel.com>
---
 lib/igt_dummyload.c              | 9 ++++-----
 lib/igt_dummyload.h              | 1 -
 tests/i915/gem_ctx_persistence.c | 2 +-
 tests/i915/gem_ctx_shared.c      | 2 +-
 tests/i915/gem_exec_balancer.c   | 2 +-
 tests/i915/gem_exec_schedule.c   | 6 +++---
 tests/i915/gem_spin_batch.c      | 2 +-
 tests/i915/gem_watchdog.c        | 2 +-
 tests/i915/i915_hangman.c        | 2 +-
 9 files changed, 13 insertions(+), 15 deletions(-)

diff --git a/lib/igt_dummyload.c b/lib/igt_dummyload.c
index 0b2be154dd97..dc1bd51e0823 100644
--- a/lib/igt_dummyload.c
+++ b/lib/igt_dummyload.c
@@ -128,7 +128,6 @@ emit_recursive_batch(igt_spin_t *spin,
 		addr += random() % addr / 2;
 		addr &= -4096;
 	} else {
-		spin->ahnd = ahnd;
 		objflags |= EXEC_OBJECT_PINNED;
 	}
 
@@ -612,14 +611,14 @@ static void __igt_spin_free(int fd, igt_spin_t *spin)
 
 	if (spin->poll_handle) {
 		gem_close(fd, spin->poll_handle);
-		if (spin->ahnd)
-			intel_allocator_free(spin->ahnd, spin->poll_handle);
+		if (spin->opts.ahnd)
+			intel_allocator_free(spin->opts.ahnd, spin->poll_handle);
 	}
 
 	if (spin->handle) {
 		gem_close(fd, spin->handle);
-		if (spin->ahnd)
-			intel_allocator_free(spin->ahnd, spin->handle);
+		if (spin->opts.ahnd)
+			intel_allocator_free(spin->opts.ahnd, spin->handle);
 	}
 
 	if (spin->out_fence >= 0)
diff --git a/lib/igt_dummyload.h b/lib/igt_dummyload.h
index af9e6a435dc3..b33507971b65 100644
--- a/lib/igt_dummyload.h
+++ b/lib/igt_dummyload.h
@@ -81,7 +81,6 @@ typedef struct igt_spin {
 	unsigned int flags;
 #define SPIN_CLFLUSH (1 << 0)
 
-	uint64_t ahnd;
 	struct igt_spin_factory opts;
 } igt_spin_t;
 
diff --git a/tests/i915/gem_ctx_persistence.c b/tests/i915/gem_ctx_persistence.c
index 9312aec302a5..00dda3a8b52d 100644
--- a/tests/i915/gem_ctx_persistence.c
+++ b/tests/i915/gem_ctx_persistence.c
@@ -524,7 +524,7 @@ static void test_noheartbeat_many(int i915, int count, unsigned int flags)
 		}
 
 		for (int n = 0; n < ARRAY_SIZE(spin); n++) {
-			ahnd = spin[n]->ahnd;
+			ahnd = spin[n]->opts.ahnd;
 			igt_spin_free(i915, spin[n]);
 			put_ahnd(ahnd);
 		}
diff --git a/tests/i915/gem_ctx_shared.c b/tests/i915/gem_ctx_shared.c
index 37444185649c..cc547b87b065 100644
--- a/tests/i915/gem_ctx_shared.c
+++ b/tests/i915/gem_ctx_shared.c
@@ -628,7 +628,7 @@ static void unplug_show_queue(int i915, struct igt_cork *c, uint64_t ahnd,
 	usleep(25000);
 
 	for (int n = 0; n < ARRAY_SIZE(spin); n++) {
-		ahnd = spin[n]->ahnd;
+		ahnd = spin[n]->opts.ahnd;
 		igt_spin_free(i915, spin[n]);
 		if (!cfg->vm)
 			put_ahnd(ahnd);
diff --git a/tests/i915/gem_exec_balancer.c b/tests/i915/gem_exec_balancer.c
index 42fd0a5220bf..857d008563da 100644
--- a/tests/i915/gem_exec_balancer.c
+++ b/tests/i915/gem_exec_balancer.c
@@ -2387,7 +2387,7 @@ static void hangme(int i915)
 			igt_assert_eq(sync_fence_status(c->spin[1]->out_fence),
 				      -EIO);
 
-			ahnd = c->spin[0]->ahnd;
+			ahnd = c->spin[0]->opts.ahnd;
 			igt_spin_free(i915, c->spin[0]);
 			igt_spin_free(i915, c->spin[1]);
 			put_ahnd(ahnd);
diff --git a/tests/i915/gem_exec_schedule.c b/tests/i915/gem_exec_schedule.c
index 119ec2d4369d..cf7e4d4eb3b6 100644
--- a/tests/i915/gem_exec_schedule.c
+++ b/tests/i915/gem_exec_schedule.c
@@ -252,7 +252,7 @@ static void unplug_show_queue(int fd, struct igt_cork *c,
 	usleep(25000);
 
 	for (int n = 0; n < max; n++) {
-		uint64_t ahnd = spin[n]->ahnd;
+		uint64_t ahnd = spin[n]->opts.ahnd;
 		igt_spin_free(fd, spin[n]);
 		put_ahnd(ahnd);
 	}
@@ -1042,7 +1042,7 @@ static void semaphore_codependency(int i915, const intel_ctx_t *ctx,
 	}
 
 	for (i = 0; i < ARRAY_SIZE(task); i++) {
-		ahnd = task[i].rcs->ahnd;
+		ahnd = task[i].rcs->opts.ahnd;
 		igt_spin_free(i915, task[i].xcs);
 		igt_spin_free(i915, task[i].rcs);
 		put_ahnd(ahnd);
@@ -1994,7 +1994,7 @@ static void preemptive_hang(int fd, const intel_ctx_cfg_t *cfg,
 		 * This is subject to change as the scheduler evolve. The test should
 		 * be updated to reflect such changes.
 		 */
-		ahnd_lo = spin[n]->ahnd;
+		ahnd_lo = spin[n]->opts.ahnd;
 		igt_assert(gem_bo_busy(fd, spin[n]->handle));
 		igt_spin_free(fd, spin[n]);
 		put_ahnd(ahnd_lo);
diff --git a/tests/i915/gem_spin_batch.c b/tests/i915/gem_spin_batch.c
index 707d69b6f497..9b39bfc78620 100644
--- a/tests/i915/gem_spin_batch.c
+++ b/tests/i915/gem_spin_batch.c
@@ -168,7 +168,7 @@ static void spin_all(int i915, const intel_ctx_t *ctx, unsigned int flags)
 
 	igt_list_for_each_entry_safe(spin, n, &list, link) {
 		igt_assert(gem_bo_busy(i915, spin->handle));
-		ahnd = spin->ahnd;
+		ahnd = spin->opts.ahnd;
 		igt_spin_end(spin);
 		if (flags & PARALLEL_SPIN_NEW_CTX)
 			intel_ctx_destroy(i915, spin->opts.ctx);
diff --git a/tests/i915/gem_watchdog.c b/tests/i915/gem_watchdog.c
index a9d7f9da7feb..fc1ba007128c 100644
--- a/tests/i915/gem_watchdog.c
+++ b/tests/i915/gem_watchdog.c
@@ -261,7 +261,7 @@ static void virtual(int i915, const intel_ctx_cfg_t *base_cfg)
 	count = wait_timeout(i915, spin, num_engines, wait_us, expect);
 
 	for (i = 0; i < num_engines && spin[i]; i++) {
-		ahnd = spin[i]->ahnd;
+		ahnd = spin[i]->opts.ahnd;
 		igt_spin_free(i915, spin[i]);
 		intel_ctx_destroy(i915, ctx[i]);
 		put_ahnd(ahnd);
diff --git a/tests/i915/i915_hangman.c b/tests/i915/i915_hangman.c
index 23055c2715f1..c7d69fdd69b8 100644
--- a/tests/i915/i915_hangman.c
+++ b/tests/i915/i915_hangman.c
@@ -346,7 +346,7 @@ test_engine_hang(const intel_ctx_t *ctx,
 
 	/* But no other engines/clients should be affected */
 	igt_list_for_each_entry_safe(spin, next, &list, link) {
-		ahndN = spin->ahnd;
+		ahndN = spin->opts.ahnd;
 		igt_assert(sync_fence_wait(spin->out_fence, 0) == -ETIME);
 		igt_spin_end(spin);
 
-- 
2.34.1

^ permalink raw reply related	[flat|nested] 5+ messages in thread

* [igt-dev] ✗ Fi.CI.BAT: failure for lib/igt_dummyload: Drop ahnd from igt_spin_t
  2022-02-24 22:02 [igt-dev] [PATCH i-g-t] lib/igt_dummyload: Drop ahnd from igt_spin_t Ashutosh Dixit
@ 2022-02-24 23:09 ` Patchwork
  2022-02-25 11:29 ` [igt-dev] [PATCH i-g-t] " Zbigniew Kempczyński
  1 sibling, 0 replies; 5+ messages in thread
From: Patchwork @ 2022-02-24 23:09 UTC (permalink / raw)
  To: Ashutosh Dixit; +Cc: igt-dev

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

== Series Details ==

Series: lib/igt_dummyload: Drop ahnd from igt_spin_t
URL   : https://patchwork.freedesktop.org/series/100703/
State : failure

== Summary ==

CI Bug Log - changes from CI_DRM_11283 -> IGTPW_6701
====================================================

Summary
-------

  **FAILURE**

  Serious unknown changes coming with IGTPW_6701 absolutely need to be
  verified manually.
  
  If you think the reported changes have nothing to do with the changes
  introduced in IGTPW_6701, please notify your bug team to allow them
  to document this new failure mode, which will reduce false positives in CI.

  External URL: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_6701/index.html

Participating hosts (42 -> 42)
------------------------------

  Additional (2): fi-cml-u2 fi-pnv-d510 
  Missing    (2): fi-bsw-cyan fi-bdw-samus 

Possible new issues
-------------------

  Here are the unknown changes that may have been introduced in IGTPW_6701:

### IGT changes ###

#### Possible regressions ####

  * igt@i915_selftest@live@gt_heartbeat:
    - fi-bsw-nick:        [PASS][1] -> [INCOMPLETE][2]
   [1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11283/fi-bsw-nick/igt@i915_selftest@live@gt_heartbeat.html
   [2]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_6701/fi-bsw-nick/igt@i915_selftest@live@gt_heartbeat.html

  
Known issues
------------

  Here are the changes found in IGTPW_6701 that come from known issues:

### IGT changes ###

#### Issues hit ####

  * igt@amdgpu/amd_basic@memory-alloc:
    - fi-cml-u2:          NOTRUN -> [SKIP][3] ([fdo#109315]) +17 similar issues
   [3]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_6701/fi-cml-u2/igt@amdgpu/amd_basic@memory-alloc.html

  * igt@gem_exec_fence@basic-busy@bcs0:
    - fi-cml-u2:          NOTRUN -> [SKIP][4] ([i915#1208]) +1 similar issue
   [4]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_6701/fi-cml-u2/igt@gem_exec_fence@basic-busy@bcs0.html

  * igt@gem_flink_basic@bad-flink:
    - fi-skl-6600u:       [PASS][5] -> [INCOMPLETE][6] ([i915#4547])
   [5]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11283/fi-skl-6600u/igt@gem_flink_basic@bad-flink.html
   [6]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_6701/fi-skl-6600u/igt@gem_flink_basic@bad-flink.html

  * igt@gem_huc_copy@huc-copy:
    - fi-cml-u2:          NOTRUN -> [SKIP][7] ([i915#2190])
   [7]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_6701/fi-cml-u2/igt@gem_huc_copy@huc-copy.html

  * igt@gem_lmem_swapping@parallel-random-engines:
    - fi-cml-u2:          NOTRUN -> [SKIP][8] ([i915#4613]) +3 similar issues
   [8]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_6701/fi-cml-u2/igt@gem_lmem_swapping@parallel-random-engines.html

  * igt@i915_selftest@live@gt_heartbeat:
    - fi-skl-guc:         [PASS][9] -> [DMESG-FAIL][10] ([i915#2291] / [i915#541])
   [9]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11283/fi-skl-guc/igt@i915_selftest@live@gt_heartbeat.html
   [10]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_6701/fi-skl-guc/igt@i915_selftest@live@gt_heartbeat.html

  * igt@i915_selftest@live@hangcheck:
    - fi-snb-2600:        [PASS][11] -> [INCOMPLETE][12] ([i915#3921])
   [11]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11283/fi-snb-2600/igt@i915_selftest@live@hangcheck.html
   [12]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_6701/fi-snb-2600/igt@i915_selftest@live@hangcheck.html

  * igt@kms_chamelium@dp-hpd-fast:
    - fi-cml-u2:          NOTRUN -> [SKIP][13] ([fdo#109284] / [fdo#111827]) +8 similar issues
   [13]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_6701/fi-cml-u2/igt@kms_chamelium@dp-hpd-fast.html

  * igt@kms_cursor_legacy@basic-busy-flip-before-cursor-atomic:
    - fi-cml-u2:          NOTRUN -> [SKIP][14] ([fdo#109278]) +1 similar issue
   [14]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_6701/fi-cml-u2/igt@kms_cursor_legacy@basic-busy-flip-before-cursor-atomic.html

  * igt@kms_force_connector_basic@force-load-detect:
    - fi-cml-u2:          NOTRUN -> [SKIP][15] ([fdo#109285])
   [15]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_6701/fi-cml-u2/igt@kms_force_connector_basic@force-load-detect.html

  * igt@kms_pipe_crc_basic@compare-crc-sanitycheck-pipe-d:
    - fi-cml-u2:          NOTRUN -> [SKIP][16] ([fdo#109278] / [i915#533])
   [16]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_6701/fi-cml-u2/igt@kms_pipe_crc_basic@compare-crc-sanitycheck-pipe-d.html

  * igt@prime_vgem@basic-userptr:
    - fi-pnv-d510:        NOTRUN -> [SKIP][17] ([fdo#109271]) +57 similar issues
   [17]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_6701/fi-pnv-d510/igt@prime_vgem@basic-userptr.html
    - fi-cml-u2:          NOTRUN -> [SKIP][18] ([i915#3301])
   [18]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_6701/fi-cml-u2/igt@prime_vgem@basic-userptr.html

  * igt@runner@aborted:
    - fi-skl-6600u:       NOTRUN -> [FAIL][19] ([i915#2722] / [i915#4312])
   [19]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_6701/fi-skl-6600u/igt@runner@aborted.html
    - fi-bsw-nick:        NOTRUN -> [FAIL][20] ([fdo#109271] / [i915#1436] / [i915#3428] / [i915#3690] / [i915#4312])
   [20]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_6701/fi-bsw-nick/igt@runner@aborted.html

  
#### Possible fixes ####

  * igt@kms_busy@basic@flip:
    - {bat-adlp-6}:       [DMESG-WARN][21] ([i915#3576]) -> [PASS][22] +1 similar issue
   [21]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11283/bat-adlp-6/igt@kms_busy@basic@flip.html
   [22]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_6701/bat-adlp-6/igt@kms_busy@basic@flip.html

  
#### Warnings ####

  * igt@i915_selftest@live@hangcheck:
    - bat-dg1-5:          [DMESG-FAIL][23] ([i915#4494] / [i915#4957]) -> [DMESG-FAIL][24] ([i915#4957])
   [23]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11283/bat-dg1-5/igt@i915_selftest@live@hangcheck.html
   [24]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_6701/bat-dg1-5/igt@i915_selftest@live@hangcheck.html

  
  {name}: This element is suppressed. This means it is ignored when computing
          the status of the difference (SUCCESS, WARNING, or FAILURE).

  [fdo#109271]: https://bugs.freedesktop.org/show_bug.cgi?id=109271
  [fdo#109278]: https://bugs.freedesktop.org/show_bug.cgi?id=109278
  [fdo#109284]: https://bugs.freedesktop.org/show_bug.cgi?id=109284
  [fdo#109285]: https://bugs.freedesktop.org/show_bug.cgi?id=109285
  [fdo#109315]: https://bugs.freedesktop.org/show_bug.cgi?id=109315
  [fdo#111827]: https://bugs.freedesktop.org/show_bug.cgi?id=111827
  [i915#1208]: https://gitlab.freedesktop.org/drm/intel/issues/1208
  [i915#1436]: https://gitlab.freedesktop.org/drm/intel/issues/1436
  [i915#2190]: https://gitlab.freedesktop.org/drm/intel/issues/2190
  [i915#2291]: https://gitlab.freedesktop.org/drm/intel/issues/2291
  [i915#2722]: https://gitlab.freedesktop.org/drm/intel/issues/2722
  [i915#3301]: https://gitlab.freedesktop.org/drm/intel/issues/3301
  [i915#3428]: https://gitlab.freedesktop.org/drm/intel/issues/3428
  [i915#3576]: https://gitlab.freedesktop.org/drm/intel/issues/3576
  [i915#3690]: https://gitlab.freedesktop.org/drm/intel/issues/3690
  [i915#3921]: https://gitlab.freedesktop.org/drm/intel/issues/3921
  [i915#4103]: https://gitlab.freedesktop.org/drm/intel/issues/4103
  [i915#4312]: https://gitlab.freedesktop.org/drm/intel/issues/4312
  [i915#4494]: https://gitlab.freedesktop.org/drm/intel/issues/4494
  [i915#4547]: https://gitlab.freedesktop.org/drm/intel/issues/4547
  [i915#4613]: https://gitlab.freedesktop.org/drm/intel/issues/4613
  [i915#4957]: https://gitlab.freedesktop.org/drm/intel/issues/4957
  [i915#533]: https://gitlab.freedesktop.org/drm/intel/issues/533
  [i915#541]: https://gitlab.freedesktop.org/drm/intel/issues/541


Build changes
-------------

  * CI: CI-20190529 -> None
  * IGT: IGT_6357 -> IGTPW_6701

  CI-20190529: 20190529
  CI_DRM_11283: 5d17a0d32cc09a2f2b20d2aeb1cadf7f2d6e13d2 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGTPW_6701: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_6701/index.html
  IGT_6357: 6546304ecf053b9c5ec278ee3c210d2c6d50a3a6 @ https://gitlab.freedesktop.org/drm/igt-gpu-tools.git

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_6701/index.html

[-- Attachment #2: Type: text/html, Size: 9467 bytes --]

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [igt-dev] [PATCH i-g-t] lib/igt_dummyload: Drop ahnd from igt_spin_t
  2022-02-24 22:02 [igt-dev] [PATCH i-g-t] lib/igt_dummyload: Drop ahnd from igt_spin_t Ashutosh Dixit
  2022-02-24 23:09 ` [igt-dev] ✗ Fi.CI.BAT: failure for " Patchwork
@ 2022-02-25 11:29 ` Zbigniew Kempczyński
  2022-02-25 22:18   ` Dixit, Ashutosh
  1 sibling, 1 reply; 5+ messages in thread
From: Zbigniew Kempczyński @ 2022-02-25 11:29 UTC (permalink / raw)
  To: Ashutosh Dixit; +Cc: igt-dev, jasmine.newsome

On Thu, Feb 24, 2022 at 02:02:29PM -0800, Ashutosh Dixit wrote:
> In 4d9396e67930 we have started storing the opts with which the spin was
> created as part of igt_spin_t. The ahnd stored as part of igt_spin_t is
> therefore redundant. We can get ahnd from opts.ahnd.

More than redundancy I like to keep opts which comes from the caller
and thus suggests spinner code that's immune. Unfortunatelly we cannot
block this after memcmp using c-syntax (I'm not able to do this).
Anyway, keeping caller vars in opts suggests this so:

Reviewed-by: Zbigniew Kempczyński <zbigniew.kempczynski@intel.com>

--
Zbigniew

> 
> Cc: Zbigniew Kempczynski <zbigniew.kempczynski@intel.com>
> Cc: Jasmine Newsome <jasmine.newsome@intel.com>
> Signed-off-by: Ashutosh Dixit <ashutosh.dixit@intel.com>
> ---
>  lib/igt_dummyload.c              | 9 ++++-----
>  lib/igt_dummyload.h              | 1 -
>  tests/i915/gem_ctx_persistence.c | 2 +-
>  tests/i915/gem_ctx_shared.c      | 2 +-
>  tests/i915/gem_exec_balancer.c   | 2 +-
>  tests/i915/gem_exec_schedule.c   | 6 +++---
>  tests/i915/gem_spin_batch.c      | 2 +-
>  tests/i915/gem_watchdog.c        | 2 +-
>  tests/i915/i915_hangman.c        | 2 +-
>  9 files changed, 13 insertions(+), 15 deletions(-)
> 
> diff --git a/lib/igt_dummyload.c b/lib/igt_dummyload.c
> index 0b2be154dd97..dc1bd51e0823 100644
> --- a/lib/igt_dummyload.c
> +++ b/lib/igt_dummyload.c
> @@ -128,7 +128,6 @@ emit_recursive_batch(igt_spin_t *spin,
>  		addr += random() % addr / 2;
>  		addr &= -4096;
>  	} else {
> -		spin->ahnd = ahnd;
>  		objflags |= EXEC_OBJECT_PINNED;
>  	}
>  
> @@ -612,14 +611,14 @@ static void __igt_spin_free(int fd, igt_spin_t *spin)
>  
>  	if (spin->poll_handle) {
>  		gem_close(fd, spin->poll_handle);
> -		if (spin->ahnd)
> -			intel_allocator_free(spin->ahnd, spin->poll_handle);
> +		if (spin->opts.ahnd)
> +			intel_allocator_free(spin->opts.ahnd, spin->poll_handle);
>  	}
>  
>  	if (spin->handle) {
>  		gem_close(fd, spin->handle);
> -		if (spin->ahnd)
> -			intel_allocator_free(spin->ahnd, spin->handle);
> +		if (spin->opts.ahnd)
> +			intel_allocator_free(spin->opts.ahnd, spin->handle);
>  	}
>  
>  	if (spin->out_fence >= 0)
> diff --git a/lib/igt_dummyload.h b/lib/igt_dummyload.h
> index af9e6a435dc3..b33507971b65 100644
> --- a/lib/igt_dummyload.h
> +++ b/lib/igt_dummyload.h
> @@ -81,7 +81,6 @@ typedef struct igt_spin {
>  	unsigned int flags;
>  #define SPIN_CLFLUSH (1 << 0)
>  
> -	uint64_t ahnd;
>  	struct igt_spin_factory opts;
>  } igt_spin_t;
>  
> diff --git a/tests/i915/gem_ctx_persistence.c b/tests/i915/gem_ctx_persistence.c
> index 9312aec302a5..00dda3a8b52d 100644
> --- a/tests/i915/gem_ctx_persistence.c
> +++ b/tests/i915/gem_ctx_persistence.c
> @@ -524,7 +524,7 @@ static void test_noheartbeat_many(int i915, int count, unsigned int flags)
>  		}
>  
>  		for (int n = 0; n < ARRAY_SIZE(spin); n++) {
> -			ahnd = spin[n]->ahnd;
> +			ahnd = spin[n]->opts.ahnd;
>  			igt_spin_free(i915, spin[n]);
>  			put_ahnd(ahnd);
>  		}
> diff --git a/tests/i915/gem_ctx_shared.c b/tests/i915/gem_ctx_shared.c
> index 37444185649c..cc547b87b065 100644
> --- a/tests/i915/gem_ctx_shared.c
> +++ b/tests/i915/gem_ctx_shared.c
> @@ -628,7 +628,7 @@ static void unplug_show_queue(int i915, struct igt_cork *c, uint64_t ahnd,
>  	usleep(25000);
>  
>  	for (int n = 0; n < ARRAY_SIZE(spin); n++) {
> -		ahnd = spin[n]->ahnd;
> +		ahnd = spin[n]->opts.ahnd;
>  		igt_spin_free(i915, spin[n]);
>  		if (!cfg->vm)
>  			put_ahnd(ahnd);
> diff --git a/tests/i915/gem_exec_balancer.c b/tests/i915/gem_exec_balancer.c
> index 42fd0a5220bf..857d008563da 100644
> --- a/tests/i915/gem_exec_balancer.c
> +++ b/tests/i915/gem_exec_balancer.c
> @@ -2387,7 +2387,7 @@ static void hangme(int i915)
>  			igt_assert_eq(sync_fence_status(c->spin[1]->out_fence),
>  				      -EIO);
>  
> -			ahnd = c->spin[0]->ahnd;
> +			ahnd = c->spin[0]->opts.ahnd;
>  			igt_spin_free(i915, c->spin[0]);
>  			igt_spin_free(i915, c->spin[1]);
>  			put_ahnd(ahnd);
> diff --git a/tests/i915/gem_exec_schedule.c b/tests/i915/gem_exec_schedule.c
> index 119ec2d4369d..cf7e4d4eb3b6 100644
> --- a/tests/i915/gem_exec_schedule.c
> +++ b/tests/i915/gem_exec_schedule.c
> @@ -252,7 +252,7 @@ static void unplug_show_queue(int fd, struct igt_cork *c,
>  	usleep(25000);
>  
>  	for (int n = 0; n < max; n++) {
> -		uint64_t ahnd = spin[n]->ahnd;
> +		uint64_t ahnd = spin[n]->opts.ahnd;
>  		igt_spin_free(fd, spin[n]);
>  		put_ahnd(ahnd);
>  	}
> @@ -1042,7 +1042,7 @@ static void semaphore_codependency(int i915, const intel_ctx_t *ctx,
>  	}
>  
>  	for (i = 0; i < ARRAY_SIZE(task); i++) {
> -		ahnd = task[i].rcs->ahnd;
> +		ahnd = task[i].rcs->opts.ahnd;
>  		igt_spin_free(i915, task[i].xcs);
>  		igt_spin_free(i915, task[i].rcs);
>  		put_ahnd(ahnd);
> @@ -1994,7 +1994,7 @@ static void preemptive_hang(int fd, const intel_ctx_cfg_t *cfg,
>  		 * This is subject to change as the scheduler evolve. The test should
>  		 * be updated to reflect such changes.
>  		 */
> -		ahnd_lo = spin[n]->ahnd;
> +		ahnd_lo = spin[n]->opts.ahnd;
>  		igt_assert(gem_bo_busy(fd, spin[n]->handle));
>  		igt_spin_free(fd, spin[n]);
>  		put_ahnd(ahnd_lo);
> diff --git a/tests/i915/gem_spin_batch.c b/tests/i915/gem_spin_batch.c
> index 707d69b6f497..9b39bfc78620 100644
> --- a/tests/i915/gem_spin_batch.c
> +++ b/tests/i915/gem_spin_batch.c
> @@ -168,7 +168,7 @@ static void spin_all(int i915, const intel_ctx_t *ctx, unsigned int flags)
>  
>  	igt_list_for_each_entry_safe(spin, n, &list, link) {
>  		igt_assert(gem_bo_busy(i915, spin->handle));
> -		ahnd = spin->ahnd;
> +		ahnd = spin->opts.ahnd;
>  		igt_spin_end(spin);
>  		if (flags & PARALLEL_SPIN_NEW_CTX)
>  			intel_ctx_destroy(i915, spin->opts.ctx);
> diff --git a/tests/i915/gem_watchdog.c b/tests/i915/gem_watchdog.c
> index a9d7f9da7feb..fc1ba007128c 100644
> --- a/tests/i915/gem_watchdog.c
> +++ b/tests/i915/gem_watchdog.c
> @@ -261,7 +261,7 @@ static void virtual(int i915, const intel_ctx_cfg_t *base_cfg)
>  	count = wait_timeout(i915, spin, num_engines, wait_us, expect);
>  
>  	for (i = 0; i < num_engines && spin[i]; i++) {
> -		ahnd = spin[i]->ahnd;
> +		ahnd = spin[i]->opts.ahnd;
>  		igt_spin_free(i915, spin[i]);
>  		intel_ctx_destroy(i915, ctx[i]);
>  		put_ahnd(ahnd);
> diff --git a/tests/i915/i915_hangman.c b/tests/i915/i915_hangman.c
> index 23055c2715f1..c7d69fdd69b8 100644
> --- a/tests/i915/i915_hangman.c
> +++ b/tests/i915/i915_hangman.c
> @@ -346,7 +346,7 @@ test_engine_hang(const intel_ctx_t *ctx,
>  
>  	/* But no other engines/clients should be affected */
>  	igt_list_for_each_entry_safe(spin, next, &list, link) {
> -		ahndN = spin->ahnd;
> +		ahndN = spin->opts.ahnd;
>  		igt_assert(sync_fence_wait(spin->out_fence, 0) == -ETIME);
>  		igt_spin_end(spin);
>  
> -- 
> 2.34.1
> 

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [igt-dev] [PATCH i-g-t] lib/igt_dummyload: Drop ahnd from igt_spin_t
  2022-02-25 11:29 ` [igt-dev] [PATCH i-g-t] " Zbigniew Kempczyński
@ 2022-02-25 22:18   ` Dixit, Ashutosh
  2022-03-01  6:25     ` Zbigniew Kempczyński
  0 siblings, 1 reply; 5+ messages in thread
From: Dixit, Ashutosh @ 2022-02-25 22:18 UTC (permalink / raw)
  To: Zbigniew Kempczyński; +Cc: igt-dev, jasmine.newsome

On Fri, 25 Feb 2022 03:29:59 -0800, Zbigniew Kempczyński wrote:
>
> On Thu, Feb 24, 2022 at 02:02:29PM -0800, Ashutosh Dixit wrote:
> > In 4d9396e67930 we have started storing the opts with which the spin was
> > created as part of igt_spin_t. The ahnd stored as part of igt_spin_t is
> > therefore redundant. We can get ahnd from opts.ahnd.
>
> More than redundancy I like to keep opts which comes from the caller
> and thus suggests spinner code that's immune. Unfortunatelly we cannot
> block this after memcmp using c-syntax (I'm not able to do this).

Sorry, didn't follow what you are referring to here?

> Anyway, keeping caller vars in opts suggests this so:
>
> Reviewed-by: Zbigniew Kempczyński <zbigniew.kempczynski@intel.com>

Thanks, but will wait for your reply before merging.

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [igt-dev] [PATCH i-g-t] lib/igt_dummyload: Drop ahnd from igt_spin_t
  2022-02-25 22:18   ` Dixit, Ashutosh
@ 2022-03-01  6:25     ` Zbigniew Kempczyński
  0 siblings, 0 replies; 5+ messages in thread
From: Zbigniew Kempczyński @ 2022-03-01  6:25 UTC (permalink / raw)
  To: Dixit, Ashutosh; +Cc: igt-dev, jasmine.newsome

On Fri, Feb 25, 2022 at 02:18:21PM -0800, Dixit, Ashutosh wrote:
> On Fri, 25 Feb 2022 03:29:59 -0800, Zbigniew Kempczyński wrote:
> >
> > On Thu, Feb 24, 2022 at 02:02:29PM -0800, Ashutosh Dixit wrote:
> > > In 4d9396e67930 we have started storing the opts with which the spin was
> > > created as part of igt_spin_t. The ahnd stored as part of igt_spin_t is
> > > therefore redundant. We can get ahnd from opts.ahnd.
> >
> > More than redundancy I like to keep opts which comes from the caller
> > and thus suggests spinner code that's immune. Unfortunatelly we cannot
> > block this after memcmp using c-syntax (I'm not able to do this).
> 
> Sorry, didn't follow what you are referring to here?

I mean - opts come from caller and should be immune within spinner
with const keyword. But memcpy() to it could be only done via pointer
tricks. 

Go ahead and merge :)

--
Zbigniew

> 
> > Anyway, keeping caller vars in opts suggests this so:
> >
> > Reviewed-by: Zbigniew Kempczyński <zbigniew.kempczynski@intel.com>
> 
> Thanks, but will wait for your reply before merging.

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2022-03-01  6:25 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2022-02-24 22:02 [igt-dev] [PATCH i-g-t] lib/igt_dummyload: Drop ahnd from igt_spin_t Ashutosh Dixit
2022-02-24 23:09 ` [igt-dev] ✗ Fi.CI.BAT: failure for " Patchwork
2022-02-25 11:29 ` [igt-dev] [PATCH i-g-t] " Zbigniew Kempczyński
2022-02-25 22:18   ` Dixit, Ashutosh
2022-03-01  6:25     ` Zbigniew Kempczyński

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox