* [BrownBag] drm/i915/gtt: Neuter the deferred unbind callback from gen6_ppgtt_cleanup
@ 2019-05-24 6:45 Chris Wilson
2019-05-24 8:13 ` Tvrtko Ursulin
2019-05-24 8:31 ` ✗ Fi.CI.BAT: failure for " Patchwork
0 siblings, 2 replies; 13+ messages in thread
From: Chris Wilson @ 2019-05-24 6:45 UTC (permalink / raw)
To: intel-gfx; +Cc: Tomi Sarvela
Having deferred the vma destruction to a worker where we can acquire the
struct_mutex, we have to avoid chasing back into the now destroyed
ppgtt. The pd_vma is special in having a custom unbind function to scan
for unused pages despite the VMA itself being notionally part of the
GGTT. As such, we need to disable that callback to avoid a
use-after-free.
This unfortunately blew up so early during boot that CI declared the
machine unreachable as opposed to being the major failure it was. Oops.
Fixes: d3622099c76f ("drm/i915/gtt: Always acquire struct_mutex for gen6_ppgtt_cleanup")
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Cc: Tomi Sarvela <tomi.p.sarvela@intel.com>
---
drivers/gpu/drm/i915/i915_gem_gtt.c | 28 ++++++++++++++++++++++++++++
1 file changed, 28 insertions(+)
diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c
index 8d8a4b0ad4d9..266baa11df64 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.c
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
@@ -1847,6 +1847,33 @@ static void gen6_ppgtt_cleanup_work(struct work_struct *wrk)
kfree(work);
}
+static int nop_set_pages(struct i915_vma *vma)
+{
+ return -ENODEV;
+}
+
+static void nop_clear_pages(struct i915_vma *vma)
+{
+}
+
+static int nop_bind(struct i915_vma *vma,
+ enum i915_cache_level cache_level,
+ u32 unused)
+{
+ return -ENODEV;
+}
+
+static void nop_unbind(struct i915_vma *vma)
+{
+}
+
+static const struct i915_vma_ops nop_vma_ops = {
+ .set_pages = nop_set_pages,
+ .clear_pages = nop_clear_pages,
+ .bind_vma = nop_bind,
+ .unbind_vma = nop_unbind,
+};
+
static void gen6_ppgtt_cleanup(struct i915_address_space *vm)
{
struct gen6_hw_ppgtt *ppgtt = to_gen6_ppgtt(i915_vm_to_ppgtt(vm));
@@ -1855,6 +1882,7 @@ static void gen6_ppgtt_cleanup(struct i915_address_space *vm)
/* FIXME remove the struct_mutex to bring the locking under control */
INIT_WORK(&work->base, gen6_ppgtt_cleanup_work);
work->vma = ppgtt->vma;
+ work->vma->ops = &nop_vma_ops;
schedule_work(&work->base);
gen6_ppgtt_free_pd(ppgtt);
--
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] 13+ messages in thread
* Re: [BrownBag] drm/i915/gtt: Neuter the deferred unbind callback from gen6_ppgtt_cleanup
2019-05-24 6:45 [BrownBag] drm/i915/gtt: Neuter the deferred unbind callback from gen6_ppgtt_cleanup Chris Wilson
@ 2019-05-24 8:13 ` Tvrtko Ursulin
2019-05-24 8:17 ` Chris Wilson
2019-05-24 8:31 ` ✗ Fi.CI.BAT: failure for " Patchwork
1 sibling, 1 reply; 13+ messages in thread
From: Tvrtko Ursulin @ 2019-05-24 8:13 UTC (permalink / raw)
To: Chris Wilson, intel-gfx; +Cc: Tomi Sarvela
On 24/05/2019 07:45, Chris Wilson wrote:
> Having deferred the vma destruction to a worker where we can acquire the
> struct_mutex, we have to avoid chasing back into the now destroyed
> ppgtt. The pd_vma is special in having a custom unbind function to scan
> for unused pages despite the VMA itself being notionally part of the
> GGTT. As such, we need to disable that callback to avoid a
> use-after-free.
>
> This unfortunately blew up so early during boot that CI declared the
> machine unreachable as opposed to being the major failure it was. Oops.
>
> Fixes: d3622099c76f ("drm/i915/gtt: Always acquire struct_mutex for gen6_ppgtt_cleanup")
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> Cc: Tomi Sarvela <tomi.p.sarvela@intel.com>
> ---
> drivers/gpu/drm/i915/i915_gem_gtt.c | 28 ++++++++++++++++++++++++++++
> 1 file changed, 28 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c
> index 8d8a4b0ad4d9..266baa11df64 100644
> --- a/drivers/gpu/drm/i915/i915_gem_gtt.c
> +++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
> @@ -1847,6 +1847,33 @@ static void gen6_ppgtt_cleanup_work(struct work_struct *wrk)
> kfree(work);
> }
>
> +static int nop_set_pages(struct i915_vma *vma)
> +{
> + return -ENODEV;
> +}
> +
> +static void nop_clear_pages(struct i915_vma *vma)
> +{
> +}
> +
> +static int nop_bind(struct i915_vma *vma,
> + enum i915_cache_level cache_level,
> + u32 unused)
> +{
> + return -ENODEV;
> +}
> +
> +static void nop_unbind(struct i915_vma *vma)
> +{
> +}
> +
> +static const struct i915_vma_ops nop_vma_ops = {
> + .set_pages = nop_set_pages,
> + .clear_pages = nop_clear_pages,
> + .bind_vma = nop_bind,
> + .unbind_vma = nop_unbind,
> +};
> +
> static void gen6_ppgtt_cleanup(struct i915_address_space *vm)
> {
> struct gen6_hw_ppgtt *ppgtt = to_gen6_ppgtt(i915_vm_to_ppgtt(vm));
> @@ -1855,6 +1882,7 @@ static void gen6_ppgtt_cleanup(struct i915_address_space *vm)
> /* FIXME remove the struct_mutex to bring the locking under control */
> INIT_WORK(&work->base, gen6_ppgtt_cleanup_work);
> work->vma = ppgtt->vma;
> + work->vma->ops = &nop_vma_ops;
Could we use some asserts before overriding the vma ops? Like
GEM_BUG_ON(vma->pages)? And something for still bound?
> schedule_work(&work->base);
>
> gen6_ppgtt_free_pd(ppgtt);
>
Regards,
Tvrtko
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [BrownBag] drm/i915/gtt: Neuter the deferred unbind callback from gen6_ppgtt_cleanup
2019-05-24 8:13 ` Tvrtko Ursulin
@ 2019-05-24 8:17 ` Chris Wilson
2019-05-24 8:23 ` Tvrtko Ursulin
0 siblings, 1 reply; 13+ messages in thread
From: Chris Wilson @ 2019-05-24 8:17 UTC (permalink / raw)
To: Tvrtko Ursulin, intel-gfx; +Cc: Tomi Sarvela
Quoting Tvrtko Ursulin (2019-05-24 09:13:14)
>
> On 24/05/2019 07:45, Chris Wilson wrote:
> > Having deferred the vma destruction to a worker where we can acquire the
> > struct_mutex, we have to avoid chasing back into the now destroyed
> > ppgtt. The pd_vma is special in having a custom unbind function to scan
> > for unused pages despite the VMA itself being notionally part of the
> > GGTT. As such, we need to disable that callback to avoid a
> > use-after-free.
> >
> > This unfortunately blew up so early during boot that CI declared the
> > machine unreachable as opposed to being the major failure it was. Oops.
> >
> > Fixes: d3622099c76f ("drm/i915/gtt: Always acquire struct_mutex for gen6_ppgtt_cleanup")
> > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> > Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> > Cc: Tomi Sarvela <tomi.p.sarvela@intel.com>
> > ---
> > drivers/gpu/drm/i915/i915_gem_gtt.c | 28 ++++++++++++++++++++++++++++
> > 1 file changed, 28 insertions(+)
> >
> > diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c
> > index 8d8a4b0ad4d9..266baa11df64 100644
> > --- a/drivers/gpu/drm/i915/i915_gem_gtt.c
> > +++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
> > @@ -1847,6 +1847,33 @@ static void gen6_ppgtt_cleanup_work(struct work_struct *wrk)
> > kfree(work);
> > }
> >
> > +static int nop_set_pages(struct i915_vma *vma)
> > +{
> > + return -ENODEV;
> > +}
> > +
> > +static void nop_clear_pages(struct i915_vma *vma)
> > +{
> > +}
> > +
> > +static int nop_bind(struct i915_vma *vma,
> > + enum i915_cache_level cache_level,
> > + u32 unused)
> > +{
> > + return -ENODEV;
> > +}
> > +
> > +static void nop_unbind(struct i915_vma *vma)
> > +{
> > +}
> > +
> > +static const struct i915_vma_ops nop_vma_ops = {
> > + .set_pages = nop_set_pages,
> > + .clear_pages = nop_clear_pages,
> > + .bind_vma = nop_bind,
> > + .unbind_vma = nop_unbind,
> > +};
> > +
> > static void gen6_ppgtt_cleanup(struct i915_address_space *vm)
> > {
> > struct gen6_hw_ppgtt *ppgtt = to_gen6_ppgtt(i915_vm_to_ppgtt(vm));
> > @@ -1855,6 +1882,7 @@ static void gen6_ppgtt_cleanup(struct i915_address_space *vm)
> > /* FIXME remove the struct_mutex to bring the locking under control */
> > INIT_WORK(&work->base, gen6_ppgtt_cleanup_work);
> > work->vma = ppgtt->vma;
> > + work->vma->ops = &nop_vma_ops;
>
> Could we use some asserts before overriding the vma ops? Like
> GEM_BUG_ON(vma->pages)? And something for still bound?
It technically still is bound as it is in the GGTT but currently
unpinned -- that will be checked on destroy, it's just we also get an
unbind callback. vma->pages doesn't exist for this (set to ERR_PTR).
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [BrownBag] drm/i915/gtt: Neuter the deferred unbind callback from gen6_ppgtt_cleanup
2019-05-24 8:17 ` Chris Wilson
@ 2019-05-24 8:23 ` Tvrtko Ursulin
2019-05-24 8:29 ` Chris Wilson
0 siblings, 1 reply; 13+ messages in thread
From: Tvrtko Ursulin @ 2019-05-24 8:23 UTC (permalink / raw)
To: Chris Wilson, intel-gfx; +Cc: Tomi Sarvela
On 24/05/2019 09:17, Chris Wilson wrote:
> Quoting Tvrtko Ursulin (2019-05-24 09:13:14)
>>
>> On 24/05/2019 07:45, Chris Wilson wrote:
>>> Having deferred the vma destruction to a worker where we can acquire the
>>> struct_mutex, we have to avoid chasing back into the now destroyed
>>> ppgtt. The pd_vma is special in having a custom unbind function to scan
>>> for unused pages despite the VMA itself being notionally part of the
>>> GGTT. As such, we need to disable that callback to avoid a
>>> use-after-free.
>>>
>>> This unfortunately blew up so early during boot that CI declared the
>>> machine unreachable as opposed to being the major failure it was. Oops.
>>>
>>> Fixes: d3622099c76f ("drm/i915/gtt: Always acquire struct_mutex for gen6_ppgtt_cleanup")
>>> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
>>> Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
>>> Cc: Tomi Sarvela <tomi.p.sarvela@intel.com>
>>> ---
>>> drivers/gpu/drm/i915/i915_gem_gtt.c | 28 ++++++++++++++++++++++++++++
>>> 1 file changed, 28 insertions(+)
>>>
>>> diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c
>>> index 8d8a4b0ad4d9..266baa11df64 100644
>>> --- a/drivers/gpu/drm/i915/i915_gem_gtt.c
>>> +++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
>>> @@ -1847,6 +1847,33 @@ static void gen6_ppgtt_cleanup_work(struct work_struct *wrk)
>>> kfree(work);
>>> }
>>>
>>> +static int nop_set_pages(struct i915_vma *vma)
>>> +{
>>> + return -ENODEV;
>>> +}
>>> +
>>> +static void nop_clear_pages(struct i915_vma *vma)
>>> +{
>>> +}
>>> +
>>> +static int nop_bind(struct i915_vma *vma,
>>> + enum i915_cache_level cache_level,
>>> + u32 unused)
>>> +{
>>> + return -ENODEV;
>>> +}
>>> +
>>> +static void nop_unbind(struct i915_vma *vma)
>>> +{
>>> +}
>>> +
>>> +static const struct i915_vma_ops nop_vma_ops = {
>>> + .set_pages = nop_set_pages,
>>> + .clear_pages = nop_clear_pages,
>>> + .bind_vma = nop_bind,
>>> + .unbind_vma = nop_unbind,
>>> +};
>>> +
>>> static void gen6_ppgtt_cleanup(struct i915_address_space *vm)
>>> {
>>> struct gen6_hw_ppgtt *ppgtt = to_gen6_ppgtt(i915_vm_to_ppgtt(vm));
>>> @@ -1855,6 +1882,7 @@ static void gen6_ppgtt_cleanup(struct i915_address_space *vm)
>>> /* FIXME remove the struct_mutex to bring the locking under control */
>>> INIT_WORK(&work->base, gen6_ppgtt_cleanup_work);
>>> work->vma = ppgtt->vma;
>>> + work->vma->ops = &nop_vma_ops;
>>
>> Could we use some asserts before overriding the vma ops? Like
>> GEM_BUG_ON(vma->pages)? And something for still bound?
>
> It technically still is bound as it is in the GGTT but currently
> unpinned -- that will be checked on destroy, it's just we also get an
> unbind callback. vma->pages doesn't exist for this (set to ERR_PTR).
If we are getting the unbind callback and we nop-ed it, who will
actually do it's job?
Regards,
Tvrtko
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [BrownBag] drm/i915/gtt: Neuter the deferred unbind callback from gen6_ppgtt_cleanup
2019-05-24 8:23 ` Tvrtko Ursulin
@ 2019-05-24 8:29 ` Chris Wilson
2019-05-24 8:31 ` Tvrtko Ursulin
0 siblings, 1 reply; 13+ messages in thread
From: Chris Wilson @ 2019-05-24 8:29 UTC (permalink / raw)
To: Tvrtko Ursulin, intel-gfx; +Cc: Tomi Sarvela
Quoting Tvrtko Ursulin (2019-05-24 09:23:40)
>
> On 24/05/2019 09:17, Chris Wilson wrote:
> > Quoting Tvrtko Ursulin (2019-05-24 09:13:14)
> >>
> >> On 24/05/2019 07:45, Chris Wilson wrote:
> >>> Having deferred the vma destruction to a worker where we can acquire the
> >>> struct_mutex, we have to avoid chasing back into the now destroyed
> >>> ppgtt. The pd_vma is special in having a custom unbind function to scan
> >>> for unused pages despite the VMA itself being notionally part of the
> >>> GGTT. As such, we need to disable that callback to avoid a
> >>> use-after-free.
> >>>
> >>> This unfortunately blew up so early during boot that CI declared the
> >>> machine unreachable as opposed to being the major failure it was. Oops.
> >>>
> >>> Fixes: d3622099c76f ("drm/i915/gtt: Always acquire struct_mutex for gen6_ppgtt_cleanup")
> >>> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> >>> Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> >>> Cc: Tomi Sarvela <tomi.p.sarvela@intel.com>
> >>> ---
> >>> drivers/gpu/drm/i915/i915_gem_gtt.c | 28 ++++++++++++++++++++++++++++
> >>> 1 file changed, 28 insertions(+)
> >>>
> >>> diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c
> >>> index 8d8a4b0ad4d9..266baa11df64 100644
> >>> --- a/drivers/gpu/drm/i915/i915_gem_gtt.c
> >>> +++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
> >>> @@ -1847,6 +1847,33 @@ static void gen6_ppgtt_cleanup_work(struct work_struct *wrk)
> >>> kfree(work);
> >>> }
> >>>
> >>> +static int nop_set_pages(struct i915_vma *vma)
> >>> +{
> >>> + return -ENODEV;
> >>> +}
> >>> +
> >>> +static void nop_clear_pages(struct i915_vma *vma)
> >>> +{
> >>> +}
> >>> +
> >>> +static int nop_bind(struct i915_vma *vma,
> >>> + enum i915_cache_level cache_level,
> >>> + u32 unused)
> >>> +{
> >>> + return -ENODEV;
> >>> +}
> >>> +
> >>> +static void nop_unbind(struct i915_vma *vma)
> >>> +{
> >>> +}
> >>> +
> >>> +static const struct i915_vma_ops nop_vma_ops = {
> >>> + .set_pages = nop_set_pages,
> >>> + .clear_pages = nop_clear_pages,
> >>> + .bind_vma = nop_bind,
> >>> + .unbind_vma = nop_unbind,
> >>> +};
> >>> +
> >>> static void gen6_ppgtt_cleanup(struct i915_address_space *vm)
> >>> {
> >>> struct gen6_hw_ppgtt *ppgtt = to_gen6_ppgtt(i915_vm_to_ppgtt(vm));
> >>> @@ -1855,6 +1882,7 @@ static void gen6_ppgtt_cleanup(struct i915_address_space *vm)
> >>> /* FIXME remove the struct_mutex to bring the locking under control */
> >>> INIT_WORK(&work->base, gen6_ppgtt_cleanup_work);
> >>> work->vma = ppgtt->vma;
> >>> + work->vma->ops = &nop_vma_ops;
> >>
> >> Could we use some asserts before overriding the vma ops? Like
> >> GEM_BUG_ON(vma->pages)? And something for still bound?
> >
> > It technically still is bound as it is in the GGTT but currently
> > unpinned -- that will be checked on destroy, it's just we also get an
> > unbind callback. vma->pages doesn't exist for this (set to ERR_PTR).
>
> If we are getting the unbind callback and we nop-ed it, who will
> actually do it's job?
The callback is just a hook for us to prune within the ppgtt.
It still is removed from GGTT by i915_vma_unbind().
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [BrownBag] drm/i915/gtt: Neuter the deferred unbind callback from gen6_ppgtt_cleanup
2019-05-24 8:29 ` Chris Wilson
@ 2019-05-24 8:31 ` Tvrtko Ursulin
2019-05-24 8:36 ` Chris Wilson
0 siblings, 1 reply; 13+ messages in thread
From: Tvrtko Ursulin @ 2019-05-24 8:31 UTC (permalink / raw)
To: Chris Wilson, intel-gfx; +Cc: Tomi Sarvela
On 24/05/2019 09:29, Chris Wilson wrote:
> Quoting Tvrtko Ursulin (2019-05-24 09:23:40)
>>
>> On 24/05/2019 09:17, Chris Wilson wrote:
>>> Quoting Tvrtko Ursulin (2019-05-24 09:13:14)
>>>>
>>>> On 24/05/2019 07:45, Chris Wilson wrote:
>>>>> Having deferred the vma destruction to a worker where we can acquire the
>>>>> struct_mutex, we have to avoid chasing back into the now destroyed
>>>>> ppgtt. The pd_vma is special in having a custom unbind function to scan
>>>>> for unused pages despite the VMA itself being notionally part of the
>>>>> GGTT. As such, we need to disable that callback to avoid a
>>>>> use-after-free.
>>>>>
>>>>> This unfortunately blew up so early during boot that CI declared the
>>>>> machine unreachable as opposed to being the major failure it was. Oops.
>>>>>
>>>>> Fixes: d3622099c76f ("drm/i915/gtt: Always acquire struct_mutex for gen6_ppgtt_cleanup")
>>>>> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
>>>>> Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
>>>>> Cc: Tomi Sarvela <tomi.p.sarvela@intel.com>
>>>>> ---
>>>>> drivers/gpu/drm/i915/i915_gem_gtt.c | 28 ++++++++++++++++++++++++++++
>>>>> 1 file changed, 28 insertions(+)
>>>>>
>>>>> diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c
>>>>> index 8d8a4b0ad4d9..266baa11df64 100644
>>>>> --- a/drivers/gpu/drm/i915/i915_gem_gtt.c
>>>>> +++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
>>>>> @@ -1847,6 +1847,33 @@ static void gen6_ppgtt_cleanup_work(struct work_struct *wrk)
>>>>> kfree(work);
>>>>> }
>>>>>
>>>>> +static int nop_set_pages(struct i915_vma *vma)
>>>>> +{
>>>>> + return -ENODEV;
>>>>> +}
>>>>> +
>>>>> +static void nop_clear_pages(struct i915_vma *vma)
>>>>> +{
>>>>> +}
>>>>> +
>>>>> +static int nop_bind(struct i915_vma *vma,
>>>>> + enum i915_cache_level cache_level,
>>>>> + u32 unused)
>>>>> +{
>>>>> + return -ENODEV;
>>>>> +}
>>>>> +
>>>>> +static void nop_unbind(struct i915_vma *vma)
>>>>> +{
>>>>> +}
>>>>> +
>>>>> +static const struct i915_vma_ops nop_vma_ops = {
>>>>> + .set_pages = nop_set_pages,
>>>>> + .clear_pages = nop_clear_pages,
>>>>> + .bind_vma = nop_bind,
>>>>> + .unbind_vma = nop_unbind,
>>>>> +};
>>>>> +
>>>>> static void gen6_ppgtt_cleanup(struct i915_address_space *vm)
>>>>> {
>>>>> struct gen6_hw_ppgtt *ppgtt = to_gen6_ppgtt(i915_vm_to_ppgtt(vm));
>>>>> @@ -1855,6 +1882,7 @@ static void gen6_ppgtt_cleanup(struct i915_address_space *vm)
>>>>> /* FIXME remove the struct_mutex to bring the locking under control */
>>>>> INIT_WORK(&work->base, gen6_ppgtt_cleanup_work);
>>>>> work->vma = ppgtt->vma;
>>>>> + work->vma->ops = &nop_vma_ops;
>>>>
>>>> Could we use some asserts before overriding the vma ops? Like
>>>> GEM_BUG_ON(vma->pages)? And something for still bound?
>>>
>>> It technically still is bound as it is in the GGTT but currently
>>> unpinned -- that will be checked on destroy, it's just we also get an
>>> unbind callback. vma->pages doesn't exist for this (set to ERR_PTR).
>>
>> If we are getting the unbind callback and we nop-ed it, who will
>> actually do it's job?
>
> The callback is just a hook for us to prune within the ppgtt.
> It still is removed from GGTT by i915_vma_unbind().
So it needs GEM_BUG_ON(ppgtt->scan_for_unused_pt) before overriding the
unbind?
Regards,
Tvrtko
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [BrownBag] drm/i915/gtt: Neuter the deferred unbind callback from gen6_ppgtt_cleanup
2019-05-24 8:31 ` Tvrtko Ursulin
@ 2019-05-24 8:36 ` Chris Wilson
2019-05-24 8:51 ` Tvrtko Ursulin
0 siblings, 1 reply; 13+ messages in thread
From: Chris Wilson @ 2019-05-24 8:36 UTC (permalink / raw)
To: Tvrtko Ursulin, intel-gfx; +Cc: Tomi Sarvela
Quoting Tvrtko Ursulin (2019-05-24 09:31:45)
>
> On 24/05/2019 09:29, Chris Wilson wrote:
> > Quoting Tvrtko Ursulin (2019-05-24 09:23:40)
> >>
> >> On 24/05/2019 09:17, Chris Wilson wrote:
> >>> Quoting Tvrtko Ursulin (2019-05-24 09:13:14)
> >>>>
> >>>> On 24/05/2019 07:45, Chris Wilson wrote:
> >>>>> Having deferred the vma destruction to a worker where we can acquire the
> >>>>> struct_mutex, we have to avoid chasing back into the now destroyed
> >>>>> ppgtt. The pd_vma is special in having a custom unbind function to scan
> >>>>> for unused pages despite the VMA itself being notionally part of the
> >>>>> GGTT. As such, we need to disable that callback to avoid a
> >>>>> use-after-free.
> >>>>>
> >>>>> This unfortunately blew up so early during boot that CI declared the
> >>>>> machine unreachable as opposed to being the major failure it was. Oops.
> >>>>>
> >>>>> Fixes: d3622099c76f ("drm/i915/gtt: Always acquire struct_mutex for gen6_ppgtt_cleanup")
> >>>>> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> >>>>> Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> >>>>> Cc: Tomi Sarvela <tomi.p.sarvela@intel.com>
> >>>>> ---
> >>>>> drivers/gpu/drm/i915/i915_gem_gtt.c | 28 ++++++++++++++++++++++++++++
> >>>>> 1 file changed, 28 insertions(+)
> >>>>>
> >>>>> diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c
> >>>>> index 8d8a4b0ad4d9..266baa11df64 100644
> >>>>> --- a/drivers/gpu/drm/i915/i915_gem_gtt.c
> >>>>> +++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
> >>>>> @@ -1847,6 +1847,33 @@ static void gen6_ppgtt_cleanup_work(struct work_struct *wrk)
> >>>>> kfree(work);
> >>>>> }
> >>>>>
> >>>>> +static int nop_set_pages(struct i915_vma *vma)
> >>>>> +{
> >>>>> + return -ENODEV;
> >>>>> +}
> >>>>> +
> >>>>> +static void nop_clear_pages(struct i915_vma *vma)
> >>>>> +{
> >>>>> +}
> >>>>> +
> >>>>> +static int nop_bind(struct i915_vma *vma,
> >>>>> + enum i915_cache_level cache_level,
> >>>>> + u32 unused)
> >>>>> +{
> >>>>> + return -ENODEV;
> >>>>> +}
> >>>>> +
> >>>>> +static void nop_unbind(struct i915_vma *vma)
> >>>>> +{
> >>>>> +}
> >>>>> +
> >>>>> +static const struct i915_vma_ops nop_vma_ops = {
> >>>>> + .set_pages = nop_set_pages,
> >>>>> + .clear_pages = nop_clear_pages,
> >>>>> + .bind_vma = nop_bind,
> >>>>> + .unbind_vma = nop_unbind,
> >>>>> +};
> >>>>> +
> >>>>> static void gen6_ppgtt_cleanup(struct i915_address_space *vm)
> >>>>> {
> >>>>> struct gen6_hw_ppgtt *ppgtt = to_gen6_ppgtt(i915_vm_to_ppgtt(vm));
> >>>>> @@ -1855,6 +1882,7 @@ static void gen6_ppgtt_cleanup(struct i915_address_space *vm)
> >>>>> /* FIXME remove the struct_mutex to bring the locking under control */
> >>>>> INIT_WORK(&work->base, gen6_ppgtt_cleanup_work);
> >>>>> work->vma = ppgtt->vma;
> >>>>> + work->vma->ops = &nop_vma_ops;
> >>>>
> >>>> Could we use some asserts before overriding the vma ops? Like
> >>>> GEM_BUG_ON(vma->pages)? And something for still bound?
> >>>
> >>> It technically still is bound as it is in the GGTT but currently
> >>> unpinned -- that will be checked on destroy, it's just we also get an
> >>> unbind callback. vma->pages doesn't exist for this (set to ERR_PTR).
> >>
> >> If we are getting the unbind callback and we nop-ed it, who will
> >> actually do it's job?
> >
> > The callback is just a hook for us to prune within the ppgtt.
> > It still is removed from GGTT by i915_vma_unbind().
>
> So it needs GEM_BUG_ON(ppgtt->scan_for_unused_pt) before overriding the
> unbind?
No. They get freed by the cleanup itself. The scan is just an
opportunistic prune if either the context/mm is evicted but still alive.
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [BrownBag] drm/i915/gtt: Neuter the deferred unbind callback from gen6_ppgtt_cleanup
2019-05-24 8:36 ` Chris Wilson
@ 2019-05-24 8:51 ` Tvrtko Ursulin
2019-05-24 8:55 ` Chris Wilson
2019-05-24 8:57 ` Tvrtko Ursulin
0 siblings, 2 replies; 13+ messages in thread
From: Tvrtko Ursulin @ 2019-05-24 8:51 UTC (permalink / raw)
To: Chris Wilson, intel-gfx; +Cc: Tomi Sarvela
On 24/05/2019 09:36, Chris Wilson wrote:
> Quoting Tvrtko Ursulin (2019-05-24 09:31:45)
>>
>> On 24/05/2019 09:29, Chris Wilson wrote:
>>> Quoting Tvrtko Ursulin (2019-05-24 09:23:40)
>>>>
>>>> On 24/05/2019 09:17, Chris Wilson wrote:
>>>>> Quoting Tvrtko Ursulin (2019-05-24 09:13:14)
>>>>>>
>>>>>> On 24/05/2019 07:45, Chris Wilson wrote:
>>>>>>> Having deferred the vma destruction to a worker where we can acquire the
>>>>>>> struct_mutex, we have to avoid chasing back into the now destroyed
>>>>>>> ppgtt. The pd_vma is special in having a custom unbind function to scan
>>>>>>> for unused pages despite the VMA itself being notionally part of the
>>>>>>> GGTT. As such, we need to disable that callback to avoid a
>>>>>>> use-after-free.
>>>>>>>
>>>>>>> This unfortunately blew up so early during boot that CI declared the
>>>>>>> machine unreachable as opposed to being the major failure it was. Oops.
>>>>>>>
>>>>>>> Fixes: d3622099c76f ("drm/i915/gtt: Always acquire struct_mutex for gen6_ppgtt_cleanup")
>>>>>>> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
>>>>>>> Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
>>>>>>> Cc: Tomi Sarvela <tomi.p.sarvela@intel.com>
>>>>>>> ---
>>>>>>> drivers/gpu/drm/i915/i915_gem_gtt.c | 28 ++++++++++++++++++++++++++++
>>>>>>> 1 file changed, 28 insertions(+)
>>>>>>>
>>>>>>> diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c
>>>>>>> index 8d8a4b0ad4d9..266baa11df64 100644
>>>>>>> --- a/drivers/gpu/drm/i915/i915_gem_gtt.c
>>>>>>> +++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
>>>>>>> @@ -1847,6 +1847,33 @@ static void gen6_ppgtt_cleanup_work(struct work_struct *wrk)
>>>>>>> kfree(work);
>>>>>>> }
>>>>>>>
>>>>>>> +static int nop_set_pages(struct i915_vma *vma)
>>>>>>> +{
>>>>>>> + return -ENODEV;
>>>>>>> +}
>>>>>>> +
>>>>>>> +static void nop_clear_pages(struct i915_vma *vma)
>>>>>>> +{
>>>>>>> +}
>>>>>>> +
>>>>>>> +static int nop_bind(struct i915_vma *vma,
>>>>>>> + enum i915_cache_level cache_level,
>>>>>>> + u32 unused)
>>>>>>> +{
>>>>>>> + return -ENODEV;
>>>>>>> +}
>>>>>>> +
>>>>>>> +static void nop_unbind(struct i915_vma *vma)
>>>>>>> +{
>>>>>>> +}
>>>>>>> +
>>>>>>> +static const struct i915_vma_ops nop_vma_ops = {
>>>>>>> + .set_pages = nop_set_pages,
>>>>>>> + .clear_pages = nop_clear_pages,
>>>>>>> + .bind_vma = nop_bind,
>>>>>>> + .unbind_vma = nop_unbind,
>>>>>>> +};
>>>>>>> +
>>>>>>> static void gen6_ppgtt_cleanup(struct i915_address_space *vm)
>>>>>>> {
>>>>>>> struct gen6_hw_ppgtt *ppgtt = to_gen6_ppgtt(i915_vm_to_ppgtt(vm));
>>>>>>> @@ -1855,6 +1882,7 @@ static void gen6_ppgtt_cleanup(struct i915_address_space *vm)
>>>>>>> /* FIXME remove the struct_mutex to bring the locking under control */
>>>>>>> INIT_WORK(&work->base, gen6_ppgtt_cleanup_work);
>>>>>>> work->vma = ppgtt->vma;
>>>>>>> + work->vma->ops = &nop_vma_ops;
>>>>>>
>>>>>> Could we use some asserts before overriding the vma ops? Like
>>>>>> GEM_BUG_ON(vma->pages)? And something for still bound?
>>>>>
>>>>> It technically still is bound as it is in the GGTT but currently
>>>>> unpinned -- that will be checked on destroy, it's just we also get an
>>>>> unbind callback. vma->pages doesn't exist for this (set to ERR_PTR).
>>>>
>>>> If we are getting the unbind callback and we nop-ed it, who will
>>>> actually do it's job?
>>>
>>> The callback is just a hook for us to prune within the ppgtt.
>>> It still is removed from GGTT by i915_vma_unbind().
>>
>> So it needs GEM_BUG_ON(ppgtt->scan_for_unused_pt) before overriding the
>> unbind?
>
> No. They get freed by the cleanup itself. The scan is just an
> opportunistic prune if either the context/mm is evicted but still alive.
Then the same assert in gen6_ppgtt_cleanup_work? :)
Regards,
Tvrtko
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [BrownBag] drm/i915/gtt: Neuter the deferred unbind callback from gen6_ppgtt_cleanup
2019-05-24 8:51 ` Tvrtko Ursulin
@ 2019-05-24 8:55 ` Chris Wilson
2019-05-24 8:57 ` Tvrtko Ursulin
1 sibling, 0 replies; 13+ messages in thread
From: Chris Wilson @ 2019-05-24 8:55 UTC (permalink / raw)
To: Tvrtko Ursulin, intel-gfx; +Cc: Tomi Sarvela
Quoting Tvrtko Ursulin (2019-05-24 09:51:46)
>
> On 24/05/2019 09:36, Chris Wilson wrote:
> > Quoting Tvrtko Ursulin (2019-05-24 09:31:45)
> >>
> >> On 24/05/2019 09:29, Chris Wilson wrote:
> >>> Quoting Tvrtko Ursulin (2019-05-24 09:23:40)
> >>>>
> >>>> On 24/05/2019 09:17, Chris Wilson wrote:
> >>>>> Quoting Tvrtko Ursulin (2019-05-24 09:13:14)
> >>>>>>
> >>>>>> On 24/05/2019 07:45, Chris Wilson wrote:
> >>>>>>> Having deferred the vma destruction to a worker where we can acquire the
> >>>>>>> struct_mutex, we have to avoid chasing back into the now destroyed
> >>>>>>> ppgtt. The pd_vma is special in having a custom unbind function to scan
> >>>>>>> for unused pages despite the VMA itself being notionally part of the
> >>>>>>> GGTT. As such, we need to disable that callback to avoid a
> >>>>>>> use-after-free.
> >>>>>>>
> >>>>>>> This unfortunately blew up so early during boot that CI declared the
> >>>>>>> machine unreachable as opposed to being the major failure it was. Oops.
> >>>>>>>
> >>>>>>> Fixes: d3622099c76f ("drm/i915/gtt: Always acquire struct_mutex for gen6_ppgtt_cleanup")
> >>>>>>> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> >>>>>>> Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> >>>>>>> Cc: Tomi Sarvela <tomi.p.sarvela@intel.com>
> >>>>>>> ---
> >>>>>>> drivers/gpu/drm/i915/i915_gem_gtt.c | 28 ++++++++++++++++++++++++++++
> >>>>>>> 1 file changed, 28 insertions(+)
> >>>>>>>
> >>>>>>> diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c
> >>>>>>> index 8d8a4b0ad4d9..266baa11df64 100644
> >>>>>>> --- a/drivers/gpu/drm/i915/i915_gem_gtt.c
> >>>>>>> +++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
> >>>>>>> @@ -1847,6 +1847,33 @@ static void gen6_ppgtt_cleanup_work(struct work_struct *wrk)
> >>>>>>> kfree(work);
> >>>>>>> }
> >>>>>>>
> >>>>>>> +static int nop_set_pages(struct i915_vma *vma)
> >>>>>>> +{
> >>>>>>> + return -ENODEV;
> >>>>>>> +}
> >>>>>>> +
> >>>>>>> +static void nop_clear_pages(struct i915_vma *vma)
> >>>>>>> +{
> >>>>>>> +}
> >>>>>>> +
> >>>>>>> +static int nop_bind(struct i915_vma *vma,
> >>>>>>> + enum i915_cache_level cache_level,
> >>>>>>> + u32 unused)
> >>>>>>> +{
> >>>>>>> + return -ENODEV;
> >>>>>>> +}
> >>>>>>> +
> >>>>>>> +static void nop_unbind(struct i915_vma *vma)
> >>>>>>> +{
> >>>>>>> +}
> >>>>>>> +
> >>>>>>> +static const struct i915_vma_ops nop_vma_ops = {
> >>>>>>> + .set_pages = nop_set_pages,
> >>>>>>> + .clear_pages = nop_clear_pages,
> >>>>>>> + .bind_vma = nop_bind,
> >>>>>>> + .unbind_vma = nop_unbind,
> >>>>>>> +};
> >>>>>>> +
> >>>>>>> static void gen6_ppgtt_cleanup(struct i915_address_space *vm)
> >>>>>>> {
> >>>>>>> struct gen6_hw_ppgtt *ppgtt = to_gen6_ppgtt(i915_vm_to_ppgtt(vm));
> >>>>>>> @@ -1855,6 +1882,7 @@ static void gen6_ppgtt_cleanup(struct i915_address_space *vm)
> >>>>>>> /* FIXME remove the struct_mutex to bring the locking under control */
> >>>>>>> INIT_WORK(&work->base, gen6_ppgtt_cleanup_work);
> >>>>>>> work->vma = ppgtt->vma;
> >>>>>>> + work->vma->ops = &nop_vma_ops;
> >>>>>>
> >>>>>> Could we use some asserts before overriding the vma ops? Like
> >>>>>> GEM_BUG_ON(vma->pages)? And something for still bound?
> >>>>>
> >>>>> It technically still is bound as it is in the GGTT but currently
> >>>>> unpinned -- that will be checked on destroy, it's just we also get an
> >>>>> unbind callback. vma->pages doesn't exist for this (set to ERR_PTR).
> >>>>
> >>>> If we are getting the unbind callback and we nop-ed it, who will
> >>>> actually do it's job?
> >>>
> >>> The callback is just a hook for us to prune within the ppgtt.
> >>> It still is removed from GGTT by i915_vma_unbind().
> >>
> >> So it needs GEM_BUG_ON(ppgtt->scan_for_unused_pt) before overriding the
> >> unbind?
> >
> > No. They get freed by the cleanup itself. The scan is just an
> > opportunistic prune if either the context/mm is evicted but still alive.
>
> Then the same assert in gen6_ppgtt_cleanup_work? :)
ppgtt is dead.
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [BrownBag] drm/i915/gtt: Neuter the deferred unbind callback from gen6_ppgtt_cleanup
2019-05-24 8:51 ` Tvrtko Ursulin
2019-05-24 8:55 ` Chris Wilson
@ 2019-05-24 8:57 ` Tvrtko Ursulin
2019-05-24 9:01 ` Chris Wilson
1 sibling, 1 reply; 13+ messages in thread
From: Tvrtko Ursulin @ 2019-05-24 8:57 UTC (permalink / raw)
To: Chris Wilson, intel-gfx; +Cc: Tomi Sarvela
On 24/05/2019 09:51, Tvrtko Ursulin wrote:
>
> On 24/05/2019 09:36, Chris Wilson wrote:
>> Quoting Tvrtko Ursulin (2019-05-24 09:31:45)
>>>
>>> On 24/05/2019 09:29, Chris Wilson wrote:
>>>> Quoting Tvrtko Ursulin (2019-05-24 09:23:40)
>>>>>
>>>>> On 24/05/2019 09:17, Chris Wilson wrote:
>>>>>> Quoting Tvrtko Ursulin (2019-05-24 09:13:14)
>>>>>>>
>>>>>>> On 24/05/2019 07:45, Chris Wilson wrote:
>>>>>>>> Having deferred the vma destruction to a worker where we can
>>>>>>>> acquire the
>>>>>>>> struct_mutex, we have to avoid chasing back into the now destroyed
>>>>>>>> ppgtt. The pd_vma is special in having a custom unbind function
>>>>>>>> to scan
>>>>>>>> for unused pages despite the VMA itself being notionally part of
>>>>>>>> the
>>>>>>>> GGTT. As such, we need to disable that callback to avoid a
>>>>>>>> use-after-free.
>>>>>>>>
>>>>>>>> This unfortunately blew up so early during boot that CI declared
>>>>>>>> the
>>>>>>>> machine unreachable as opposed to being the major failure it
>>>>>>>> was. Oops.
>>>>>>>>
>>>>>>>> Fixes: d3622099c76f ("drm/i915/gtt: Always acquire struct_mutex
>>>>>>>> for gen6_ppgtt_cleanup")
>>>>>>>> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
>>>>>>>> Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
>>>>>>>> Cc: Tomi Sarvela <tomi.p.sarvela@intel.com>
>>>>>>>> ---
>>>>>>>> drivers/gpu/drm/i915/i915_gem_gtt.c | 28
>>>>>>>> ++++++++++++++++++++++++++++
>>>>>>>> 1 file changed, 28 insertions(+)
>>>>>>>>
>>>>>>>> diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c
>>>>>>>> b/drivers/gpu/drm/i915/i915_gem_gtt.c
>>>>>>>> index 8d8a4b0ad4d9..266baa11df64 100644
>>>>>>>> --- a/drivers/gpu/drm/i915/i915_gem_gtt.c
>>>>>>>> +++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
>>>>>>>> @@ -1847,6 +1847,33 @@ static void
>>>>>>>> gen6_ppgtt_cleanup_work(struct work_struct *wrk)
>>>>>>>> kfree(work);
>>>>>>>> }
>>>>>>>> +static int nop_set_pages(struct i915_vma *vma)
>>>>>>>> +{
>>>>>>>> + return -ENODEV;
>>>>>>>> +}
>>>>>>>> +
>>>>>>>> +static void nop_clear_pages(struct i915_vma *vma)
>>>>>>>> +{
>>>>>>>> +}
>>>>>>>> +
>>>>>>>> +static int nop_bind(struct i915_vma *vma,
>>>>>>>> + enum i915_cache_level cache_level,
>>>>>>>> + u32 unused)
>>>>>>>> +{
>>>>>>>> + return -ENODEV;
>>>>>>>> +}
>>>>>>>> +
>>>>>>>> +static void nop_unbind(struct i915_vma *vma)
>>>>>>>> +{
>>>>>>>> +}
>>>>>>>> +
>>>>>>>> +static const struct i915_vma_ops nop_vma_ops = {
>>>>>>>> + .set_pages = nop_set_pages,
>>>>>>>> + .clear_pages = nop_clear_pages,
>>>>>>>> + .bind_vma = nop_bind,
>>>>>>>> + .unbind_vma = nop_unbind,
>>>>>>>> +};
>>>>>>>> +
>>>>>>>> static void gen6_ppgtt_cleanup(struct i915_address_space *vm)
>>>>>>>> {
>>>>>>>> struct gen6_hw_ppgtt *ppgtt =
>>>>>>>> to_gen6_ppgtt(i915_vm_to_ppgtt(vm));
>>>>>>>> @@ -1855,6 +1882,7 @@ static void gen6_ppgtt_cleanup(struct
>>>>>>>> i915_address_space *vm)
>>>>>>>> /* FIXME remove the struct_mutex to bring the locking
>>>>>>>> under control */
>>>>>>>> INIT_WORK(&work->base, gen6_ppgtt_cleanup_work);
>>>>>>>> work->vma = ppgtt->vma;
>>>>>>>> + work->vma->ops = &nop_vma_ops;
>>>>>>>
>>>>>>> Could we use some asserts before overriding the vma ops? Like
>>>>>>> GEM_BUG_ON(vma->pages)? And something for still bound?
>>>>>>
>>>>>> It technically still is bound as it is in the GGTT but currently
>>>>>> unpinned -- that will be checked on destroy, it's just we also get an
>>>>>> unbind callback. vma->pages doesn't exist for this (set to ERR_PTR).
>>>>>
>>>>> If we are getting the unbind callback and we nop-ed it, who will
>>>>> actually do it's job?
>>>>
>>>> The callback is just a hook for us to prune within the ppgtt.
>>>> It still is removed from GGTT by i915_vma_unbind().
>>>
>>> So it needs GEM_BUG_ON(ppgtt->scan_for_unused_pt) before overriding the
>>> unbind?
>>
>> No. They get freed by the cleanup itself. The scan is just an
>> opportunistic prune if either the context/mm is evicted but still alive.
>
> Then the same assert in gen6_ppgtt_cleanup_work? :)
Okay ppgtt is gone so can't do it.. annoying.. Cleanup seems to support
your claims but I think we need a BFC (big fat comment) above the vma
ops override to explains this. With that:
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] 13+ messages in thread
* Re: [BrownBag] drm/i915/gtt: Neuter the deferred unbind callback from gen6_ppgtt_cleanup
2019-05-24 8:57 ` Tvrtko Ursulin
@ 2019-05-24 9:01 ` Chris Wilson
2019-05-24 9:08 ` Tvrtko Ursulin
0 siblings, 1 reply; 13+ messages in thread
From: Chris Wilson @ 2019-05-24 9:01 UTC (permalink / raw)
To: Tvrtko Ursulin, intel-gfx; +Cc: Tomi Sarvela
Quoting Tvrtko Ursulin (2019-05-24 09:57:42)
>
> On 24/05/2019 09:51, Tvrtko Ursulin wrote:
> >
> > On 24/05/2019 09:36, Chris Wilson wrote:
> >> Quoting Tvrtko Ursulin (2019-05-24 09:31:45)
> >>>
> >>> On 24/05/2019 09:29, Chris Wilson wrote:
> >>>> Quoting Tvrtko Ursulin (2019-05-24 09:23:40)
> >>>>>
> >>>>> On 24/05/2019 09:17, Chris Wilson wrote:
> >>>>>> Quoting Tvrtko Ursulin (2019-05-24 09:13:14)
> >>>>>>>
> >>>>>>> On 24/05/2019 07:45, Chris Wilson wrote:
> >>>>>>>> Having deferred the vma destruction to a worker where we can
> >>>>>>>> acquire the
> >>>>>>>> struct_mutex, we have to avoid chasing back into the now destroyed
> >>>>>>>> ppgtt. The pd_vma is special in having a custom unbind function
> >>>>>>>> to scan
> >>>>>>>> for unused pages despite the VMA itself being notionally part of
> >>>>>>>> the
> >>>>>>>> GGTT. As such, we need to disable that callback to avoid a
> >>>>>>>> use-after-free.
> >>>>>>>>
> >>>>>>>> This unfortunately blew up so early during boot that CI declared
> >>>>>>>> the
> >>>>>>>> machine unreachable as opposed to being the major failure it
> >>>>>>>> was. Oops.
> >>>>>>>>
> >>>>>>>> Fixes: d3622099c76f ("drm/i915/gtt: Always acquire struct_mutex
> >>>>>>>> for gen6_ppgtt_cleanup")
> >>>>>>>> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> >>>>>>>> Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> >>>>>>>> Cc: Tomi Sarvela <tomi.p.sarvela@intel.com>
> >>>>>>>> ---
> >>>>>>>> drivers/gpu/drm/i915/i915_gem_gtt.c | 28
> >>>>>>>> ++++++++++++++++++++++++++++
> >>>>>>>> 1 file changed, 28 insertions(+)
> >>>>>>>>
> >>>>>>>> diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c
> >>>>>>>> b/drivers/gpu/drm/i915/i915_gem_gtt.c
> >>>>>>>> index 8d8a4b0ad4d9..266baa11df64 100644
> >>>>>>>> --- a/drivers/gpu/drm/i915/i915_gem_gtt.c
> >>>>>>>> +++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
> >>>>>>>> @@ -1847,6 +1847,33 @@ static void
> >>>>>>>> gen6_ppgtt_cleanup_work(struct work_struct *wrk)
> >>>>>>>> kfree(work);
> >>>>>>>> }
> >>>>>>>> +static int nop_set_pages(struct i915_vma *vma)
> >>>>>>>> +{
> >>>>>>>> + return -ENODEV;
> >>>>>>>> +}
> >>>>>>>> +
> >>>>>>>> +static void nop_clear_pages(struct i915_vma *vma)
> >>>>>>>> +{
> >>>>>>>> +}
> >>>>>>>> +
> >>>>>>>> +static int nop_bind(struct i915_vma *vma,
> >>>>>>>> + enum i915_cache_level cache_level,
> >>>>>>>> + u32 unused)
> >>>>>>>> +{
> >>>>>>>> + return -ENODEV;
> >>>>>>>> +}
> >>>>>>>> +
> >>>>>>>> +static void nop_unbind(struct i915_vma *vma)
> >>>>>>>> +{
> >>>>>>>> +}
> >>>>>>>> +
> >>>>>>>> +static const struct i915_vma_ops nop_vma_ops = {
> >>>>>>>> + .set_pages = nop_set_pages,
> >>>>>>>> + .clear_pages = nop_clear_pages,
> >>>>>>>> + .bind_vma = nop_bind,
> >>>>>>>> + .unbind_vma = nop_unbind,
> >>>>>>>> +};
> >>>>>>>> +
> >>>>>>>> static void gen6_ppgtt_cleanup(struct i915_address_space *vm)
> >>>>>>>> {
> >>>>>>>> struct gen6_hw_ppgtt *ppgtt =
> >>>>>>>> to_gen6_ppgtt(i915_vm_to_ppgtt(vm));
> >>>>>>>> @@ -1855,6 +1882,7 @@ static void gen6_ppgtt_cleanup(struct
> >>>>>>>> i915_address_space *vm)
> >>>>>>>> /* FIXME remove the struct_mutex to bring the locking
> >>>>>>>> under control */
> >>>>>>>> INIT_WORK(&work->base, gen6_ppgtt_cleanup_work);
> >>>>>>>> work->vma = ppgtt->vma;
> >>>>>>>> + work->vma->ops = &nop_vma_ops;
> >>>>>>>
> >>>>>>> Could we use some asserts before overriding the vma ops? Like
> >>>>>>> GEM_BUG_ON(vma->pages)? And something for still bound?
> >>>>>>
> >>>>>> It technically still is bound as it is in the GGTT but currently
> >>>>>> unpinned -- that will be checked on destroy, it's just we also get an
> >>>>>> unbind callback. vma->pages doesn't exist for this (set to ERR_PTR).
> >>>>>
> >>>>> If we are getting the unbind callback and we nop-ed it, who will
> >>>>> actually do it's job?
> >>>>
> >>>> The callback is just a hook for us to prune within the ppgtt.
> >>>> It still is removed from GGTT by i915_vma_unbind().
> >>>
> >>> So it needs GEM_BUG_ON(ppgtt->scan_for_unused_pt) before overriding the
> >>> unbind?
> >>
> >> No. They get freed by the cleanup itself. The scan is just an
> >> opportunistic prune if either the context/mm is evicted but still alive.
> >
> > Then the same assert in gen6_ppgtt_cleanup_work? :)
>
> Okay ppgtt is gone so can't do it.. annoying.. Cleanup seems to support
> your claims but I think we need a BFC (big fat comment) above the vma
> ops override to explains this. With that:
It has FIXME! I really do hope this is short term...
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [BrownBag] drm/i915/gtt: Neuter the deferred unbind callback from gen6_ppgtt_cleanup
2019-05-24 9:01 ` Chris Wilson
@ 2019-05-24 9:08 ` Tvrtko Ursulin
0 siblings, 0 replies; 13+ messages in thread
From: Tvrtko Ursulin @ 2019-05-24 9:08 UTC (permalink / raw)
To: Chris Wilson, intel-gfx; +Cc: Tomi Sarvela
On 24/05/2019 10:01, Chris Wilson wrote:
> Quoting Tvrtko Ursulin (2019-05-24 09:57:42)
>>
>> On 24/05/2019 09:51, Tvrtko Ursulin wrote:
>>>
>>> On 24/05/2019 09:36, Chris Wilson wrote:
>>>> Quoting Tvrtko Ursulin (2019-05-24 09:31:45)
>>>>>
>>>>> On 24/05/2019 09:29, Chris Wilson wrote:
>>>>>> Quoting Tvrtko Ursulin (2019-05-24 09:23:40)
>>>>>>>
>>>>>>> On 24/05/2019 09:17, Chris Wilson wrote:
>>>>>>>> Quoting Tvrtko Ursulin (2019-05-24 09:13:14)
>>>>>>>>>
>>>>>>>>> On 24/05/2019 07:45, Chris Wilson wrote:
>>>>>>>>>> Having deferred the vma destruction to a worker where we can
>>>>>>>>>> acquire the
>>>>>>>>>> struct_mutex, we have to avoid chasing back into the now destroyed
>>>>>>>>>> ppgtt. The pd_vma is special in having a custom unbind function
>>>>>>>>>> to scan
>>>>>>>>>> for unused pages despite the VMA itself being notionally part of
>>>>>>>>>> the
>>>>>>>>>> GGTT. As such, we need to disable that callback to avoid a
>>>>>>>>>> use-after-free.
>>>>>>>>>>
>>>>>>>>>> This unfortunately blew up so early during boot that CI declared
>>>>>>>>>> the
>>>>>>>>>> machine unreachable as opposed to being the major failure it
>>>>>>>>>> was. Oops.
>>>>>>>>>>
>>>>>>>>>> Fixes: d3622099c76f ("drm/i915/gtt: Always acquire struct_mutex
>>>>>>>>>> for gen6_ppgtt_cleanup")
>>>>>>>>>> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
>>>>>>>>>> Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
>>>>>>>>>> Cc: Tomi Sarvela <tomi.p.sarvela@intel.com>
>>>>>>>>>> ---
>>>>>>>>>> drivers/gpu/drm/i915/i915_gem_gtt.c | 28
>>>>>>>>>> ++++++++++++++++++++++++++++
>>>>>>>>>> 1 file changed, 28 insertions(+)
>>>>>>>>>>
>>>>>>>>>> diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c
>>>>>>>>>> b/drivers/gpu/drm/i915/i915_gem_gtt.c
>>>>>>>>>> index 8d8a4b0ad4d9..266baa11df64 100644
>>>>>>>>>> --- a/drivers/gpu/drm/i915/i915_gem_gtt.c
>>>>>>>>>> +++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
>>>>>>>>>> @@ -1847,6 +1847,33 @@ static void
>>>>>>>>>> gen6_ppgtt_cleanup_work(struct work_struct *wrk)
>>>>>>>>>> kfree(work);
>>>>>>>>>> }
>>>>>>>>>> +static int nop_set_pages(struct i915_vma *vma)
>>>>>>>>>> +{
>>>>>>>>>> + return -ENODEV;
>>>>>>>>>> +}
>>>>>>>>>> +
>>>>>>>>>> +static void nop_clear_pages(struct i915_vma *vma)
>>>>>>>>>> +{
>>>>>>>>>> +}
>>>>>>>>>> +
>>>>>>>>>> +static int nop_bind(struct i915_vma *vma,
>>>>>>>>>> + enum i915_cache_level cache_level,
>>>>>>>>>> + u32 unused)
>>>>>>>>>> +{
>>>>>>>>>> + return -ENODEV;
>>>>>>>>>> +}
>>>>>>>>>> +
>>>>>>>>>> +static void nop_unbind(struct i915_vma *vma)
>>>>>>>>>> +{
>>>>>>>>>> +}
>>>>>>>>>> +
>>>>>>>>>> +static const struct i915_vma_ops nop_vma_ops = {
>>>>>>>>>> + .set_pages = nop_set_pages,
>>>>>>>>>> + .clear_pages = nop_clear_pages,
>>>>>>>>>> + .bind_vma = nop_bind,
>>>>>>>>>> + .unbind_vma = nop_unbind,
>>>>>>>>>> +};
>>>>>>>>>> +
>>>>>>>>>> static void gen6_ppgtt_cleanup(struct i915_address_space *vm)
>>>>>>>>>> {
>>>>>>>>>> struct gen6_hw_ppgtt *ppgtt =
>>>>>>>>>> to_gen6_ppgtt(i915_vm_to_ppgtt(vm));
>>>>>>>>>> @@ -1855,6 +1882,7 @@ static void gen6_ppgtt_cleanup(struct
>>>>>>>>>> i915_address_space *vm)
>>>>>>>>>> /* FIXME remove the struct_mutex to bring the locking
>>>>>>>>>> under control */
>>>>>>>>>> INIT_WORK(&work->base, gen6_ppgtt_cleanup_work);
>>>>>>>>>> work->vma = ppgtt->vma;
>>>>>>>>>> + work->vma->ops = &nop_vma_ops;
>>>>>>>>>
>>>>>>>>> Could we use some asserts before overriding the vma ops? Like
>>>>>>>>> GEM_BUG_ON(vma->pages)? And something for still bound?
>>>>>>>>
>>>>>>>> It technically still is bound as it is in the GGTT but currently
>>>>>>>> unpinned -- that will be checked on destroy, it's just we also get an
>>>>>>>> unbind callback. vma->pages doesn't exist for this (set to ERR_PTR).
>>>>>>>
>>>>>>> If we are getting the unbind callback and we nop-ed it, who will
>>>>>>> actually do it's job?
>>>>>>
>>>>>> The callback is just a hook for us to prune within the ppgtt.
>>>>>> It still is removed from GGTT by i915_vma_unbind().
>>>>>
>>>>> So it needs GEM_BUG_ON(ppgtt->scan_for_unused_pt) before overriding the
>>>>> unbind?
>>>>
>>>> No. They get freed by the cleanup itself. The scan is just an
>>>> opportunistic prune if either the context/mm is evicted but still alive.
>>>
>>> Then the same assert in gen6_ppgtt_cleanup_work? :)
>>
>> Okay ppgtt is gone so can't do it.. annoying.. Cleanup seems to support
>> your claims but I think we need a BFC (big fat comment) above the vma
>> ops override to explains this. With that:
>
> It has FIXME! I really do hope this is short term...
Ok, that's good enough.
Regards,
Tvrtko
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 13+ messages in thread
* ✗ Fi.CI.BAT: failure for drm/i915/gtt: Neuter the deferred unbind callback from gen6_ppgtt_cleanup
2019-05-24 6:45 [BrownBag] drm/i915/gtt: Neuter the deferred unbind callback from gen6_ppgtt_cleanup Chris Wilson
2019-05-24 8:13 ` Tvrtko Ursulin
@ 2019-05-24 8:31 ` Patchwork
1 sibling, 0 replies; 13+ messages in thread
From: Patchwork @ 2019-05-24 8:31 UTC (permalink / raw)
To: Chris Wilson; +Cc: intel-gfx
== Series Details ==
Series: drm/i915/gtt: Neuter the deferred unbind callback from gen6_ppgtt_cleanup
URL : https://patchwork.freedesktop.org/series/61084/
State : failure
== Summary ==
CI Bug Log - changes from CI_DRM_6138 -> Patchwork_13087
====================================================
Summary
-------
**FAILURE**
Serious unknown changes coming with Patchwork_13087 absolutely need to be
verified manually.
If you think the reported changes have nothing to do with the changes
introduced in Patchwork_13087, 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_13087/
Possible new issues
-------------------
Here are the unknown changes that may have been introduced in Patchwork_13087:
### IGT changes ###
#### Possible regressions ####
* igt@i915_module_load@reload:
- fi-apl-guc: NOTRUN -> [DMESG-WARN][1]
[1]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13087/fi-apl-guc/igt@i915_module_load@reload.html
* igt@runner@aborted:
- fi-apl-guc: NOTRUN -> [FAIL][2]
[2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13087/fi-apl-guc/igt@runner@aborted.html
Known issues
------------
Here are the changes found in Patchwork_13087 that come from known issues:
### IGT changes ###
#### Issues hit ####
* igt@gem_exec_suspend@basic-s4-devices:
- fi-blb-e6850: [PASS][3] -> [INCOMPLETE][4] ([fdo#107718])
[3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6138/fi-blb-e6850/igt@gem_exec_suspend@basic-s4-devices.html
[4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13087/fi-blb-e6850/igt@gem_exec_suspend@basic-s4-devices.html
* igt@i915_pm_rpm@module-reload:
- fi-skl-6770hq: [PASS][5] -> [FAIL][6] ([fdo#108511])
[5]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6138/fi-skl-6770hq/igt@i915_pm_rpm@module-reload.html
[6]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13087/fi-skl-6770hq/igt@i915_pm_rpm@module-reload.html
* igt@kms_flip@basic-flip-vs-dpms:
- fi-bxt-dsi: [PASS][7] -> [INCOMPLETE][8] ([fdo#103927])
[7]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6138/fi-bxt-dsi/igt@kms_flip@basic-flip-vs-dpms.html
[8]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13087/fi-bxt-dsi/igt@kms_flip@basic-flip-vs-dpms.html
#### Possible fixes ####
* {igt@gem_busy@busy-all}:
- {fi-icl-y}: [INCOMPLETE][9] ([fdo#107713]) -> [PASS][10]
[9]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6138/fi-icl-y/igt@gem_busy@busy-all.html
[10]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13087/fi-icl-y/igt@gem_busy@busy-all.html
* igt@gem_ctx_create@basic-files:
- {fi-icl-u2}: [INCOMPLETE][11] ([fdo#107713] / [fdo#109100]) -> [PASS][12]
[11]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6138/fi-icl-u2/igt@gem_ctx_create@basic-files.html
[12]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13087/fi-icl-u2/igt@gem_ctx_create@basic-files.html
* igt@i915_selftest@live_contexts:
- fi-skl-gvtdvm: [DMESG-FAIL][13] ([fdo#110235]) -> [PASS][14]
[13]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6138/fi-skl-gvtdvm/igt@i915_selftest@live_contexts.html
[14]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13087/fi-skl-gvtdvm/igt@i915_selftest@live_contexts.html
* igt@i915_selftest@live_hangcheck:
- {fi-icl-u3}: [INCOMPLETE][15] ([fdo#107713] / [fdo#108569]) -> [PASS][16]
[15]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6138/fi-icl-u3/igt@i915_selftest@live_hangcheck.html
[16]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13087/fi-icl-u3/igt@i915_selftest@live_hangcheck.html
* {igt@i915_selftest@live_reset}:
- fi-skl-iommu: [INCOMPLETE][17] -> [PASS][18]
[17]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6138/fi-skl-iommu/igt@i915_selftest@live_reset.html
[18]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13087/fi-skl-iommu/igt@i915_selftest@live_reset.html
* igt@kms_frontbuffer_tracking@basic:
- {fi-icl-u3}: [FAIL][19] ([fdo#103167]) -> [PASS][20]
[19]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6138/fi-icl-u3/igt@kms_frontbuffer_tracking@basic.html
[20]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13087/fi-icl-u3/igt@kms_frontbuffer_tracking@basic.html
* igt@prime_vgem@basic-sync-default:
- fi-pnv-d510: [INCOMPLETE][21] ([fdo#110740]) -> [PASS][22]
[21]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6138/fi-pnv-d510/igt@prime_vgem@basic-sync-default.html
[22]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13087/fi-pnv-d510/igt@prime_vgem@basic-sync-default.html
{name}: This element is suppressed. This means it is ignored when computing
the status of the difference (SUCCESS, WARNING, or FAILURE).
[fdo#103167]: https://bugs.freedesktop.org/show_bug.cgi?id=103167
[fdo#103927]: https://bugs.freedesktop.org/show_bug.cgi?id=103927
[fdo#107713]: https://bugs.freedesktop.org/show_bug.cgi?id=107713
[fdo#107718]: https://bugs.freedesktop.org/show_bug.cgi?id=107718
[fdo#107724]: https://bugs.freedesktop.org/show_bug.cgi?id=107724
[fdo#108511]: https://bugs.freedesktop.org/show_bug.cgi?id=108511
[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
[fdo#110580 ]: https://bugs.freedesktop.org/show_bug.cgi?id=110580
[fdo#110718]: https://bugs.freedesktop.org/show_bug.cgi?id=110718
[fdo#110740]: https://bugs.freedesktop.org/show_bug.cgi?id=110740
[fdo#110753]: https://bugs.freedesktop.org/show_bug.cgi?id=110753
Participating hosts (42 -> 45)
------------------------------
Additional (7): fi-hsw-4770r fi-byt-j1900 fi-hsw-peppy fi-apl-guc fi-hsw-4770 fi-ivb-3770 fi-byt-n2820
Missing (4): fi-skl-guc fi-ilk-m540 fi-bsw-cyan fi-bdw-samus
Build changes
-------------
* Linux: CI_DRM_6138 -> Patchwork_13087
CI_DRM_6138: a17cee540f7ffd493c7944dea5ea6e2d32fdfdb4 @ git://anongit.freedesktop.org/gfx-ci/linux
IGT_5011: 7f120c5f1bff2727d50f3c392d81c0f6878b61d6 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
Patchwork_13087: 25610cdceea315bd495e3fe7533db997fa64c6f5 @ git://anongit.freedesktop.org/gfx-ci/linux
== Linux commits ==
25610cdceea3 drm/i915/gtt: Neuter the deferred unbind callback from gen6_ppgtt_cleanup
== Logs ==
For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13087/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 13+ messages in thread
end of thread, other threads:[~2019-05-24 9:08 UTC | newest]
Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-05-24 6:45 [BrownBag] drm/i915/gtt: Neuter the deferred unbind callback from gen6_ppgtt_cleanup Chris Wilson
2019-05-24 8:13 ` Tvrtko Ursulin
2019-05-24 8:17 ` Chris Wilson
2019-05-24 8:23 ` Tvrtko Ursulin
2019-05-24 8:29 ` Chris Wilson
2019-05-24 8:31 ` Tvrtko Ursulin
2019-05-24 8:36 ` Chris Wilson
2019-05-24 8:51 ` Tvrtko Ursulin
2019-05-24 8:55 ` Chris Wilson
2019-05-24 8:57 ` Tvrtko Ursulin
2019-05-24 9:01 ` Chris Wilson
2019-05-24 9:08 ` Tvrtko Ursulin
2019-05-24 8:31 ` ✗ Fi.CI.BAT: failure for " Patchwork
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.