All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] drm/i915: Serialise i915_active_wait() with its retirement
@ 2019-11-29  9:39 ` Chris Wilson
  0 siblings, 0 replies; 10+ messages in thread
From: Chris Wilson @ 2019-11-29  9:39 UTC (permalink / raw)
  To: intel-gfx

As the i915_active.retire() may be running on another CPU as we detect
that the i915_active is idle, we may not wait for the retirement itself.
Wait for the remote callback by waiting for the retirement worker.

Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=112424
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
---
 drivers/gpu/drm/i915/i915_active.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/gpu/drm/i915/i915_active.c b/drivers/gpu/drm/i915/i915_active.c
index 479195ecbc6c..e8630ee33336 100644
--- a/drivers/gpu/drm/i915/i915_active.c
+++ b/drivers/gpu/drm/i915/i915_active.c
@@ -469,6 +469,7 @@ int i915_active_wait(struct i915_active *ref)
 	if (wait_var_event_interruptible(ref, i915_active_is_idle(ref)))
 		return -EINTR;
 
+	flush_work(&ref->work);
 	return 0;
 }
 
-- 
2.24.0

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

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

* [Intel-gfx] [PATCH] drm/i915: Serialise i915_active_wait() with its retirement
@ 2019-11-29  9:39 ` Chris Wilson
  0 siblings, 0 replies; 10+ messages in thread
From: Chris Wilson @ 2019-11-29  9:39 UTC (permalink / raw)
  To: intel-gfx

As the i915_active.retire() may be running on another CPU as we detect
that the i915_active is idle, we may not wait for the retirement itself.
Wait for the remote callback by waiting for the retirement worker.

Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=112424
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
---
 drivers/gpu/drm/i915/i915_active.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/gpu/drm/i915/i915_active.c b/drivers/gpu/drm/i915/i915_active.c
index 479195ecbc6c..e8630ee33336 100644
--- a/drivers/gpu/drm/i915/i915_active.c
+++ b/drivers/gpu/drm/i915/i915_active.c
@@ -469,6 +469,7 @@ int i915_active_wait(struct i915_active *ref)
 	if (wait_var_event_interruptible(ref, i915_active_is_idle(ref)))
 		return -EINTR;
 
+	flush_work(&ref->work);
 	return 0;
 }
 
-- 
2.24.0

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

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

* ✗ Fi.CI.BAT: failure for drm/i915: Serialise i915_active_wait() with its retirement
@ 2019-11-29 10:42   ` Patchwork
  0 siblings, 0 replies; 10+ messages in thread
From: Patchwork @ 2019-11-29 10:42 UTC (permalink / raw)
  To: Chris Wilson; +Cc: intel-gfx

== Series Details ==

Series: drm/i915: Serialise i915_active_wait() with its retirement
URL   : https://patchwork.freedesktop.org/series/70195/
State : failure

== Summary ==

CI Bug Log - changes from CI_DRM_7440 -> Patchwork_15505
====================================================

Summary
-------

  **FAILURE**

  Serious unknown changes coming with Patchwork_15505 absolutely need to be
  verified manually.
  
  If you think the reported changes have nothing to do with the changes
  introduced in Patchwork_15505, 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_15505/index.html

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

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

### IGT changes ###

#### Possible regressions ####

  * igt@i915_pm_rpm@basic-pci-d3-state:
    - fi-icl-u2:          [PASS][1] -> [DMESG-WARN][2]
   [1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7440/fi-icl-u2/igt@i915_pm_rpm@basic-pci-d3-state.html
   [2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15505/fi-icl-u2/igt@i915_pm_rpm@basic-pci-d3-state.html
    - fi-icl-u3:          [PASS][3] -> [DMESG-WARN][4]
   [3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7440/fi-icl-u3/igt@i915_pm_rpm@basic-pci-d3-state.html
   [4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15505/fi-icl-u3/igt@i915_pm_rpm@basic-pci-d3-state.html

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

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

### IGT changes ###

#### Issues hit ####

  * igt@i915_pm_rpm@module-reload:
    - fi-kbl-guc:         [PASS][5] -> [FAIL][6] ([fdo#107707])
   [5]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7440/fi-kbl-guc/igt@i915_pm_rpm@module-reload.html
   [6]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15505/fi-kbl-guc/igt@i915_pm_rpm@module-reload.html

  
#### Possible fixes ####

  * igt@i915_pm_rpm@module-reload:
    - fi-skl-6770hq:      [FAIL][7] ([fdo#108511]) -> [PASS][8]
   [7]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7440/fi-skl-6770hq/igt@i915_pm_rpm@module-reload.html
   [8]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15505/fi-skl-6770hq/igt@i915_pm_rpm@module-reload.html

  * igt@i915_selftest@live_gt_heartbeat:
    - fi-kbl-guc:         [DMESG-FAIL][9] ([fdo#112405]) -> [PASS][10]
   [9]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7440/fi-kbl-guc/igt@i915_selftest@live_gt_heartbeat.html
   [10]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15505/fi-kbl-guc/igt@i915_selftest@live_gt_heartbeat.html

  * igt@kms_chamelium@hdmi-hpd-fast:
    - fi-icl-u2:          [FAIL][11] ([fdo#109483]) -> [PASS][12]
   [11]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7440/fi-icl-u2/igt@kms_chamelium@hdmi-hpd-fast.html
   [12]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15505/fi-icl-u2/igt@kms_chamelium@hdmi-hpd-fast.html
    - fi-kbl-7500u:       [FAIL][13] ([fdo#111045] / [fdo#111096]) -> [PASS][14]
   [13]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7440/fi-kbl-7500u/igt@kms_chamelium@hdmi-hpd-fast.html
   [14]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15505/fi-kbl-7500u/igt@kms_chamelium@hdmi-hpd-fast.html

  * igt@kms_frontbuffer_tracking@basic:
    - fi-tgl-y:           [FAIL][15] ([fdo# 112163] / [fdo#103167]) -> [PASS][16]
   [15]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7440/fi-tgl-y/igt@kms_frontbuffer_tracking@basic.html
   [16]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15505/fi-tgl-y/igt@kms_frontbuffer_tracking@basic.html

  
#### Warnings ####

  * igt@kms_cursor_legacy@basic-busy-flip-before-cursor-legacy:
    - fi-kbl-x1275:       [DMESG-WARN][17] ([fdo#103558] / [fdo#105602]) -> [DMESG-WARN][18] ([fdo#103558] / [fdo#105602] / [fdo#105763]) +4 similar issues
   [17]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7440/fi-kbl-x1275/igt@kms_cursor_legacy@basic-busy-flip-before-cursor-legacy.html
   [18]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15505/fi-kbl-x1275/igt@kms_cursor_legacy@basic-busy-flip-before-cursor-legacy.html

  * igt@kms_flip@basic-flip-vs-modeset:
    - fi-kbl-x1275:       [DMESG-WARN][19] ([fdo#103558] / [fdo#105602] / [fdo#105763]) -> [DMESG-WARN][20] ([fdo#103558] / [fdo#105602]) +9 similar issues
   [19]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7440/fi-kbl-x1275/igt@kms_flip@basic-flip-vs-modeset.html
   [20]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15505/fi-kbl-x1275/igt@kms_flip@basic-flip-vs-modeset.html

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

  [fdo# 112163]: https://bugs.freedesktop.org/show_bug.cgi?id= 112163
  [fdo#103167]: https://bugs.freedesktop.org/show_bug.cgi?id=103167
  [fdo#103558]: https://bugs.freedesktop.org/show_bug.cgi?id=103558
  [fdo#105602]: https://bugs.freedesktop.org/show_bug.cgi?id=105602
  [fdo#105763]: https://bugs.freedesktop.org/show_bug.cgi?id=105763
  [fdo#107707]: https://bugs.freedesktop.org/show_bug.cgi?id=107707
  [fdo#108511]: https://bugs.freedesktop.org/show_bug.cgi?id=108511
  [fdo#109483]: https://bugs.freedesktop.org/show_bug.cgi?id=109483
  [fdo#109964]: https://bugs.freedesktop.org/show_bug.cgi?id=109964
  [fdo#111045]: https://bugs.freedesktop.org/show_bug.cgi?id=111045
  [fdo#111096]: https://bugs.freedesktop.org/show_bug.cgi?id=111096
  [fdo#112298]: https://bugs.freedesktop.org/show_bug.cgi?id=112298
  [fdo#112405]: https://bugs.freedesktop.org/show_bug.cgi?id=112405


Participating hosts (52 -> 46)
------------------------------

  Missing    (6): fi-ilk-m540 fi-hsw-4200u fi-bsw-cyan fi-ctg-p8600 fi-byt-clapper fi-bdw-samus 


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

  * CI: CI-20190529 -> None
  * Linux: CI_DRM_7440 -> Patchwork_15505

  CI-20190529: 20190529
  CI_DRM_7440: 7b08e6efdddfcde8ea60a96d5a818af032b52b4d @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_5315: 58705eb1bd29414244f4d0cfa08a9f0ce42d6545 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_15505: 1d4aaedc3c5188faa858c90145565737af08b153 @ git://anongit.freedesktop.org/gfx-ci/linux


== Linux commits ==

1d4aaedc3c51 drm/i915: Serialise i915_active_wait() with its retirement

== Logs ==

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

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

* [Intel-gfx] ✗ Fi.CI.BAT: failure for drm/i915: Serialise i915_active_wait() with its retirement
@ 2019-11-29 10:42   ` Patchwork
  0 siblings, 0 replies; 10+ messages in thread
From: Patchwork @ 2019-11-29 10:42 UTC (permalink / raw)
  To: Chris Wilson; +Cc: intel-gfx

== Series Details ==

Series: drm/i915: Serialise i915_active_wait() with its retirement
URL   : https://patchwork.freedesktop.org/series/70195/
State : failure

== Summary ==

CI Bug Log - changes from CI_DRM_7440 -> Patchwork_15505
====================================================

Summary
-------

  **FAILURE**

  Serious unknown changes coming with Patchwork_15505 absolutely need to be
  verified manually.
  
  If you think the reported changes have nothing to do with the changes
  introduced in Patchwork_15505, 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_15505/index.html

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

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

### IGT changes ###

#### Possible regressions ####

  * igt@i915_pm_rpm@basic-pci-d3-state:
    - fi-icl-u2:          [PASS][1] -> [DMESG-WARN][2]
   [1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7440/fi-icl-u2/igt@i915_pm_rpm@basic-pci-d3-state.html
   [2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15505/fi-icl-u2/igt@i915_pm_rpm@basic-pci-d3-state.html
    - fi-icl-u3:          [PASS][3] -> [DMESG-WARN][4]
   [3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7440/fi-icl-u3/igt@i915_pm_rpm@basic-pci-d3-state.html
   [4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15505/fi-icl-u3/igt@i915_pm_rpm@basic-pci-d3-state.html

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

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

### IGT changes ###

#### Issues hit ####

  * igt@i915_pm_rpm@module-reload:
    - fi-kbl-guc:         [PASS][5] -> [FAIL][6] ([fdo#107707])
   [5]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7440/fi-kbl-guc/igt@i915_pm_rpm@module-reload.html
   [6]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15505/fi-kbl-guc/igt@i915_pm_rpm@module-reload.html

  
#### Possible fixes ####

  * igt@i915_pm_rpm@module-reload:
    - fi-skl-6770hq:      [FAIL][7] ([fdo#108511]) -> [PASS][8]
   [7]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7440/fi-skl-6770hq/igt@i915_pm_rpm@module-reload.html
   [8]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15505/fi-skl-6770hq/igt@i915_pm_rpm@module-reload.html

  * igt@i915_selftest@live_gt_heartbeat:
    - fi-kbl-guc:         [DMESG-FAIL][9] ([fdo#112405]) -> [PASS][10]
   [9]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7440/fi-kbl-guc/igt@i915_selftest@live_gt_heartbeat.html
   [10]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15505/fi-kbl-guc/igt@i915_selftest@live_gt_heartbeat.html

  * igt@kms_chamelium@hdmi-hpd-fast:
    - fi-icl-u2:          [FAIL][11] ([fdo#109483]) -> [PASS][12]
   [11]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7440/fi-icl-u2/igt@kms_chamelium@hdmi-hpd-fast.html
   [12]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15505/fi-icl-u2/igt@kms_chamelium@hdmi-hpd-fast.html
    - fi-kbl-7500u:       [FAIL][13] ([fdo#111045] / [fdo#111096]) -> [PASS][14]
   [13]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7440/fi-kbl-7500u/igt@kms_chamelium@hdmi-hpd-fast.html
   [14]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15505/fi-kbl-7500u/igt@kms_chamelium@hdmi-hpd-fast.html

  * igt@kms_frontbuffer_tracking@basic:
    - fi-tgl-y:           [FAIL][15] ([fdo# 112163] / [fdo#103167]) -> [PASS][16]
   [15]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7440/fi-tgl-y/igt@kms_frontbuffer_tracking@basic.html
   [16]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15505/fi-tgl-y/igt@kms_frontbuffer_tracking@basic.html

  
#### Warnings ####

  * igt@kms_cursor_legacy@basic-busy-flip-before-cursor-legacy:
    - fi-kbl-x1275:       [DMESG-WARN][17] ([fdo#103558] / [fdo#105602]) -> [DMESG-WARN][18] ([fdo#103558] / [fdo#105602] / [fdo#105763]) +4 similar issues
   [17]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7440/fi-kbl-x1275/igt@kms_cursor_legacy@basic-busy-flip-before-cursor-legacy.html
   [18]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15505/fi-kbl-x1275/igt@kms_cursor_legacy@basic-busy-flip-before-cursor-legacy.html

  * igt@kms_flip@basic-flip-vs-modeset:
    - fi-kbl-x1275:       [DMESG-WARN][19] ([fdo#103558] / [fdo#105602] / [fdo#105763]) -> [DMESG-WARN][20] ([fdo#103558] / [fdo#105602]) +9 similar issues
   [19]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7440/fi-kbl-x1275/igt@kms_flip@basic-flip-vs-modeset.html
   [20]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15505/fi-kbl-x1275/igt@kms_flip@basic-flip-vs-modeset.html

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

  [fdo# 112163]: https://bugs.freedesktop.org/show_bug.cgi?id= 112163
  [fdo#103167]: https://bugs.freedesktop.org/show_bug.cgi?id=103167
  [fdo#103558]: https://bugs.freedesktop.org/show_bug.cgi?id=103558
  [fdo#105602]: https://bugs.freedesktop.org/show_bug.cgi?id=105602
  [fdo#105763]: https://bugs.freedesktop.org/show_bug.cgi?id=105763
  [fdo#107707]: https://bugs.freedesktop.org/show_bug.cgi?id=107707
  [fdo#108511]: https://bugs.freedesktop.org/show_bug.cgi?id=108511
  [fdo#109483]: https://bugs.freedesktop.org/show_bug.cgi?id=109483
  [fdo#109964]: https://bugs.freedesktop.org/show_bug.cgi?id=109964
  [fdo#111045]: https://bugs.freedesktop.org/show_bug.cgi?id=111045
  [fdo#111096]: https://bugs.freedesktop.org/show_bug.cgi?id=111096
  [fdo#112298]: https://bugs.freedesktop.org/show_bug.cgi?id=112298
  [fdo#112405]: https://bugs.freedesktop.org/show_bug.cgi?id=112405


Participating hosts (52 -> 46)
------------------------------

  Missing    (6): fi-ilk-m540 fi-hsw-4200u fi-bsw-cyan fi-ctg-p8600 fi-byt-clapper fi-bdw-samus 


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

  * CI: CI-20190529 -> None
  * Linux: CI_DRM_7440 -> Patchwork_15505

  CI-20190529: 20190529
  CI_DRM_7440: 7b08e6efdddfcde8ea60a96d5a818af032b52b4d @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_5315: 58705eb1bd29414244f4d0cfa08a9f0ce42d6545 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_15505: 1d4aaedc3c5188faa858c90145565737af08b153 @ git://anongit.freedesktop.org/gfx-ci/linux


== Linux commits ==

1d4aaedc3c51 drm/i915: Serialise i915_active_wait() with its retirement

== Logs ==

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

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

* Re: [PATCH] drm/i915: Serialise i915_active_wait() with its retirement
@ 2019-11-29 11:15   ` Tvrtko Ursulin
  0 siblings, 0 replies; 10+ messages in thread
From: Tvrtko Ursulin @ 2019-11-29 11:15 UTC (permalink / raw)
  To: Chris Wilson, intel-gfx


On 29/11/2019 09:39, Chris Wilson wrote:
> As the i915_active.retire() may be running on another CPU as we detect
> that the i915_active is idle, we may not wait for the retirement itself.
> Wait for the remote callback by waiting for the retirement worker.
> 
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=112424
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> ---
>   drivers/gpu/drm/i915/i915_active.c | 1 +
>   1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/gpu/drm/i915/i915_active.c b/drivers/gpu/drm/i915/i915_active.c
> index 479195ecbc6c..e8630ee33336 100644
> --- a/drivers/gpu/drm/i915/i915_active.c
> +++ b/drivers/gpu/drm/i915/i915_active.c
> @@ -469,6 +469,7 @@ int i915_active_wait(struct i915_active *ref)
>   	if (wait_var_event_interruptible(ref, i915_active_is_idle(ref)))
>   		return -EINTR;
>   
> +	flush_work(&ref->work);
>   	return 0;
>   }
>   
> 

Hm, but wake_up_war is in the worker so how does wait_var_event wake the 
waiter up before it has been retired?

Regards,

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

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

* Re: [Intel-gfx] [PATCH] drm/i915: Serialise i915_active_wait() with its retirement
@ 2019-11-29 11:15   ` Tvrtko Ursulin
  0 siblings, 0 replies; 10+ messages in thread
From: Tvrtko Ursulin @ 2019-11-29 11:15 UTC (permalink / raw)
  To: Chris Wilson, intel-gfx


On 29/11/2019 09:39, Chris Wilson wrote:
> As the i915_active.retire() may be running on another CPU as we detect
> that the i915_active is idle, we may not wait for the retirement itself.
> Wait for the remote callback by waiting for the retirement worker.
> 
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=112424
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> ---
>   drivers/gpu/drm/i915/i915_active.c | 1 +
>   1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/gpu/drm/i915/i915_active.c b/drivers/gpu/drm/i915/i915_active.c
> index 479195ecbc6c..e8630ee33336 100644
> --- a/drivers/gpu/drm/i915/i915_active.c
> +++ b/drivers/gpu/drm/i915/i915_active.c
> @@ -469,6 +469,7 @@ int i915_active_wait(struct i915_active *ref)
>   	if (wait_var_event_interruptible(ref, i915_active_is_idle(ref)))
>   		return -EINTR;
>   
> +	flush_work(&ref->work);
>   	return 0;
>   }
>   
> 

Hm, but wake_up_war is in the worker so how does wait_var_event wake the 
waiter up before it has been retired?

Regards,

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

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

* Re: [PATCH] drm/i915: Serialise i915_active_wait() with its retirement
@ 2019-11-29 11:28     ` Chris Wilson
  0 siblings, 0 replies; 10+ messages in thread
From: Chris Wilson @ 2019-11-29 11:28 UTC (permalink / raw)
  To: Tvrtko Ursulin, intel-gfx

Quoting Tvrtko Ursulin (2019-11-29 11:15:46)
> 
> On 29/11/2019 09:39, Chris Wilson wrote:
> > As the i915_active.retire() may be running on another CPU as we detect
> > that the i915_active is idle, we may not wait for the retirement itself.
> > Wait for the remote callback by waiting for the retirement worker.
> > 
> > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=112424
> > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> > ---
> >   drivers/gpu/drm/i915/i915_active.c | 1 +
> >   1 file changed, 1 insertion(+)
> > 
> > diff --git a/drivers/gpu/drm/i915/i915_active.c b/drivers/gpu/drm/i915/i915_active.c
> > index 479195ecbc6c..e8630ee33336 100644
> > --- a/drivers/gpu/drm/i915/i915_active.c
> > +++ b/drivers/gpu/drm/i915/i915_active.c
> > @@ -469,6 +469,7 @@ int i915_active_wait(struct i915_active *ref)
> >       if (wait_var_event_interruptible(ref, i915_active_is_idle(ref)))
> >               return -EINTR;
> >   
> > +     flush_work(&ref->work);
> >       return 0;
> >   }
> >   
> > 
> 
> Hm, but wake_up_war is in the worker so how does wait_var_event wake the 
> waiter up before it has been retired?

Remember the wait_event pattern is to skip the wait if COND is already
met. So since the first thing the retirement does is the
atomic_dec_and_test(), we can see ref->count == 0 very early, long
before ref->retire() is called. Our selftest is checking that if
i915_active_wait() reports completion, the callback has also run and
that the i915_active can then be destroyed.
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

* Re: [Intel-gfx] [PATCH] drm/i915: Serialise i915_active_wait() with its retirement
@ 2019-11-29 11:28     ` Chris Wilson
  0 siblings, 0 replies; 10+ messages in thread
From: Chris Wilson @ 2019-11-29 11:28 UTC (permalink / raw)
  To: Tvrtko Ursulin, intel-gfx

Quoting Tvrtko Ursulin (2019-11-29 11:15:46)
> 
> On 29/11/2019 09:39, Chris Wilson wrote:
> > As the i915_active.retire() may be running on another CPU as we detect
> > that the i915_active is idle, we may not wait for the retirement itself.
> > Wait for the remote callback by waiting for the retirement worker.
> > 
> > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=112424
> > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> > ---
> >   drivers/gpu/drm/i915/i915_active.c | 1 +
> >   1 file changed, 1 insertion(+)
> > 
> > diff --git a/drivers/gpu/drm/i915/i915_active.c b/drivers/gpu/drm/i915/i915_active.c
> > index 479195ecbc6c..e8630ee33336 100644
> > --- a/drivers/gpu/drm/i915/i915_active.c
> > +++ b/drivers/gpu/drm/i915/i915_active.c
> > @@ -469,6 +469,7 @@ int i915_active_wait(struct i915_active *ref)
> >       if (wait_var_event_interruptible(ref, i915_active_is_idle(ref)))
> >               return -EINTR;
> >   
> > +     flush_work(&ref->work);
> >       return 0;
> >   }
> >   
> > 
> 
> Hm, but wake_up_war is in the worker so how does wait_var_event wake the 
> waiter up before it has been retired?

Remember the wait_event pattern is to skip the wait if COND is already
met. So since the first thing the retirement does is the
atomic_dec_and_test(), we can see ref->count == 0 very early, long
before ref->retire() is called. Our selftest is checking that if
i915_active_wait() reports completion, the callback has also run and
that the i915_active can then be destroyed.
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

* Re: [PATCH] drm/i915: Serialise i915_active_wait() with its retirement
@ 2019-11-29 11:38       ` Tvrtko Ursulin
  0 siblings, 0 replies; 10+ messages in thread
From: Tvrtko Ursulin @ 2019-11-29 11:38 UTC (permalink / raw)
  To: Chris Wilson, intel-gfx


On 29/11/2019 11:28, Chris Wilson wrote:
> Quoting Tvrtko Ursulin (2019-11-29 11:15:46)
>>
>> On 29/11/2019 09:39, Chris Wilson wrote:
>>> As the i915_active.retire() may be running on another CPU as we detect
>>> that the i915_active is idle, we may not wait for the retirement itself.
>>> Wait for the remote callback by waiting for the retirement worker.
>>>
>>> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=112424
>>> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
>>> ---
>>>    drivers/gpu/drm/i915/i915_active.c | 1 +
>>>    1 file changed, 1 insertion(+)
>>>
>>> diff --git a/drivers/gpu/drm/i915/i915_active.c b/drivers/gpu/drm/i915/i915_active.c
>>> index 479195ecbc6c..e8630ee33336 100644
>>> --- a/drivers/gpu/drm/i915/i915_active.c
>>> +++ b/drivers/gpu/drm/i915/i915_active.c
>>> @@ -469,6 +469,7 @@ int i915_active_wait(struct i915_active *ref)
>>>        if (wait_var_event_interruptible(ref, i915_active_is_idle(ref)))
>>>                return -EINTR;
>>>    
>>> +     flush_work(&ref->work);
>>>        return 0;
>>>    }
>>>    
>>>
>>
>> Hm, but wake_up_war is in the worker so how does wait_var_event wake the
>> waiter up before it has been retired?
> 
> Remember the wait_event pattern is to skip the wait if COND is already
> met. So since the first thing the retirement does is the
> atomic_dec_and_test(), we can see ref->count == 0 very early, long
> before ref->retire() is called. Our selftest is checking that if
> i915_active_wait() reports completion, the callback has also run and
> that the i915_active can then be destroyed.

True!

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] 10+ messages in thread

* Re: [Intel-gfx] [PATCH] drm/i915: Serialise i915_active_wait() with its retirement
@ 2019-11-29 11:38       ` Tvrtko Ursulin
  0 siblings, 0 replies; 10+ messages in thread
From: Tvrtko Ursulin @ 2019-11-29 11:38 UTC (permalink / raw)
  To: Chris Wilson, intel-gfx


On 29/11/2019 11:28, Chris Wilson wrote:
> Quoting Tvrtko Ursulin (2019-11-29 11:15:46)
>>
>> On 29/11/2019 09:39, Chris Wilson wrote:
>>> As the i915_active.retire() may be running on another CPU as we detect
>>> that the i915_active is idle, we may not wait for the retirement itself.
>>> Wait for the remote callback by waiting for the retirement worker.
>>>
>>> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=112424
>>> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
>>> ---
>>>    drivers/gpu/drm/i915/i915_active.c | 1 +
>>>    1 file changed, 1 insertion(+)
>>>
>>> diff --git a/drivers/gpu/drm/i915/i915_active.c b/drivers/gpu/drm/i915/i915_active.c
>>> index 479195ecbc6c..e8630ee33336 100644
>>> --- a/drivers/gpu/drm/i915/i915_active.c
>>> +++ b/drivers/gpu/drm/i915/i915_active.c
>>> @@ -469,6 +469,7 @@ int i915_active_wait(struct i915_active *ref)
>>>        if (wait_var_event_interruptible(ref, i915_active_is_idle(ref)))
>>>                return -EINTR;
>>>    
>>> +     flush_work(&ref->work);
>>>        return 0;
>>>    }
>>>    
>>>
>>
>> Hm, but wake_up_war is in the worker so how does wait_var_event wake the
>> waiter up before it has been retired?
> 
> Remember the wait_event pattern is to skip the wait if COND is already
> met. So since the first thing the retirement does is the
> atomic_dec_and_test(), we can see ref->count == 0 very early, long
> before ref->retire() is called. Our selftest is checking that if
> i915_active_wait() reports completion, the callback has also run and
> that the i915_active can then be destroyed.

True!

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] 10+ messages in thread

end of thread, other threads:[~2019-11-29 11:38 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2019-11-29  9:39 [PATCH] drm/i915: Serialise i915_active_wait() with its retirement Chris Wilson
2019-11-29  9:39 ` [Intel-gfx] " Chris Wilson
2019-11-29 10:42 ` ✗ Fi.CI.BAT: failure for " Patchwork
2019-11-29 10:42   ` [Intel-gfx] " Patchwork
2019-11-29 11:15 ` [PATCH] " Tvrtko Ursulin
2019-11-29 11:15   ` [Intel-gfx] " Tvrtko Ursulin
2019-11-29 11:28   ` Chris Wilson
2019-11-29 11:28     ` [Intel-gfx] " Chris Wilson
2019-11-29 11:38     ` Tvrtko Ursulin
2019-11-29 11:38       ` [Intel-gfx] " Tvrtko Ursulin

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.