* [PATCH] drm: Release driver tracking before making the object available again
@ 2017-08-19 12:05 Chris Wilson
2017-08-19 12:27 ` ✓ Fi.CI.BAT: success for " Patchwork
2017-08-21 16:16 ` Daniel Vetter
0 siblings, 2 replies; 7+ messages in thread
From: Chris Wilson @ 2017-08-19 12:05 UTC (permalink / raw)
To: dri-devel
Cc: intel-gfx, Chris Wilson, David Airlie, Daniel Vetter, Rob Clark,
Ville Syrjälä,
Thierry Reding, stable
This is the same bug as we fixed in commit f6cd7daecff5 ("drm: Release
driver references to handle before making it available again"), but now
the exposure is via the PRIME lookup tables. If we remove the
object/handle from the PRIME lut, then a new request for the same
object/fd will generate a new handle, thus for a short window that
object is known to userspace by two different handles. Fix this by
releasing the driver tracking before PRIME.
Fixes: 0ff926c7d4f0 ("drm/prime: add exported buffers to current fprivs
imported buffer list (v2)")
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: David Airlie <airlied@linux.ie>
Cc: Daniel Vetter <daniel.vetter@intel.com>
Cc: Rob Clark <robdclark@gmail.com>
Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Cc: Thierry Reding <treding@nvidia.com>
Cc: stable@vger.kernel.org
---
drivers/gpu/drm/drm_gem.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/drm_gem.c b/drivers/gpu/drm/drm_gem.c
index ad4e9cfe48a2..88c6d78ee2d5 100644
--- a/drivers/gpu/drm/drm_gem.c
+++ b/drivers/gpu/drm/drm_gem.c
@@ -255,13 +255,13 @@ drm_gem_object_release_handle(int id, void *ptr, void *data)
struct drm_gem_object *obj = ptr;
struct drm_device *dev = obj->dev;
+ if (dev->driver->gem_close_object)
+ dev->driver->gem_close_object(obj, file_priv);
+
if (drm_core_check_feature(dev, DRIVER_PRIME))
drm_gem_remove_prime_handles(obj, file_priv);
drm_vma_node_revoke(&obj->vma_node, file_priv);
- if (dev->driver->gem_close_object)
- dev->driver->gem_close_object(obj, file_priv);
-
drm_gem_object_handle_put_unlocked(obj);
return 0;
--
2.14.1
^ permalink raw reply related [flat|nested] 7+ messages in thread
* ✓ Fi.CI.BAT: success for drm: Release driver tracking before making the object available again
2017-08-19 12:05 [PATCH] drm: Release driver tracking before making the object available again Chris Wilson
@ 2017-08-19 12:27 ` Patchwork
2017-08-21 16:16 ` Daniel Vetter
1 sibling, 0 replies; 7+ messages in thread
From: Patchwork @ 2017-08-19 12:27 UTC (permalink / raw)
To: Chris Wilson; +Cc: intel-gfx
== Series Details ==
Series: drm: Release driver tracking before making the object available again
URL : https://patchwork.freedesktop.org/series/29039/
State : success
== Summary ==
Series 29039v1 drm: Release driver tracking before making the object available again
https://patchwork.freedesktop.org/api/1.0/series/29039/revisions/1/mbox/
Test gem_exec_flush:
Subgroup basic-batch-kernel-default-uc:
pass -> FAIL (fi-snb-2600) fdo#100007
Test drv_module_reload:
Subgroup basic-no-display:
dmesg-warn -> PASS (fi-kbl-7260u)
Subgroup basic-reload-inject:
dmesg-warn -> PASS (fi-kbl-7260u) fdo#102295
fdo#100007 https://bugs.freedesktop.org/show_bug.cgi?id=100007
fdo#102295 https://bugs.freedesktop.org/show_bug.cgi?id=102295
fi-bdw-5557u total:279 pass:268 dwarn:0 dfail:0 fail:0 skip:11 time:456s
fi-blb-e6850 total:279 pass:224 dwarn:1 dfail:0 fail:0 skip:54 time:359s
fi-bsw-n3050 total:279 pass:243 dwarn:0 dfail:0 fail:0 skip:36 time:558s
fi-bxt-j4205 total:279 pass:260 dwarn:0 dfail:0 fail:0 skip:19 time:519s
fi-byt-j1900 total:279 pass:254 dwarn:1 dfail:0 fail:0 skip:24 time:522s
fi-byt-n2820 total:279 pass:251 dwarn:0 dfail:0 fail:0 skip:28 time:522s
fi-glk-2a total:279 pass:260 dwarn:0 dfail:0 fail:0 skip:19 time:612s
fi-hsw-4770 total:279 pass:263 dwarn:0 dfail:0 fail:0 skip:16 time:443s
fi-hsw-4770r total:279 pass:263 dwarn:0 dfail:0 fail:0 skip:16 time:424s
fi-ilk-650 total:279 pass:229 dwarn:0 dfail:0 fail:0 skip:50 time:420s
fi-ivb-3520m total:279 pass:261 dwarn:0 dfail:0 fail:0 skip:18 time:507s
fi-ivb-3770 total:279 pass:261 dwarn:0 dfail:0 fail:0 skip:18 time:477s
fi-kbl-7260u total:279 pass:268 dwarn:1 dfail:0 fail:0 skip:10 time:495s
fi-kbl-7500u total:279 pass:261 dwarn:0 dfail:0 fail:0 skip:18 time:481s
fi-kbl-7560u total:279 pass:269 dwarn:0 dfail:0 fail:0 skip:10 time:598s
fi-kbl-r total:279 pass:261 dwarn:0 dfail:0 fail:0 skip:18 time:595s
fi-pnv-d510 total:279 pass:223 dwarn:1 dfail:0 fail:0 skip:55 time:527s
fi-skl-6260u total:279 pass:269 dwarn:0 dfail:0 fail:0 skip:10 time:468s
fi-skl-6700k total:279 pass:261 dwarn:0 dfail:0 fail:0 skip:18 time:473s
fi-skl-6770hq total:279 pass:269 dwarn:0 dfail:0 fail:0 skip:10 time:486s
fi-skl-gvtdvm total:279 pass:266 dwarn:0 dfail:0 fail:0 skip:13 time:445s
fi-skl-x1585l total:279 pass:268 dwarn:0 dfail:0 fail:0 skip:11 time:473s
fi-snb-2520m total:279 pass:251 dwarn:0 dfail:0 fail:0 skip:28 time:548s
fi-snb-2600 total:279 pass:249 dwarn:0 dfail:0 fail:1 skip:29 time:405s
fi-bdw-gvtdvm failed to connect after reboot
5b151fd0dafb8130ce4b809e3795146a5ef6bc2e drm-tip: 2017y-08m-18d-22h-38m-58s UTC integration manifest
fde442024d1e drm: Release driver tracking before making the object available again
== Logs ==
For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_5447/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [Intel-gfx] [PATCH] drm: Release driver tracking before making the object available again
2017-08-19 12:05 [PATCH] drm: Release driver tracking before making the object available again Chris Wilson
@ 2017-08-21 16:16 ` Daniel Vetter
2017-08-21 16:16 ` Daniel Vetter
1 sibling, 0 replies; 7+ messages in thread
From: Daniel Vetter @ 2017-08-21 16:16 UTC (permalink / raw)
To: Chris Wilson
Cc: dri-devel, David Airlie, intel-gfx, stable, Daniel Vetter,
Thierry Reding
On Sat, Aug 19, 2017 at 01:05:58PM +0100, Chris Wilson wrote:
> This is the same bug as we fixed in commit f6cd7daecff5 ("drm: Release
> driver references to handle before making it available again"), but now
> the exposure is via the PRIME lookup tables. If we remove the
> object/handle from the PRIME lut, then a new request for the same
> object/fd will generate a new handle, thus for a short window that
> object is known to userspace by two different handles. Fix this by
> releasing the driver tracking before PRIME.
>
> Fixes: 0ff926c7d4f0 ("drm/prime: add exported buffers to current fprivs
> imported buffer list (v2)")
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: David Airlie <airlied@linux.ie>
> Cc: Daniel Vetter <daniel.vetter@intel.com>
> Cc: Rob Clark <robdclark@gmail.com>
> Cc: Ville Syrj�l� <ville.syrjala@linux.intel.com>
> Cc: Thierry Reding <treding@nvidia.com>
> Cc: stable@vger.kernel.org
Do we have an evil igt for this? I guess since the old one didn't have
one, this new race is also hard to reproduce ...
Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> ---
> drivers/gpu/drm/drm_gem.c | 6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_gem.c b/drivers/gpu/drm/drm_gem.c
> index ad4e9cfe48a2..88c6d78ee2d5 100644
> --- a/drivers/gpu/drm/drm_gem.c
> +++ b/drivers/gpu/drm/drm_gem.c
> @@ -255,13 +255,13 @@ drm_gem_object_release_handle(int id, void *ptr, void *data)
> struct drm_gem_object *obj = ptr;
> struct drm_device *dev = obj->dev;
>
> + if (dev->driver->gem_close_object)
> + dev->driver->gem_close_object(obj, file_priv);
> +
> if (drm_core_check_feature(dev, DRIVER_PRIME))
> drm_gem_remove_prime_handles(obj, file_priv);
> drm_vma_node_revoke(&obj->vma_node, file_priv);
>
> - if (dev->driver->gem_close_object)
> - dev->driver->gem_close_object(obj, file_priv);
> -
> drm_gem_object_handle_put_unlocked(obj);
>
> return 0;
> --
> 2.14.1
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] drm: Release driver tracking before making the object available again
@ 2017-08-21 16:16 ` Daniel Vetter
0 siblings, 0 replies; 7+ messages in thread
From: Daniel Vetter @ 2017-08-21 16:16 UTC (permalink / raw)
To: Chris Wilson
Cc: David Airlie, intel-gfx, stable, dri-devel, Daniel Vetter,
Thierry Reding
On Sat, Aug 19, 2017 at 01:05:58PM +0100, Chris Wilson wrote:
> This is the same bug as we fixed in commit f6cd7daecff5 ("drm: Release
> driver references to handle before making it available again"), but now
> the exposure is via the PRIME lookup tables. If we remove the
> object/handle from the PRIME lut, then a new request for the same
> object/fd will generate a new handle, thus for a short window that
> object is known to userspace by two different handles. Fix this by
> releasing the driver tracking before PRIME.
>
> Fixes: 0ff926c7d4f0 ("drm/prime: add exported buffers to current fprivs
> imported buffer list (v2)")
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: David Airlie <airlied@linux.ie>
> Cc: Daniel Vetter <daniel.vetter@intel.com>
> Cc: Rob Clark <robdclark@gmail.com>
> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Cc: Thierry Reding <treding@nvidia.com>
> Cc: stable@vger.kernel.org
Do we have an evil igt for this? I guess since the old one didn't have
one, this new race is also hard to reproduce ...
Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> ---
> drivers/gpu/drm/drm_gem.c | 6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_gem.c b/drivers/gpu/drm/drm_gem.c
> index ad4e9cfe48a2..88c6d78ee2d5 100644
> --- a/drivers/gpu/drm/drm_gem.c
> +++ b/drivers/gpu/drm/drm_gem.c
> @@ -255,13 +255,13 @@ drm_gem_object_release_handle(int id, void *ptr, void *data)
> struct drm_gem_object *obj = ptr;
> struct drm_device *dev = obj->dev;
>
> + if (dev->driver->gem_close_object)
> + dev->driver->gem_close_object(obj, file_priv);
> +
> if (drm_core_check_feature(dev, DRIVER_PRIME))
> drm_gem_remove_prime_handles(obj, file_priv);
> drm_vma_node_revoke(&obj->vma_node, file_priv);
>
> - if (dev->driver->gem_close_object)
> - dev->driver->gem_close_object(obj, file_priv);
> -
> drm_gem_object_handle_put_unlocked(obj);
>
> return 0;
> --
> 2.14.1
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] drm: Release driver tracking before making the object available again
2017-08-21 16:16 ` Daniel Vetter
(?)
@ 2017-08-21 16:48 ` Chris Wilson
-1 siblings, 0 replies; 7+ messages in thread
From: Chris Wilson @ 2017-08-21 16:48 UTC (permalink / raw)
To: Daniel Vetter
Cc: David Airlie, intel-gfx, stable, dri-devel, Daniel Vetter,
Thierry Reding
Quoting Daniel Vetter (2017-08-21 17:16:24)
> On Sat, Aug 19, 2017 at 01:05:58PM +0100, Chris Wilson wrote:
> > This is the same bug as we fixed in commit f6cd7daecff5 ("drm: Release
> > driver references to handle before making it available again"), but now
> > the exposure is via the PRIME lookup tables. If we remove the
> > object/handle from the PRIME lut, then a new request for the same
> > object/fd will generate a new handle, thus for a short window that
> > object is known to userspace by two different handles. Fix this by
> > releasing the driver tracking before PRIME.
> >
> > Fixes: 0ff926c7d4f0 ("drm/prime: add exported buffers to current fprivs
> > imported buffer list (v2)")
> > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> > Cc: David Airlie <airlied@linux.ie>
> > Cc: Daniel Vetter <daniel.vetter@intel.com>
> > Cc: Rob Clark <robdclark@gmail.com>
> > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > Cc: Thierry Reding <treding@nvidia.com>
> > Cc: stable@vger.kernel.org
>
> Do we have an evil igt for this? I guess since the old one didn't have
> one, this new race is also hard to reproduce ...
The old one we did hit in igt (gem_concurrent_blit), but only by virtue
of it running for long enough to spot the race (ending with two handles
to the same object in an execbuf call).
This one requires us racing dma-buf import/close vs execbuf on the same
handles. It's the type of race the gem_close_race is looking for (except
that it doesn't cover dmabuf yet), but we are reliant on having a means
to detect the race. At the moment, we would detect it if you ended up
with two handles to the same object within the execbuf (which is
plausible as you can currently create that second handle before we mark
the first as closed, but the race will require some unfair queueing on
struct_mutex), or if we end up with two handles to the vma on close.
Hmm, the one way to make the race easier to hit is to add a sleep to
i915_gem_close_object before we take the struct_mutex.
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [Intel-gfx] [PATCH] drm: Release driver tracking before making the object available again
2017-08-21 16:16 ` Daniel Vetter
@ 2017-08-22 13:20 ` Joonas Lahtinen
-1 siblings, 0 replies; 7+ messages in thread
From: Joonas Lahtinen @ 2017-08-22 13:20 UTC (permalink / raw)
To: Daniel Vetter, Chris Wilson, Sean Paul
Cc: David Airlie, intel-gfx, stable, dri-devel, Daniel Vetter,
Thierry Reding
+ Sean
On Mon, 2017-08-21 at 18:16 +0200, Daniel Vetter wrote:
> On Sat, Aug 19, 2017 at 01:05:58PM +0100, Chris Wilson wrote:
> > This is the same bug as we fixed in commit f6cd7daecff5 ("drm: Release
> > driver references to handle before making it available again"), but now
> > the exposure is via the PRIME lookup tables. If we remove the
> > object/handle from the PRIME lut, then a new request for the same
> > object/fd will generate a new handle, thus for a short window that
> > object is known to userspace by two different handles. Fix this by
> > releasing the driver tracking before PRIME.
> >
> > Fixes: 0ff926c7d4f0 ("drm/prime: add exported buffers to current fprivs
> > imported buffer list (v2)")
> > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> > Cc: David Airlie <airlied@linux.ie>
> > Cc: Daniel Vetter <daniel.vetter@intel.com>
> > Cc: Rob Clark <robdclark@gmail.com>
> > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > Cc: Thierry Reding <treding@nvidia.com>
> > Cc: stable@vger.kernel.org
>
> Do we have an evil igt for this? I guess since the old one didn't have
> one, this new race is also hard to reproduce ...
>
> Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Pushed this to drm-misc-fixes (and drm-misc-next for I am a monkey with
a keyboard), thanks for the patch and review.
Sean, you can blame it on me when/if there is trouble caused by the
patch being in both branches. Hopefully next merge will cause less
headache.
Regards, Joonas
--
Joonas Lahtinen
Open Source Technology Center
Intel Corporation
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [Intel-gfx] [PATCH] drm: Release driver tracking before making the object available again
@ 2017-08-22 13:20 ` Joonas Lahtinen
0 siblings, 0 replies; 7+ messages in thread
From: Joonas Lahtinen @ 2017-08-22 13:20 UTC (permalink / raw)
To: Daniel Vetter, Chris Wilson, Sean Paul
Cc: intel-gfx, dri-devel, stable, Daniel Vetter, Thierry Reding
+ Sean
On Mon, 2017-08-21 at 18:16 +0200, Daniel Vetter wrote:
> On Sat, Aug 19, 2017 at 01:05:58PM +0100, Chris Wilson wrote:
> > This is the same bug as we fixed in commit f6cd7daecff5 ("drm: Release
> > driver references to handle before making it available again"), but now
> > the exposure is via the PRIME lookup tables. If we remove the
> > object/handle from the PRIME lut, then a new request for the same
> > object/fd will generate a new handle, thus for a short window that
> > object is known to userspace by two different handles. Fix this by
> > releasing the driver tracking before PRIME.
> >
> > Fixes: 0ff926c7d4f0 ("drm/prime: add exported buffers to current fprivs
> > imported buffer list (v2)")
> > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> > Cc: David Airlie <airlied@linux.ie>
> > Cc: Daniel Vetter <daniel.vetter@intel.com>
> > Cc: Rob Clark <robdclark@gmail.com>
> > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > Cc: Thierry Reding <treding@nvidia.com>
> > Cc: stable@vger.kernel.org
>
> Do we have an evil igt for this? I guess since the old one didn't have
> one, this new race is also hard to reproduce ...
>
> Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Pushed this to drm-misc-fixes (and drm-misc-next for I am a monkey with
a keyboard), thanks for the patch and review.
Sean, you can blame it on me when/if there is trouble caused by the
patch being in both branches. Hopefully next merge will cause less
headache.
Regards, Joonas
--
Joonas Lahtinen
Open Source Technology Center
Intel Corporation
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2017-08-22 13:20 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-08-19 12:05 [PATCH] drm: Release driver tracking before making the object available again Chris Wilson
2017-08-19 12:27 ` ✓ Fi.CI.BAT: success for " Patchwork
2017-08-21 16:16 ` [Intel-gfx] [PATCH] " Daniel Vetter
2017-08-21 16:16 ` Daniel Vetter
2017-08-21 16:48 ` Chris Wilson
2017-08-22 13:20 ` [Intel-gfx] " Joonas Lahtinen
2017-08-22 13:20 ` Joonas Lahtinen
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.