[PATCH] drm/i915/selftests: Let other struct_mutex users have their gpu time

[PATCH] drm/i915/selftests: Let other struct_mutex users have their gpu time

[Chris Wilson]

live_gtt is a very slow test to run, simply because it tries to allocate
and use as much as the 48b address space as possibly can and in the
process will try to own all of the system memory. This leads to resource
exhaustion and CPU starvation; the latter impacts us when the NMI
watchdog declares a task hung due to a mutex contention with ourselves.
This we can prevent by releasing the struct_mutex around cond_resched(),
to allow e.g. the i915_gem_free_objects_work to acquire the mutex and
cleanup.

References: https://bugs.freedesktop.org/show_bug.cgi?id=107094
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
---
 drivers/gpu/drm/i915/selftests/i915_gem_gtt.c | 58 ++++++++++++++-----
 1 file changed, 45 insertions(+), 13 deletions(-)

diff --git a/drivers/gpu/drm/i915/selftests/i915_gem_gtt.c b/drivers/gpu/drm/i915/selftests/i915_gem_gtt.c
index a28ee0cc6a63..789add200720 100644
--- a/drivers/gpu/drm/i915/selftests/i915_gem_gtt.c
+++ b/drivers/gpu/drm/i915/selftests/i915_gem_gtt.c
@@ -32,6 +32,14 @@
 #include "mock_drm.h"
 #include "mock_gem_device.h"
 
+static void schedule_locked(struct drm_i915_private *i915)
+{
+	/* Let anyone else who wants the context take it */
+	mutex_unlock(&i915->drm.struct_mutex);
+	cond_resched();
+	mutex_lock(&i915->drm.struct_mutex);
+}
+
 static void fake_free_pages(struct drm_i915_gem_object *obj,
 			    struct sg_table *pages)
 {
@@ -132,18 +140,18 @@ fake_dma_object(struct drm_i915_private *i915, u64 size)
 
 static int igt_ppgtt_alloc(void *arg)
 {
-	struct drm_i915_private *dev_priv = arg;
+	struct drm_i915_private *i915 = arg;
 	struct i915_hw_ppgtt *ppgtt;
 	u64 size, last;
 	int err = 0;
 
 	/* Allocate a ppggt and try to fill the entire range */
 
-	if (!USES_PPGTT(dev_priv))
+	if (!USES_PPGTT(i915))
 		return 0;
 
-	mutex_lock(&dev_priv->drm.struct_mutex);
-	ppgtt = __hw_ppgtt_create(dev_priv);
+	mutex_lock(&i915->drm.struct_mutex);
+	ppgtt = __hw_ppgtt_create(i915);
 	if (IS_ERR(ppgtt)) {
 		err = PTR_ERR(ppgtt);
 		goto err_unlock;
@@ -169,6 +177,8 @@ static int igt_ppgtt_alloc(void *arg)
 		ppgtt->vm.clear_range(&ppgtt->vm, 0, size);
 	}
 
+	schedule_locked(i915);
+
 	/* Check we can incrementally allocate the entire range */
 	for (last = 0, size = 4096;
 	     size <= ppgtt->vm.total;
@@ -189,7 +199,7 @@ static int igt_ppgtt_alloc(void *arg)
 	ppgtt->vm.cleanup(&ppgtt->vm);
 	kfree(ppgtt);
 err_unlock:
-	mutex_unlock(&dev_priv->drm.struct_mutex);
+	mutex_unlock(&i915->drm.struct_mutex);
 	return err;
 }
 
@@ -291,6 +301,8 @@ static int lowlevel_hole(struct drm_i915_private *i915,
 		i915_gem_object_put(obj);
 
 		kfree(order);
+
+		schedule_locked(i915);
 	}
 
 	return 0;
@@ -519,6 +531,7 @@ static int fill_hole(struct drm_i915_private *i915,
 		}
 
 		close_object_list(&objects, vm);
+		schedule_locked(i915);
 	}
 
 	return 0;
@@ -605,6 +618,8 @@ static int walk_hole(struct drm_i915_private *i915,
 		i915_gem_object_put(obj);
 		if (err)
 			return err;
+
+		schedule_locked(i915);
 	}
 
 	return 0;
@@ -676,6 +691,8 @@ static int pot_hole(struct drm_i915_private *i915,
 			err = -EINTR;
 			goto err;
 		}
+
+		schedule_locked(i915);
 	}
 
 err:
@@ -789,6 +806,8 @@ static int drunk_hole(struct drm_i915_private *i915,
 		kfree(order);
 		if (err)
 			return err;
+
+		schedule_locked(i915);
 	}
 
 	return 0;
@@ -854,6 +873,8 @@ static int __shrink_hole(struct drm_i915_private *i915,
 			err = -EINTR;
 			break;
 		}
+
+		schedule_locked(i915);
 	}
 
 	close_object_list(&objects, vm);
@@ -949,6 +970,7 @@ static int shrink_boom(struct drm_i915_private *i915,
 		i915_gem_object_put(explode);
 
 		memset(&vm->fault_attr, 0, sizeof(vm->fault_attr));
+		schedule_locked(i915);
 	}
 
 	return 0;
@@ -961,7 +983,7 @@ static int shrink_boom(struct drm_i915_private *i915,
 	return err;
 }
 
-static int exercise_ppgtt(struct drm_i915_private *dev_priv,
+static int exercise_ppgtt(struct drm_i915_private *i915,
 			  int (*func)(struct drm_i915_private *i915,
 				      struct i915_address_space *vm,
 				      u64 hole_start, u64 hole_end,
@@ -972,15 +994,15 @@ static int exercise_ppgtt(struct drm_i915_private *dev_priv,
 	IGT_TIMEOUT(end_time);
 	int err;
 
-	if (!USES_FULL_PPGTT(dev_priv))
+	if (!USES_FULL_PPGTT(i915))
 		return 0;
 
-	file = mock_file(dev_priv);
+	file = mock_file(i915);
 	if (IS_ERR(file))
 		return PTR_ERR(file);
 
-	mutex_lock(&dev_priv->drm.struct_mutex);
-	ppgtt = i915_ppgtt_create(dev_priv, file->driver_priv, "mock");
+	mutex_lock(&i915->drm.struct_mutex);
+	ppgtt = i915_ppgtt_create(i915, file->driver_priv, "mock");
 	if (IS_ERR(ppgtt)) {
 		err = PTR_ERR(ppgtt);
 		goto out_unlock;
@@ -988,14 +1010,14 @@ static int exercise_ppgtt(struct drm_i915_private *dev_priv,
 	GEM_BUG_ON(offset_in_page(ppgtt->vm.total));
 	GEM_BUG_ON(ppgtt->vm.closed);
 
-	err = func(dev_priv, &ppgtt->vm, 0, ppgtt->vm.total, end_time);
+	err = func(i915, &ppgtt->vm, 0, ppgtt->vm.total, end_time);
 
 	i915_ppgtt_close(&ppgtt->vm);
 	i915_ppgtt_put(ppgtt);
 out_unlock:
-	mutex_unlock(&dev_priv->drm.struct_mutex);
+	mutex_unlock(&i915->drm.struct_mutex);
 
-	mock_file_free(dev_priv, file);
+	mock_file_free(i915, file);
 	return err;
 }
 
@@ -1315,6 +1337,8 @@ static int igt_gtt_reserve(void *arg)
 		}
 	}
 
+	schedule_locked(i915);
+
 	/* Now we start forcing evictions */
 	for (total = I915_GTT_PAGE_SIZE;
 	     total + 2*I915_GTT_PAGE_SIZE <= i915->ggtt.vm.total;
@@ -1364,6 +1388,8 @@ static int igt_gtt_reserve(void *arg)
 		}
 	}
 
+	schedule_locked(i915);
+
 	/* And then try at random */
 	list_for_each_entry_safe(obj, on, &objects, st_link) {
 		struct i915_vma *vma;
@@ -1517,6 +1543,8 @@ static int igt_gtt_insert(void *arg)
 		GEM_BUG_ON(!drm_mm_node_allocated(&vma->node));
 	}
 
+	schedule_locked(i915);
+
 	list_for_each_entry(obj, &objects, st_link) {
 		struct i915_vma *vma;
 
@@ -1535,6 +1563,8 @@ static int igt_gtt_insert(void *arg)
 		__i915_vma_unpin(vma);
 	}
 
+	schedule_locked(i915);
+
 	/* If we then reinsert, we should find the same hole */
 	list_for_each_entry_safe(obj, on, &objects, st_link) {
 		struct i915_vma *vma;
@@ -1575,6 +1605,8 @@ static int igt_gtt_insert(void *arg)
 		}
 	}
 
+	schedule_locked(i915);
+
 	/* And then force evictions */
 	for (total = 0;
 	     total + 2*I915_GTT_PAGE_SIZE <= i915->ggtt.vm.total;
-- 
2.18.0

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

✓ Fi.CI.BAT: success for drm/i915/selftests: Let other struct_mutex users have their gpu time

[Patchwork]

== Series Details ==

Series: drm/i915/selftests: Let other struct_mutex users have their gpu time
URL   : https://patchwork.freedesktop.org/series/45810/
State : success

== Summary ==

= CI Bug Log - changes from CI_DRM_4418 -> Patchwork_9504 =

== Summary - WARNING ==

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

  External URL: https://patchwork.freedesktop.org/api/1.0/series/45810/revisions/1/mbox/

== Possible new issues ==

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

  === IGT changes ===

    ==== Warnings ====

    igt@gem_exec_gttfill@basic:
      fi-pnv-d510:        SKIP -> PASS

    
== Known issues ==

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

  === IGT changes ===

    ==== Issues hit ====

    igt@kms_pipe_crc_basic@suspend-read-crc-pipe-c:
      fi-kbl-7567u:       PASS -> FAIL (fdo#103191, fdo#104724)

    
  fdo#103191 https://bugs.freedesktop.org/show_bug.cgi?id=103191
  fdo#104724 https://bugs.freedesktop.org/show_bug.cgi?id=104724


== Participating hosts (45 -> 40) ==

  Missing    (5): fi-ctg-p8600 fi-ilk-m540 fi-byt-squawks fi-bsw-cyan fi-hsw-4200u 


== Build changes ==

    * Linux: CI_DRM_4418 -> Patchwork_9504

  CI_DRM_4418: 1b4261461fc3fc25b77f32dd3c80e21d676a2a16 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_4532: 840d12e2f050b784552197403d6575a57b6e896d @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_9504: 5b15ca8c259dd71aeb71410a0ed67fd9a6569556 @ git://anongit.freedesktop.org/gfx-ci/linux


== Linux commits ==

5b15ca8c259d drm/i915/selftests: Let other struct_mutex users have their gpu time

== Logs ==

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

Re: [PATCH] drm/i915/selftests: Let other struct_mutex users have their gpu time

[Tvrtko Ursulin]

On 03/07/2018 09:30, Chris Wilson wrote:
> live_gtt is a very slow test to run, simply because it tries to allocate
> and use as much as the 48b address space as possibly can and in the
> process will try to own all of the system memory. This leads to resource
> exhaustion and CPU starvation; the latter impacts us when the NMI
> watchdog declares a task hung due to a mutex contention with ourselves.
> This we can prevent by releasing the struct_mutex around cond_resched(),
> to allow e.g. the i915_gem_free_objects_work to acquire the mutex and
> cleanup.
> 
> References: https://bugs.freedesktop.org/show_bug.cgi?id=107094
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> ---
>   drivers/gpu/drm/i915/selftests/i915_gem_gtt.c | 58 ++++++++++++++-----
>   1 file changed, 45 insertions(+), 13 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/selftests/i915_gem_gtt.c b/drivers/gpu/drm/i915/selftests/i915_gem_gtt.c
> index a28ee0cc6a63..789add200720 100644
> --- a/drivers/gpu/drm/i915/selftests/i915_gem_gtt.c
> +++ b/drivers/gpu/drm/i915/selftests/i915_gem_gtt.c
> @@ -32,6 +32,14 @@
>   #include "mock_drm.h"
>   #include "mock_gem_device.h"
>   
> +static void schedule_locked(struct drm_i915_private *i915)
> +{
> +	/* Let anyone else who wants the context take it */
> +	mutex_unlock(&i915->drm.struct_mutex);
> +	cond_resched();
> +	mutex_lock(&i915->drm.struct_mutex);
> +}
> +
>   static void fake_free_pages(struct drm_i915_gem_object *obj,
>   			    struct sg_table *pages)
>   {
> @@ -132,18 +140,18 @@ fake_dma_object(struct drm_i915_private *i915, u64 size)
>   
>   static int igt_ppgtt_alloc(void *arg)
>   {
> -	struct drm_i915_private *dev_priv = arg;
> +	struct drm_i915_private *i915 = arg;

Adding some flavour to reviewers lives! :I

>   	struct i915_hw_ppgtt *ppgtt;
>   	u64 size, last;
>   	int err = 0;
>   
>   	/* Allocate a ppggt and try to fill the entire range */
>   
> -	if (!USES_PPGTT(dev_priv))
> +	if (!USES_PPGTT(i915))
>   		return 0;
>   
> -	mutex_lock(&dev_priv->drm.struct_mutex);
> -	ppgtt = __hw_ppgtt_create(dev_priv);
> +	mutex_lock(&i915->drm.struct_mutex);
> +	ppgtt = __hw_ppgtt_create(i915);
>   	if (IS_ERR(ppgtt)) {
>   		err = PTR_ERR(ppgtt);
>   		goto err_unlock;
> @@ -169,6 +177,8 @@ static int igt_ppgtt_alloc(void *arg)
>   		ppgtt->vm.clear_range(&ppgtt->vm, 0, size);
>   	}
>   
> +	schedule_locked(i915);
> +

Is it needed in this test? I glanced over and couldn't spot anything 
struct mutexy in page table only manipulations it does.

>   	/* Check we can incrementally allocate the entire range */
>   	for (last = 0, size = 4096;
>   	     size <= ppgtt->vm.total;
> @@ -189,7 +199,7 @@ static int igt_ppgtt_alloc(void *arg)
>   	ppgtt->vm.cleanup(&ppgtt->vm);
>   	kfree(ppgtt);
>   err_unlock:
> -	mutex_unlock(&dev_priv->drm.struct_mutex);
> +	mutex_unlock(&i915->drm.struct_mutex);
>   	return err;
>   }
>   
> @@ -291,6 +301,8 @@ static int lowlevel_hole(struct drm_i915_private *i915,
>   		i915_gem_object_put(obj);
>   
>   		kfree(order);
> +
> +		schedule_locked(i915);
>   	}
>   
>   	return 0;
> @@ -519,6 +531,7 @@ static int fill_hole(struct drm_i915_private *i915,
>   		}
>   
>   		close_object_list(&objects, vm);
> +		schedule_locked(i915);
>   	}
>   
>   	return 0;
> @@ -605,6 +618,8 @@ static int walk_hole(struct drm_i915_private *i915,
>   		i915_gem_object_put(obj);
>   		if (err)
>   			return err;
> +
> +		schedule_locked(i915);
>   	}
>   
>   	return 0;
> @@ -676,6 +691,8 @@ static int pot_hole(struct drm_i915_private *i915,
>   			err = -EINTR;
>   			goto err;
>   		}
> +
> +		schedule_locked(i915);
>   	}
>   
>   err:
> @@ -789,6 +806,8 @@ static int drunk_hole(struct drm_i915_private *i915,
>   		kfree(order);
>   		if (err)
>   			return err;
> +
> +		schedule_locked(i915);
>   	}
>   
>   	return 0;
> @@ -854,6 +873,8 @@ static int __shrink_hole(struct drm_i915_private *i915,
>   			err = -EINTR;
>   			break;
>   		}
> +
> +		schedule_locked(i915);
>   	}
>   
>   	close_object_list(&objects, vm);
> @@ -949,6 +970,7 @@ static int shrink_boom(struct drm_i915_private *i915,
>   		i915_gem_object_put(explode);
>   
>   		memset(&vm->fault_attr, 0, sizeof(vm->fault_attr));
> +		schedule_locked(i915);
>   	}
>   
>   	return 0;
> @@ -961,7 +983,7 @@ static int shrink_boom(struct drm_i915_private *i915,
>   	return err;
>   }
>   
> -static int exercise_ppgtt(struct drm_i915_private *dev_priv,
> +static int exercise_ppgtt(struct drm_i915_private *i915,
>   			  int (*func)(struct drm_i915_private *i915,
>   				      struct i915_address_space *vm,
>   				      u64 hole_start, u64 hole_end,
> @@ -972,15 +994,15 @@ static int exercise_ppgtt(struct drm_i915_private *dev_priv,
>   	IGT_TIMEOUT(end_time);
>   	int err;
>   
> -	if (!USES_FULL_PPGTT(dev_priv))
> +	if (!USES_FULL_PPGTT(i915))
>   		return 0;
>   
> -	file = mock_file(dev_priv);
> +	file = mock_file(i915);
>   	if (IS_ERR(file))
>   		return PTR_ERR(file);
>   
> -	mutex_lock(&dev_priv->drm.struct_mutex);
> -	ppgtt = i915_ppgtt_create(dev_priv, file->driver_priv, "mock");
> +	mutex_lock(&i915->drm.struct_mutex);
> +	ppgtt = i915_ppgtt_create(i915, file->driver_priv, "mock");
>   	if (IS_ERR(ppgtt)) {
>   		err = PTR_ERR(ppgtt);
>   		goto out_unlock;
> @@ -988,14 +1010,14 @@ static int exercise_ppgtt(struct drm_i915_private *dev_priv,
>   	GEM_BUG_ON(offset_in_page(ppgtt->vm.total));
>   	GEM_BUG_ON(ppgtt->vm.closed);
>   
> -	err = func(dev_priv, &ppgtt->vm, 0, ppgtt->vm.total, end_time);
> +	err = func(i915, &ppgtt->vm, 0, ppgtt->vm.total, end_time);

How about a less hacky solution where func is called unlocked it is 
responsible to take struct_mutex across the parts which need it?

Regards,

Tvrtko

>   
>   	i915_ppgtt_close(&ppgtt->vm);
>   	i915_ppgtt_put(ppgtt);
>   out_unlock:
> -	mutex_unlock(&dev_priv->drm.struct_mutex);
> +	mutex_unlock(&i915->drm.struct_mutex);
>   
> -	mock_file_free(dev_priv, file);
> +	mock_file_free(i915, file);
>   	return err;
>   }
>   
> @@ -1315,6 +1337,8 @@ static int igt_gtt_reserve(void *arg)
>   		}
>   	}
>   
> +	schedule_locked(i915);
> +
>   	/* Now we start forcing evictions */
>   	for (total = I915_GTT_PAGE_SIZE;
>   	     total + 2*I915_GTT_PAGE_SIZE <= i915->ggtt.vm.total;
> @@ -1364,6 +1388,8 @@ static int igt_gtt_reserve(void *arg)
>   		}
>   	}
>   
> +	schedule_locked(i915);
> +
>   	/* And then try at random */
>   	list_for_each_entry_safe(obj, on, &objects, st_link) {
>   		struct i915_vma *vma;
> @@ -1517,6 +1543,8 @@ static int igt_gtt_insert(void *arg)
>   		GEM_BUG_ON(!drm_mm_node_allocated(&vma->node));
>   	}
>   
> +	schedule_locked(i915);
> +
>   	list_for_each_entry(obj, &objects, st_link) {
>   		struct i915_vma *vma;
>   
> @@ -1535,6 +1563,8 @@ static int igt_gtt_insert(void *arg)
>   		__i915_vma_unpin(vma);
>   	}
>   
> +	schedule_locked(i915);
> +
>   	/* If we then reinsert, we should find the same hole */
>   	list_for_each_entry_safe(obj, on, &objects, st_link) {
>   		struct i915_vma *vma;
> @@ -1575,6 +1605,8 @@ static int igt_gtt_insert(void *arg)
>   		}
>   	}
>   
> +	schedule_locked(i915);
> +
>   	/* And then force evictions */
>   	for (total = 0;
>   	     total + 2*I915_GTT_PAGE_SIZE <= i915->ggtt.vm.total;
> 
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH] drm/i915/selftests: Let other struct_mutex users have their gpu time

[Chris Wilson]

Quoting Tvrtko Ursulin (2018-07-03 10:27:47)
> 
> On 03/07/2018 09:30, Chris Wilson wrote:
> > @@ -169,6 +177,8 @@ static int igt_ppgtt_alloc(void *arg)
> >               ppgtt->vm.clear_range(&ppgtt->vm, 0, size);
> >       }
> >   
> > +     schedule_locked(i915);
> > +
> 
> Is it needed in this test? I glanced over and couldn't spot anything 
> struct mutexy in page table only manipulations it does.

struct_mutex is the guard for all drm_mm/i915_address_space, so while we
may not need it exactly in this instance, it is the current mutex to
use. I do have per-vm mutexes in the queue, it's quite scary.

> > @@ -988,14 +1010,14 @@ static int exercise_ppgtt(struct drm_i915_private *dev_priv,
> >       GEM_BUG_ON(offset_in_page(ppgtt->vm.total));
> >       GEM_BUG_ON(ppgtt->vm.closed);
> >   
> > -     err = func(dev_priv, &ppgtt->vm, 0, ppgtt->vm.total, end_time);
> > +     err = func(i915, &ppgtt->vm, 0, ppgtt->vm.total, end_time);
> 
> How about a less hacky solution where func is called unlocked it is 
> responsible to take struct_mutex across the parts which need it?

Convenience. Single threaded tests that didn't want to care about the
details of struct_mutex. And then CI sets itself up in annoying ways!
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

✓ Fi.CI.IGT: success for drm/i915/selftests: Let other struct_mutex users have their gpu time

[Patchwork]

== Series Details ==

Series: drm/i915/selftests: Let other struct_mutex users have their gpu time
URL   : https://patchwork.freedesktop.org/series/45810/
State : success

== Summary ==

= CI Bug Log - changes from CI_DRM_4418_full -> Patchwork_9504_full =

== Summary - WARNING ==

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

  

== Possible new issues ==

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

  === IGT changes ===

    ==== Warnings ====

    igt@gem_exec_schedule@deep-bsd1:
      shard-kbl:          SKIP -> PASS +1

    
== Known issues ==

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

  === IGT changes ===

    ==== Issues hit ====

    igt@drv_selftest@live_gtt:
      shard-apl:          PASS -> INCOMPLETE (fdo#103927)

    igt@gem_workarounds@suspend-resume-context:
      shard-kbl:          PASS -> INCOMPLETE (fdo#103665)

    igt@kms_flip_tiling@flip-to-x-tiled:
      shard-glk:          PASS -> FAIL (fdo#104724) +1

    igt@kms_rotation_crc@sprite-rotation-180:
      shard-hsw:          PASS -> FAIL (fdo#104724, fdo#103925)

    
    ==== Possible fixes ====

    igt@drv_selftest@live_gtt:
      shard-kbl:          FAIL (fdo#105347) -> PASS

    igt@kms_cursor_legacy@2x-nonblocking-modeset-vs-cursor-atomic:
      shard-glk:          FAIL (fdo#106509, fdo#105454) -> PASS

    igt@kms_cursor_legacy@cursor-vs-flip-atomic-transitions:
      shard-hsw:          FAIL (fdo#103355) -> PASS

    igt@kms_flip_tiling@flip-x-tiled:
      shard-glk:          FAIL (fdo#104724, fdo#103822) -> PASS

    igt@kms_setmode@basic:
      shard-apl:          FAIL (fdo#99912) -> PASS

    
    ==== Warnings ====

    igt@drv_selftest@live_gtt:
      shard-glk:          FAIL (fdo#105347) -> INCOMPLETE (fdo#103359, k.org#198133)

    
  fdo#103355 https://bugs.freedesktop.org/show_bug.cgi?id=103355
  fdo#103359 https://bugs.freedesktop.org/show_bug.cgi?id=103359
  fdo#103665 https://bugs.freedesktop.org/show_bug.cgi?id=103665
  fdo#103822 https://bugs.freedesktop.org/show_bug.cgi?id=103822
  fdo#103925 https://bugs.freedesktop.org/show_bug.cgi?id=103925
  fdo#103927 https://bugs.freedesktop.org/show_bug.cgi?id=103927
  fdo#104724 https://bugs.freedesktop.org/show_bug.cgi?id=104724
  fdo#105347 https://bugs.freedesktop.org/show_bug.cgi?id=105347
  fdo#105454 https://bugs.freedesktop.org/show_bug.cgi?id=105454
  fdo#106509 https://bugs.freedesktop.org/show_bug.cgi?id=106509
  fdo#99912 https://bugs.freedesktop.org/show_bug.cgi?id=99912
  k.org#198133 https://bugzilla.kernel.org/show_bug.cgi?id=198133


== Participating hosts (5 -> 5) ==

  No changes in participating hosts


== Build changes ==

    * Linux: CI_DRM_4418 -> Patchwork_9504

  CI_DRM_4418: 1b4261461fc3fc25b77f32dd3c80e21d676a2a16 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_4532: 840d12e2f050b784552197403d6575a57b6e896d @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_9504: 5b15ca8c259dd71aeb71410a0ed67fd9a6569556 @ 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_9504/shards.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH] drm/i915/selftests: Let other struct_mutex users have their gpu time

[Chris Wilson]

Quoting Chris Wilson (2018-07-03 09:30:15)
> @@ -132,18 +140,18 @@ fake_dma_object(struct drm_i915_private *i915, u64 size)
>  
>  static int igt_ppgtt_alloc(void *arg)
>  {
> -       struct drm_i915_private *dev_priv = arg;
> +       struct drm_i915_private *i915 = arg;
>         struct i915_hw_ppgtt *ppgtt;
>         u64 size, last;
>         int err = 0;
>  
>         /* Allocate a ppggt and try to fill the entire range */
>  
> -       if (!USES_PPGTT(dev_priv))
> +       if (!USES_PPGTT(i915))
>                 return 0;
>  
> -       mutex_lock(&dev_priv->drm.struct_mutex);
> -       ppgtt = __hw_ppgtt_create(dev_priv);
> +       mutex_lock(&i915->drm.struct_mutex);
> +       ppgtt = __hw_ppgtt_create(i915);
>         if (IS_ERR(ppgtt)) {
>                 err = PTR_ERR(ppgtt);
>                 goto err_unlock;
> @@ -169,6 +177,8 @@ static int igt_ppgtt_alloc(void *arg)
>                 ppgtt->vm.clear_range(&ppgtt->vm, 0, size);
>         }
>  
> +       schedule_locked(i915);

This still appears to be too coarse for glk.
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH] drm/i915/selftests: Let other struct_mutex users have their gpu time

[Tvrtko Ursulin]

On 03/07/2018 10:52, Chris Wilson wrote:
> Quoting Tvrtko Ursulin (2018-07-03 10:27:47)
>>
>> On 03/07/2018 09:30, Chris Wilson wrote:
>>> @@ -169,6 +177,8 @@ static int igt_ppgtt_alloc(void *arg)
>>>                ppgtt->vm.clear_range(&ppgtt->vm, 0, size);
>>>        }
>>>    
>>> +     schedule_locked(i915);
>>> +
>>
>> Is it needed in this test? I glanced over and couldn't spot anything
>> struct mutexy in page table only manipulations it does.
> 
> struct_mutex is the guard for all drm_mm/i915_address_space, so while we
> may not need it exactly in this instance, it is the current mutex to
> use. I do have per-vm mutexes in the queue, it's quite scary.

I meant mutex dropping doesn't seem to be needed for this test, unless I 
am missing something.

>>> @@ -988,14 +1010,14 @@ static int exercise_ppgtt(struct drm_i915_private *dev_priv,
>>>        GEM_BUG_ON(offset_in_page(ppgtt->vm.total));
>>>        GEM_BUG_ON(ppgtt->vm.closed);
>>>    
>>> -     err = func(dev_priv, &ppgtt->vm, 0, ppgtt->vm.total, end_time);
>>> +     err = func(i915, &ppgtt->vm, 0, ppgtt->vm.total, end_time);
>>
>> How about a less hacky solution where func is called unlocked it is
>> responsible to take struct_mutex across the parts which need it?
> 
> Convenience. Single threaded tests that didn't want to care about the
> details of struct_mutex. And then CI sets itself up in annoying ways!

Yeah, I get the convenience just worry it ends up that little bit harder 
to maintain. Okay I guess for testing..

Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>

Regards,

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

[PATCH v2] drm/i915/selftests: Release the struct_mutex to free the objects

[Chris Wilson]

live_gtt is a very slow test to run, simply because it tries to allocate
and use as much as the 48b address space as possibly can and in the
process will try to own all of the system memory. This leads to resource
exhaustion and CPU starvation; the latter impacts us when the NMI
watchdog declares a task hung due to a mutex contention with ourselves.
This we can prevent by releasing the struct_mutex and forcing our
i915/rcu workers to run, and in particular flushing the freed object
worker that is the cause for concern.

References: https://bugs.freedesktop.org/show_bug.cgi?id=107094
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
---
 drivers/gpu/drm/i915/selftests/i915_gem_gtt.c | 23 +++++++++++++++++++
 1 file changed, 23 insertions(+)

diff --git a/drivers/gpu/drm/i915/selftests/i915_gem_gtt.c b/drivers/gpu/drm/i915/selftests/i915_gem_gtt.c
index fff26bd05f71..eefcf7b84054 100644
--- a/drivers/gpu/drm/i915/selftests/i915_gem_gtt.c
+++ b/drivers/gpu/drm/i915/selftests/i915_gem_gtt.c
@@ -32,6 +32,20 @@
 #include "mock_drm.h"
 #include "mock_gem_device.h"
 
+static void cleanup_freed_objects(struct drm_i915_private *i915)
+{
+	/*
+	 * As we may hold onto the struct_mutex for inordinate lengths of
+	 * time, the NMI khungtaskd detector may fire for the free objects
+	 * worker.
+	 */
+	mutex_unlock(&i915->drm.struct_mutex);
+
+	i915_gem_drain_freed_objects(i915);
+
+	mutex_lock(&i915->drm.struct_mutex);
+}
+
 static void fake_free_pages(struct drm_i915_gem_object *obj,
 			    struct sg_table *pages)
 {
@@ -290,6 +304,8 @@ static int lowlevel_hole(struct drm_i915_private *i915,
 		i915_gem_object_put(obj);
 
 		kfree(order);
+
+		cleanup_freed_objects(i915);
 	}
 
 	return 0;
@@ -518,6 +534,7 @@ static int fill_hole(struct drm_i915_private *i915,
 		}
 
 		close_object_list(&objects, vm);
+		cleanup_freed_objects(i915);
 	}
 
 	return 0;
@@ -604,6 +621,8 @@ static int walk_hole(struct drm_i915_private *i915,
 		i915_gem_object_put(obj);
 		if (err)
 			return err;
+
+		cleanup_freed_objects(i915);
 	}
 
 	return 0;
@@ -788,6 +807,8 @@ static int drunk_hole(struct drm_i915_private *i915,
 		kfree(order);
 		if (err)
 			return err;
+
+		cleanup_freed_objects(i915);
 	}
 
 	return 0;
@@ -856,6 +877,7 @@ static int __shrink_hole(struct drm_i915_private *i915,
 	}
 
 	close_object_list(&objects, vm);
+	cleanup_freed_objects(i915);
 	return err;
 }
 
@@ -948,6 +970,7 @@ static int shrink_boom(struct drm_i915_private *i915,
 		i915_gem_object_put(explode);
 
 		memset(&vm->fault_attr, 0, sizeof(vm->fault_attr));
+		cleanup_freed_objects(i915);
 	}
 
 	return 0;
-- 
2.18.0

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

Re: [PATCH v2] drm/i915/selftests: Release the struct_mutex to free the objects

[Tvrtko Ursulin]

On 03/07/2018 11:18, Chris Wilson wrote:
> live_gtt is a very slow test to run, simply because it tries to allocate
> and use as much as the 48b address space as possibly can and in the
> process will try to own all of the system memory. This leads to resource
> exhaustion and CPU starvation; the latter impacts us when the NMI
> watchdog declares a task hung due to a mutex contention with ourselves.
> This we can prevent by releasing the struct_mutex and forcing our
> i915/rcu workers to run, and in particular flushing the freed object
> worker that is the cause for concern.
> 
> References: https://bugs.freedesktop.org/show_bug.cgi?id=107094
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> ---
>   drivers/gpu/drm/i915/selftests/i915_gem_gtt.c | 23 +++++++++++++++++++
>   1 file changed, 23 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/selftests/i915_gem_gtt.c b/drivers/gpu/drm/i915/selftests/i915_gem_gtt.c
> index fff26bd05f71..eefcf7b84054 100644
> --- a/drivers/gpu/drm/i915/selftests/i915_gem_gtt.c
> +++ b/drivers/gpu/drm/i915/selftests/i915_gem_gtt.c
> @@ -32,6 +32,20 @@
>   #include "mock_drm.h"
>   #include "mock_gem_device.h"
>   
> +static void cleanup_freed_objects(struct drm_i915_private *i915)
> +{
> +	/*
> +	 * As we may hold onto the struct_mutex for inordinate lengths of
> +	 * time, the NMI khungtaskd detector may fire for the free objects
> +	 * worker.
> +	 */
> +	mutex_unlock(&i915->drm.struct_mutex);
> +
> +	i915_gem_drain_freed_objects(i915);
> +
> +	mutex_lock(&i915->drm.struct_mutex);
> +}
> +
>   static void fake_free_pages(struct drm_i915_gem_object *obj,
>   			    struct sg_table *pages)
>   {
> @@ -290,6 +304,8 @@ static int lowlevel_hole(struct drm_i915_private *i915,
>   		i915_gem_object_put(obj);
>   
>   		kfree(order);
> +
> +		cleanup_freed_objects(i915);
>   	}
>   
>   	return 0;
> @@ -518,6 +534,7 @@ static int fill_hole(struct drm_i915_private *i915,
>   		}
>   
>   		close_object_list(&objects, vm);
> +		cleanup_freed_objects(i915);
>   	}
>   
>   	return 0;
> @@ -604,6 +621,8 @@ static int walk_hole(struct drm_i915_private *i915,
>   		i915_gem_object_put(obj);
>   		if (err)
>   			return err;
> +
> +		cleanup_freed_objects(i915);
>   	}
>   
>   	return 0;
> @@ -788,6 +807,8 @@ static int drunk_hole(struct drm_i915_private *i915,
>   		kfree(order);
>   		if (err)
>   			return err;
> +
> +		cleanup_freed_objects(i915);
>   	}
>   
>   	return 0;
> @@ -856,6 +877,7 @@ static int __shrink_hole(struct drm_i915_private *i915,
>   	}
>   
>   	close_object_list(&objects, vm);
> +	cleanup_freed_objects(i915);
>   	return err;
>   }
>   
> @@ -948,6 +970,7 @@ static int shrink_boom(struct drm_i915_private *i915,
>   		i915_gem_object_put(explode);
>   
>   		memset(&vm->fault_attr, 0, sizeof(vm->fault_attr));
> +		cleanup_freed_objects(i915);
>   	}
>   
>   	return 0;
> 

Okay this is nicer.

Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>

Regards,

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

✓ Fi.CI.BAT: success for drm/i915/selftests: Let other struct_mutex users have their gpu time (rev2)

[Patchwork]

== Series Details ==

Series: drm/i915/selftests: Let other struct_mutex users have their gpu time (rev2)
URL   : https://patchwork.freedesktop.org/series/45810/
State : success

== Summary ==

= CI Bug Log - changes from CI_DRM_4420 -> Patchwork_9507 =

== Summary - WARNING ==

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

  External URL: https://patchwork.freedesktop.org/api/1.0/series/45810/revisions/2/mbox/

== Possible new issues ==

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

  === IGT changes ===

    ==== Possible regressions ====

    igt@kms_pipe_crc_basic@suspend-read-crc-pipe-c:
      {fi-cfl-8109u}:     SKIP -> INCOMPLETE

    
    ==== Warnings ====

    igt@gem_exec_gttfill@basic:
      fi-pnv-d510:        SKIP -> PASS

    igt@kms_pipe_crc_basic@read-crc-pipe-b:
      {fi-cfl-8109u}:     SKIP -> PASS +32

    
== Known issues ==

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

  === IGT changes ===

    ==== Issues hit ====

    igt@drv_module_reload@basic-no-display:
      {fi-skl-iommu}:     NOTRUN -> FAIL (fdo#106066) +2

    igt@kms_pipe_crc_basic@hang-read-crc-pipe-b:
      {fi-skl-iommu}:     NOTRUN -> FAIL (fdo#106686) +2

    igt@prime_vgem@basic-fence-flip:
      fi-ilk-650:         PASS -> FAIL (fdo#104008)

    
    ==== Possible fixes ====

    igt@kms_pipe_crc_basic@suspend-read-crc-pipe-b:
      fi-snb-2520m:       INCOMPLETE (fdo#103713) -> PASS

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

  fdo#103713 https://bugs.freedesktop.org/show_bug.cgi?id=103713
  fdo#104008 https://bugs.freedesktop.org/show_bug.cgi?id=104008
  fdo#106066 https://bugs.freedesktop.org/show_bug.cgi?id=106066
  fdo#106686 https://bugs.freedesktop.org/show_bug.cgi?id=106686


== Participating hosts (44 -> 41) ==

  Additional (2): fi-glk-j4005 fi-skl-iommu 
  Missing    (5): fi-ctg-p8600 fi-ilk-m540 fi-byt-squawks fi-bsw-cyan fi-hsw-4200u 


== Build changes ==

    * Linux: CI_DRM_4420 -> Patchwork_9507

  CI_DRM_4420: d9596d1a02b553958c684ef2ea0d3e64e8f68efe @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_4532: 840d12e2f050b784552197403d6575a57b6e896d @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_9507: d88f3379db8f08decce3f51c9737e41221f3debc @ git://anongit.freedesktop.org/gfx-ci/linux


== Linux commits ==

d88f3379db8f drm/i915/selftests: Release the struct_mutex to free the objects

== Logs ==

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

✓ Fi.CI.IGT: success for drm/i915/selftests: Let other struct_mutex users have their gpu time (rev2)

[Patchwork]

== Series Details ==

Series: drm/i915/selftests: Let other struct_mutex users have their gpu time (rev2)
URL   : https://patchwork.freedesktop.org/series/45810/
State : success

== Summary ==

= CI Bug Log - changes from CI_DRM_4420_full -> Patchwork_9507_full =

== Summary - WARNING ==

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

  

== Possible new issues ==

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

  === IGT changes ===

    ==== Warnings ====

    igt@gem_exec_schedule@deep-bsd2:
      shard-kbl:          PASS -> SKIP

    igt@gem_mocs_settings@mocs-rc6-vebox:
      shard-kbl:          SKIP -> PASS

    
== Known issues ==

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

  === IGT changes ===

    ==== Issues hit ====

    igt@gem_exec_big:
      shard-hsw:          PASS -> INCOMPLETE (fdo#103540)

    igt@kms_cursor_legacy@2x-nonblocking-modeset-vs-cursor-atomic:
      shard-glk:          PASS -> FAIL (fdo#105454, fdo#106509)

    igt@kms_flip@2x-flip-vs-expired-vblank-interruptible:
      shard-glk:          PASS -> FAIL (fdo#105363)

    igt@kms_flip_tiling@flip-y-tiled:
      shard-glk:          PASS -> FAIL (fdo#103822, fdo#104724)

    igt@kms_setmode@basic:
      shard-apl:          PASS -> FAIL (fdo#99912)

    
    ==== Possible fixes ====

    igt@drv_selftest@live_gtt:
      shard-glk:          INCOMPLETE (fdo#103359, k.org#198133) -> PASS

    igt@drv_suspend@shrink:
      shard-kbl:          FAIL (fdo#106886) -> PASS

    igt@kms_cursor_legacy@cursor-vs-flip-atomic-transitions-varying-size:
      shard-hsw:          FAIL (fdo#103355) -> PASS

    igt@kms_flip@2x-modeset-vs-vblank-race-interruptible:
      shard-hsw:          FAIL (fdo#103060) -> PASS

    
  fdo#103060 https://bugs.freedesktop.org/show_bug.cgi?id=103060
  fdo#103355 https://bugs.freedesktop.org/show_bug.cgi?id=103355
  fdo#103359 https://bugs.freedesktop.org/show_bug.cgi?id=103359
  fdo#103540 https://bugs.freedesktop.org/show_bug.cgi?id=103540
  fdo#103822 https://bugs.freedesktop.org/show_bug.cgi?id=103822
  fdo#104724 https://bugs.freedesktop.org/show_bug.cgi?id=104724
  fdo#105363 https://bugs.freedesktop.org/show_bug.cgi?id=105363
  fdo#105454 https://bugs.freedesktop.org/show_bug.cgi?id=105454
  fdo#106509 https://bugs.freedesktop.org/show_bug.cgi?id=106509
  fdo#106886 https://bugs.freedesktop.org/show_bug.cgi?id=106886
  fdo#99912 https://bugs.freedesktop.org/show_bug.cgi?id=99912
  k.org#198133 https://bugzilla.kernel.org/show_bug.cgi?id=198133


== Participating hosts (5 -> 5) ==

  No changes in participating hosts


== Build changes ==

    * Linux: CI_DRM_4420 -> Patchwork_9507

  CI_DRM_4420: d9596d1a02b553958c684ef2ea0d3e64e8f68efe @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_4532: 840d12e2f050b784552197403d6575a57b6e896d @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_9507: d88f3379db8f08decce3f51c9737e41221f3debc @ 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_9507/shards.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx