* [PATCH] drm/i915: Avoid refcount_inc on known zero count
@ 2019-05-28 15:40 Chris Wilson
2019-05-28 16:46 ` ✗ Fi.CI.BAT: failure for " Patchwork
2019-05-29 12:13 ` [PATCH] " Mika Kuoppala
0 siblings, 2 replies; 3+ messages in thread
From: Chris Wilson @ 2019-05-28 15:40 UTC (permalink / raw)
To: intel-gfx
In intel_wakeref_auto, we use refcount_inc_not_zero to detect the first
use and initialise the timer. On doing so, we have to avoid using
refcount_inc on that zero count as the debug code flags that as an
error:
refcount_t: increment on 0; use-after-free.
Rearrange the code so that if we know the count is 0 and we are
initialising, we explicitly set it to 1.
Fixes: b27e35ae5b18 ("drm/i915: Keep user GGTT alive for a minimum of 250ms")
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Mika Kuoppala <mika.kuoppala@linux.intel.com>
---
drivers/gpu/drm/i915/intel_wakeref.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_wakeref.c b/drivers/gpu/drm/i915/intel_wakeref.c
index c2dda5a375f0..c25ba1b5e8ba 100644
--- a/drivers/gpu/drm/i915/intel_wakeref.c
+++ b/drivers/gpu/drm/i915/intel_wakeref.c
@@ -114,11 +114,11 @@ void intel_wakeref_auto(struct intel_wakeref_auto *wf, unsigned long timeout)
if (!refcount_inc_not_zero(&wf->count)) {
spin_lock_irqsave(&wf->lock, flags);
- if (!refcount_read(&wf->count)) {
+ if (!refcount_inc_not_zero(&wf->count)) {
GEM_BUG_ON(wf->wakeref);
wf->wakeref = intel_runtime_pm_get_if_in_use(wf->i915);
+ refcount_set(&wf->count, 1);
}
- refcount_inc(&wf->count);
spin_unlock_irqrestore(&wf->lock, flags);
}
--
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] 3+ messages in thread
* ✗ Fi.CI.BAT: failure for drm/i915: Avoid refcount_inc on known zero count
2019-05-28 15:40 [PATCH] drm/i915: Avoid refcount_inc on known zero count Chris Wilson
@ 2019-05-28 16:46 ` Patchwork
2019-05-29 12:13 ` [PATCH] " Mika Kuoppala
1 sibling, 0 replies; 3+ messages in thread
From: Patchwork @ 2019-05-28 16:46 UTC (permalink / raw)
To: Chris Wilson; +Cc: intel-gfx
== Series Details ==
Series: drm/i915: Avoid refcount_inc on known zero count
URL : https://patchwork.freedesktop.org/series/61255/
State : failure
== Summary ==
CI Bug Log - changes from CI_DRM_6157 -> Patchwork_13116
====================================================
Summary
-------
**FAILURE**
Serious unknown changes coming with Patchwork_13116 absolutely need to be
verified manually.
If you think the reported changes have nothing to do with the changes
introduced in Patchwork_13116, 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_13116/
Possible new issues
-------------------
Here are the unknown changes that may have been introduced in Patchwork_13116:
### IGT changes ###
#### Possible regressions ####
* igt@i915_selftest@live_sanitycheck:
- fi-kbl-7567u: [PASS][1] -> [FAIL][2]
[1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6157/fi-kbl-7567u/igt@i915_selftest@live_sanitycheck.html
[2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13116/fi-kbl-7567u/igt@i915_selftest@live_sanitycheck.html
* igt@runner@aborted:
- fi-kbl-7567u: NOTRUN -> [FAIL][3]
[3]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13116/fi-kbl-7567u/igt@runner@aborted.html
Known issues
------------
Here are the changes found in Patchwork_13116 that come from known issues:
### IGT changes ###
#### Issues hit ####
* igt@i915_selftest@live_contexts:
- fi-bdw-gvtdvm: [PASS][4] -> [DMESG-FAIL][5] ([fdo#110235])
[4]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6157/fi-bdw-gvtdvm/igt@i915_selftest@live_contexts.html
[5]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13116/fi-bdw-gvtdvm/igt@i915_selftest@live_contexts.html
* igt@i915_selftest@live_hangcheck:
- fi-icl-u3: [PASS][6] -> [INCOMPLETE][7] ([fdo#107713] / [fdo#108569])
[6]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6157/fi-icl-u3/igt@i915_selftest@live_hangcheck.html
[7]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13116/fi-icl-u3/igt@i915_selftest@live_hangcheck.html
* igt@kms_frontbuffer_tracking@basic:
- fi-hsw-peppy: [PASS][8] -> [DMESG-WARN][9] ([fdo#102614])
[8]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6157/fi-hsw-peppy/igt@kms_frontbuffer_tracking@basic.html
[9]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13116/fi-hsw-peppy/igt@kms_frontbuffer_tracking@basic.html
#### Possible fixes ####
* igt@kms_busy@basic-flip-c:
- fi-icl-u3: [DMESG-WARN][10] ([fdo#107724]) -> [PASS][11]
[10]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6157/fi-icl-u3/igt@kms_busy@basic-flip-c.html
[11]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13116/fi-icl-u3/igt@kms_busy@basic-flip-c.html
{name}: This element is suppressed. This means it is ignored when computing
the status of the difference (SUCCESS, WARNING, or FAILURE).
[fdo#102614]: https://bugs.freedesktop.org/show_bug.cgi?id=102614
[fdo#107713]: https://bugs.freedesktop.org/show_bug.cgi?id=107713
[fdo#107724]: https://bugs.freedesktop.org/show_bug.cgi?id=107724
[fdo#108569]: https://bugs.freedesktop.org/show_bug.cgi?id=108569
[fdo#109100]: https://bugs.freedesktop.org/show_bug.cgi?id=109100
[fdo#110235]: https://bugs.freedesktop.org/show_bug.cgi?id=110235
Participating hosts (54 -> 47)
------------------------------
Missing (7): fi-kbl-soraka fi-ilk-m540 fi-hsw-4200u fi-byt-squawks fi-bsw-cyan fi-byt-clapper fi-bdw-samus
Build changes
-------------
* Linux: CI_DRM_6157 -> Patchwork_13116
CI_DRM_6157: 8991a80f85f227dc1a1ef76396ca87693ad217a6 @ git://anongit.freedesktop.org/gfx-ci/linux
IGT_5021: 2d64cb6808075b0d0696a89d2ce290220e6eff8e @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
Patchwork_13116: 669e72a095cee1f77d41107e282565daa0394c26 @ git://anongit.freedesktop.org/gfx-ci/linux
== Linux commits ==
669e72a095ce drm/i915: Avoid refcount_inc on known zero count
== Logs ==
For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13116/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] drm/i915: Avoid refcount_inc on known zero count
2019-05-28 15:40 [PATCH] drm/i915: Avoid refcount_inc on known zero count Chris Wilson
2019-05-28 16:46 ` ✗ Fi.CI.BAT: failure for " Patchwork
@ 2019-05-29 12:13 ` Mika Kuoppala
1 sibling, 0 replies; 3+ messages in thread
From: Mika Kuoppala @ 2019-05-29 12:13 UTC (permalink / raw)
To: Chris Wilson, intel-gfx
Chris Wilson <chris@chris-wilson.co.uk> writes:
> In intel_wakeref_auto, we use refcount_inc_not_zero to detect the first
> use and initialise the timer. On doing so, we have to avoid using
> refcount_inc on that zero count as the debug code flags that as an
> error:
> refcount_t: increment on 0; use-after-free.
>
Yeah there are reinforced version: refcount_inc_checked, which
I failed to notice.
I guess the good news is that now we have proof that there is
someone watching our six.
> Rearrange the code so that if we know the count is 0 and we are
> initialising, we explicitly set it to 1.
>
> Fixes: b27e35ae5b18 ("drm/i915: Keep user GGTT alive for a minimum of 250ms")
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Mika Kuoppala <mika.kuoppala@linux.intel.com>
> ---
> drivers/gpu/drm/i915/intel_wakeref.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_wakeref.c b/drivers/gpu/drm/i915/intel_wakeref.c
> index c2dda5a375f0..c25ba1b5e8ba 100644
> --- a/drivers/gpu/drm/i915/intel_wakeref.c
> +++ b/drivers/gpu/drm/i915/intel_wakeref.c
> @@ -114,11 +114,11 @@ void intel_wakeref_auto(struct intel_wakeref_auto *wf, unsigned long timeout)
>
> if (!refcount_inc_not_zero(&wf->count)) {
> spin_lock_irqsave(&wf->lock, flags);
> - if (!refcount_read(&wf->count)) {
> + if (!refcount_inc_not_zero(&wf->count)) {
Ok, overflow is checked with this.
Reviewed-by: Mika Kuoppala <mika.kuoppala@linux.intel.com>
> GEM_BUG_ON(wf->wakeref);
> wf->wakeref = intel_runtime_pm_get_if_in_use(wf->i915);
> + refcount_set(&wf->count, 1);
> }
> - refcount_inc(&wf->count);
> spin_unlock_irqrestore(&wf->lock, flags);
> }
>
> --
> 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] 3+ messages in thread
end of thread, other threads:[~2019-05-29 12:13 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-05-28 15:40 [PATCH] drm/i915: Avoid refcount_inc on known zero count Chris Wilson
2019-05-28 16:46 ` ✗ Fi.CI.BAT: failure for " Patchwork
2019-05-29 12:13 ` [PATCH] " Mika Kuoppala
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.