On Tue, 21 Nov 2017 10:30:15 -0800
Eric Anholt <eric@xxxxxxxxxx> wrote:
> Boris Brezillon <boris.brezillon@xxxxxxxxxxxxxxxxxx> writes:
>
> > Signed-off-by: Boris Brezillon <boris.brezillon@xxxxxxxxxxxxxxxxxx>
> > ---
> > 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?
Hm, I think we can get rid of the else and unconditionally execute
igt_assert(ret == SIGBUS).
> Do we need a volatile write in that case, to make sure
> the compiler doesn't move *map=0 after igt_assert(false)?
That's a good question. I'm not even sure making map a volatile
variable would guarantee ordering here.
>
> > + 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
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
