intel-gfx.lists.freedesktop.org archive mirror
 help / color / mirror / Atom feed
* [Intel-gfx] [PATCH] drm/i915/gt: Wait for RCUs frees before asserting idle on unload
@ 2020-03-12 11:53 Chris Wilson
  2020-03-12 13:38 ` [Intel-gfx] ✗ Fi.CI.BAT: failure for " Patchwork
                   ` (3 more replies)
  0 siblings, 4 replies; 6+ messages in thread
From: Chris Wilson @ 2020-03-12 11:53 UTC (permalink / raw)
  To: intel-gfx

During driver unload, we have many asserts that we have released our
bookkeeping structs and are idle. In some cases, these struct are
protected by RCU and we do not release them until after an RCU grace
period.

Reported-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Fixes: 130a95e9098e ("drm/i915/gem: Consolidate ctx->engines[] release")
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Cc: Mika Kuoppala <mika.kuoppala@linux.intel.com>
---
 drivers/gpu/drm/i915/gt/intel_gt.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/gpu/drm/i915/gt/intel_gt.c b/drivers/gpu/drm/i915/gt/intel_gt.c
index 3dea8881e915..d09f7596cb98 100644
--- a/drivers/gpu/drm/i915/gt/intel_gt.c
+++ b/drivers/gpu/drm/i915/gt/intel_gt.c
@@ -667,6 +667,9 @@ void intel_gt_driver_release(struct intel_gt *gt)
 
 void intel_gt_driver_late_release(struct intel_gt *gt)
 {
+	/* We need to wait for inflight RCU frees to release their grip */
+	rcu_barrier();
+
 	intel_uc_driver_late_release(&gt->uc);
 	intel_gt_fini_requests(gt);
 	intel_gt_fini_reset(gt);
-- 
2.20.1

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

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

* [Intel-gfx] ✗ Fi.CI.BAT: failure for drm/i915/gt: Wait for RCUs frees before asserting idle on unload
  2020-03-12 11:53 [Intel-gfx] [PATCH] drm/i915/gt: Wait for RCUs frees before asserting idle on unload Chris Wilson
@ 2020-03-12 13:38 ` Patchwork
  2020-03-12 14:18 ` [Intel-gfx] [PATCH] " Tvrtko Ursulin
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 6+ messages in thread
From: Patchwork @ 2020-03-12 13:38 UTC (permalink / raw)
  To: Chris Wilson; +Cc: intel-gfx

== Series Details ==

Series: drm/i915/gt: Wait for RCUs frees before asserting idle on unload
URL   : https://patchwork.freedesktop.org/series/74645/
State : failure

== Summary ==

CI Bug Log - changes from CI_DRM_8126 -> Patchwork_16951
====================================================

Summary
-------

  **FAILURE**

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

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

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

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

### IGT changes ###

#### Possible regressions ####

  * igt@runner@aborted:
    - fi-kbl-soraka:      NOTRUN -> [FAIL][1]
   [1]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16951/fi-kbl-soraka/igt@runner@aborted.html

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

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

### IGT changes ###

#### Issues hit ####

  * igt@i915_selftest@live@execlists:
    - fi-kbl-soraka:      [PASS][2] -> [INCOMPLETE][3] ([fdo#112259] / [i915#656])
   [2]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8126/fi-kbl-soraka/igt@i915_selftest@live@execlists.html
   [3]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16951/fi-kbl-soraka/igt@i915_selftest@live@execlists.html

  * igt@i915_selftest@live@gem_contexts:
    - fi-cml-s:           [PASS][4] -> [DMESG-FAIL][5] ([i915#877])
   [4]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8126/fi-cml-s/igt@i915_selftest@live@gem_contexts.html
   [5]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16951/fi-cml-s/igt@i915_selftest@live@gem_contexts.html

  
#### Possible fixes ####

  * igt@i915_pm_rpm@basic-rte:
    - fi-hsw-4770:        [SKIP][6] ([fdo#109271]) -> [PASS][7] +1 similar issue
   [6]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8126/fi-hsw-4770/igt@i915_pm_rpm@basic-rte.html
   [7]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16951/fi-hsw-4770/igt@i915_pm_rpm@basic-rte.html

  * igt@kms_chamelium@hdmi-hpd-fast:
    - fi-kbl-7500u:       [FAIL][8] ([i915#323]) -> [PASS][9]
   [8]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8126/fi-kbl-7500u/igt@kms_chamelium@hdmi-hpd-fast.html
   [9]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16951/fi-kbl-7500u/igt@kms_chamelium@hdmi-hpd-fast.html

  
  [fdo#109271]: https://bugs.freedesktop.org/show_bug.cgi?id=109271
  [fdo#112259]: https://bugs.freedesktop.org/show_bug.cgi?id=112259
  [i915#323]: https://gitlab.freedesktop.org/drm/intel/issues/323
  [i915#656]: https://gitlab.freedesktop.org/drm/intel/issues/656
  [i915#877]: https://gitlab.freedesktop.org/drm/intel/issues/877


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

  Additional (1): fi-kbl-7560u 
  Missing    (5): fi-hsw-4200u fi-byt-squawks fi-bsw-cyan fi-bsw-kefka fi-byt-clapper 


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

  * CI: CI-20190529 -> None
  * Linux: CI_DRM_8126 -> Patchwork_16951

  CI-20190529: 20190529
  CI_DRM_8126: 2bd9e989a5653d4cd710e9dd2b42b0a080f1add8 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_5505: 8973d811f3fdfb4ace4aabab2095ce0309881648 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_16951: 30a0c44ecbd19a1c219265bb3ceb650a9bfd037d @ git://anongit.freedesktop.org/gfx-ci/linux


== Linux commits ==

30a0c44ecbd1 drm/i915/gt: Wait for RCUs frees before asserting idle on unload

== Logs ==

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

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

* Re: [Intel-gfx] [PATCH] drm/i915/gt: Wait for RCUs frees before asserting idle on unload
  2020-03-12 11:53 [Intel-gfx] [PATCH] drm/i915/gt: Wait for RCUs frees before asserting idle on unload Chris Wilson
  2020-03-12 13:38 ` [Intel-gfx] ✗ Fi.CI.BAT: failure for " Patchwork
@ 2020-03-12 14:18 ` Tvrtko Ursulin
  2020-03-12 16:16   ` [Intel-gfx] ***UNCHECKED*** " Chris Wilson
  2020-03-12 15:53 ` [Intel-gfx] " Mika Kuoppala
  2020-03-13 11:08 ` Maarten Lankhorst
  3 siblings, 1 reply; 6+ messages in thread
From: Tvrtko Ursulin @ 2020-03-12 14:18 UTC (permalink / raw)
  To: Chris Wilson, intel-gfx


On 12/03/2020 11:53, Chris Wilson wrote:
> During driver unload, we have many asserts that we have released our
> bookkeeping structs and are idle. In some cases, these struct are
> protected by RCU and we do not release them until after an RCU grace
> period.
> 
> Reported-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> Fixes: 130a95e9098e ("drm/i915/gem: Consolidate ctx->engines[] release")
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> Cc: Mika Kuoppala <mika.kuoppala@linux.intel.com>
> ---
>   drivers/gpu/drm/i915/gt/intel_gt.c | 3 +++
>   1 file changed, 3 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/gt/intel_gt.c b/drivers/gpu/drm/i915/gt/intel_gt.c
> index 3dea8881e915..d09f7596cb98 100644
> --- a/drivers/gpu/drm/i915/gt/intel_gt.c
> +++ b/drivers/gpu/drm/i915/gt/intel_gt.c
> @@ -667,6 +667,9 @@ void intel_gt_driver_release(struct intel_gt *gt)
>   
>   void intel_gt_driver_late_release(struct intel_gt *gt)
>   {
> +	/* We need to wait for inflight RCU frees to release their grip */
> +	rcu_barrier();
> +
>   	intel_uc_driver_late_release(&gt->uc);
>   	intel_gt_fini_requests(gt);
>   	intel_gt_fini_reset(gt);
> 

Not sure if GT or GEM is the place, but liberal application seems 
required anyway nowadays.

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

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

* Re: [Intel-gfx] [PATCH] drm/i915/gt: Wait for RCUs frees before asserting idle on unload
  2020-03-12 11:53 [Intel-gfx] [PATCH] drm/i915/gt: Wait for RCUs frees before asserting idle on unload Chris Wilson
  2020-03-12 13:38 ` [Intel-gfx] ✗ Fi.CI.BAT: failure for " Patchwork
  2020-03-12 14:18 ` [Intel-gfx] [PATCH] " Tvrtko Ursulin
@ 2020-03-12 15:53 ` Mika Kuoppala
  2020-03-13 11:08 ` Maarten Lankhorst
  3 siblings, 0 replies; 6+ messages in thread
From: Mika Kuoppala @ 2020-03-12 15:53 UTC (permalink / raw)
  To: Chris Wilson, intel-gfx

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

> During driver unload, we have many asserts that we have released our
> bookkeeping structs and are idle. In some cases, these struct are
> protected by RCU and we do not release them until after an RCU grace
> period.
>
> Reported-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> Fixes: 130a95e9098e ("drm/i915/gem: Consolidate ctx->engines[] release")
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> Cc: Mika Kuoppala <mika.kuoppala@linux.intel.com>

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

> ---
>  drivers/gpu/drm/i915/gt/intel_gt.c | 3 +++
>  1 file changed, 3 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/gt/intel_gt.c b/drivers/gpu/drm/i915/gt/intel_gt.c
> index 3dea8881e915..d09f7596cb98 100644
> --- a/drivers/gpu/drm/i915/gt/intel_gt.c
> +++ b/drivers/gpu/drm/i915/gt/intel_gt.c
> @@ -667,6 +667,9 @@ void intel_gt_driver_release(struct intel_gt *gt)
>  
>  void intel_gt_driver_late_release(struct intel_gt *gt)
>  {
> +	/* We need to wait for inflight RCU frees to release their grip */
> +	rcu_barrier();
> +
>  	intel_uc_driver_late_release(&gt->uc);
>  	intel_gt_fini_requests(gt);
>  	intel_gt_fini_reset(gt);
> -- 
> 2.20.1
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

* Re: [Intel-gfx] ***UNCHECKED*** Re: [PATCH] drm/i915/gt: Wait for RCUs frees before asserting idle on unload
  2020-03-12 14:18 ` [Intel-gfx] [PATCH] " Tvrtko Ursulin
@ 2020-03-12 16:16   ` Chris Wilson
  0 siblings, 0 replies; 6+ messages in thread
From: Chris Wilson @ 2020-03-12 16:16 UTC (permalink / raw)
  To: Tvrtko Ursulin, intel-gfx

Quoting Tvrtko Ursulin (2020-03-12 14:18:26)
> 
> On 12/03/2020 11:53, Chris Wilson wrote:
> > During driver unload, we have many asserts that we have released our
> > bookkeeping structs and are idle. In some cases, these struct are
> > protected by RCU and we do not release them until after an RCU grace
> > period.
> > 
> > Reported-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> > Fixes: 130a95e9098e ("drm/i915/gem: Consolidate ctx->engines[] release")
> > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> > Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> > Cc: Mika Kuoppala <mika.kuoppala@linux.intel.com>
> > ---
> >   drivers/gpu/drm/i915/gt/intel_gt.c | 3 +++
> >   1 file changed, 3 insertions(+)
> > 
> > diff --git a/drivers/gpu/drm/i915/gt/intel_gt.c b/drivers/gpu/drm/i915/gt/intel_gt.c
> > index 3dea8881e915..d09f7596cb98 100644
> > --- a/drivers/gpu/drm/i915/gt/intel_gt.c
> > +++ b/drivers/gpu/drm/i915/gt/intel_gt.c
> > @@ -667,6 +667,9 @@ void intel_gt_driver_release(struct intel_gt *gt)
> >   
> >   void intel_gt_driver_late_release(struct intel_gt *gt)
> >   {
> > +     /* We need to wait for inflight RCU frees to release their grip */
> > +     rcu_barrier();
> > +
> >       intel_uc_driver_late_release(&gt->uc);
> >       intel_gt_fini_requests(gt);
> >       intel_gt_fini_reset(gt);
> > 
> 
> Not sure if GT or GEM is the place, but liberal application seems 
> required anyway nowadays.
> 
> Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>

Either after gem_fini_contexts or before here, both have reasonable
arguments. One being the point of assertion, the other the point of
expected release.
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

* Re: [Intel-gfx] [PATCH] drm/i915/gt: Wait for RCUs frees before asserting idle on unload
  2020-03-12 11:53 [Intel-gfx] [PATCH] drm/i915/gt: Wait for RCUs frees before asserting idle on unload Chris Wilson
                   ` (2 preceding siblings ...)
  2020-03-12 15:53 ` [Intel-gfx] " Mika Kuoppala
@ 2020-03-13 11:08 ` Maarten Lankhorst
  3 siblings, 0 replies; 6+ messages in thread
From: Maarten Lankhorst @ 2020-03-13 11:08 UTC (permalink / raw)
  To: Chris Wilson, intel-gfx

Op 12-03-2020 om 12:53 schreef Chris Wilson:
> During driver unload, we have many asserts that we have released our
> bookkeeping structs and are idle. In some cases, these struct are
> protected by RCU and we do not release them until after an RCU grace
> period.
>
> Reported-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> Fixes: 130a95e9098e ("drm/i915/gem: Consolidate ctx->engines[] release")
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> Cc: Mika Kuoppala <mika.kuoppala@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/gt/intel_gt.c | 3 +++
>  1 file changed, 3 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/gt/intel_gt.c b/drivers/gpu/drm/i915/gt/intel_gt.c
> index 3dea8881e915..d09f7596cb98 100644
> --- a/drivers/gpu/drm/i915/gt/intel_gt.c
> +++ b/drivers/gpu/drm/i915/gt/intel_gt.c
> @@ -667,6 +667,9 @@ void intel_gt_driver_release(struct intel_gt *gt)
>  
>  void intel_gt_driver_late_release(struct intel_gt *gt)
>  {
> +	/* We need to wait for inflight RCU frees to release their grip */
> +	rcu_barrier();
> +
>  	intel_uc_driver_late_release(&gt->uc);
>  	intel_gt_fini_requests(gt);
>  	intel_gt_fini_reset(gt);

Is it possible this is causing a hang on exit in gem_exec_parallel when I have an extra bunch of debug options enabled?

Also potentially seems to break selftests on cml :)

otherwise looks sane,

Reviewed-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>

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

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

end of thread, other threads:[~2020-03-13 11:08 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-03-12 11:53 [Intel-gfx] [PATCH] drm/i915/gt: Wait for RCUs frees before asserting idle on unload Chris Wilson
2020-03-12 13:38 ` [Intel-gfx] ✗ Fi.CI.BAT: failure for " Patchwork
2020-03-12 14:18 ` [Intel-gfx] [PATCH] " Tvrtko Ursulin
2020-03-12 16:16   ` [Intel-gfx] ***UNCHECKED*** " Chris Wilson
2020-03-12 15:53 ` [Intel-gfx] " Mika Kuoppala
2020-03-13 11:08 ` Maarten Lankhorst

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).