Just a selection of some fine brown paper bags

Just a selection of some fine brown paper bags

[Chris Wilson]

Some ugly bugs.
-Chris


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

[PATCH 1/3] drm/i915/gem: Defer obj->base.resv fini until RCU callback

[Chris Wilson]

Since reservation_object_fini() does an immediate free, rather than
kfree_rcu as normal, we have to delay the release until after the RCU
grace period has elapsed (i.e. from the rcu cleanup callback) so that we
can rely on the RCU protected access to the fences while the object is a
zombie.

i915_gem_busy_ioctl relies on having an RCU barrier to protect the
reservation in order to avoid having to take a reference and strong
memory barriers.

Fixes: c03467ba40f7 ("drm/i915/gem: Free pages before rcu-freeing the object")
Testcase: igt/gem_busy/close-race
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Matthew Auld <matthew.auld@intel.com>
---
 drivers/gpu/drm/i915/gem/i915_gem_object.c | 3 ++-
 drivers/gpu/drm/i915/gem/i915_gem_shmem.c  | 7 +++++++
 2 files changed, 9 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/gem/i915_gem_object.c b/drivers/gpu/drm/i915/gem/i915_gem_object.c
index d3e96f09c6b7..0dced4a20e20 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_object.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_object.c
@@ -152,6 +152,7 @@ static void __i915_gem_free_object_rcu(struct rcu_head *head)
 		container_of(head, typeof(*obj), rcu);
 	struct drm_i915_private *i915 = to_i915(obj->base.dev);
 
+	reservation_object_fini(&obj->base._resv);
 	i915_gem_object_free(obj);
 
 	GEM_BUG_ON(!atomic_read(&i915->mm.free_count));
@@ -198,7 +199,7 @@ static void __i915_gem_free_objects(struct drm_i915_private *i915,
 		if (obj->base.import_attach)
 			drm_prime_gem_destroy(&obj->base, NULL);
 
-		drm_gem_object_release(&obj->base);
+		drm_gem_free_mmap_offset(&obj->base);
 
 		/* But keep the pointer alive for RCU-protected lookups */
 		call_rcu(&obj->rcu, __i915_gem_free_object_rcu);
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_shmem.c b/drivers/gpu/drm/i915/gem/i915_gem_shmem.c
index 19d9ecdb2894..d2a1158868e7 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_shmem.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_shmem.c
@@ -414,6 +414,11 @@ shmem_pwrite(struct drm_i915_gem_object *obj,
 	return 0;
 }
 
+static void shmem_release(struct drm_i915_gem_object *obj)
+{
+	fput(obj->base.filp);
+}
+
 const struct drm_i915_gem_object_ops i915_gem_shmem_ops = {
 	.flags = I915_GEM_OBJECT_HAS_STRUCT_PAGE |
 		 I915_GEM_OBJECT_IS_SHRINKABLE,
@@ -424,6 +429,8 @@ const struct drm_i915_gem_object_ops i915_gem_shmem_ops = {
 	.writeback = shmem_writeback,
 
 	.pwrite = shmem_pwrite,
+
+	.release = shmem_release,
 };
 
 static int create_shmem(struct drm_i915_private *i915,
-- 
2.20.1

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

[PATCH 2/3] drm/i915/gtt: Defer the free for alloc error paths

[Chris Wilson]

If we hit an error while allocating the page tables, we have to unwind
the incomplete updates, and wish to free the unused pd. However, we are
not allowed to be hoding the spinlock at that point, and so must use the
later free to defer it until after we drop the lock.

<3> [414.363795] BUG: sleeping function called from invalid context at drivers/gpu/drm/i915/i915_gem_gtt.c:472
<3> [414.364167] in_atomic(): 1, irqs_disabled(): 0, pid: 3905, name: i915_selftest
<4> [414.364406] 3 locks held by i915_selftest/3905:
<4> [414.364408]  #0: 0000000034fe8aa8 (&dev->mutex){....}, at: device_driver_attach+0x18/0x50
<4> [414.364415]  #1: 000000006bd8a560 (&dev->struct_mutex){+.+.}, at: igt_ctx_exec+0xb7/0x410 [i915]
<4> [414.364476]  #2: 000000003dfdc766 (&(&pd->lock)->rlock){+.+.}, at: gen8_ppgtt_alloc_pdp+0x448/0x540 [i915]
<3> [414.364529] Preemption disabled at:
<4> [414.364530] [<0000000000000000>] 0x0
<4> [414.364696] CPU: 0 PID: 3905 Comm: i915_selftest Tainted: G     U            5.2.0-rc7-CI-CI_DRM_6403+ #1
<4> [414.364698] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS rel-1.10.1-0-g8891697-prebuilt.qemu-project.org 04/01/2014
<4> [414.364699] Call Trace:
<4> [414.364704]  dump_stack+0x67/0x9b
<4> [414.364708]  ___might_sleep+0x167/0x250
<4> [414.364777]  vm_free_page+0x24/0xc0 [i915]
<4> [414.364852]  free_pd+0xf/0x20 [i915]
<4> [414.364897]  gen8_ppgtt_alloc_pdp+0x489/0x540 [i915]
<4> [414.364946]  gen8_ppgtt_alloc_4lvl+0x8e/0x2e0 [i915]
<4> [414.364992]  ppgtt_bind_vma+0x2e/0x60 [i915]
<4> [414.365039]  i915_vma_bind+0xe8/0x2c0 [i915]
<4> [414.365088]  __i915_vma_do_pin+0xa1/0xd20 [i915]

Fixes: 1d1b5490b91c ("drm/i915/gtt: Replace struct_mutex serialisation for allocation")
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Matthew Auld <matthew.auld@intel.com>
Cc: Mika Kuoppala <mika.kuoppala@intel.com>
---
 drivers/gpu/drm/i915/i915_gem_gtt.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c
index 9e76347e039e..1065753e86fb 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.c
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
@@ -1489,7 +1489,8 @@ static int gen8_ppgtt_alloc_pdp(struct i915_address_space *vm,
 		gen8_ppgtt_set_pdpe(pdp, vm->scratch_pd, pdpe);
 		GEM_BUG_ON(!atomic_read(&pdp->used));
 		atomic_dec(&pdp->used);
-		free_pd(vm, pd);
+		GEM_BUG_ON(alloc);
+		alloc = pd; /* defer the free to after the lock */
 	}
 	spin_unlock(&pdp->lock);
 unwind:
@@ -1558,7 +1559,8 @@ static int gen8_ppgtt_alloc_4lvl(struct i915_address_space *vm,
 	spin_lock(&pml4->lock);
 	if (atomic_dec_and_test(&pdp->used)) {
 		gen8_ppgtt_set_pml4e(pml4, vm->scratch_pdp, pml4e);
-		free_pd(vm, pdp);
+		GEM_BUG_ON(alloc);
+		alloc = pdp; /* defer the free until after the lock */
 	}
 	spin_unlock(&pml4->lock);
 unwind:
-- 
2.20.1

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

[PATCH 3/3] drm/i915: Flush the workqueue before draining

[Chris Wilson]

Trying to drain a workqueue while we may still be adding to it from
background tasks is, according to kernel/workqueue.c, verboten. So, add
a flush_workqueue() at the start of our cleanup procedure.

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
---
 drivers/gpu/drm/i915/i915_drv.h | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 9d132c9d17b0..d2f9af3a16dc 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -2472,6 +2472,7 @@ static inline void i915_gem_drain_workqueue(struct drm_i915_private *i915)
 	 */
 	int pass = 3;
 	do {
+		flush_workqueue(i915->wq);
 		rcu_barrier();
 		i915_gem_drain_freed_objects(i915);
 	} while (--pass);
-- 
2.20.1

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

[PATCH v2] drm/i915/gem: Defer obj->base.resv fini until RCU callback

[Chris Wilson]

Since reservation_object_fini() does an immediate free, rather than
kfree_rcu as normal, we have to delay the release until after the RCU
grace period has elapsed (i.e. from the rcu cleanup callback) so that we
can rely on the RCU protected access to the fences while the object is a
zombie.

i915_gem_busy_ioctl relies on having an RCU barrier to protect the
reservation in order to avoid having to take a reference and strong
memory barriers.

v2: Order is important; only release after putting the pages!

Fixes: c03467ba40f7 ("drm/i915/gem: Free pages before rcu-freeing the object")
Testcase: igt/gem_busy/close-race
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Matthew Auld <matthew.auld@intel.com>
---
 drivers/gpu/drm/i915/gem/i915_gem_object.c | 9 +++++----
 drivers/gpu/drm/i915/gem/i915_gem_phys.c   | 7 -------
 drivers/gpu/drm/i915/gem/i915_gem_shmem.c  | 7 +++++++
 drivers/gpu/drm/i915/gem/i915_gem_stolen.c | 2 --
 4 files changed, 12 insertions(+), 13 deletions(-)

diff --git a/drivers/gpu/drm/i915/gem/i915_gem_object.c b/drivers/gpu/drm/i915/gem/i915_gem_object.c
index d3e96f09c6b7..d5197a2a106f 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_object.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_object.c
@@ -152,6 +152,7 @@ static void __i915_gem_free_object_rcu(struct rcu_head *head)
 		container_of(head, typeof(*obj), rcu);
 	struct drm_i915_private *i915 = to_i915(obj->base.dev);
 
+	reservation_object_fini(&obj->base._resv);
 	i915_gem_object_free(obj);
 
 	GEM_BUG_ON(!atomic_read(&i915->mm.free_count));
@@ -187,9 +188,6 @@ static void __i915_gem_free_objects(struct drm_i915_private *i915,
 		GEM_BUG_ON(atomic_read(&obj->frontbuffer_bits));
 		GEM_BUG_ON(!list_empty(&obj->lut_list));
 
-		if (obj->ops->release)
-			obj->ops->release(obj);
-
 		atomic_set(&obj->mm.pages_pin_count, 0);
 		__i915_gem_object_put_pages(obj, I915_MM_NORMAL);
 		GEM_BUG_ON(i915_gem_object_has_pages(obj));
@@ -198,7 +196,10 @@ static void __i915_gem_free_objects(struct drm_i915_private *i915,
 		if (obj->base.import_attach)
 			drm_prime_gem_destroy(&obj->base, NULL);
 
-		drm_gem_object_release(&obj->base);
+		drm_gem_free_mmap_offset(&obj->base);
+
+		if (obj->ops->release)
+			obj->ops->release(obj);
 
 		/* But keep the pointer alive for RCU-protected lookups */
 		call_rcu(&obj->rcu, __i915_gem_free_object_rcu);
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_phys.c b/drivers/gpu/drm/i915/gem/i915_gem_phys.c
index b9fab22ada6f..102fd7a23d3d 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_phys.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_phys.c
@@ -133,16 +133,9 @@ i915_gem_object_put_pages_phys(struct drm_i915_gem_object *obj,
 	drm_pci_free(obj->base.dev, obj->phys_handle);
 }
 
-static void
-i915_gem_object_release_phys(struct drm_i915_gem_object *obj)
-{
-	i915_gem_object_unpin_pages(obj);
-}
-
 static const struct drm_i915_gem_object_ops i915_gem_phys_ops = {
 	.get_pages = i915_gem_object_get_pages_phys,
 	.put_pages = i915_gem_object_put_pages_phys,
-	.release = i915_gem_object_release_phys,
 };
 
 int i915_gem_object_attach_phys(struct drm_i915_gem_object *obj, int align)
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_shmem.c b/drivers/gpu/drm/i915/gem/i915_gem_shmem.c
index 19d9ecdb2894..d2a1158868e7 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_shmem.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_shmem.c
@@ -414,6 +414,11 @@ shmem_pwrite(struct drm_i915_gem_object *obj,
 	return 0;
 }
 
+static void shmem_release(struct drm_i915_gem_object *obj)
+{
+	fput(obj->base.filp);
+}
+
 const struct drm_i915_gem_object_ops i915_gem_shmem_ops = {
 	.flags = I915_GEM_OBJECT_HAS_STRUCT_PAGE |
 		 I915_GEM_OBJECT_IS_SHRINKABLE,
@@ -424,6 +429,8 @@ const struct drm_i915_gem_object_ops i915_gem_shmem_ops = {
 	.writeback = shmem_writeback,
 
 	.pwrite = shmem_pwrite,
+
+	.release = shmem_release,
 };
 
 static int create_shmem(struct drm_i915_private *i915,
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_stolen.c b/drivers/gpu/drm/i915/gem/i915_gem_stolen.c
index de1fab2058ec..639c852bad12 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_stolen.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_stolen.c
@@ -529,8 +529,6 @@ i915_gem_object_release_stolen(struct drm_i915_gem_object *obj)
 
 	GEM_BUG_ON(!stolen);
 
-	__i915_gem_object_unpin_pages(obj);
-
 	i915_gem_stolen_remove_node(dev_priv, stolen);
 	kfree(stolen);
 }
-- 
2.20.1

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

✗ Fi.CI.CHECKPATCH: warning for series starting with [v2] drm/i915/gem: Defer obj->base.resv fini until RCU callback (rev2)

[Patchwork]

== Series Details ==

Series: series starting with [v2] drm/i915/gem: Defer obj->base.resv fini until RCU callback (rev2)
URL   : https://patchwork.freedesktop.org/series/63160/
State : warning

== Summary ==

$ dim checkpatch origin/drm-tip
cd5ff9ac095e drm/i915/gem: Defer obj->base.resv fini until RCU callback
46ca078fad16 drm/i915/gtt: Defer the free for alloc error paths
-:11: WARNING:COMMIT_LOG_LONG_LINE: Possible unwrapped commit description (prefer a maximum 75 chars per line)
#11: 
<3> [414.363795] BUG: sleeping function called from invalid context at drivers/gpu/drm/i915/i915_gem_gtt.c:472

total: 0 errors, 1 warnings, 0 checks, 18 lines checked
c2bdbb50c9c7 drm/i915: Flush the workqueue before draining

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

✓ Fi.CI.BAT: success for series starting with [v2] drm/i915/gem: Defer obj->base.resv fini until RCU callback (rev2)

[Patchwork]

== Series Details ==

Series: series starting with [v2] drm/i915/gem: Defer obj->base.resv fini until RCU callback (rev2)
URL   : https://patchwork.freedesktop.org/series/63160/
State : success

== Summary ==

CI Bug Log - changes from CI_DRM_6405 -> Patchwork_13514
====================================================

Summary
-------

  **SUCCESS**

  No regressions found.

  External URL: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13514/


Changes
-------

  No changes found


Participating hosts (53 -> 45)
------------------------------

  Additional (2): fi-hsw-4770r fi-cml-u2 
  Missing    (10): fi-kbl-soraka fi-icl-u4 fi-ilk-m540 fi-hsw-4200u fi-byt-squawks fi-bsw-cyan fi-icl-u3 fi-icl-y fi-byt-clapper fi-bdw-samus 


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

  * Linux: CI_DRM_6405 -> Patchwork_13514

  CI_DRM_6405: d395f3e20d154dfeabb95117f388f2e953c12ac9 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_5082: f7c51e6fbf8da0784b64a1edaee5266aa9b9f829 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_13514: c2bdbb50c9c7ad6e57705cfaf6473d68546ad4bf @ git://anongit.freedesktop.org/gfx-ci/linux


== Kernel 32bit build ==

Warning: Kernel 32bit buildtest failed:
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13514/build_32bit.log

  CALL    scripts/checksyscalls.sh
  CALL    scripts/atomic/check-atomics.sh
  CHK     include/generated/compile.h
Kernel: arch/x86/boot/bzImage is ready  (#1)
  Building modules, stage 2.
  MODPOST 112 modules
ERROR: "__udivdi3" [drivers/gpu/drm/amd/amdgpu/amdgpu.ko] undefined!
ERROR: "__divdi3" [drivers/gpu/drm/amd/amdgpu/amdgpu.ko] undefined!
scripts/Makefile.modpost:91: recipe for target '__modpost' failed
make[1]: *** [__modpost] Error 1
Makefile:1287: recipe for target 'modules' failed
make: *** [modules] Error 2


== Linux commits ==

c2bdbb50c9c7 drm/i915: Flush the workqueue before draining
46ca078fad16 drm/i915/gtt: Defer the free for alloc error paths
cd5ff9ac095e drm/i915/gem: Defer obj->base.resv fini until RCU callback

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13514/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 2/3] drm/i915/gtt: Defer the free for alloc error paths

[Mika Kuoppala]

Chris Wilson <chris@chris-wilson.co.uk> writes:

> If we hit an error while allocating the page tables, we have to unwind
> the incomplete updates, and wish to free the unused pd. However, we are
> not allowed to be hoding the spinlock at that point, and so must use the
> later free to defer it until after we drop the lock.
>
> <3> [414.363795] BUG: sleeping function called from invalid context at drivers/gpu/drm/i915/i915_gem_gtt.c:472
> <3> [414.364167] in_atomic(): 1, irqs_disabled(): 0, pid: 3905, name: i915_selftest
> <4> [414.364406] 3 locks held by i915_selftest/3905:
> <4> [414.364408]  #0: 0000000034fe8aa8 (&dev->mutex){....}, at: device_driver_attach+0x18/0x50
> <4> [414.364415]  #1: 000000006bd8a560 (&dev->struct_mutex){+.+.}, at: igt_ctx_exec+0xb7/0x410 [i915]
> <4> [414.364476]  #2: 000000003dfdc766 (&(&pd->lock)->rlock){+.+.}, at: gen8_ppgtt_alloc_pdp+0x448/0x540 [i915]
> <3> [414.364529] Preemption disabled at:
> <4> [414.364530] [<0000000000000000>] 0x0
> <4> [414.364696] CPU: 0 PID: 3905 Comm: i915_selftest Tainted: G     U            5.2.0-rc7-CI-CI_DRM_6403+ #1
> <4> [414.364698] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS rel-1.10.1-0-g8891697-prebuilt.qemu-project.org 04/01/2014
> <4> [414.364699] Call Trace:
> <4> [414.364704]  dump_stack+0x67/0x9b
> <4> [414.364708]  ___might_sleep+0x167/0x250
> <4> [414.364777]  vm_free_page+0x24/0xc0 [i915]
> <4> [414.364852]  free_pd+0xf/0x20 [i915]
> <4> [414.364897]  gen8_ppgtt_alloc_pdp+0x489/0x540 [i915]
> <4> [414.364946]  gen8_ppgtt_alloc_4lvl+0x8e/0x2e0 [i915]
> <4> [414.364992]  ppgtt_bind_vma+0x2e/0x60 [i915]
> <4> [414.365039]  i915_vma_bind+0xe8/0x2c0 [i915]
> <4> [414.365088]  __i915_vma_do_pin+0xa1/0xd20 [i915]
>
> Fixes: 1d1b5490b91c ("drm/i915/gtt: Replace struct_mutex serialisation for allocation")
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Matthew Auld <matthew.auld@intel.com>
> Cc: Mika Kuoppala <mika.kuoppala@intel.com>

From another thread,

Reviewed-by: Mika Kuoppala <mika.kuoppala@linux.intel.com>

> ---
>  drivers/gpu/drm/i915/i915_gem_gtt.c | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c
> index 9e76347e039e..1065753e86fb 100644
> --- a/drivers/gpu/drm/i915/i915_gem_gtt.c
> +++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
> @@ -1489,7 +1489,8 @@ static int gen8_ppgtt_alloc_pdp(struct i915_address_space *vm,
>  		gen8_ppgtt_set_pdpe(pdp, vm->scratch_pd, pdpe);
>  		GEM_BUG_ON(!atomic_read(&pdp->used));
>  		atomic_dec(&pdp->used);
> -		free_pd(vm, pd);
> +		GEM_BUG_ON(alloc);
> +		alloc = pd; /* defer the free to after the lock */
>  	}
>  	spin_unlock(&pdp->lock);
>  unwind:
> @@ -1558,7 +1559,8 @@ static int gen8_ppgtt_alloc_4lvl(struct i915_address_space *vm,
>  	spin_lock(&pml4->lock);
>  	if (atomic_dec_and_test(&pdp->used)) {
>  		gen8_ppgtt_set_pml4e(pml4, vm->scratch_pdp, pml4e);
> -		free_pd(vm, pdp);
> +		GEM_BUG_ON(alloc);
> +		alloc = pdp; /* defer the free until after the lock */
>  	}
>  	spin_unlock(&pml4->lock);
>  unwind:
> -- 
> 2.20.1
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 3/3] drm/i915: Flush the workqueue before draining

[Mika Kuoppala]

Chris Wilson <chris@chris-wilson.co.uk> writes:

> Trying to drain a workqueue while we may still be adding to it from
> background tasks is, according to kernel/workqueue.c, verboten. So, add
> a flush_workqueue() at the start of our cleanup procedure.

I don't get it. drain_workqueue does it's own flushing.
-Mika

>
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> ---
>  drivers/gpu/drm/i915/i915_drv.h | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 9d132c9d17b0..d2f9af3a16dc 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -2472,6 +2472,7 @@ static inline void i915_gem_drain_workqueue(struct drm_i915_private *i915)
>  	 */
>  	int pass = 3;
>  	do {
> +		flush_workqueue(i915->wq);
>  		rcu_barrier();
>  		i915_gem_drain_freed_objects(i915);
>  	} while (--pass);
> -- 
> 2.20.1
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 3/3] drm/i915: Flush the workqueue before draining

[Chris Wilson]

Quoting Mika Kuoppala (2019-07-04 11:22:17)
> Chris Wilson <chris@chris-wilson.co.uk> writes:
> 
> > Trying to drain a workqueue while we may still be adding to it from
> > background tasks is, according to kernel/workqueue.c, verboten. So, add
> > a flush_workqueue() at the start of our cleanup procedure.
> 
> I don't get it. drain_workqueue does it's own flushing.

Ordering is important here. The problem with drain_workqueue() is that
is forbids us from adding more tasks into the workqueue as it drains, so
before we drain we must plug.

It's just adding more hammers. Eventually it'll break.
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 2/3] drm/i915/gtt: Defer the free for alloc error paths

[Matthew Auld]

On 03/07/2019 18:19, Chris Wilson wrote:
> If we hit an error while allocating the page tables, we have to unwind
> the incomplete updates, and wish to free the unused pd. However, we are
> not allowed to be hoding the spinlock at that point, and so must use the

holding

> later free to defer it until after we drop the lock.
> 
> <3> [414.363795] BUG: sleeping function called from invalid context at drivers/gpu/drm/i915/i915_gem_gtt.c:472
> <3> [414.364167] in_atomic(): 1, irqs_disabled(): 0, pid: 3905, name: i915_selftest
> <4> [414.364406] 3 locks held by i915_selftest/3905:
> <4> [414.364408]  #0: 0000000034fe8aa8 (&dev->mutex){....}, at: device_driver_attach+0x18/0x50
> <4> [414.364415]  #1: 000000006bd8a560 (&dev->struct_mutex){+.+.}, at: igt_ctx_exec+0xb7/0x410 [i915]
> <4> [414.364476]  #2: 000000003dfdc766 (&(&pd->lock)->rlock){+.+.}, at: gen8_ppgtt_alloc_pdp+0x448/0x540 [i915]
> <3> [414.364529] Preemption disabled at:
> <4> [414.364530] [<0000000000000000>] 0x0
> <4> [414.364696] CPU: 0 PID: 3905 Comm: i915_selftest Tainted: G     U            5.2.0-rc7-CI-CI_DRM_6403+ #1
> <4> [414.364698] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS rel-1.10.1-0-g8891697-prebuilt.qemu-project.org 04/01/2014
> <4> [414.364699] Call Trace:
> <4> [414.364704]  dump_stack+0x67/0x9b
> <4> [414.364708]  ___might_sleep+0x167/0x250
> <4> [414.364777]  vm_free_page+0x24/0xc0 [i915]
> <4> [414.364852]  free_pd+0xf/0x20 [i915]
> <4> [414.364897]  gen8_ppgtt_alloc_pdp+0x489/0x540 [i915]
> <4> [414.364946]  gen8_ppgtt_alloc_4lvl+0x8e/0x2e0 [i915]
> <4> [414.364992]  ppgtt_bind_vma+0x2e/0x60 [i915]
> <4> [414.365039]  i915_vma_bind+0xe8/0x2c0 [i915]
> <4> [414.365088]  __i915_vma_do_pin+0xa1/0xd20 [i915]
> 
> Fixes: 1d1b5490b91c ("drm/i915/gtt: Replace struct_mutex serialisation for allocation")
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Matthew Auld <matthew.auld@intel.com>
> Cc: Mika Kuoppala <mika.kuoppala@intel.com>
> ---
>   drivers/gpu/drm/i915/i915_gem_gtt.c | 6 ++++--
>   1 file changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c
> index 9e76347e039e..1065753e86fb 100644
> --- a/drivers/gpu/drm/i915/i915_gem_gtt.c
> +++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
> @@ -1489,7 +1489,8 @@ static int gen8_ppgtt_alloc_pdp(struct i915_address_space *vm,
>   		gen8_ppgtt_set_pdpe(pdp, vm->scratch_pd, pdpe);
>   		GEM_BUG_ON(!atomic_read(&pdp->used));
>   		atomic_dec(&pdp->used);
> -		free_pd(vm, pd);
> +		GEM_BUG_ON(alloc);

Pretty sure it's possible for alloc != NULL at this point...two threads, 
one is late to the party, we are unlucky and hit the unwind_pd path. No?

> +		alloc = pd; /* defer the free to after the lock */
>   	}
>   	spin_unlock(&pdp->lock);
>   unwind:
> @@ -1558,7 +1559,8 @@ static int gen8_ppgtt_alloc_4lvl(struct i915_address_space *vm,
>   	spin_lock(&pml4->lock);
>   	if (atomic_dec_and_test(&pdp->used)) {
>   		gen8_ppgtt_set_pml4e(pml4, vm->scratch_pdp, pml4e);
> -		free_pd(vm, pdp);
> +		GEM_BUG_ON(alloc);
> +		alloc = pdp; /* defer the free until after the lock */
>   	}
>   	spin_unlock(&pml4->lock);
>   unwind:
> 
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 2/3] drm/i915/gtt: Defer the free for alloc error paths

[Chris Wilson]

Quoting Matthew Auld (2019-07-04 11:28:18)
> On 03/07/2019 18:19, Chris Wilson wrote:
> > If we hit an error while allocating the page tables, we have to unwind
> > the incomplete updates, and wish to free the unused pd. However, we are
> > not allowed to be hoding the spinlock at that point, and so must use the
> 
> holding
> 
> > later free to defer it until after we drop the lock.
> > 
> > <3> [414.363795] BUG: sleeping function called from invalid context at drivers/gpu/drm/i915/i915_gem_gtt.c:472
> > <3> [414.364167] in_atomic(): 1, irqs_disabled(): 0, pid: 3905, name: i915_selftest
> > <4> [414.364406] 3 locks held by i915_selftest/3905:
> > <4> [414.364408]  #0: 0000000034fe8aa8 (&dev->mutex){....}, at: device_driver_attach+0x18/0x50
> > <4> [414.364415]  #1: 000000006bd8a560 (&dev->struct_mutex){+.+.}, at: igt_ctx_exec+0xb7/0x410 [i915]
> > <4> [414.364476]  #2: 000000003dfdc766 (&(&pd->lock)->rlock){+.+.}, at: gen8_ppgtt_alloc_pdp+0x448/0x540 [i915]
> > <3> [414.364529] Preemption disabled at:
> > <4> [414.364530] [<0000000000000000>] 0x0
> > <4> [414.364696] CPU: 0 PID: 3905 Comm: i915_selftest Tainted: G     U            5.2.0-rc7-CI-CI_DRM_6403+ #1
> > <4> [414.364698] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS rel-1.10.1-0-g8891697-prebuilt.qemu-project.org 04/01/2014
> > <4> [414.364699] Call Trace:
> > <4> [414.364704]  dump_stack+0x67/0x9b
> > <4> [414.364708]  ___might_sleep+0x167/0x250
> > <4> [414.364777]  vm_free_page+0x24/0xc0 [i915]
> > <4> [414.364852]  free_pd+0xf/0x20 [i915]
> > <4> [414.364897]  gen8_ppgtt_alloc_pdp+0x489/0x540 [i915]
> > <4> [414.364946]  gen8_ppgtt_alloc_4lvl+0x8e/0x2e0 [i915]
> > <4> [414.364992]  ppgtt_bind_vma+0x2e/0x60 [i915]
> > <4> [414.365039]  i915_vma_bind+0xe8/0x2c0 [i915]
> > <4> [414.365088]  __i915_vma_do_pin+0xa1/0xd20 [i915]
> > 
> > Fixes: 1d1b5490b91c ("drm/i915/gtt: Replace struct_mutex serialisation for allocation")
> > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> > Cc: Matthew Auld <matthew.auld@intel.com>
> > Cc: Mika Kuoppala <mika.kuoppala@intel.com>
> > ---
> >   drivers/gpu/drm/i915/i915_gem_gtt.c | 6 ++++--
> >   1 file changed, 4 insertions(+), 2 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c
> > index 9e76347e039e..1065753e86fb 100644
> > --- a/drivers/gpu/drm/i915/i915_gem_gtt.c
> > +++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
> > @@ -1489,7 +1489,8 @@ static int gen8_ppgtt_alloc_pdp(struct i915_address_space *vm,
> >               gen8_ppgtt_set_pdpe(pdp, vm->scratch_pd, pdpe);
> >               GEM_BUG_ON(!atomic_read(&pdp->used));
> >               atomic_dec(&pdp->used);
> > -             free_pd(vm, pd);
> > +             GEM_BUG_ON(alloc);
> 
> Pretty sure it's possible for alloc != NULL at this point...two threads, 
> one is late to the party, we are unlucky and hit the unwind_pd path. No?

Hmm. I was thinking we would only get here on an alloc failure path, but
yeah, the BUG_ON was a case for doubt.

Drat.
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 2/3] drm/i915/gtt: Defer the free for alloc error paths

[Mika Kuoppala]

Chris Wilson <chris@chris-wilson.co.uk> writes:

> Quoting Matthew Auld (2019-07-04 11:28:18)
>> On 03/07/2019 18:19, Chris Wilson wrote:
>> > If we hit an error while allocating the page tables, we have to unwind
>> > the incomplete updates, and wish to free the unused pd. However, we are
>> > not allowed to be hoding the spinlock at that point, and so must use the
>> 
>> holding
>> 
>> > later free to defer it until after we drop the lock.
>> > 
>> > <3> [414.363795] BUG: sleeping function called from invalid context at drivers/gpu/drm/i915/i915_gem_gtt.c:472
>> > <3> [414.364167] in_atomic(): 1, irqs_disabled(): 0, pid: 3905, name: i915_selftest
>> > <4> [414.364406] 3 locks held by i915_selftest/3905:
>> > <4> [414.364408]  #0: 0000000034fe8aa8 (&dev->mutex){....}, at: device_driver_attach+0x18/0x50
>> > <4> [414.364415]  #1: 000000006bd8a560 (&dev->struct_mutex){+.+.}, at: igt_ctx_exec+0xb7/0x410 [i915]
>> > <4> [414.364476]  #2: 000000003dfdc766 (&(&pd->lock)->rlock){+.+.}, at: gen8_ppgtt_alloc_pdp+0x448/0x540 [i915]
>> > <3> [414.364529] Preemption disabled at:
>> > <4> [414.364530] [<0000000000000000>] 0x0
>> > <4> [414.364696] CPU: 0 PID: 3905 Comm: i915_selftest Tainted: G     U            5.2.0-rc7-CI-CI_DRM_6403+ #1
>> > <4> [414.364698] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS rel-1.10.1-0-g8891697-prebuilt.qemu-project.org 04/01/2014
>> > <4> [414.364699] Call Trace:
>> > <4> [414.364704]  dump_stack+0x67/0x9b
>> > <4> [414.364708]  ___might_sleep+0x167/0x250
>> > <4> [414.364777]  vm_free_page+0x24/0xc0 [i915]
>> > <4> [414.364852]  free_pd+0xf/0x20 [i915]
>> > <4> [414.364897]  gen8_ppgtt_alloc_pdp+0x489/0x540 [i915]
>> > <4> [414.364946]  gen8_ppgtt_alloc_4lvl+0x8e/0x2e0 [i915]
>> > <4> [414.364992]  ppgtt_bind_vma+0x2e/0x60 [i915]
>> > <4> [414.365039]  i915_vma_bind+0xe8/0x2c0 [i915]
>> > <4> [414.365088]  __i915_vma_do_pin+0xa1/0xd20 [i915]
>> > 
>> > Fixes: 1d1b5490b91c ("drm/i915/gtt: Replace struct_mutex serialisation for allocation")
>> > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
>> > Cc: Matthew Auld <matthew.auld@intel.com>
>> > Cc: Mika Kuoppala <mika.kuoppala@intel.com>
>> > ---
>> >   drivers/gpu/drm/i915/i915_gem_gtt.c | 6 ++++--
>> >   1 file changed, 4 insertions(+), 2 deletions(-)
>> > 
>> > diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c
>> > index 9e76347e039e..1065753e86fb 100644
>> > --- a/drivers/gpu/drm/i915/i915_gem_gtt.c
>> > +++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
>> > @@ -1489,7 +1489,8 @@ static int gen8_ppgtt_alloc_pdp(struct i915_address_space *vm,
>> >               gen8_ppgtt_set_pdpe(pdp, vm->scratch_pd, pdpe);
>> >               GEM_BUG_ON(!atomic_read(&pdp->used));
>> >               atomic_dec(&pdp->used);
>> > -             free_pd(vm, pd);
>> > +             GEM_BUG_ON(alloc);
>> 
>> Pretty sure it's possible for alloc != NULL at this point...two threads, 
>> one is late to the party, we are unlucky and hit the unwind_pd path. No?
>
> Hmm. I was thinking we would only get here on an alloc failure path, but
> yeah, the BUG_ON was a case for doubt.

Am I staring at the wrong spot then. I thought the atomic_inc(&pd_used)
saves us.

-Mika


>
> Drat.
> -Chris
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 2/3] drm/i915/gtt: Defer the free for alloc error paths

[Chris Wilson]

Quoting Mika Kuoppala (2019-07-04 11:58:38)
> Chris Wilson <chris@chris-wilson.co.uk> writes:
> 
> > Quoting Matthew Auld (2019-07-04 11:28:18)
> >> On 03/07/2019 18:19, Chris Wilson wrote:
> >> > If we hit an error while allocating the page tables, we have to unwind
> >> > the incomplete updates, and wish to free the unused pd. However, we are
> >> > not allowed to be hoding the spinlock at that point, and so must use the
> >> 
> >> holding
> >> 
> >> > later free to defer it until after we drop the lock.
> >> > 
> >> > <3> [414.363795] BUG: sleeping function called from invalid context at drivers/gpu/drm/i915/i915_gem_gtt.c:472
> >> > <3> [414.364167] in_atomic(): 1, irqs_disabled(): 0, pid: 3905, name: i915_selftest
> >> > <4> [414.364406] 3 locks held by i915_selftest/3905:
> >> > <4> [414.364408]  #0: 0000000034fe8aa8 (&dev->mutex){....}, at: device_driver_attach+0x18/0x50
> >> > <4> [414.364415]  #1: 000000006bd8a560 (&dev->struct_mutex){+.+.}, at: igt_ctx_exec+0xb7/0x410 [i915]
> >> > <4> [414.364476]  #2: 000000003dfdc766 (&(&pd->lock)->rlock){+.+.}, at: gen8_ppgtt_alloc_pdp+0x448/0x540 [i915]
> >> > <3> [414.364529] Preemption disabled at:
> >> > <4> [414.364530] [<0000000000000000>] 0x0
> >> > <4> [414.364696] CPU: 0 PID: 3905 Comm: i915_selftest Tainted: G     U            5.2.0-rc7-CI-CI_DRM_6403+ #1
> >> > <4> [414.364698] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS rel-1.10.1-0-g8891697-prebuilt.qemu-project.org 04/01/2014
> >> > <4> [414.364699] Call Trace:
> >> > <4> [414.364704]  dump_stack+0x67/0x9b
> >> > <4> [414.364708]  ___might_sleep+0x167/0x250
> >> > <4> [414.364777]  vm_free_page+0x24/0xc0 [i915]
> >> > <4> [414.364852]  free_pd+0xf/0x20 [i915]
> >> > <4> [414.364897]  gen8_ppgtt_alloc_pdp+0x489/0x540 [i915]
> >> > <4> [414.364946]  gen8_ppgtt_alloc_4lvl+0x8e/0x2e0 [i915]
> >> > <4> [414.364992]  ppgtt_bind_vma+0x2e/0x60 [i915]
> >> > <4> [414.365039]  i915_vma_bind+0xe8/0x2c0 [i915]
> >> > <4> [414.365088]  __i915_vma_do_pin+0xa1/0xd20 [i915]
> >> > 
> >> > Fixes: 1d1b5490b91c ("drm/i915/gtt: Replace struct_mutex serialisation for allocation")
> >> > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> >> > Cc: Matthew Auld <matthew.auld@intel.com>
> >> > Cc: Mika Kuoppala <mika.kuoppala@intel.com>
> >> > ---
> >> >   drivers/gpu/drm/i915/i915_gem_gtt.c | 6 ++++--
> >> >   1 file changed, 4 insertions(+), 2 deletions(-)
> >> > 
> >> > diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c
> >> > index 9e76347e039e..1065753e86fb 100644
> >> > --- a/drivers/gpu/drm/i915/i915_gem_gtt.c
> >> > +++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
> >> > @@ -1489,7 +1489,8 @@ static int gen8_ppgtt_alloc_pdp(struct i915_address_space *vm,
> >> >               gen8_ppgtt_set_pdpe(pdp, vm->scratch_pd, pdpe);
> >> >               GEM_BUG_ON(!atomic_read(&pdp->used));
> >> >               atomic_dec(&pdp->used);
> >> > -             free_pd(vm, pd);
> >> > +             GEM_BUG_ON(alloc);
> >> 
> >> Pretty sure it's possible for alloc != NULL at this point...two threads, 
> >> one is late to the party, we are unlucky and hit the unwind_pd path. No?
> >
> > Hmm. I was thinking we would only get here on an alloc failure path, but
> > yeah, the BUG_ON was a case for doubt.
> 
> Am I staring at the wrong spot then. I thought the atomic_inc(&pd_used)
> saves us.

It's on another entry in our table. So we threads A|B fighting over
pdpe:0 and B|C fighting over pdpe:1, and if B loses the first race and
the race with C results in an error...
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 2/3] drm/i915/gtt: Defer the free for alloc error paths

[Mika Kuoppala]

Chris Wilson <chris@chris-wilson.co.uk> writes:

> Quoting Mika Kuoppala (2019-07-04 11:58:38)
>> Chris Wilson <chris@chris-wilson.co.uk> writes:
>> 
>> > Quoting Matthew Auld (2019-07-04 11:28:18)
>> >> On 03/07/2019 18:19, Chris Wilson wrote:
>> >> > If we hit an error while allocating the page tables, we have to unwind
>> >> > the incomplete updates, and wish to free the unused pd. However, we are
>> >> > not allowed to be hoding the spinlock at that point, and so must use the
>> >> 
>> >> holding
>> >> 
>> >> > later free to defer it until after we drop the lock.
>> >> > 
>> >> > <3> [414.363795] BUG: sleeping function called from invalid context at drivers/gpu/drm/i915/i915_gem_gtt.c:472
>> >> > <3> [414.364167] in_atomic(): 1, irqs_disabled(): 0, pid: 3905, name: i915_selftest
>> >> > <4> [414.364406] 3 locks held by i915_selftest/3905:
>> >> > <4> [414.364408]  #0: 0000000034fe8aa8 (&dev->mutex){....}, at: device_driver_attach+0x18/0x50
>> >> > <4> [414.364415]  #1: 000000006bd8a560 (&dev->struct_mutex){+.+.}, at: igt_ctx_exec+0xb7/0x410 [i915]
>> >> > <4> [414.364476]  #2: 000000003dfdc766 (&(&pd->lock)->rlock){+.+.}, at: gen8_ppgtt_alloc_pdp+0x448/0x540 [i915]
>> >> > <3> [414.364529] Preemption disabled at:
>> >> > <4> [414.364530] [<0000000000000000>] 0x0
>> >> > <4> [414.364696] CPU: 0 PID: 3905 Comm: i915_selftest Tainted: G     U            5.2.0-rc7-CI-CI_DRM_6403+ #1
>> >> > <4> [414.364698] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS rel-1.10.1-0-g8891697-prebuilt.qemu-project.org 04/01/2014
>> >> > <4> [414.364699] Call Trace:
>> >> > <4> [414.364704]  dump_stack+0x67/0x9b
>> >> > <4> [414.364708]  ___might_sleep+0x167/0x250
>> >> > <4> [414.364777]  vm_free_page+0x24/0xc0 [i915]
>> >> > <4> [414.364852]  free_pd+0xf/0x20 [i915]
>> >> > <4> [414.364897]  gen8_ppgtt_alloc_pdp+0x489/0x540 [i915]
>> >> > <4> [414.364946]  gen8_ppgtt_alloc_4lvl+0x8e/0x2e0 [i915]
>> >> > <4> [414.364992]  ppgtt_bind_vma+0x2e/0x60 [i915]
>> >> > <4> [414.365039]  i915_vma_bind+0xe8/0x2c0 [i915]
>> >> > <4> [414.365088]  __i915_vma_do_pin+0xa1/0xd20 [i915]
>> >> > 
>> >> > Fixes: 1d1b5490b91c ("drm/i915/gtt: Replace struct_mutex serialisation for allocation")
>> >> > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
>> >> > Cc: Matthew Auld <matthew.auld@intel.com>
>> >> > Cc: Mika Kuoppala <mika.kuoppala@intel.com>
>> >> > ---
>> >> >   drivers/gpu/drm/i915/i915_gem_gtt.c | 6 ++++--
>> >> >   1 file changed, 4 insertions(+), 2 deletions(-)
>> >> > 
>> >> > diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c
>> >> > index 9e76347e039e..1065753e86fb 100644
>> >> > --- a/drivers/gpu/drm/i915/i915_gem_gtt.c
>> >> > +++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
>> >> > @@ -1489,7 +1489,8 @@ static int gen8_ppgtt_alloc_pdp(struct i915_address_space *vm,
>> >> >               gen8_ppgtt_set_pdpe(pdp, vm->scratch_pd, pdpe);
>> >> >               GEM_BUG_ON(!atomic_read(&pdp->used));
>> >> >               atomic_dec(&pdp->used);
>> >> > -             free_pd(vm, pd);
>> >> > +             GEM_BUG_ON(alloc);
>> >> 
>> >> Pretty sure it's possible for alloc != NULL at this point...two threads, 
>> >> one is late to the party, we are unlucky and hit the unwind_pd path. No?
>> >
>> > Hmm. I was thinking we would only get here on an alloc failure path, but
>> > yeah, the BUG_ON was a case for doubt.
>> 
>> Am I staring at the wrong spot then. I thought the atomic_inc(&pd_used)
>> saves us.
>
> It's on another entry in our table. So we threads A|B fighting over
> pdpe:0 and B|C fighting over pdpe:1, and if B loses the first race and
> the race with C results in an error...

/o\

I do see it now, thanks.
-MIka
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 3/3] drm/i915: Flush the workqueue before draining

[Mika Kuoppala]

Chris Wilson <chris@chris-wilson.co.uk> writes:

> Quoting Mika Kuoppala (2019-07-04 11:22:17)
>> Chris Wilson <chris@chris-wilson.co.uk> writes:
>> 
>> > Trying to drain a workqueue while we may still be adding to it from
>> > background tasks is, according to kernel/workqueue.c, verboten. So, add
>> > a flush_workqueue() at the start of our cleanup procedure.
>> 
>> I don't get it. drain_workqueue does it's own flushing.
>
> Ordering is important here. The problem with drain_workqueue() is that
> is forbids us from adding more tasks into the workqueue as it drains, so
> before we drain we must plug.
>
> It's just adding more hammers. Eventually it'll break.

If so, then we just increase passes? :)

Ok, I was going to say we don't add to free work from
any of it's handlers.

But then realized this is not about the free list handling,
even tho the freed objects is drained along.

And yes, drain only handles flushing for it's chain.

Reviewed-by: Mika Kuoppala <mika.kuoppala@linux.intel.com>

> -Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 1/3] drm/i915/gem: Defer obj->base.resv fini until RCU callback

[Mika Kuoppala]

Chris Wilson <chris@chris-wilson.co.uk> writes:

> Since reservation_object_fini() does an immediate free, rather than
> kfree_rcu as normal, we have to delay the release until after the RCU
> grace period has elapsed (i.e. from the rcu cleanup callback) so that we
> can rely on the RCU protected access to the fences while the object is a
> zombie.
>
> i915_gem_busy_ioctl relies on having an RCU barrier to protect the
> reservation in order to avoid having to take a reference and strong
> memory barriers.

Ok so for gem busy to be able to operate on a 'to be freed' object
we need to keep the reservation object alive?

>
> Fixes: c03467ba40f7 ("drm/i915/gem: Free pages before rcu-freeing the object")
> Testcase: igt/gem_busy/close-race
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Matthew Auld <matthew.auld@intel.com>
> ---
>  drivers/gpu/drm/i915/gem/i915_gem_object.c | 3 ++-
>  drivers/gpu/drm/i915/gem/i915_gem_shmem.c  | 7 +++++++
>  2 files changed, 9 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/i915/gem/i915_gem_object.c b/drivers/gpu/drm/i915/gem/i915_gem_object.c
> index d3e96f09c6b7..0dced4a20e20 100644
> --- a/drivers/gpu/drm/i915/gem/i915_gem_object.c
> +++ b/drivers/gpu/drm/i915/gem/i915_gem_object.c
> @@ -152,6 +152,7 @@ static void __i915_gem_free_object_rcu(struct rcu_head *head)
>  		container_of(head, typeof(*obj), rcu);
>  	struct drm_i915_private *i915 = to_i915(obj->base.dev);
>  
> +	reservation_object_fini(&obj->base._resv);
>  	i915_gem_object_free(obj);
>  
>  	GEM_BUG_ON(!atomic_read(&i915->mm.free_count));
> @@ -198,7 +199,7 @@ static void __i915_gem_free_objects(struct drm_i915_private *i915,
>  		if (obj->base.import_attach)
>  			drm_prime_gem_destroy(&obj->base, NULL);
>  
> -		drm_gem_object_release(&obj->base);
> +		drm_gem_free_mmap_offset(&obj->base);
>  
>  		/* But keep the pointer alive for RCU-protected lookups */
>  		call_rcu(&obj->rcu, __i915_gem_free_object_rcu);
> diff --git a/drivers/gpu/drm/i915/gem/i915_gem_shmem.c b/drivers/gpu/drm/i915/gem/i915_gem_shmem.c
> index 19d9ecdb2894..d2a1158868e7 100644
> --- a/drivers/gpu/drm/i915/gem/i915_gem_shmem.c
> +++ b/drivers/gpu/drm/i915/gem/i915_gem_shmem.c
> @@ -414,6 +414,11 @@ shmem_pwrite(struct drm_i915_gem_object *obj,
>  	return 0;
>  }
>  
> +static void shmem_release(struct drm_i915_gem_object *obj)
> +{
> +	fput(obj->base.filp);

We lose the check for filp existence. But as internal
ops have their own mechanics, we should always have the filp.

We lose a warn for dma_buf existence tho.
-Mika

> +}
> +
>  const struct drm_i915_gem_object_ops i915_gem_shmem_ops = {
>  	.flags = I915_GEM_OBJECT_HAS_STRUCT_PAGE |
>  		 I915_GEM_OBJECT_IS_SHRINKABLE,
> @@ -424,6 +429,8 @@ const struct drm_i915_gem_object_ops i915_gem_shmem_ops = {
>  	.writeback = shmem_writeback,
>  
>  	.pwrite = shmem_pwrite,
> +
> +	.release = shmem_release,
>  };
>  
>  static int create_shmem(struct drm_i915_private *i915,
> -- 
> 2.20.1
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 1/3] drm/i915/gem: Defer obj->base.resv fini until RCU callback

[Chris Wilson]

Quoting Mika Kuoppala (2019-07-04 14:53:09)
> Chris Wilson <chris@chris-wilson.co.uk> writes:
> 
> > Since reservation_object_fini() does an immediate free, rather than
> > kfree_rcu as normal, we have to delay the release until after the RCU
> > grace period has elapsed (i.e. from the rcu cleanup callback) so that we
> > can rely on the RCU protected access to the fences while the object is a
> > zombie.
> >
> > i915_gem_busy_ioctl relies on having an RCU barrier to protect the
> > reservation in order to avoid having to take a reference and strong
> > memory barriers.
> 
> Ok so for gem busy to be able to operate on a 'to be freed' object
> we need to keep the reservation object alive?

Yup. It could equally be kept alive if resv_obj_fini used kfree_rcu()
instead, but we already need an RCU barrier for our object lookup so
might as well use one stone for both birds.

> > Fixes: c03467ba40f7 ("drm/i915/gem: Free pages before rcu-freeing the object")
> > Testcase: igt/gem_busy/close-race
> > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> > Cc: Matthew Auld <matthew.auld@intel.com>
> > ---
> >  drivers/gpu/drm/i915/gem/i915_gem_object.c | 3 ++-
> >  drivers/gpu/drm/i915/gem/i915_gem_shmem.c  | 7 +++++++
> >  2 files changed, 9 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/gpu/drm/i915/gem/i915_gem_object.c b/drivers/gpu/drm/i915/gem/i915_gem_object.c
> > index d3e96f09c6b7..0dced4a20e20 100644
> > --- a/drivers/gpu/drm/i915/gem/i915_gem_object.c
> > +++ b/drivers/gpu/drm/i915/gem/i915_gem_object.c
> > @@ -152,6 +152,7 @@ static void __i915_gem_free_object_rcu(struct rcu_head *head)
> >               container_of(head, typeof(*obj), rcu);
> >       struct drm_i915_private *i915 = to_i915(obj->base.dev);
> >  
> > +     reservation_object_fini(&obj->base._resv);
> >       i915_gem_object_free(obj);
> >  
> >       GEM_BUG_ON(!atomic_read(&i915->mm.free_count));
> > @@ -198,7 +199,7 @@ static void __i915_gem_free_objects(struct drm_i915_private *i915,
> >               if (obj->base.import_attach)
> >                       drm_prime_gem_destroy(&obj->base, NULL);
> >  
> > -             drm_gem_object_release(&obj->base);
> > +             drm_gem_free_mmap_offset(&obj->base);
> >  
> >               /* But keep the pointer alive for RCU-protected lookups */
> >               call_rcu(&obj->rcu, __i915_gem_free_object_rcu);
> > diff --git a/drivers/gpu/drm/i915/gem/i915_gem_shmem.c b/drivers/gpu/drm/i915/gem/i915_gem_shmem.c
> > index 19d9ecdb2894..d2a1158868e7 100644
> > --- a/drivers/gpu/drm/i915/gem/i915_gem_shmem.c
> > +++ b/drivers/gpu/drm/i915/gem/i915_gem_shmem.c
> > @@ -414,6 +414,11 @@ shmem_pwrite(struct drm_i915_gem_object *obj,
> >       return 0;
> >  }
> >  
> > +static void shmem_release(struct drm_i915_gem_object *obj)
> > +{
> > +     fput(obj->base.filp);
> 
> We lose the check for filp existence. But as internal
> ops have their own mechanics, we should always have the filp.

Exactly. drm_gem_object should not have filp anymore.

> We lose a warn for dma_buf existence tho.

Good. Let me hand you a tiny violin ;)
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 1/3] drm/i915/gem: Defer obj->base.resv fini until RCU callback

[Mika Kuoppala]

Chris Wilson <chris@chris-wilson.co.uk> writes:

> Quoting Mika Kuoppala (2019-07-04 14:53:09)
>> Chris Wilson <chris@chris-wilson.co.uk> writes:
>> 
>> > Since reservation_object_fini() does an immediate free, rather than
>> > kfree_rcu as normal, we have to delay the release until after the RCU
>> > grace period has elapsed (i.e. from the rcu cleanup callback) so that we
>> > can rely on the RCU protected access to the fences while the object is a
>> > zombie.
>> >
>> > i915_gem_busy_ioctl relies on having an RCU barrier to protect the
>> > reservation in order to avoid having to take a reference and strong
>> > memory barriers.
>> 
>> Ok so for gem busy to be able to operate on a 'to be freed' object
>> we need to keep the reservation object alive?
>
> Yup. It could equally be kept alive if resv_obj_fini used kfree_rcu()
> instead, but we already need an RCU barrier for our object lookup so
> might as well use one stone for both birds.
>
>> > Fixes: c03467ba40f7 ("drm/i915/gem: Free pages before rcu-freeing the object")
>> > Testcase: igt/gem_busy/close-race
>> > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
>> > Cc: Matthew Auld <matthew.auld@intel.com>
>> > ---
>> >  drivers/gpu/drm/i915/gem/i915_gem_object.c | 3 ++-
>> >  drivers/gpu/drm/i915/gem/i915_gem_shmem.c  | 7 +++++++
>> >  2 files changed, 9 insertions(+), 1 deletion(-)
>> >
>> > diff --git a/drivers/gpu/drm/i915/gem/i915_gem_object.c b/drivers/gpu/drm/i915/gem/i915_gem_object.c
>> > index d3e96f09c6b7..0dced4a20e20 100644
>> > --- a/drivers/gpu/drm/i915/gem/i915_gem_object.c
>> > +++ b/drivers/gpu/drm/i915/gem/i915_gem_object.c
>> > @@ -152,6 +152,7 @@ static void __i915_gem_free_object_rcu(struct rcu_head *head)
>> >               container_of(head, typeof(*obj), rcu);
>> >       struct drm_i915_private *i915 = to_i915(obj->base.dev);
>> >  
>> > +     reservation_object_fini(&obj->base._resv);
>> >       i915_gem_object_free(obj);
>> >  
>> >       GEM_BUG_ON(!atomic_read(&i915->mm.free_count));
>> > @@ -198,7 +199,7 @@ static void __i915_gem_free_objects(struct drm_i915_private *i915,
>> >               if (obj->base.import_attach)
>> >                       drm_prime_gem_destroy(&obj->base, NULL);
>> >  
>> > -             drm_gem_object_release(&obj->base);
>> > +             drm_gem_free_mmap_offset(&obj->base);
>> >  
>> >               /* But keep the pointer alive for RCU-protected lookups */
>> >               call_rcu(&obj->rcu, __i915_gem_free_object_rcu);
>> > diff --git a/drivers/gpu/drm/i915/gem/i915_gem_shmem.c b/drivers/gpu/drm/i915/gem/i915_gem_shmem.c
>> > index 19d9ecdb2894..d2a1158868e7 100644
>> > --- a/drivers/gpu/drm/i915/gem/i915_gem_shmem.c
>> > +++ b/drivers/gpu/drm/i915/gem/i915_gem_shmem.c
>> > @@ -414,6 +414,11 @@ shmem_pwrite(struct drm_i915_gem_object *obj,
>> >       return 0;
>> >  }
>> >  
>> > +static void shmem_release(struct drm_i915_gem_object *obj)
>> > +{
>> > +     fput(obj->base.filp);
>> 
>> We lose the check for filp existence. But as internal
>> ops have their own mechanics, we should always have the filp.
>
> Exactly. drm_gem_object should not have filp anymore.

..for internal objects.

>
>> We lose a warn for dma_buf existence tho.
>
> Good. Let me hand you a tiny violin ;)

Let's see how it plays out.

Reviewed-by: Mika Kuoppala <mika.kuoppala@linux.intel.com>

> -Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 1/3] drm/i915/gem: Defer obj->base.resv fini until RCU callback

[Chris Wilson]

Quoting Mika Kuoppala (2019-07-04 15:18:40)
> Chris Wilson <chris@chris-wilson.co.uk> writes:
> 
> > Quoting Mika Kuoppala (2019-07-04 14:53:09)
> >> Chris Wilson <chris@chris-wilson.co.uk> writes:
> >> > +static void shmem_release(struct drm_i915_gem_object *obj)
> >> > +{
> >> > +     fput(obj->base.filp);
> >> 
> >> We lose the check for filp existence. But as internal
> >> ops have their own mechanics, we should always have the filp.
> >
> > Exactly. drm_gem_object should not have filp anymore.
> 
> ..for internal objects.

I mean the struct drm_gem_object should not include a struct file *filp
anymore as not all subclasses are shmem based. And where people want to
use it, it raises more questions than answers.
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

✓ Fi.CI.IGT: success for series starting with [v2] drm/i915/gem: Defer obj->base.resv fini until RCU callback (rev2)

[Patchwork]

== Series Details ==

Series: series starting with [v2] drm/i915/gem: Defer obj->base.resv fini until RCU callback (rev2)
URL   : https://patchwork.freedesktop.org/series/63160/
State : success

== Summary ==

CI Bug Log - changes from CI_DRM_6405_full -> Patchwork_13514_full
====================================================

Summary
-------

  **SUCCESS**

  No regressions found.

  

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

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

### IGT changes ###

#### Issues hit ####

  * igt@gem_exec_balancer@smoke:
    - shard-iclb:         [PASS][1] -> [SKIP][2] ([fdo#110854])
   [1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6405/shard-iclb4/igt@gem_exec_balancer@smoke.html
   [2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13514/shard-iclb5/igt@gem_exec_balancer@smoke.html

  * igt@gem_tiled_swapping@non-threaded:
    - shard-apl:          [PASS][3] -> [DMESG-WARN][4] ([fdo#108686])
   [3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6405/shard-apl4/igt@gem_tiled_swapping@non-threaded.html
   [4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13514/shard-apl2/igt@gem_tiled_swapping@non-threaded.html

  * igt@kms_flip@flip-vs-expired-vblank-interruptible:
    - shard-glk:          [PASS][5] -> [FAIL][6] ([fdo#105363])
   [5]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6405/shard-glk7/igt@kms_flip@flip-vs-expired-vblank-interruptible.html
   [6]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13514/shard-glk8/igt@kms_flip@flip-vs-expired-vblank-interruptible.html

  * igt@kms_flip@flip-vs-suspend-interruptible:
    - shard-hsw:          [PASS][7] -> [INCOMPLETE][8] ([fdo#103540]) +1 similar issue
   [7]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6405/shard-hsw7/igt@kms_flip@flip-vs-suspend-interruptible.html
   [8]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13514/shard-hsw1/igt@kms_flip@flip-vs-suspend-interruptible.html

  * igt@kms_frontbuffer_tracking@fbc-suspend:
    - shard-apl:          [PASS][9] -> [DMESG-WARN][10] ([fdo#108566]) +4 similar issues
   [9]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6405/shard-apl2/igt@kms_frontbuffer_tracking@fbc-suspend.html
   [10]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13514/shard-apl4/igt@kms_frontbuffer_tracking@fbc-suspend.html

  * igt@kms_frontbuffer_tracking@fbcpsr-1p-primscrn-shrfb-plflip-blt:
    - shard-iclb:         [PASS][11] -> [FAIL][12] ([fdo#103167]) +3 similar issues
   [11]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6405/shard-iclb2/igt@kms_frontbuffer_tracking@fbcpsr-1p-primscrn-shrfb-plflip-blt.html
   [12]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13514/shard-iclb1/igt@kms_frontbuffer_tracking@fbcpsr-1p-primscrn-shrfb-plflip-blt.html

  * igt@kms_plane_alpha_blend@pipe-b-coverage-7efc:
    - shard-skl:          [PASS][13] -> [FAIL][14] ([fdo#108145] / [fdo#110403])
   [13]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6405/shard-skl10/igt@kms_plane_alpha_blend@pipe-b-coverage-7efc.html
   [14]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13514/shard-skl10/igt@kms_plane_alpha_blend@pipe-b-coverage-7efc.html

  * igt@kms_plane_alpha_blend@pipe-c-constant-alpha-min:
    - shard-skl:          [PASS][15] -> [FAIL][16] ([fdo#108145])
   [15]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6405/shard-skl8/igt@kms_plane_alpha_blend@pipe-c-constant-alpha-min.html
   [16]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13514/shard-skl8/igt@kms_plane_alpha_blend@pipe-c-constant-alpha-min.html

  * igt@kms_psr@psr2_primary_mmap_cpu:
    - shard-iclb:         [PASS][17] -> [SKIP][18] ([fdo#109441]) +3 similar issues
   [17]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6405/shard-iclb2/igt@kms_psr@psr2_primary_mmap_cpu.html
   [18]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13514/shard-iclb7/igt@kms_psr@psr2_primary_mmap_cpu.html

  * igt@kms_setmode@basic:
    - shard-apl:          [PASS][19] -> [FAIL][20] ([fdo#99912])
   [19]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6405/shard-apl4/igt@kms_setmode@basic.html
   [20]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13514/shard-apl8/igt@kms_setmode@basic.html

  
#### Possible fixes ####

  * igt@gem_eio@in-flight-suspend:
    - shard-apl:          [DMESG-WARN][21] ([fdo#108566]) -> [PASS][22]
   [21]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6405/shard-apl5/igt@gem_eio@in-flight-suspend.html
   [22]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13514/shard-apl7/igt@gem_eio@in-flight-suspend.html

  * igt@kms_frontbuffer_tracking@fbc-1p-offscren-pri-shrfb-draw-pwrite:
    - shard-iclb:         [FAIL][23] ([fdo#103167]) -> [PASS][24]
   [23]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6405/shard-iclb6/igt@kms_frontbuffer_tracking@fbc-1p-offscren-pri-shrfb-draw-pwrite.html
   [24]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13514/shard-iclb6/igt@kms_frontbuffer_tracking@fbc-1p-offscren-pri-shrfb-draw-pwrite.html

  * igt@kms_psr@psr2_basic:
    - shard-iclb:         [SKIP][25] ([fdo#109441]) -> [PASS][26]
   [25]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6405/shard-iclb8/igt@kms_psr@psr2_basic.html
   [26]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13514/shard-iclb2/igt@kms_psr@psr2_basic.html

  * igt@kms_setmode@basic:
    - shard-kbl:          [FAIL][27] ([fdo#99912]) -> [PASS][28]
   [27]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6405/shard-kbl4/igt@kms_setmode@basic.html
   [28]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13514/shard-kbl6/igt@kms_setmode@basic.html

  
#### Warnings ####

  * igt@kms_frontbuffer_tracking@fbc-tilingchange:
    - shard-skl:          [FAIL][29] ([fdo#108040]) -> [FAIL][30] ([fdo#103167])
   [29]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6405/shard-skl1/igt@kms_frontbuffer_tracking@fbc-tilingchange.html
   [30]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13514/shard-skl8/igt@kms_frontbuffer_tracking@fbc-tilingchange.html

  
  [fdo#103167]: https://bugs.freedesktop.org/show_bug.cgi?id=103167
  [fdo#103540]: https://bugs.freedesktop.org/show_bug.cgi?id=103540
  [fdo#105363]: https://bugs.freedesktop.org/show_bug.cgi?id=105363
  [fdo#108040]: https://bugs.freedesktop.org/show_bug.cgi?id=108040
  [fdo#108145]: https://bugs.freedesktop.org/show_bug.cgi?id=108145
  [fdo#108566]: https://bugs.freedesktop.org/show_bug.cgi?id=108566
  [fdo#108686]: https://bugs.freedesktop.org/show_bug.cgi?id=108686
  [fdo#109441]: https://bugs.freedesktop.org/show_bug.cgi?id=109441
  [fdo#110403]: https://bugs.freedesktop.org/show_bug.cgi?id=110403
  [fdo#110854]: https://bugs.freedesktop.org/show_bug.cgi?id=110854
  [fdo#99912]: https://bugs.freedesktop.org/show_bug.cgi?id=99912


Participating hosts (10 -> 10)
------------------------------

  No changes in participating hosts


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

  * Linux: CI_DRM_6405 -> Patchwork_13514

  CI_DRM_6405: d395f3e20d154dfeabb95117f388f2e953c12ac9 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_5082: f7c51e6fbf8da0784b64a1edaee5266aa9b9f829 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_13514: c2bdbb50c9c7ad6e57705cfaf6473d68546ad4bf @ git://anongit.freedesktop.org/gfx-ci/linux
  piglit_4509: fdc5a4ca11124ab8413c7988896eec4c97336694 @ git://anongit.freedesktop.org/piglit

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13514/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx