From mboxrd@z Thu Jan 1 00:00:00 1970 From: Eric Anholt Subject: Re: [PATCH i-g-t v2 3/3] igt: Add VC4 purgeable BO tests Date: Tue, 21 Nov 2017 10:30:15 -0800 Message-ID: <87tvxnv614.fsf@anholt.net> References: <20171004102221.23085-1-boris.brezillon@free-electrons.com> <20171004102221.23085-4-boris.brezillon@free-electrons.com> Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="===============1762893893==" Return-path: Received: from anholt.net (anholt.net [50.246.234.109]) by gabe.freedesktop.org (Postfix) with ESMTP id 5A7476E1D8 for ; Tue, 21 Nov 2017 18:30:19 +0000 (UTC) In-Reply-To: <20171004102221.23085-4-boris.brezillon@free-electrons.com> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: intel-gfx-bounces@lists.freedesktop.org Sender: "Intel-gfx" To: Boris Brezillon Cc: Intel GFX discussion List-Id: intel-gfx@lists.freedesktop.org --===============1762893893== Content-Type: multipart/signed; boundary="=-=-="; micalg=pgp-sha512; protocol="application/pgp-signature" --=-=-= Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable Boris Brezillon writes: > Signed-off-by: Boris Brezillon > --- > 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 =3D \ > 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 =C2=A9 2017 Broadcom > + * > + * Permission is hereby granted, free of charge, to any person obtaining= a > + * copy of this software and associated documentation files (the "Softwa= re"), > + * to deal in the Software without restriction, including without limita= tion > + * the rights to use, copy, modify, merge, publish, distribute, sublicen= se, > + * 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, EXPRE= SS OR > + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILI= TY, > + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SH= ALL > + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR = OTHER > + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISI= NG > + * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER D= EALINGS > + * IN THE SOFTWARE. > + */ > + > +#include "igt.h" > +#include "igt_vc4.h" > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#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 =3D { > + .size =3D size, > + }; > + > + while (true) { > + if (igt_ioctl(fd, DRM_IOCTL_VC4_CREATE_BO, &create)) > + break; > + > + bo =3D malloc(sizeof(*bo)); > + igt_assert(bo); > + bo->handle =3D create.handle; > + bo->size =3D create.size; > + bo->map =3D 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 =3D 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 *l= ist) > +{ > + 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 =3D 0; > + > + fd =3D 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 =3D 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 =3D 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 =3D igt_list_first_entry(&list, bo, node); > + map =3D (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 =3D setjmp(jmp); > + if (!ret) > + *map =3D 0xdeadbeef; > + else > + igt_assert(ret =3D=3D 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 =3D igt_list_first_entry(&list, bo, node); > + map =3D (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 =3D setjmp(jmp); > + if (!ret) > + *map =3D 0; > + else > + igt_assert(ret =3D=3D 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 =3D 0? Do we need a volatile write in that case, to make sure the compiler doesn't move *map=3D0 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 =3D (uint32_t *)bo->map; > + *map =3D 0xdeadbeef; > + igt_vc4_purgeable_bo(fd, bo->handle, true); > + } > + > + igt_list_for_each(bo, &list, node) { > + map =3D (uint32_t *)bo->map; > + if (igt_vc4_purgeable_bo(fd, bo->handle, false)) > + igt_assert(*map =3D=3D 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 =3D igt_list_first_entry(&list, bo, node); > + map =3D (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 =3D setjmp(jmp); > + if (!ret) > + *map =3D 0; > + else > + igt_assert(ret =3D=3D SIGBUS); > + signal(SIGBUS, SIG_DFL); > + signal(SIGSEGV, SIG_DFL); > + } > + > + igt_subtest("free-purged-bo") { > + igt_vc4_purgeable_subtest_prepare(fd, &list); > + bo =3D 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); > +} > --=20 > 2.11.0 --=-=-= Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- iQIzBAEBCgAdFiEE/JuuFDWp9/ZkuCBXtdYpNtH8nugFAloUcLcACgkQtdYpNtH8 nuihIA//apCs/3CNifuc4XFYDfjQS7WkFfnjYQqp+AzoPtMfUVkylnUVDc+06OcJ Yq+MoyLLb/zMq26volAZ8y0OVxXZcqKtMsON9NoXjtJ3vpQKsH/QFZe5LwMlW+9J +ntW+3Cqw1EfEm0oF1h+aDZn/r+HqDq+9g3IPy1t3qxzBOuo0hZGDtBPCYNSZxDO o06e7Fh8CDgstSykXD1Y3jAcTpfPlOacKLDV86AAHh4VOyXWr5Qsq4eMJ+M2f2IX DT0QxJhaC9IfIFX9w1RSy0aj3yIoOp97DxA5mH26KFf2Op0mLKvBpl9rbCrPLTZQ mkRJNlZmYKEKBawnDfII3iDJwbOvqjyvqXBKWJBJITpSE5CrCj+wMqcpZ6f59GWd ntY14oW1H4KuFpCfSASxOcc84jj4M6Ry7Uh5HtHX8Tfs/HwHtt3ulWv5prqr8ZQ7 VFACmyl9p/nQpx4aVoaBMT1EL6SciMdsKwWAiOJt+hgJOewxGRMzdkkLWQZ5Nq9u ahq7uIGd0GL0hYfOoKrOIQcXnDzqwdcMe7vGvFmqZpx9Y/V8JjoBGgxrLj9WCwE1 0QdqAHaqJbpvI3hxb1TecciWF9DJ6cBIbHR69xILyvbFQhds60idHYDiBes85xSI Fd76I00/rCdq90yMz8Fct5ND4+Nb2DKzk56NleDWxzaKZBqyftA= =mI+I -----END PGP SIGNATURE----- --=-=-=-- --===============1762893893== Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: base64 Content-Disposition: inline X19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX18KSW50ZWwtZ2Z4 IG1haWxpbmcgbGlzdApJbnRlbC1nZnhAbGlzdHMuZnJlZWRlc2t0b3Aub3JnCmh0dHBzOi8vbGlz dHMuZnJlZWRlc2t0b3Aub3JnL21haWxtYW4vbGlzdGluZm8vaW50ZWwtZ2Z4Cg== --===============1762893893==--