diff mbox

drm/i915/selftests: Create mock_engine() under struct_mutex

Message ID 20180508211056.17151-1-chris@chris-wilson.co.uk (mailing list archive)
State New, archived
Headers show

Commit Message

Chris Wilson May 8, 2018, 9:10 p.m. UTC
Calling mock_engine() calls i915_timeline_init() and that requires
struct_mutex to be held as it adds itself to the global list of
timelines. This error was introduced by commit a89d1f921c15 ("drm/i915:
Split i915_gem_timeline into individual timelines") but the issue was
masked in CI by the earlier lockdep spam.

Fixes: a89d1f921c15 ("drm/i915: Split i915_gem_timeline into individual timelines")
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
Cc: Michel Thierry <michel.thierry@intel.com>
---
 drivers/gpu/drm/i915/selftests/mock_gem_device.c | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

Comments

Michel Thierry May 8, 2018, 9:25 p.m. UTC | #1
On 05/08/2018 02:10 PM, Chris Wilson wrote:
> Calling mock_engine() calls i915_timeline_init() and that requires
> struct_mutex to be held as it adds itself to the global list of
> timelines. This error was introduced by commit a89d1f921c15 ("drm/i915:
> Split i915_gem_timeline into individual timelines") but the issue was
> masked in CI by the earlier lockdep spam.
> 
> Fixes: a89d1f921c15 ("drm/i915: Split i915_gem_timeline into individual timelines")
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
> Cc: Michel Thierry <michel.thierry@intel.com>

Double checked that mock_ring (the other caller of i915_timeline_init) 
is covered by this same lock.

Reviewed-by: Michel Thierry <michel.thierry@intel.com>

> ---
>   drivers/gpu/drm/i915/selftests/mock_gem_device.c | 9 ++++++---
>   1 file changed, 6 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/selftests/mock_gem_device.c b/drivers/gpu/drm/i915/selftests/mock_gem_device.c
> index 4b6622c6986a..94baedfa0f74 100644
> --- a/drivers/gpu/drm/i915/selftests/mock_gem_device.c
> +++ b/drivers/gpu/drm/i915/selftests/mock_gem_device.c
> @@ -229,18 +229,20 @@ struct drm_i915_private *mock_gem_device(void)
>   	INIT_LIST_HEAD(&i915->gt.closed_vma);
>   
>   	mutex_lock(&i915->drm.struct_mutex);
> +
>   	mock_init_ggtt(i915);
> -	mutex_unlock(&i915->drm.struct_mutex);
>   
>   	mkwrite_device_info(i915)->ring_mask = BIT(0);
>   	i915->engine[RCS] = mock_engine(i915, "mock", RCS);
>   	if (!i915->engine[RCS])
> -		goto err_priorities;
> +		goto err_unlock;
>   
>   	i915->kernel_context = mock_context(i915, NULL);
>   	if (!i915->kernel_context)
>   		goto err_engine;
>   
> +	mutex_unlock(&i915->drm.struct_mutex);
> +
>   	WARN_ON(i915_gemfs_init(i915));
>   
>   	return i915;
> @@ -248,7 +250,8 @@ struct drm_i915_private *mock_gem_device(void)
>   err_engine:
>   	for_each_engine(engine, i915, id)
>   		mock_engine_free(engine);
> -err_priorities:
> +err_unlock:
> +	mutex_unlock(&i915->drm.struct_mutex);
>   	kmem_cache_destroy(i915->priorities);
>   err_dependencies:
>   	kmem_cache_destroy(i915->dependencies);
>
Chris Wilson May 9, 2018, 7:25 a.m. UTC | #2
Quoting Patchwork (2018-05-09 03:41:59)
> == Series Details ==
> 
> Series: drm/i915/selftests: Create mock_engine() under struct_mutex
> URL   : https://patchwork.freedesktop.org/series/42898/
> State : success
> 
> == Summary ==
> 
> = CI Bug Log - changes from CI_DRM_4160_full -> Patchwork_8951_full =
> 
> == Summary - WARNING ==
> 
>   Minor unknown changes coming with Patchwork_8951_full need to be verified
>   manually.
>   
>   If you think the reported changes have nothing to do with the changes
>   introduced in Patchwork_8951_full, 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/42898/revisions/1/mbox/
> 
> == Possible new issues ==
> 
>   Here are the unknown changes that may have been introduced in Patchwork_8951_full:
> 
>   === IGT changes ===
> 
>     ==== Warnings ====
> 
>     igt@drv_selftest@live_execlists:
>       shard-apl:          PASS -> SKIP +1
> 
>     igt@drv_selftest@live_hangcheck:
>       shard-apl:          DMESG-WARN -> DMESG-FAIL

Applied, but more fallout to fix. Thanks,
-Chris
diff mbox

Patch

diff --git a/drivers/gpu/drm/i915/selftests/mock_gem_device.c b/drivers/gpu/drm/i915/selftests/mock_gem_device.c
index 4b6622c6986a..94baedfa0f74 100644
--- a/drivers/gpu/drm/i915/selftests/mock_gem_device.c
+++ b/drivers/gpu/drm/i915/selftests/mock_gem_device.c
@@ -229,18 +229,20 @@  struct drm_i915_private *mock_gem_device(void)
 	INIT_LIST_HEAD(&i915->gt.closed_vma);
 
 	mutex_lock(&i915->drm.struct_mutex);
+
 	mock_init_ggtt(i915);
-	mutex_unlock(&i915->drm.struct_mutex);
 
 	mkwrite_device_info(i915)->ring_mask = BIT(0);
 	i915->engine[RCS] = mock_engine(i915, "mock", RCS);
 	if (!i915->engine[RCS])
-		goto err_priorities;
+		goto err_unlock;
 
 	i915->kernel_context = mock_context(i915, NULL);
 	if (!i915->kernel_context)
 		goto err_engine;
 
+	mutex_unlock(&i915->drm.struct_mutex);
+
 	WARN_ON(i915_gemfs_init(i915));
 
 	return i915;
@@ -248,7 +250,8 @@  struct drm_i915_private *mock_gem_device(void)
 err_engine:
 	for_each_engine(engine, i915, id)
 		mock_engine_free(engine);
-err_priorities:
+err_unlock:
+	mutex_unlock(&i915->drm.struct_mutex);
 	kmem_cache_destroy(i915->priorities);
 err_dependencies:
 	kmem_cache_destroy(i915->dependencies);