Re: [PATCH i-g-t v2 3/3] igt: Add VC4 purgeable BO tests

[Eric Anholt <eric@anholt.net>]

[-- Attachment #1.1: Type: text/plain, Size: 10477 bytes --]

Boris Brezillon <boris.brezillon@free-electrons.com> writes:

> Signed-off-by: Boris Brezillon <boris.brezillon@free-electrons.com>
> ---
> Changes in v2:
> - Add a rule to build vc4 purgeable tests when using Meson (suggested by
>   Petri)
> - Rework the subtests to avoid uncertainty (suggested by Chris)
> - Add a subtest to make sure new BOs have their content retained by
>   default (suggested by Chris)
> - Make sure we are in a known state before starting a subtest even if
>   the previous subtest failed (suggested by Chris)
> - Reworked the tests to adjust to the new MADV behavior (WILLNEED no
>   longer re-allocated the BO if it's been purged)
> - Make the purgeable test dependent on the MADVISE feature (checked
>   with the GET_PARAM ioctl)

I've pushed a rebased version of your series as of
f9b0d7ac84b6579c5248357ab45b0a65b27c6e54 up to the "purgeable" branch of
my tree.  Something I noticed as I was reviewing, though...

> ---
>  tests/Makefile.sources   |   1 +
>  tests/meson.build        |   1 +
>  tests/vc4_purgeable_bo.c | 265 +++++++++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 267 insertions(+)
>  create mode 100644 tests/vc4_purgeable_bo.c
>
> diff --git a/tests/Makefile.sources b/tests/Makefile.sources
> index 0adc28a014d2..c78ac9d27921 100644
> --- a/tests/Makefile.sources
> +++ b/tests/Makefile.sources
> @@ -8,6 +8,7 @@ VC4_TESTS = \
>  	vc4_create_bo \
>  	vc4_dmabuf_poll \
>  	vc4_lookup_fail \
> +	vc4_purgeable_bo \
>  	vc4_wait_bo \
>  	vc4_wait_seqno \
>  	$(NULL)
> diff --git a/tests/meson.build b/tests/meson.build
> index 1c619179fe6a..8f94e3c4385b 100644
> --- a/tests/meson.build
> +++ b/tests/meson.build
> @@ -243,6 +243,7 @@ if libdrm_vc4.found()
>  		'vc4_create_bo',
>  		'vc4_dmabuf_poll',
>  		'vc4_lookup_fail',
> +		'vc4_purgeable_bo',
>  		'vc4_wait_bo',
>  		'vc4_wait_seqno',
>  	]
> diff --git a/tests/vc4_purgeable_bo.c b/tests/vc4_purgeable_bo.c
> new file mode 100644
> index 000000000000..b25581cc532e
> --- /dev/null
> +++ b/tests/vc4_purgeable_bo.c
> @@ -0,0 +1,265 @@
> +/*
> + * Copyright © 2017 Broadcom
> + *
> + * Permission is hereby granted, free of charge, to any person obtaining a
> + * copy of this software and associated documentation files (the "Software"),
> + * to deal in the Software without restriction, including without limitation
> + * the rights to use, copy, modify, merge, publish, distribute, sublicense,
> + * and/or sell copies of the Software, and to permit persons to whom the
> + * Software is furnished to do so, subject to the following conditions:
> + *
> + * The above copyright notice and this permission notice (including the next
> + * paragraph) shall be included in all copies or substantial portions of the
> + * Software.
> + *
> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT SHALL
> + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
> + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
> + * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS
> + * IN THE SOFTWARE.
> + */
> +
> +#include "igt.h"
> +#include "igt_vc4.h"
> +#include <unistd.h>
> +#include <stdlib.h>
> +#include <stdio.h>
> +#include <string.h>
> +#include <fcntl.h>
> +#include <inttypes.h>
> +#include <errno.h>
> +#include <sys/stat.h>
> +#include <sys/ioctl.h>
> +#include "vc4_drm.h"
> +
> +struct igt_vc4_bo {
> +	struct igt_list node;
> +	int handle;
> +	void *map;
> +	size_t size;
> +};
> +
> +static jmp_buf jmp;
> +
> +static void __attribute__((noreturn)) sigtrap(int sig)
> +{
> +	longjmp(jmp, sig);
> +}
> +
> +static void igt_vc4_alloc_mmap_max_bo(int fd, struct igt_list *list,
> +				      size_t size)
> +{
> +	struct igt_vc4_bo *bo;
> +	struct drm_vc4_create_bo create = {
> +		.size = size,
> +	};
> +
> +	while (true) {
> +		if (igt_ioctl(fd, DRM_IOCTL_VC4_CREATE_BO, &create))
> +			break;
> +
> +		bo = malloc(sizeof(*bo));
> +		igt_assert(bo);
> +		bo->handle = create.handle;
> +		bo->size = create.size;
> +		bo->map = igt_vc4_mmap_bo(fd, bo->handle, bo->size,
> +					  PROT_READ | PROT_WRITE);
> +		igt_list_add_tail(&bo->node, list);
> +	}
> +}
> +
> +static void igt_vc4_unmap_free_bo_pool(int fd, struct igt_list *list)
> +{
> +	struct igt_vc4_bo *bo;
> +
> +	while (!igt_list_empty(list)) {
> +		bo = igt_list_first_entry(list, bo, node);
> +		igt_assert(bo);
> +		igt_list_del(&bo->node);
> +		munmap(bo->map, bo->size);
> +		gem_close(fd, bo->handle);
> +		free(bo);
> +	}
> +}
> +
> +static void igt_vc4_trigger_purge(int fd)
> +{
> +	struct igt_list list;
> +
> +	igt_list_init(&list);
> +
> +	/* Try to allocate as much as we can to trigger a purge. */
> +	igt_vc4_alloc_mmap_max_bo(fd, &list, 64 * 1024);
> +	igt_assert(!igt_list_empty(&list));
> +	igt_vc4_unmap_free_bo_pool(fd, &list);
> +}
> +
> +static void igt_vc4_purgeable_subtest_prepare(int fd, struct igt_list *list)
> +{
> +	igt_vc4_unmap_free_bo_pool(fd, list);
> +	igt_vc4_alloc_mmap_max_bo(fd, list, 64 * 1024);
> +	igt_assert(!igt_list_empty(list));
> +}
> +
> +igt_main
> +{
> +	struct igt_vc4_bo *bo;
> +	struct igt_list list;
> +	uint32_t *map;
> +	int fd, ret;
> +
> +	igt_fixture {
> +		uint64_t val = 0;
> +
> +		fd = drm_open_driver(DRIVER_VC4);
> +		igt_vc4_get_param(fd, DRM_VC4_PARAM_SUPPORTS_MADVISE, &val);
> +		igt_require(val);
> +		igt_list_init(&list);
> +	}
> +
> +	igt_subtest("mark-willneed") {
> +		igt_vc4_purgeable_subtest_prepare(fd, &list);
> +		igt_list_for_each(bo, &list, node)
> +			igt_assert(igt_vc4_purgeable_bo(fd, bo->handle,
> +							false));
> +	}
> +
> +	igt_subtest("mark-purgeable") {
> +		igt_vc4_purgeable_subtest_prepare(fd, &list);
> +		igt_list_for_each(bo, &list, node)
> +			igt_vc4_purgeable_bo(fd, bo->handle, true);
> +
> +		igt_list_for_each(bo, &list, node)
> +			igt_vc4_purgeable_bo(fd, bo->handle, false);
> +	}
> +
> +	igt_subtest("mark-purgeable-twice") {
> +		igt_vc4_purgeable_subtest_prepare(fd, &list);
> +		bo = igt_list_first_entry(&list, bo, node);
> +		igt_vc4_purgeable_bo(fd, bo->handle, true);
> +		igt_vc4_purgeable_bo(fd, bo->handle, true);
> +		igt_vc4_purgeable_bo(fd, bo->handle, false);
> +	}
> +
> +	igt_subtest("mark-unpurgeable-twice") {
> +		igt_vc4_purgeable_subtest_prepare(fd, &list);
> +		bo = igt_list_first_entry(&list, bo, node);
> +		igt_vc4_purgeable_bo(fd, bo->handle, true);
> +		igt_vc4_purgeable_bo(fd, bo->handle, false);
> +		igt_vc4_purgeable_bo(fd, bo->handle, false);
> +	}
> +
> +	igt_subtest("access-purgeable-bo-mem") {
> +		igt_vc4_purgeable_subtest_prepare(fd, &list);
> +		bo = igt_list_first_entry(&list, bo, node);
> +		map = (uint32_t *)bo->map;
> +
> +		/* Mark the BO as purgeable, but do not try to allocate a new
> +		 * BO. This should leave the BO in a non-purged state unless
> +		 * someone else tries to allocated a new BO.
> +		 */
> +		igt_vc4_purgeable_bo(fd, bo->handle, true);
> +
> +		/* Accessing a purgeable BO may generate a SIGBUS event if the
> +		 * BO has been purged by the system in the meantime.
> +		 */
> +		signal(SIGSEGV, sigtrap);
> +		signal(SIGBUS, sigtrap);
> +		ret = setjmp(jmp);
> +		if (!ret)
> +			*map = 0xdeadbeef;
> +		else
> +			igt_assert(ret == SIGBUS);
> +		signal(SIGBUS, SIG_DFL);
> +		signal(SIGSEGV, SIG_DFL);
> +	}
> +
> +	igt_subtest("access-purged-bo-mem") {
> +		igt_vc4_purgeable_subtest_prepare(fd, &list);
> +
> +		/* Mark the first BO in our list as purgeable and try to
> +		 * allocate a new one. This should trigger a purge and render
> +		 * the first BO inaccessible.
> +		 */
> +		bo = igt_list_first_entry(&list, bo, node);
> +		map = (uint32_t *)bo->map;
> +		igt_vc4_purgeable_bo(fd, bo->handle, true);
> +
> +		/* Trigger a purge. */
> +		igt_vc4_trigger_purge(fd);
> +
> +		/* Accessing a purged BO should generate a SIGBUS event. */
> +		signal(SIGSEGV, sigtrap);
> +		signal(SIGBUS, sigtrap);
> +		ret = setjmp(jmp);
> +		if (!ret)
> +			*map = 0;
> +		else
> +			igt_assert(ret == SIGBUS);

If we didn't take a signal at all, I think this test would exit
successfully, when we don't want it to.  Maybe igt_assert(false) after
the *map = 0?  Do we need a volatile write in that case, to make sure
the compiler doesn't move *map=0 after igt_assert(false)?

> +		signal(SIGBUS, SIG_DFL);
> +		signal(SIGSEGV, SIG_DFL);
> +		igt_vc4_purgeable_bo(fd, bo->handle, false);
> +	}
> +
> +	igt_subtest("mark-unpurgeable-check-retained") {
> +		igt_vc4_purgeable_subtest_prepare(fd, &list);
> +		igt_list_for_each(bo, &list, node) {
> +			map = (uint32_t *)bo->map;
> +			*map = 0xdeadbeef;
> +			igt_vc4_purgeable_bo(fd, bo->handle, true);
> +		}
> +
> +		igt_list_for_each(bo, &list, node) {
> +			map = (uint32_t *)bo->map;
> +			if (igt_vc4_purgeable_bo(fd, bo->handle, false))
> +				igt_assert(*map == 0xdeadbeef);
> +		}
> +	}
> +
> +	igt_subtest("mark-unpurgeable-purged") {
> +		igt_vc4_purgeable_subtest_prepare(fd, &list);
> +
> +		igt_list_for_each(bo, &list, node)
> +			igt_vc4_purgeable_bo(fd, bo->handle, true);
> +
> +		/* Trigger a purge. */
> +		igt_vc4_trigger_purge(fd);
> +
> +		bo = igt_list_first_entry(&list, bo, node);
> +		map = (uint32_t *)bo->map;
> +
> +		igt_assert(!igt_vc4_purgeable_bo(fd, bo->handle, false));
> +
> +		/* Purged BOs are unusable and any access to their
> +		 * mmap-ed region should trigger a SIGBUS.
> +		 */
> +		signal(SIGSEGV, sigtrap);
> +		signal(SIGBUS, sigtrap);
> +		ret = setjmp(jmp);
> +		if (!ret)
> +			*map = 0;
> +		else
> +			igt_assert(ret == SIGBUS);
> +		signal(SIGBUS, SIG_DFL);
> +		signal(SIGSEGV, SIG_DFL);
> +	}
> +
> +	igt_subtest("free-purged-bo") {
> +		igt_vc4_purgeable_subtest_prepare(fd, &list);
> +		bo = igt_list_first_entry(&list, bo, node);
> +		igt_vc4_purgeable_bo(fd, bo->handle, true);
> +
> +		/* Trigger a purge. */
> +		igt_vc4_trigger_purge(fd);
> +
> +		igt_list_del(&bo->node);
> +		munmap(bo->map, bo->size);
> +		gem_close(fd, bo->handle);
> +		free(bo);
> +	}
> +
> +	igt_fixture
> +		close(fd);
> +}
> -- 
> 2.11.0

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 832 bytes --]

[-- Attachment #2: Type: text/plain, Size: 160 bytes --]

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx