* [PATCH] drm/syncobj: Don't leak fences when WAIT_FOR_SUBMIT is set
@ 2018-09-26 7:17 Jason Ekstrand
2018-09-26 8:18 ` Chris Wilson
2018-09-26 14:55 ` Sean Paul
0 siblings, 2 replies; 6+ messages in thread
From: Jason Ekstrand @ 2018-09-26 7:17 UTC (permalink / raw)
To: dri-devel; +Cc: Jason Ekstrand, Jason Ekstrand, stable
We attempt to get fences earlier in the hopes that everything will
already have fences and no callbacks will be needed. If we do succeed
in getting a fence, getting one a second time will result in a duplicate
ref with no unref. This is causing memory leaks in Vulkan applications
that create a lot of fences; playing for a few hours can, apparently,
bring down the system.
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=107899
Signed-off-by: Jason Ekstrand <jason@jlekstrand.net>
Cc: stable@vger.kernel.org
---
drivers/gpu/drm/drm_syncobj.c | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/drivers/gpu/drm/drm_syncobj.c b/drivers/gpu/drm/drm_syncobj.c
index adb3cb27d31e..759278fef35a 100644
--- a/drivers/gpu/drm/drm_syncobj.c
+++ b/drivers/gpu/drm/drm_syncobj.c
@@ -97,6 +97,8 @@ static int drm_syncobj_fence_get_or_add_callback(struct drm_syncobj *syncobj,
{
int ret;
+ WARN_ON(*fence);
+
*fence = drm_syncobj_fence_get(syncobj);
if (*fence)
return 1;
@@ -743,6 +745,9 @@ static signed long drm_syncobj_array_wait_timeout(struct drm_syncobj **syncobjs,
if (flags & DRM_SYNCOBJ_WAIT_FLAGS_WAIT_FOR_SUBMIT) {
for (i = 0; i < count; ++i) {
+ if (entries[i].fence)
+ continue;
+
drm_syncobj_fence_get_or_add_callback(syncobjs[i],
&entries[i].fence,
&entries[i].syncobj_cb,
--
2.17.1
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH] drm/syncobj: Don't leak fences when WAIT_FOR_SUBMIT is set
2018-09-26 7:17 [PATCH] drm/syncobj: Don't leak fences when WAIT_FOR_SUBMIT is set Jason Ekstrand
@ 2018-09-26 8:18 ` Chris Wilson
2018-09-26 14:55 ` Sean Paul
1 sibling, 0 replies; 6+ messages in thread
From: Chris Wilson @ 2018-09-26 8:18 UTC (permalink / raw)
To: Jason Ekstrand, dri-devel; +Cc: Jason Ekstrand, stable, Jason Ekstrand
Quoting Jason Ekstrand (2018-09-26 08:17:03)
> We attempt to get fences earlier in the hopes that everything will
> already have fences and no callbacks will be needed. If we do succeed
> in getting a fence, getting one a second time will result in a duplicate
> ref with no unref. This is causing memory leaks in Vulkan applications
> that create a lot of fences; playing for a few hours can, apparently,
> bring down the system.
>
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=107899
> Signed-off-by: Jason Ekstrand <jason@jlekstrand.net>
> Cc: stable@vger.kernel.org
> ---
> drivers/gpu/drm/drm_syncobj.c | 5 +++++
> 1 file changed, 5 insertions(+)
>
> diff --git a/drivers/gpu/drm/drm_syncobj.c b/drivers/gpu/drm/drm_syncobj.c
> index adb3cb27d31e..759278fef35a 100644
> --- a/drivers/gpu/drm/drm_syncobj.c
> +++ b/drivers/gpu/drm/drm_syncobj.c
> @@ -97,6 +97,8 @@ static int drm_syncobj_fence_get_or_add_callback(struct drm_syncobj *syncobj,
> {
> int ret;
>
> + WARN_ON(*fence);
I would have just put if (*fence) return; since the function is tied to
the array_wait implementation.
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
-Chris
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] drm/syncobj: Don't leak fences when WAIT_FOR_SUBMIT is set
@ 2018-09-26 8:18 ` Chris Wilson
0 siblings, 0 replies; 6+ messages in thread
From: Chris Wilson @ 2018-09-26 8:18 UTC (permalink / raw)
To: dri-devel; +Cc: Jason Ekstrand, stable, Jason Ekstrand
Quoting Jason Ekstrand (2018-09-26 08:17:03)
> We attempt to get fences earlier in the hopes that everything will
> already have fences and no callbacks will be needed. If we do succeed
> in getting a fence, getting one a second time will result in a duplicate
> ref with no unref. This is causing memory leaks in Vulkan applications
> that create a lot of fences; playing for a few hours can, apparently,
> bring down the system.
>
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=107899
> Signed-off-by: Jason Ekstrand <jason@jlekstrand.net>
> Cc: stable@vger.kernel.org
> ---
> drivers/gpu/drm/drm_syncobj.c | 5 +++++
> 1 file changed, 5 insertions(+)
>
> diff --git a/drivers/gpu/drm/drm_syncobj.c b/drivers/gpu/drm/drm_syncobj.c
> index adb3cb27d31e..759278fef35a 100644
> --- a/drivers/gpu/drm/drm_syncobj.c
> +++ b/drivers/gpu/drm/drm_syncobj.c
> @@ -97,6 +97,8 @@ static int drm_syncobj_fence_get_or_add_callback(struct drm_syncobj *syncobj,
> {
> int ret;
>
> + WARN_ON(*fence);
I would have just put if (*fence) return; since the function is tied to
the array_wait implementation.
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
-Chris
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] drm/syncobj: Don't leak fences when WAIT_FOR_SUBMIT is set
2018-09-26 8:18 ` Chris Wilson
(?)
@ 2018-09-26 9:43 ` Jason Ekstrand
2018-09-26 9:50 ` Chris Wilson
-1 siblings, 1 reply; 6+ messages in thread
From: Jason Ekstrand @ 2018-09-26 9:43 UTC (permalink / raw)
To: Chris Wilson; +Cc: Jason Ekstrand, stable, Maling list - DRI developers
[-- Attachment #1.1: Type: text/plain, Size: 1446 bytes --]
On Wed, Sep 26, 2018 at 3:18 AM Chris Wilson <chris@chris-wilson.co.uk>
wrote:
> Quoting Jason Ekstrand (2018-09-26 08:17:03)
> > We attempt to get fences earlier in the hopes that everything will
> > already have fences and no callbacks will be needed. If we do succeed
> > in getting a fence, getting one a second time will result in a duplicate
> > ref with no unref. This is causing memory leaks in Vulkan applications
> > that create a lot of fences; playing for a few hours can, apparently,
> > bring down the system.
> >
> > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=107899
> > Signed-off-by: Jason Ekstrand <jason@jlekstrand.net>
> > Cc: stable@vger.kernel.org
> > ---
> > drivers/gpu/drm/drm_syncobj.c | 5 +++++
> > 1 file changed, 5 insertions(+)
> >
> > diff --git a/drivers/gpu/drm/drm_syncobj.c
> b/drivers/gpu/drm/drm_syncobj.c
> > index adb3cb27d31e..759278fef35a 100644
> > --- a/drivers/gpu/drm/drm_syncobj.c
> > +++ b/drivers/gpu/drm/drm_syncobj.c
> > @@ -97,6 +97,8 @@ static int
> drm_syncobj_fence_get_or_add_callback(struct drm_syncobj *syncobj,
> > {
> > int ret;
> >
> > + WARN_ON(*fence);
>
> I would have just put if (*fence) return; since the function is tied to
> the array_wait implementation.
>
I considered doing that but marginally liked this better. If you have a
preference, I'm happy to change itl.
--Jason
> Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
> -Chris
>
[-- Attachment #1.2: Type: text/html, Size: 2332 bytes --]
[-- Attachment #2: Type: text/plain, Size: 160 bytes --]
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] drm/syncobj: Don't leak fences when WAIT_FOR_SUBMIT is set
2018-09-26 9:43 ` Jason Ekstrand
@ 2018-09-26 9:50 ` Chris Wilson
0 siblings, 0 replies; 6+ messages in thread
From: Chris Wilson @ 2018-09-26 9:50 UTC (permalink / raw)
To: Jason Ekstrand; +Cc: Maling list - DRI developers, Jason Ekstrand, stable
Quoting Jason Ekstrand (2018-09-26 10:43:59)
> On Wed, Sep 26, 2018 at 3:18 AM Chris Wilson <chris@chris-wilson.co.uk> wrote:
>
> Quoting Jason Ekstrand (2018-09-26 08:17:03)
> > We attempt to get fences earlier in the hopes that everything will
> > already have fences and no callbacks will be needed. If we do succeed
> > in getting a fence, getting one a second time will result in a duplicate
> > ref with no unref. This is causing memory leaks in Vulkan applications
> > that create a lot of fences; playing for a few hours can, apparently,
> > bring down the system.
> >
> > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=107899
> > Signed-off-by: Jason Ekstrand <jason@jlekstrand.net>
> > Cc: stable@vger.kernel.org
> > ---
> > drivers/gpu/drm/drm_syncobj.c | 5 +++++
> > 1 file changed, 5 insertions(+)
> >
> > diff --git a/drivers/gpu/drm/drm_syncobj.c b/drivers/gpu/drm/
> drm_syncobj.c
> > index adb3cb27d31e..759278fef35a 100644
> > --- a/drivers/gpu/drm/drm_syncobj.c
> > +++ b/drivers/gpu/drm/drm_syncobj.c
> > @@ -97,6 +97,8 @@ static int drm_syncobj_fence_get_or_add_callback(struct
> drm_syncobj *syncobj,
> > {
> > int ret;
> >
> > + WARN_ON(*fence);
>
> I would have just put if (*fence) return; since the function is tied to
> the array_wait implementation.
>
>
> I considered doing that but marginally liked this better. If you have a
> preference, I'm happy to change itl.
Patch works, the difference is insignificant, land the patch and close
the bug.
-Chris
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] drm/syncobj: Don't leak fences when WAIT_FOR_SUBMIT is set
2018-09-26 7:17 [PATCH] drm/syncobj: Don't leak fences when WAIT_FOR_SUBMIT is set Jason Ekstrand
2018-09-26 8:18 ` Chris Wilson
@ 2018-09-26 14:55 ` Sean Paul
1 sibling, 0 replies; 6+ messages in thread
From: Sean Paul @ 2018-09-26 14:55 UTC (permalink / raw)
To: Jason Ekstrand; +Cc: dri-devel, Jason Ekstrand, stable
On Wed, Sep 26, 2018 at 02:17:03AM -0500, Jason Ekstrand wrote:
> We attempt to get fences earlier in the hopes that everything will
> already have fences and no callbacks will be needed. If we do succeed
> in getting a fence, getting one a second time will result in a duplicate
> ref with no unref. This is causing memory leaks in Vulkan applications
> that create a lot of fences; playing for a few hours can, apparently,
> bring down the system.
>
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=107899
> Signed-off-by: Jason Ekstrand <jason@jlekstrand.net>
Pushed to drm-misc-fixes, thanks!
Sean
> Cc: stable@vger.kernel.org
> ---
> drivers/gpu/drm/drm_syncobj.c | 5 +++++
> 1 file changed, 5 insertions(+)
>
> diff --git a/drivers/gpu/drm/drm_syncobj.c b/drivers/gpu/drm/drm_syncobj.c
> index adb3cb27d31e..759278fef35a 100644
> --- a/drivers/gpu/drm/drm_syncobj.c
> +++ b/drivers/gpu/drm/drm_syncobj.c
> @@ -97,6 +97,8 @@ static int drm_syncobj_fence_get_or_add_callback(struct drm_syncobj *syncobj,
> {
> int ret;
>
> + WARN_ON(*fence);
> +
> *fence = drm_syncobj_fence_get(syncobj);
> if (*fence)
> return 1;
> @@ -743,6 +745,9 @@ static signed long drm_syncobj_array_wait_timeout(struct drm_syncobj **syncobjs,
>
> if (flags & DRM_SYNCOBJ_WAIT_FLAGS_WAIT_FOR_SUBMIT) {
> for (i = 0; i < count; ++i) {
> + if (entries[i].fence)
> + continue;
> +
> drm_syncobj_fence_get_or_add_callback(syncobjs[i],
> &entries[i].fence,
> &entries[i].syncobj_cb,
> --
> 2.17.1
>
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
--
Sean Paul, Software Engineer, Google / Chromium OS
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2018-09-26 21:08 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-09-26 7:17 [PATCH] drm/syncobj: Don't leak fences when WAIT_FOR_SUBMIT is set Jason Ekstrand
2018-09-26 8:18 ` Chris Wilson
2018-09-26 8:18 ` Chris Wilson
2018-09-26 9:43 ` Jason Ekstrand
2018-09-26 9:50 ` Chris Wilson
2018-09-26 14:55 ` Sean Paul
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.