* [PATCH] drm/i915: Mark up i915_vma_unbind() as a potential sleeper
@ 2017-11-09 10:59 Chris Wilson
2017-11-09 11:57 ` Joonas Lahtinen
2017-11-09 12:02 ` ✗ Fi.CI.BAT: warning for " Patchwork
0 siblings, 2 replies; 5+ messages in thread
From: Chris Wilson @ 2017-11-09 10:59 UTC (permalink / raw)
To: intel-gfx; +Cc: Daniel Vetter
Whenever we want to unbind a vma, we must wait on all GPU activity to
complete first. (This is what gives us the ability to do fine grained
eviction and purging by only having to wait on the VMA that we need to
unbind to proceed; though if pushed we can make it a rule that we are
only allowed to unbind already idle VMA and move the burden of the work
and organising the sleep onto the caller.) Currently, we might only
sleep if the vma is still active on the GPU, but in principle
i915_vma_unbind() always implies a sleep, so mark it up with a
might_sleep().
Suggested-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
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: Matthew Auld <matthew.william.auld@gmail.com>
Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
---
drivers/gpu/drm/i915/i915_vma.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/gpu/drm/i915/i915_vma.c b/drivers/gpu/drm/i915/i915_vma.c
index b7ac84db1d2e..bf6d8d1eaabe 100644
--- a/drivers/gpu/drm/i915/i915_vma.c
+++ b/drivers/gpu/drm/i915/i915_vma.c
@@ -743,6 +743,7 @@ int i915_vma_unbind(struct i915_vma *vma)
/* First wait upon any activity as retiring the request may
* have side-effects such as unpinning or even unbinding this vma.
*/
+ might_sleep();
active = i915_vma_get_active(vma);
if (active) {
int idx;
--
2.15.0
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH] drm/i915: Mark up i915_vma_unbind() as a potential sleeper
2017-11-09 10:59 [PATCH] drm/i915: Mark up i915_vma_unbind() as a potential sleeper Chris Wilson
@ 2017-11-09 11:57 ` Joonas Lahtinen
2017-11-09 12:02 ` Chris Wilson
2017-11-09 12:02 ` ✗ Fi.CI.BAT: warning for " Patchwork
1 sibling, 1 reply; 5+ messages in thread
From: Joonas Lahtinen @ 2017-11-09 11:57 UTC (permalink / raw)
To: Chris Wilson, intel-gfx; +Cc: Daniel Vetter
On Thu, 2017-11-09 at 10:59 +0000, Chris Wilson wrote:
> Whenever we want to unbind a vma, we must wait on all GPU activity to
> complete first. (This is what gives us the ability to do fine grained
> eviction and purging by only having to wait on the VMA that we need to
> unbind to proceed; though if pushed we can make it a rule that we are
> only allowed to unbind already idle VMA and move the burden of the work
> and organising the sleep onto the caller.) Currently, we might only
> sleep if the vma is still active on the GPU, but in principle
> i915_vma_unbind() always implies a sleep, so mark it up with a
> might_sleep().
>
> Suggested-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> 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: Matthew Auld <matthew.william.auld@gmail.com>
> Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
<SNIP>
> +++ b/drivers/gpu/drm/i915/i915_vma.c
> @@ -743,6 +743,7 @@ int i915_vma_unbind(struct i915_vma *vma)
> /* First wait upon any activity as retiring the request may
> * have side-effects such as unpinning or even unbinding this vma.
> */
> + might_sleep();
Please lift this up, even before lockdep_assert_held, for more clarity.
might_sleep();
lockdep_assert_held(...);
Reviewed-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
Regards, Joonas
--
Joonas Lahtinen
Open Source Technology Center
Intel Corporation
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 5+ messages in thread
* ✗ Fi.CI.BAT: warning for drm/i915: Mark up i915_vma_unbind() as a potential sleeper
2017-11-09 10:59 [PATCH] drm/i915: Mark up i915_vma_unbind() as a potential sleeper Chris Wilson
2017-11-09 11:57 ` Joonas Lahtinen
@ 2017-11-09 12:02 ` Patchwork
2017-11-09 12:18 ` Chris Wilson
1 sibling, 1 reply; 5+ messages in thread
From: Patchwork @ 2017-11-09 12:02 UTC (permalink / raw)
To: Chris Wilson; +Cc: intel-gfx
== Series Details ==
Series: drm/i915: Mark up i915_vma_unbind() as a potential sleeper
URL : https://patchwork.freedesktop.org/series/33509/
State : warning
== Summary ==
Series 33509v1 drm/i915: Mark up i915_vma_unbind() as a potential sleeper
https://patchwork.freedesktop.org/api/1.0/series/33509/revisions/1/mbox/
Test gem_ctx_switch:
Subgroup basic-default:
pass -> DMESG-WARN (fi-bdw-gvtdvm)
pass -> DMESG-WARN (fi-bsw-n3050)
Subgroup basic-default-heavy:
pass -> DMESG-WARN (fi-skl-6600u)
pass -> DMESG-WARN (fi-skl-gvtdvm)
pass -> DMESG-WARN (fi-bxt-dsi)
Test gem_exec_reloc:
Subgroup basic-gtt-active:
fail -> PASS (fi-gdg-551) fdo#102582 +2
fdo#102582 https://bugs.freedesktop.org/show_bug.cgi?id=102582
fi-bdw-5557u total:289 pass:268 dwarn:0 dfail:0 fail:0 skip:21 time:444s
fi-bdw-gvtdvm total:289 pass:264 dwarn:1 dfail:0 fail:0 skip:24 time:456s
fi-blb-e6850 total:289 pass:223 dwarn:1 dfail:0 fail:0 skip:65 time:380s
fi-bsw-n3050 total:289 pass:242 dwarn:1 dfail:0 fail:0 skip:46 time:544s
fi-bwr-2160 total:289 pass:183 dwarn:0 dfail:0 fail:0 skip:106 time:275s
fi-bxt-dsi total:289 pass:258 dwarn:1 dfail:0 fail:0 skip:30 time:514s
fi-bxt-j4205 total:289 pass:260 dwarn:0 dfail:0 fail:0 skip:29 time:500s
fi-byt-j1900 total:289 pass:254 dwarn:0 dfail:0 fail:0 skip:35 time:500s
fi-byt-n2820 total:289 pass:250 dwarn:0 dfail:0 fail:0 skip:39 time:496s
fi-elk-e7500 total:289 pass:229 dwarn:0 dfail:0 fail:0 skip:60 time:431s
fi-gdg-551 total:289 pass:178 dwarn:1 dfail:0 fail:1 skip:109 time:266s
fi-glk-1 total:289 pass:261 dwarn:0 dfail:0 fail:0 skip:28 time:542s
fi-hsw-4770 total:289 pass:262 dwarn:0 dfail:0 fail:0 skip:27 time:431s
fi-hsw-4770r total:289 pass:262 dwarn:0 dfail:0 fail:0 skip:27 time:438s
fi-ilk-650 total:289 pass:228 dwarn:0 dfail:0 fail:0 skip:61 time:426s
fi-ivb-3520m total:289 pass:260 dwarn:0 dfail:0 fail:0 skip:29 time:485s
fi-ivb-3770 total:289 pass:260 dwarn:0 dfail:0 fail:0 skip:29 time:460s
fi-kbl-7500u total:289 pass:264 dwarn:1 dfail:0 fail:0 skip:24 time:484s
fi-kbl-7560u total:289 pass:270 dwarn:0 dfail:0 fail:0 skip:19 time:514s
fi-kbl-7567u total:289 pass:269 dwarn:0 dfail:0 fail:0 skip:20 time:475s
fi-kbl-r total:289 pass:262 dwarn:0 dfail:0 fail:0 skip:27 time:535s
fi-pnv-d510 total:289 pass:222 dwarn:1 dfail:0 fail:0 skip:66 time:567s
fi-skl-6260u total:289 pass:269 dwarn:0 dfail:0 fail:0 skip:20 time:455s
fi-skl-6600u total:289 pass:261 dwarn:1 dfail:0 fail:0 skip:27 time:543s
fi-skl-6700hq total:289 pass:263 dwarn:0 dfail:0 fail:0 skip:26 time:564s
fi-skl-6700k total:289 pass:265 dwarn:0 dfail:0 fail:0 skip:24 time:517s
fi-skl-6770hq total:289 pass:269 dwarn:0 dfail:0 fail:0 skip:20 time:495s
fi-skl-gvtdvm total:289 pass:265 dwarn:1 dfail:0 fail:0 skip:23 time:458s
fi-snb-2520m total:289 pass:250 dwarn:0 dfail:0 fail:0 skip:39 time:559s
fi-snb-2600 total:289 pass:249 dwarn:0 dfail:0 fail:0 skip:40 time:418s
Blacklisted hosts:
fi-cnl-y total:289 pass:262 dwarn:0 dfail:0 fail:0 skip:27 time:544s
fi-glk-dsi total:289 pass:258 dwarn:0 dfail:0 fail:1 skip:30 time:505s
65dc54b704d3ee0486f9f5b11f00c28973f783a2 drm-tip: 2017y-11m-09d-08h-53m-46s UTC integration manifest
e68dbff33cf1 drm/i915: Mark up i915_vma_unbind() as a potential sleeper
== Logs ==
For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_7034/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] drm/i915: Mark up i915_vma_unbind() as a potential sleeper
2017-11-09 11:57 ` Joonas Lahtinen
@ 2017-11-09 12:02 ` Chris Wilson
0 siblings, 0 replies; 5+ messages in thread
From: Chris Wilson @ 2017-11-09 12:02 UTC (permalink / raw)
To: Joonas Lahtinen, intel-gfx; +Cc: Daniel Vetter
Quoting Joonas Lahtinen (2017-11-09 11:57:15)
> On Thu, 2017-11-09 at 10:59 +0000, Chris Wilson wrote:
> > Whenever we want to unbind a vma, we must wait on all GPU activity to
> > complete first. (This is what gives us the ability to do fine grained
> > eviction and purging by only having to wait on the VMA that we need to
> > unbind to proceed; though if pushed we can make it a rule that we are
> > only allowed to unbind already idle VMA and move the burden of the work
> > and organising the sleep onto the caller.) Currently, we might only
> > sleep if the vma is still active on the GPU, but in principle
> > i915_vma_unbind() always implies a sleep, so mark it up with a
> > might_sleep().
> >
> > Suggested-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> > 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: Matthew Auld <matthew.william.auld@gmail.com>
> > Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
>
> <SNIP>
>
> > +++ b/drivers/gpu/drm/i915/i915_vma.c
> > @@ -743,6 +743,7 @@ int i915_vma_unbind(struct i915_vma *vma)
> > /* First wait upon any activity as retiring the request may
> > * have side-effects such as unpinning or even unbinding this vma.
> > */
> > + might_sleep();
>
> Please lift this up, even before lockdep_assert_held, for more clarity.
The comment was explaining why we might_sleep(), so I liked it here. The
pattern that's kind of been established is that we document the locks we
expect the caller to be holding first, but that pattern is easy to break.
But the comment doesn't belong before lockdep_assert_held() as it is
currently written. :|
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: ✗ Fi.CI.BAT: warning for drm/i915: Mark up i915_vma_unbind() as a potential sleeper
2017-11-09 12:02 ` ✗ Fi.CI.BAT: warning for " Patchwork
@ 2017-11-09 12:18 ` Chris Wilson
0 siblings, 0 replies; 5+ messages in thread
From: Chris Wilson @ 2017-11-09 12:18 UTC (permalink / raw)
To: Patchwork; +Cc: intel-gfx
Quoting Patchwork (2017-11-09 12:02:30)
> == Series Details ==
>
> Series: drm/i915: Mark up i915_vma_unbind() as a potential sleeper
> URL : https://patchwork.freedesktop.org/series/33509/
> State : warning
>
> == Summary ==
>
> Series 33509v1 drm/i915: Mark up i915_vma_unbind() as a potential sleeper
> https://patchwork.freedesktop.org/api/1.0/series/33509/revisions/1/mbox/
>
> Test gem_ctx_switch:
> Subgroup basic-default:
> pass -> DMESG-WARN (fi-bdw-gvtdvm)
> pass -> DMESG-WARN (fi-bsw-n3050)
> Subgroup basic-default-heavy:
> pass -> DMESG-WARN (fi-skl-6600u)
> pass -> DMESG-WARN (fi-skl-gvtdvm)
> pass -> DMESG-WARN (fi-bxt-dsi)
I must be missing something since the test doesn't vary, if it is
closing the context before calling gem_close() on every object, all
machines should find themselves with a populated ppgtt on context-close.
All full-ppgtt should be triggering the warning if one does!!!
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2017-11-09 12:24 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-11-09 10:59 [PATCH] drm/i915: Mark up i915_vma_unbind() as a potential sleeper Chris Wilson
2017-11-09 11:57 ` Joonas Lahtinen
2017-11-09 12:02 ` Chris Wilson
2017-11-09 12:02 ` ✗ Fi.CI.BAT: warning for " Patchwork
2017-11-09 12:18 ` Chris Wilson
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.