[i-g-t] gem_ppgtt: Test VMA leak on context destruction
diff mbox

Message ID 1441981871-28054-1-git-send-email-tvrtko.ursulin@linux.intel.com
State New
Headers show

Commit Message

Tvrtko Ursulin Sept. 11, 2015, 2:31 p.m. UTC
From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>

Test that VMAs associated with a context are cleaned up when
contexts are destroyed.

In practice this emulates the leak seen between fbcon and X server.
Every time the X server exits we gain one VMA on the fbcon frame
buffer object as externally visible via for example
/sys/kernel/debug/dri/0/i915_gem_gtt.

Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
---
 tests/gem_ppgtt.c | 80 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 80 insertions(+)

Comments

Thomas Wood Sept. 18, 2015, 11:17 a.m. UTC | #1
On 11 September 2015 at 15:31, Tvrtko Ursulin
<tvrtko.ursulin@linux.intel.com> wrote:
> From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
>
> Test that VMAs associated with a context are cleaned up when
> contexts are destroyed.
>
> In practice this emulates the leak seen between fbcon and X server.
> Every time the X server exits we gain one VMA on the fbcon frame
> buffer object as externally visible via for example
> /sys/kernel/debug/dri/0/i915_gem_gtt.
>
> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> ---
>  tests/gem_ppgtt.c | 80 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 80 insertions(+)
>
> diff --git a/tests/gem_ppgtt.c b/tests/gem_ppgtt.c
> index 4f6df063214a..363f9d701585 100644
> --- a/tests/gem_ppgtt.c
> +++ b/tests/gem_ppgtt.c
> @@ -265,6 +265,83 @@ static void flink_and_close(void)
>         close(fd2);
>  }
>
> +static int grep_name(char *match, int to_match)

This could be added to igt_debugfs.c, as it might be useful for other tests.


> +{
> +       int fdd, ret, matched;
> +
> +       fdd = open("/sys/kernel/debug/dri/0/i915_gem_gtt", O_RDONLY);

igt_debugfs_open would be more robust here, as it checks various
locations for the debugfs and also attempts to mount it if it can't be
found.


> +       igt_assert(fdd >= 0);
> +
> +       matched = 0;
> +       do {
> +               char ch;
> +
> +               ret = read(fdd, &ch, 1);
> +               if (ret == 0)
> +                       break;
> +               igt_assert(ret == 1);
> +
> +               if (ch == match[matched])
> +                       matched++;
> +               else
> +                       matched = 0;
> +       } while (matched < to_match);

igt_debugfs_fopen is also available, which would allow the use of
getline and strstr here instead, as a slightly simpler implementation.


> +
> +       close(fdd);
> +
> +       return matched;
> +}
> +
> +static void flink_and_exit(void)
> +{
> +       uint32_t fd, fd2;
> +       uint32_t bo, flinked_bo, name;
> +       char match[100];
> +       int matched, to_match;
> +       int retry = 0;
> +       const int retries = 50;
> +
> +       fd = drm_open_any();
> +       igt_require(uses_full_ppgtt(fd));
> +
> +       bo = gem_create(fd, 4096);
> +       name = gem_flink(fd, bo);
> +
> +       to_match  = snprintf(match, sizeof(match), "(name: %u)", name);
> +       igt_assert(to_match < sizeof(match));
> +
> +       fd2 = drm_open_any();
> +
> +       flinked_bo = gem_open(fd2, name);
> +       exec_and_get_offset(fd2, flinked_bo);
> +       gem_sync(fd2, flinked_bo);
> +
> +       /* Verify looking for string works OK. */
> +       matched = grep_name(match, to_match);
> +       igt_assert_eq(matched, to_match);
> +
> +       gem_close(fd2, flinked_bo);
> +
> +       /* Close the context. */
> +       close(fd2);
> +
> +retry:
> +       /* Give cleanup some time to run. */
> +       usleep(100000);
> +
> +       /* The flinked bo VMA should have been cleared now, so list of VMAs
> +        * in debugfs should not contain the one for the imported object.
> +        */
> +       matched = grep_name(match, to_match);
> +       if (matched >= to_match && retry++ < retries)
> +               goto retry;
> +
> +       igt_assert_lt(matched, to_match);
> +
> +       gem_close(fd, bo);
> +       close(fd);
> +}
> +
>  #define N_CHILD 8
>  int main(int argc, char **argv)
>  {
> @@ -297,5 +374,8 @@ int main(int argc, char **argv)
>         igt_subtest("flink-and-close-vma-leak")
>                 flink_and_close();
>
> +       igt_subtest("flink-and-exit-vma-leak")
> +               flink_and_exit();
> +
>         igt_exit();
>  }
> --
> 2.5.1
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Tvrtko Ursulin Sept. 23, 2015, 12:36 p.m. UTC | #2
Hi,

On 09/18/2015 12:17 PM, Thomas Wood wrote:
> On 11 September 2015 at 15:31, Tvrtko Ursulin
> <tvrtko.ursulin@linux.intel.com> wrote:
>> From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
>>
>> Test that VMAs associated with a context are cleaned up when
>> contexts are destroyed.
>>
>> In practice this emulates the leak seen between fbcon and X server.
>> Every time the X server exits we gain one VMA on the fbcon frame
>> buffer object as externally visible via for example
>> /sys/kernel/debug/dri/0/i915_gem_gtt.
>>
>> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
>> ---
>>   tests/gem_ppgtt.c | 80 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
>>   1 file changed, 80 insertions(+)
>>
>> diff --git a/tests/gem_ppgtt.c b/tests/gem_ppgtt.c
>> index 4f6df063214a..363f9d701585 100644
>> --- a/tests/gem_ppgtt.c
>> +++ b/tests/gem_ppgtt.c
>> @@ -265,6 +265,83 @@ static void flink_and_close(void)
>>          close(fd2);
>>   }
>>
>> +static int grep_name(char *match, int to_match)
>
> This could be added to igt_debugfs.c, as it might be useful for other tests.

I couldn't force myself to do it.

>
>> +{
>> +       int fdd, ret, matched;
>> +
>> +       fdd = open("/sys/kernel/debug/dri/0/i915_gem_gtt", O_RDONLY);
>
> igt_debugfs_open would be more robust here, as it checks various
> locations for the debugfs and also attempts to mount it if it can't be
> found.

Done.

>
>
>> +       igt_assert(fdd >= 0);
>> +
>> +       matched = 0;
>> +       do {
>> +               char ch;
>> +
>> +               ret = read(fdd, &ch, 1);
>> +               if (ret == 0)
>> +                       break;
>> +               igt_assert(ret == 1);
>> +
>> +               if (ch == match[matched])
>> +                       matched++;
>> +               else
>> +                       matched = 0;
>> +       } while (matched < to_match);
>
> igt_debugfs_fopen is also available, which would allow the use of
> getline and strstr here instead, as a slightly simpler implementation.

Also done. Ah not via fopen but open+fdopen... drat..

You want v3 or you can live with v2?

Regards,

Tvrtko
Thomas Wood Sept. 23, 2015, 4:16 p.m. UTC | #3
On 23 September 2015 at 13:36, Tvrtko Ursulin
<tvrtko.ursulin@linux.intel.com> wrote:
>
> Hi,
>
> On 09/18/2015 12:17 PM, Thomas Wood wrote:
>>
>> On 11 September 2015 at 15:31, Tvrtko Ursulin
>> <tvrtko.ursulin@linux.intel.com> wrote:
>>>
>>> From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
>>>
>>> Test that VMAs associated with a context are cleaned up when
>>> contexts are destroyed.
>>>
>>> In practice this emulates the leak seen between fbcon and X server.
>>> Every time the X server exits we gain one VMA on the fbcon frame
>>> buffer object as externally visible via for example
>>> /sys/kernel/debug/dri/0/i915_gem_gtt.
>>>
>>> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
>>> ---
>>>   tests/gem_ppgtt.c | 80
>>> +++++++++++++++++++++++++++++++++++++++++++++++++++++++
>>>   1 file changed, 80 insertions(+)
>>>
>>> diff --git a/tests/gem_ppgtt.c b/tests/gem_ppgtt.c
>>> index 4f6df063214a..363f9d701585 100644
>>> --- a/tests/gem_ppgtt.c
>>> +++ b/tests/gem_ppgtt.c
>>> @@ -265,6 +265,83 @@ static void flink_and_close(void)
>>>          close(fd2);
>>>   }
>>>
>>> +static int grep_name(char *match, int to_match)
>>
>>
>> This could be added to igt_debugfs.c, as it might be useful for other
>> tests.
>
>
> I couldn't force myself to do it.

I've sent a follow up patch to move the function into the library and
use it in gem_ppgtt and gem_tiled_swapping.

>
>>
>>> +{
>>> +       int fdd, ret, matched;
>>> +
>>> +       fdd = open("/sys/kernel/debug/dri/0/i915_gem_gtt", O_RDONLY);
>>
>>
>> igt_debugfs_open would be more robust here, as it checks various
>> locations for the debugfs and also attempts to mount it if it can't be
>> found.
>
>
> Done.
>
>>
>>
>>> +       igt_assert(fdd >= 0);
>>> +
>>> +       matched = 0;
>>> +       do {
>>> +               char ch;
>>> +
>>> +               ret = read(fdd, &ch, 1);
>>> +               if (ret == 0)
>>> +                       break;
>>> +               igt_assert(ret == 1);
>>> +
>>> +               if (ch == match[matched])
>>> +                       matched++;
>>> +               else
>>> +                       matched = 0;
>>> +       } while (matched < to_match);
>>
>>
>> igt_debugfs_fopen is also available, which would allow the use of
>> getline and strstr here instead, as a slightly simpler implementation.
>
>
> Also done. Ah not via fopen but open+fdopen... drat..

The follow up patch I've sent addresses this and a few other minor issues.


>
> You want v3 or you can live with v2?

Unfortunately another version will be needed, as drm_open_any has been
replaced by drm_open_driver.


>
> Regards,
>
> Tvrtko
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Patch
diff mbox

diff --git a/tests/gem_ppgtt.c b/tests/gem_ppgtt.c
index 4f6df063214a..363f9d701585 100644
--- a/tests/gem_ppgtt.c
+++ b/tests/gem_ppgtt.c
@@ -265,6 +265,83 @@  static void flink_and_close(void)
 	close(fd2);
 }
 
+static int grep_name(char *match, int to_match)
+{
+	int fdd, ret, matched;
+
+	fdd = open("/sys/kernel/debug/dri/0/i915_gem_gtt", O_RDONLY);
+	igt_assert(fdd >= 0);
+
+	matched = 0;
+	do {
+		char ch;
+
+		ret = read(fdd, &ch, 1);
+		if (ret == 0)
+			break;
+		igt_assert(ret == 1);
+
+		if (ch == match[matched])
+			matched++;
+		else
+			matched = 0;
+	} while (matched < to_match);
+
+	close(fdd);
+
+	return matched;
+}
+
+static void flink_and_exit(void)
+{
+	uint32_t fd, fd2;
+	uint32_t bo, flinked_bo, name;
+	char match[100];
+	int matched, to_match;
+	int retry = 0;
+	const int retries = 50;
+
+	fd = drm_open_any();
+	igt_require(uses_full_ppgtt(fd));
+
+	bo = gem_create(fd, 4096);
+	name = gem_flink(fd, bo);
+
+	to_match  = snprintf(match, sizeof(match), "(name: %u)", name);
+	igt_assert(to_match < sizeof(match));
+
+	fd2 = drm_open_any();
+
+	flinked_bo = gem_open(fd2, name);
+	exec_and_get_offset(fd2, flinked_bo);
+	gem_sync(fd2, flinked_bo);
+
+	/* Verify looking for string works OK. */
+	matched = grep_name(match, to_match);
+	igt_assert_eq(matched, to_match);
+
+	gem_close(fd2, flinked_bo);
+
+	/* Close the context. */
+	close(fd2);
+
+retry:
+	/* Give cleanup some time to run. */
+	usleep(100000);
+
+	/* The flinked bo VMA should have been cleared now, so list of VMAs
+	 * in debugfs should not contain the one for the imported object.
+	 */
+	matched = grep_name(match, to_match);
+	if (matched >= to_match && retry++ < retries)
+		goto retry;
+
+	igt_assert_lt(matched, to_match);
+
+	gem_close(fd, bo);
+	close(fd);
+}
+
 #define N_CHILD 8
 int main(int argc, char **argv)
 {
@@ -297,5 +374,8 @@  int main(int argc, char **argv)
 	igt_subtest("flink-and-close-vma-leak")
 		flink_and_close();
 
+	igt_subtest("flink-and-exit-vma-leak")
+		flink_and_exit();
+
 	igt_exit();
 }