* [PATCH 1/6] igt_kms: don't leak the mmap used for cairo
@ 2013-12-30 15:56 Paulo Zanoni
2013-12-30 15:56 ` [PATCH 2/6] drmtest: don't leak memory when parsing the arguments Paulo Zanoni
` (5 more replies)
0 siblings, 6 replies; 12+ messages in thread
From: Paulo Zanoni @ 2013-12-30 15:56 UTC (permalink / raw)
To: intel-gfx; +Cc: Paulo Zanoni
From: Paulo Zanoni <paulo.r.zanoni@intel.com>
When we call kmstest_get_cairo_ctx() and create a context, we do a
gem_mmap. The problem is that we lose the mmap pointer, so we leak it.
This patch stores the pointer and frees it at kmstest_remove_fb.
Huge test suites like kms_flip do this operation thousands of times,
which makes the virtual memory size increase until the test suite gets
killed. Today, without this patch, we can't even run 50% of the
kms_flip tests due to this problem. To test this, just "./kms_flip",
then run "top" and sort it by the VIRT column.
Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
---
lib/igt_kms.c | 15 ++++++++++-----
lib/igt_kms.h | 1 +
2 files changed, 11 insertions(+), 5 deletions(-)
diff --git a/lib/igt_kms.c b/lib/igt_kms.c
index 57795b1..f032cd3 100644
--- a/lib/igt_kms.c
+++ b/lib/igt_kms.c
@@ -303,6 +303,7 @@ unsigned int kmstest_create_fb(int fd, int width, int height, int bpp,
fb->height = height;
fb->tiling = tiled;
fb->drm_format = bpp_depth_to_drm_format(bpp, depth);
+ fb->fb_ptr = NULL;
return fb->fb_id;
}
@@ -353,6 +354,7 @@ unsigned int kmstest_create_fb2(int fd, int width, int height, uint32_t format,
fb->tiling = tiled;
fb->drm_format = format;
fb->fb_id = fb_id;
+ fb->fb_ptr = NULL;
return fb_id;
}
@@ -372,13 +374,14 @@ static cairo_surface_t *create_image_surface(int fd, struct kmstest_fb *fb)
{
cairo_surface_t *surface;
cairo_format_t cformat;
- void *fb_ptr;
cformat = drm_format_to_cairo(fb->drm_format);
- fb_ptr = gem_mmap(fd, fb->gem_handle, fb->size, PROT_READ | PROT_WRITE);
- surface = cairo_image_surface_create_for_data((unsigned char *)fb_ptr,
- cformat, fb->width,
- fb->height, fb->stride);
+ fb->fb_ptr = gem_mmap(fd, fb->gem_handle, fb->size,
+ PROT_READ | PROT_WRITE);
+ surface = cairo_image_surface_create_for_data(
+ (unsigned char *)fb->fb_ptr,
+ cformat, fb->width,
+ fb->height, fb->stride);
assert(surface);
return surface;
@@ -423,6 +426,8 @@ void kmstest_remove_fb(int fd, struct kmstest_fb *fb)
{
if (fb->cairo_ctx)
cairo_destroy(fb->cairo_ctx);
+ if (fb->fb_ptr)
+ munmap(fb->fb_ptr, fb->size);
do_or_die(drmModeRmFB(fd, fb->fb_id));
gem_close(fd, fb->gem_handle);
}
diff --git a/lib/igt_kms.h b/lib/igt_kms.h
index f61f8e5..f9494d0 100644
--- a/lib/igt_kms.h
+++ b/lib/igt_kms.h
@@ -55,6 +55,7 @@ struct kmstest_fb {
unsigned tiling;
unsigned size;
cairo_t *cairo_ctx;
+ void *fb_ptr;
};
enum kmstest_text_align {
--
1.8.3.1
^ permalink raw reply related [flat|nested] 12+ messages in thread* [PATCH 2/6] drmtest: don't leak memory when parsing the arguments
2013-12-30 15:56 [PATCH 1/6] igt_kms: don't leak the mmap used for cairo Paulo Zanoni
@ 2013-12-30 15:56 ` Paulo Zanoni
2013-12-30 15:56 ` [PATCH 3/6] tests/kms_flip: don't leak the connector when setting DPMS Paulo Zanoni
` (4 subsequent siblings)
5 siblings, 0 replies; 12+ messages in thread
From: Paulo Zanoni @ 2013-12-30 15:56 UTC (permalink / raw)
To: intel-gfx; +Cc: Paulo Zanoni
From: Paulo Zanoni <paulo.r.zanoni@intel.com>
Found this while investigating memory leaks on kms_flip. Detected by
Valgrind.
Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
---
lib/drmtest.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/lib/drmtest.c b/lib/drmtest.c
index 3d79a4d..7275b7f 100644
--- a/lib/drmtest.c
+++ b/lib/drmtest.c
@@ -911,6 +911,8 @@ int igt_subtest_init_parse_opts(int argc, char **argv,
oom_adjust_for_doom();
out:
+ free(short_opts);
+ free(combined_opts);
print_version();
return ret;
--
1.8.3.1
^ permalink raw reply related [flat|nested] 12+ messages in thread* [PATCH 3/6] tests/kms_flip: don't leak the connector when setting DPMS
2013-12-30 15:56 [PATCH 1/6] igt_kms: don't leak the mmap used for cairo Paulo Zanoni
2013-12-30 15:56 ` [PATCH 2/6] drmtest: don't leak memory when parsing the arguments Paulo Zanoni
@ 2013-12-30 15:56 ` Paulo Zanoni
2013-12-30 15:56 ` [PATCH 4/6] tests/kms_flip: don't leak the CRTC Paulo Zanoni
` (3 subsequent siblings)
5 siblings, 0 replies; 12+ messages in thread
From: Paulo Zanoni @ 2013-12-30 15:56 UTC (permalink / raw)
To: intel-gfx; +Cc: Paulo Zanoni
From: Paulo Zanoni <paulo.r.zanoni@intel.com>
Caught by Valgrind.
Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
---
tests/kms_flip.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/tests/kms_flip.c b/tests/kms_flip.c
index eaa42b5..80f4d76 100644
--- a/tests/kms_flip.c
+++ b/tests/kms_flip.c
@@ -295,6 +295,8 @@ static void dpms_off_other_outputs(struct test_output *o)
set_connector_dpms(connector, DRM_MODE_DPMS_ON);
set_connector_dpms(connector, DRM_MODE_DPMS_OFF);
+
+ drmModeFreeConnector(connector);
next:
;
}
--
1.8.3.1
^ permalink raw reply related [flat|nested] 12+ messages in thread* [PATCH 4/6] tests/kms_flip: don't leak the CRTC
2013-12-30 15:56 [PATCH 1/6] igt_kms: don't leak the mmap used for cairo Paulo Zanoni
2013-12-30 15:56 ` [PATCH 2/6] drmtest: don't leak memory when parsing the arguments Paulo Zanoni
2013-12-30 15:56 ` [PATCH 3/6] tests/kms_flip: don't leak the connector when setting DPMS Paulo Zanoni
@ 2013-12-30 15:56 ` Paulo Zanoni
2013-12-30 15:56 ` [PATCH 5/6] tests/kms_flip: don't leak the connector_config struct Paulo Zanoni
` (2 subsequent siblings)
5 siblings, 0 replies; 12+ messages in thread
From: Paulo Zanoni @ 2013-12-30 15:56 UTC (permalink / raw)
To: intel-gfx; +Cc: Paulo Zanoni
From: Paulo Zanoni <paulo.r.zanoni@intel.com>
The kms_flip program calls kmstest_get_connector_config, which returns
a struct containing some allocated variables, including a pointer to
the CRTC. The problem is that we copy the values returned by this
structure to the test_output struct, but we ignore the CRTC pointer.
So free the CRTC pointer instead of leaking it.
Caught by Valgrind.
Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
---
tests/kms_flip.c | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/tests/kms_flip.c b/tests/kms_flip.c
index 80f4d76..78e179e 100644
--- a/tests/kms_flip.c
+++ b/tests/kms_flip.c
@@ -921,6 +921,8 @@ static void connector_find_preferred_mode(uint32_t connector_id, int crtc_idx,
o->fb_width = o->kmode[0].hdisplay;
o->fb_height = o->kmode[0].vdisplay;
+
+ drmModeFreeCrtc(config.crtc);
}
static bool mode_compatible(const drmModeModeInfo *a, const drmModeModeInfo *b)
@@ -986,6 +988,9 @@ found:
o->kencoder[1] = config[1].encoder;
o->_crtc[1] = config[1].crtc->crtc_id;
o->kmode[1] = *mode[1];
+
+ drmModeFreeCrtc(config[0].crtc);
+ drmModeFreeCrtc(config[1].crtc);
}
static void paint_flip_mode(struct kmstest_fb *fb, bool odd_frame)
--
1.8.3.1
^ permalink raw reply related [flat|nested] 12+ messages in thread* [PATCH 5/6] tests/kms_flip: don't leak the connector_config struct
2013-12-30 15:56 [PATCH 1/6] igt_kms: don't leak the mmap used for cairo Paulo Zanoni
` (2 preceding siblings ...)
2013-12-30 15:56 ` [PATCH 4/6] tests/kms_flip: don't leak the CRTC Paulo Zanoni
@ 2013-12-30 15:56 ` Paulo Zanoni
2013-12-30 15:56 ` [PATCH 6/6] tests/kms_flip: free the test_output struct when counting modes Paulo Zanoni
2013-12-30 17:40 ` [PATCH 1/6] igt_kms: don't leak the mmap used for cairo Chris Wilson
5 siblings, 0 replies; 12+ messages in thread
From: Paulo Zanoni @ 2013-12-30 15:56 UTC (permalink / raw)
To: intel-gfx; +Cc: Paulo Zanoni
From: Paulo Zanoni <paulo.r.zanoni@intel.com>
... in case we can't find a compatible mode. We already have
config[0], we can't return without freeing it first.
Caught by Valgrind.
Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
---
tests/kms_flip.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/tests/kms_flip.c b/tests/kms_flip.c
index 78e179e..c43a26f 100644
--- a/tests/kms_flip.c
+++ b/tests/kms_flip.c
@@ -954,8 +954,10 @@ static void connector_find_compatible_mode(int crtc_idx0, int crtc_idx1,
return;
if (kmstest_get_connector_config(drm_fd, o->_connector[1],
- 1 << crtc_idx1, &config[1]) < 0)
+ 1 << crtc_idx1, &config[1]) < 0) {
+ kmstest_free_connector_config(&config[0]);
return;
+ }
mode[0] = &config[0].default_mode;
mode[1] = &config[1].default_mode;
--
1.8.3.1
^ permalink raw reply related [flat|nested] 12+ messages in thread* [PATCH 6/6] tests/kms_flip: free the test_output struct when counting modes
2013-12-30 15:56 [PATCH 1/6] igt_kms: don't leak the mmap used for cairo Paulo Zanoni
` (3 preceding siblings ...)
2013-12-30 15:56 ` [PATCH 5/6] tests/kms_flip: don't leak the connector_config struct Paulo Zanoni
@ 2013-12-30 15:56 ` Paulo Zanoni
2014-01-07 7:16 ` Daniel Vetter
2013-12-30 17:40 ` [PATCH 1/6] igt_kms: don't leak the mmap used for cairo Chris Wilson
5 siblings, 1 reply; 12+ messages in thread
From: Paulo Zanoni @ 2013-12-30 15:56 UTC (permalink / raw)
To: intel-gfx; +Cc: Paulo Zanoni
From: Paulo Zanoni <paulo.r.zanoni@intel.com>
Looks like we have to do a lot of work just to count the number of
modes...
Caught by Valgrind.
Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
---
tests/kms_flip.c | 17 +++++++++++++----
1 file changed, 13 insertions(+), 4 deletions(-)
diff --git a/tests/kms_flip.c b/tests/kms_flip.c
index c43a26f..7e5e355 100644
--- a/tests/kms_flip.c
+++ b/tests/kms_flip.c
@@ -1129,6 +1129,16 @@ static unsigned event_loop(struct test_output *o, unsigned duration_ms)
return end - start;
}
+static void free_test_output(struct test_output *o)
+{
+ int i;
+
+ for (i = 0; i < o->count; i++) {
+ drmModeFreeEncoder(o->kencoder[i]);
+ drmModeFreeConnector(o->kconnector[i]);
+ }
+}
+
static void run_test_on_crtc_set(struct test_output *o, int *crtc_idxs,
int crtc_count, int duration_ms)
{
@@ -1231,10 +1241,7 @@ out:
last_connector = NULL;
- for (i = 0; i < o->count; i++) {
- drmModeFreeEncoder(o->kencoder[i]);
- drmModeFreeConnector(o->kconnector[i]);
- }
+ free_test_output(o);
}
static int run_test(int duration, int flags)
@@ -1261,6 +1268,7 @@ static int run_test(int duration, int flags)
if (o.mode_valid)
modes++;
+ free_test_output(&o);
}
}
@@ -1318,6 +1326,7 @@ static int run_pair(int duration, int flags)
if (o.mode_valid)
modes++;
+ free_test_output(&o);
}
}
}
--
1.8.3.1
^ permalink raw reply related [flat|nested] 12+ messages in thread* Re: [PATCH 6/6] tests/kms_flip: free the test_output struct when counting modes
2013-12-30 15:56 ` [PATCH 6/6] tests/kms_flip: free the test_output struct when counting modes Paulo Zanoni
@ 2014-01-07 7:16 ` Daniel Vetter
0 siblings, 0 replies; 12+ messages in thread
From: Daniel Vetter @ 2014-01-07 7:16 UTC (permalink / raw)
To: Paulo Zanoni; +Cc: intel-gfx, Paulo Zanoni
On Mon, Dec 30, 2013 at 01:56:53PM -0200, Paulo Zanoni wrote:
> From: Paulo Zanoni <paulo.r.zanoni@intel.com>
>
> Looks like we have to do a lot of work just to count the number of
> modes...
>
> Caught by Valgrind.
>
> Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
Nice set of patches, please push. I agree that the mode/output config
handling in our kms tests is still awful and too verbose, but I didn't
come up yet with a good idea ...
-Daniel
> ---
> tests/kms_flip.c | 17 +++++++++++++----
> 1 file changed, 13 insertions(+), 4 deletions(-)
>
> diff --git a/tests/kms_flip.c b/tests/kms_flip.c
> index c43a26f..7e5e355 100644
> --- a/tests/kms_flip.c
> +++ b/tests/kms_flip.c
> @@ -1129,6 +1129,16 @@ static unsigned event_loop(struct test_output *o, unsigned duration_ms)
> return end - start;
> }
>
> +static void free_test_output(struct test_output *o)
> +{
> + int i;
> +
> + for (i = 0; i < o->count; i++) {
> + drmModeFreeEncoder(o->kencoder[i]);
> + drmModeFreeConnector(o->kconnector[i]);
> + }
> +}
> +
> static void run_test_on_crtc_set(struct test_output *o, int *crtc_idxs,
> int crtc_count, int duration_ms)
> {
> @@ -1231,10 +1241,7 @@ out:
>
> last_connector = NULL;
>
> - for (i = 0; i < o->count; i++) {
> - drmModeFreeEncoder(o->kencoder[i]);
> - drmModeFreeConnector(o->kconnector[i]);
> - }
> + free_test_output(o);
> }
>
> static int run_test(int duration, int flags)
> @@ -1261,6 +1268,7 @@ static int run_test(int duration, int flags)
> if (o.mode_valid)
> modes++;
>
> + free_test_output(&o);
> }
> }
>
> @@ -1318,6 +1326,7 @@ static int run_pair(int duration, int flags)
> if (o.mode_valid)
> modes++;
>
> + free_test_output(&o);
> }
> }
> }
> --
> 1.8.3.1
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 1/6] igt_kms: don't leak the mmap used for cairo
2013-12-30 15:56 [PATCH 1/6] igt_kms: don't leak the mmap used for cairo Paulo Zanoni
` (4 preceding siblings ...)
2013-12-30 15:56 ` [PATCH 6/6] tests/kms_flip: free the test_output struct when counting modes Paulo Zanoni
@ 2013-12-30 17:40 ` Chris Wilson
2013-12-30 17:53 ` Paulo Zanoni
5 siblings, 1 reply; 12+ messages in thread
From: Chris Wilson @ 2013-12-30 17:40 UTC (permalink / raw)
To: Paulo Zanoni; +Cc: intel-gfx, Paulo Zanoni
On Mon, Dec 30, 2013 at 01:56:48PM -0200, Paulo Zanoni wrote:
> From: Paulo Zanoni <paulo.r.zanoni@intel.com>
>
> When we call kmstest_get_cairo_ctx() and create a context, we do a
> gem_mmap. The problem is that we lose the mmap pointer, so we leak it.
> This patch stores the pointer and frees it at kmstest_remove_fb.
>
> Huge test suites like kms_flip do this operation thousands of times,
> which makes the virtual memory size increase until the test suite gets
> killed. Today, without this patch, we can't even run 50% of the
> kms_flip tests due to this problem. To test this, just "./kms_flip",
> then run "top" and sort it by the VIRT column.
Bleh. kmstest is a bad example of how to handle cairo object lifetimes
and this patch just makes it worse.
-Chris
--
Chris Wilson, Intel Open Source Technology Centre
^ permalink raw reply [flat|nested] 12+ messages in thread* Re: [PATCH 1/6] igt_kms: don't leak the mmap used for cairo
2013-12-30 17:40 ` [PATCH 1/6] igt_kms: don't leak the mmap used for cairo Chris Wilson
@ 2013-12-30 17:53 ` Paulo Zanoni
2013-12-30 18:16 ` Chris Wilson
0 siblings, 1 reply; 12+ messages in thread
From: Paulo Zanoni @ 2013-12-30 17:53 UTC (permalink / raw)
To: Chris Wilson, Paulo Zanoni, Intel Graphics Development,
Paulo Zanoni
2013/12/30 Chris Wilson <chris@chris-wilson.co.uk>:
> On Mon, Dec 30, 2013 at 01:56:48PM -0200, Paulo Zanoni wrote:
>> From: Paulo Zanoni <paulo.r.zanoni@intel.com>
>>
>> When we call kmstest_get_cairo_ctx() and create a context, we do a
>> gem_mmap. The problem is that we lose the mmap pointer, so we leak it.
>> This patch stores the pointer and frees it at kmstest_remove_fb.
>>
>> Huge test suites like kms_flip do this operation thousands of times,
>> which makes the virtual memory size increase until the test suite gets
>> killed. Today, without this patch, we can't even run 50% of the
>> kms_flip tests due to this problem. To test this, just "./kms_flip",
>> then run "top" and sort it by the VIRT column.
>
> Bleh. kmstest is a bad example of how to handle cairo object lifetimes
> and this patch just makes it worse.
I definitely can't say I love the current code, but we need a fix
somehow. Do you have any specific suggestions that I could try?
> -Chris
>
> --
> Chris Wilson, Intel Open Source Technology Centre
--
Paulo Zanoni
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 1/6] igt_kms: don't leak the mmap used for cairo
2013-12-30 17:53 ` Paulo Zanoni
@ 2013-12-30 18:16 ` Chris Wilson
2013-12-31 12:32 ` Chris Wilson
0 siblings, 1 reply; 12+ messages in thread
From: Chris Wilson @ 2013-12-30 18:16 UTC (permalink / raw)
To: Paulo Zanoni; +Cc: Intel Graphics Development, Paulo Zanoni
On Mon, Dec 30, 2013 at 03:53:10PM -0200, Paulo Zanoni wrote:
> 2013/12/30 Chris Wilson <chris@chris-wilson.co.uk>:
> > On Mon, Dec 30, 2013 at 01:56:48PM -0200, Paulo Zanoni wrote:
> >> From: Paulo Zanoni <paulo.r.zanoni@intel.com>
> >>
> >> When we call kmstest_get_cairo_ctx() and create a context, we do a
> >> gem_mmap. The problem is that we lose the mmap pointer, so we leak it.
> >> This patch stores the pointer and frees it at kmstest_remove_fb.
> >>
> >> Huge test suites like kms_flip do this operation thousands of times,
> >> which makes the virtual memory size increase until the test suite gets
> >> killed. Today, without this patch, we can't even run 50% of the
> >> kms_flip tests due to this problem. To test this, just "./kms_flip",
> >> then run "top" and sort it by the VIRT column.
> >
> > Bleh. kmstest is a bad example of how to handle cairo object lifetimes
> > and this patch just makes it worse.
>
> I definitely can't say I love the current code, but we need a fix
> somehow. Do you have any specific suggestions that I could try?
I have something I can push in the near future.
-Chris
--
Chris Wilson, Intel Open Source Technology Centre
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 1/6] igt_kms: don't leak the mmap used for cairo
2013-12-30 18:16 ` Chris Wilson
@ 2013-12-31 12:32 ` Chris Wilson
2014-01-02 16:44 ` Paulo Zanoni
0 siblings, 1 reply; 12+ messages in thread
From: Chris Wilson @ 2013-12-31 12:32 UTC (permalink / raw)
To: Paulo Zanoni, Intel Graphics Development, Paulo Zanoni
On Mon, Dec 30, 2013 at 06:16:20PM +0000, Chris Wilson wrote:
> On Mon, Dec 30, 2013 at 03:53:10PM -0200, Paulo Zanoni wrote:
> > 2013/12/30 Chris Wilson <chris@chris-wilson.co.uk>:
> > > On Mon, Dec 30, 2013 at 01:56:48PM -0200, Paulo Zanoni wrote:
> > >> From: Paulo Zanoni <paulo.r.zanoni@intel.com>
> > >>
> > >> When we call kmstest_get_cairo_ctx() and create a context, we do a
> > >> gem_mmap. The problem is that we lose the mmap pointer, so we leak it.
> > >> This patch stores the pointer and frees it at kmstest_remove_fb.
> > >>
> > >> Huge test suites like kms_flip do this operation thousands of times,
> > >> which makes the virtual memory size increase until the test suite gets
> > >> killed. Today, without this patch, we can't even run 50% of the
> > >> kms_flip tests due to this problem. To test this, just "./kms_flip",
> > >> then run "top" and sort it by the VIRT column.
> > >
> > > Bleh. kmstest is a bad example of how to handle cairo object lifetimes
> > > and this patch just makes it worse.
> >
> > I definitely can't say I love the current code, but we need a fix
> > somehow. Do you have any specific suggestions that I could try?
>
> I have something I can push in the near future.
I've taken care of this GEM bo leak, thanks.
-Chris
--
Chris Wilson, Intel Open Source Technology Centre
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 1/6] igt_kms: don't leak the mmap used for cairo
2013-12-31 12:32 ` Chris Wilson
@ 2014-01-02 16:44 ` Paulo Zanoni
0 siblings, 0 replies; 12+ messages in thread
From: Paulo Zanoni @ 2014-01-02 16:44 UTC (permalink / raw)
To: Chris Wilson, Paulo Zanoni, Intel Graphics Development,
Paulo Zanoni
2013/12/31 Chris Wilson <chris@chris-wilson.co.uk>:
> On Mon, Dec 30, 2013 at 06:16:20PM +0000, Chris Wilson wrote:
>> On Mon, Dec 30, 2013 at 03:53:10PM -0200, Paulo Zanoni wrote:
>> > 2013/12/30 Chris Wilson <chris@chris-wilson.co.uk>:
>> > > On Mon, Dec 30, 2013 at 01:56:48PM -0200, Paulo Zanoni wrote:
>> > >> From: Paulo Zanoni <paulo.r.zanoni@intel.com>
>> > >>
>> > >> When we call kmstest_get_cairo_ctx() and create a context, we do a
>> > >> gem_mmap. The problem is that we lose the mmap pointer, so we leak it.
>> > >> This patch stores the pointer and frees it at kmstest_remove_fb.
>> > >>
>> > >> Huge test suites like kms_flip do this operation thousands of times,
>> > >> which makes the virtual memory size increase until the test suite gets
>> > >> killed. Today, without this patch, we can't even run 50% of the
>> > >> kms_flip tests due to this problem. To test this, just "./kms_flip",
>> > >> then run "top" and sort it by the VIRT column.
>> > >
>> > > Bleh. kmstest is a bad example of how to handle cairo object lifetimes
>> > > and this patch just makes it worse.
>> >
>> > I definitely can't say I love the current code, but we need a fix
>> > somehow. Do you have any specific suggestions that I could try?
>>
>> I have something I can push in the near future.
>
> I've taken care of this GEM bo leak, thanks.
Yay \o/
I confirm your patch fixes the leak. Thanks!!
> -Chris
>
> --
> Chris Wilson, Intel Open Source Technology Centre
--
Paulo Zanoni
^ permalink raw reply [flat|nested] 12+ messages in thread
end of thread, other threads:[~2014-01-07 7:15 UTC | newest]
Thread overview: 12+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-12-30 15:56 [PATCH 1/6] igt_kms: don't leak the mmap used for cairo Paulo Zanoni
2013-12-30 15:56 ` [PATCH 2/6] drmtest: don't leak memory when parsing the arguments Paulo Zanoni
2013-12-30 15:56 ` [PATCH 3/6] tests/kms_flip: don't leak the connector when setting DPMS Paulo Zanoni
2013-12-30 15:56 ` [PATCH 4/6] tests/kms_flip: don't leak the CRTC Paulo Zanoni
2013-12-30 15:56 ` [PATCH 5/6] tests/kms_flip: don't leak the connector_config struct Paulo Zanoni
2013-12-30 15:56 ` [PATCH 6/6] tests/kms_flip: free the test_output struct when counting modes Paulo Zanoni
2014-01-07 7:16 ` Daniel Vetter
2013-12-30 17:40 ` [PATCH 1/6] igt_kms: don't leak the mmap used for cairo Chris Wilson
2013-12-30 17:53 ` Paulo Zanoni
2013-12-30 18:16 ` Chris Wilson
2013-12-31 12:32 ` Chris Wilson
2014-01-02 16:44 ` Paulo Zanoni
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox