| Message ID | 20170927115118.23170-3-boris.brezillon@free-electrons.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Quoting Boris Brezillon (2017-09-27 12:51:18) > Signed-off-by: Boris Brezillon <boris.brezillon@free-electrons.com> > --- > tests/Makefile.sources | 1 + > tests/vc4_purgeable_bo.c | 274 +++++++++++++++++++++++++++++++++++++++++++++++ > 2 files changed, 275 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/vc4_purgeable_bo.c b/tests/vc4_purgeable_bo.c > new file mode 100644 > index 000000000000..e3eaf0c24563 > --- /dev/null > +++ b/tests/vc4_purgeable_bo.c > @@ -0,0 +1,274 @@ > +/* > + * 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) > +{ May I suggest a /proc/sys/vm/drop_caches-esque interface? For when you want to explicitly control reclaim. > + 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); > +} > + > +igt_main > +{ > + struct igt_vc4_bo *bo; > + struct igt_list list; > + uint32_t *map; > + int fd, ret; > + > + igt_fixture { > + fd = drm_open_driver(DRIVER_VC4); > + igt_list_init(&list); > + > + igt_vc4_alloc_mmap_max_bo(fd, &list, 64 * 1024); > + igt_assert(!igt_list_empty(&list)); > + } igt_subtest("mark-willneed") { /* check that setting willneed on a new bo says retained */ } > + > + igt_subtest("mark-purgeable") { > + 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); Hmm, if this fails early, the state of the list is unknown. Subsequent tests depend upon the state being known.... I am not sure if you want to preallocate all the bo, or at least if you do you should walk the list and verify them at the start of each subtest. > + } > + > + igt_subtest("mark-purgeable-twice") { > + 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") { > + 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") { > + 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); Do you do immediate reaping of backing storage and vma on setting purgeable? If the bo has been in use in earlier tests, then it will have pages that it may not free immediately, so the mapping might succeed? (Spots the purged-bo-mem later) I personally hate "may" tests with a passion. If the expected outcome is uncertain, how do you know if what it did matches expectations. > + > + /* 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("mark-unpurgeable-not-purged") { > + igt_list_for_each(bo, &list, node) { > + map = (uint32_t *)bo->map; > + *map = 0xdeadbeef; > + igt_vc4_purgeable_bo(fd, bo->handle, true); > + } /* lots of intervening time for the system to change state? */ > + > + igt_list_for_each(bo, &list, node) { > + map = (uint32_t *)bo->map; > + igt_assert(igt_vc4_purgeable_bo(fd, bo->handle, false)); > + igt_assert(*map == 0xdeadbeef); > + } > + }
Hi Chris, On Wed, 27 Sep 2017 13:07:28 +0100 Chris Wilson <chris@chris-wilson.co.uk> wrote: > Quoting Boris Brezillon (2017-09-27 12:51:18) > > Signed-off-by: Boris Brezillon <boris.brezillon@free-electrons.com> > > --- > > tests/Makefile.sources | 1 + > > tests/vc4_purgeable_bo.c | 274 +++++++++++++++++++++++++++++++++++++++++++++++ > > 2 files changed, 275 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/vc4_purgeable_bo.c b/tests/vc4_purgeable_bo.c > > new file mode 100644 > > index 000000000000..e3eaf0c24563 > > --- /dev/null > > +++ b/tests/vc4_purgeable_bo.c > > @@ -0,0 +1,274 @@ > > +/* > > + * 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) > > +{ > > May I suggest a /proc/sys/vm/drop_caches-esque interface? > For when you want to explicitly control reclaim. Eric suggested to add a debugfs entry to control the purge, I just thought I didn't really need it since I had a way to trigger this mechanism without adding yet another userspace -> kernel interface that will become part of the ABI and will have to be maintained forever. If you think this is preferable, I'll go for the debugfs hook. > > > + 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); > > +} > > + > > +igt_main > > +{ > > + struct igt_vc4_bo *bo; > > + struct igt_list list; > > + uint32_t *map; > > + int fd, ret; > > + > > + igt_fixture { > > + fd = drm_open_driver(DRIVER_VC4); > > + igt_list_init(&list); > > + > > + igt_vc4_alloc_mmap_max_bo(fd, &list, 64 * 1024); > > + igt_assert(!igt_list_empty(&list)); > > + } > > igt_subtest("mark-willneed") { > /* check that setting willneed on a new bo says retained */ > } Will add this test in the next version. > > > + > > + igt_subtest("mark-purgeable") { > > + 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); > > Hmm, if this fails early, the state of the list is unknown. Subsequent > tests depend upon the state being known.... Well, yes, that's one of the problem I have with the current vc4_purgeable_bo testsuite. As soon as one subtest fails it's likely to impact other subtests. > > I am not sure if you want to preallocate all the bo, or at least if you > do you should walk the list and verify them at the start of each subtest. Okay, I'll allocate the BO pool at the beginning of each subtest and release it before leaving the subtest. Note that I'm not guaranteed that the release operation works, which means the next subtest might not be able to allocate new BOs and will fail, but this failure is actually caused by the previous subtest not the one that reports a failure in IGT. > > > + } > > + > > + igt_subtest("mark-purgeable-twice") { > > + 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") { > > + 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") { > > + 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); > > Do you do immediate reaping of backing storage and vma on setting > purgeable? Nope. > If the bo has been in use in earlier tests, then it will have > pages that it may not free immediately, so the mapping might succeed? It's likely what will happen, and that's actually what I intended to test initially. Eric just pointed out that someone else (external to the IGT test) might trigger a purge between the igt_vc4_purgeable_bo() and the *map access. > (Spots the purged-bo-mem later) > > I personally hate "may" tests with a passion. If the expected outcome is > uncertain, how do you know if what it did matches expectations. Well, it's not completely uncertain, we just have 2 valid results: - SIGBUS event is received => the BO has been purged - nothing happens and the *map assignments works => the BO has not been purged yet. > > > + > > + /* 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("mark-unpurgeable-not-purged") { > > + igt_list_for_each(bo, &list, node) { > > + map = (uint32_t *)bo->map; > > + *map = 0xdeadbeef; > > + igt_vc4_purgeable_bo(fd, bo->handle, true); > > + } > > /* lots of intervening time for the system to change state? */ That's true. > > > + > > + igt_list_for_each(bo, &list, node) { > > + map = (uint32_t *)bo->map; > > + igt_assert(igt_vc4_purgeable_bo(fd, bo->handle, false)); > > + igt_assert(*map == 0xdeadbeef); I can change the above 2 lines into: if (igt_vc4_purgeable_bo(fd, bo->handle, false)) igt_assert(*map == 0xdeadbeef); else igt_assert(*map != 0xdeadbeef); so that it works even if someone triggered the purge between igt_vc4_purgeable_bo(true) and igt_vc4_purgeable_bo(false). > > + } > > + } Thanks for your review. Boris
Quoting Boris Brezillon (2017-09-27 13:41:41) > Hi Chris, > > On Wed, 27 Sep 2017 13:07:28 +0100 > Chris Wilson <chris@chris-wilson.co.uk> wrote: > > > Quoting Boris Brezillon (2017-09-27 12:51:18) > > > +static void igt_vc4_trigger_purge(int fd) > > > +{ > > > > May I suggest a /proc/sys/vm/drop_caches-esque interface? > > For when you want to explicitly control reclaim. > > Eric suggested to add a debugfs entry to control the purge, I just > thought I didn't really need it since I had a way to trigger this > mechanism without adding yet another userspace -> kernel interface that > will become part of the ABI and will have to be maintained forever. > > If you think this is preferable, I'll go for the debugfs hook. I think you will find it useful in future. i915's drop-caches also has options to make sure the GPU is idle, delayed frees are flushed, etc. One thing we found useful is that through a debugfs interface, we can pretend to be the shrinker/in-reclaim, setting fs_reclaim_acquire(GFP_KERNEL) around the operation. That gives us better lockdep coverage without having to trigger the shrinker. Our experience says that you will make good use of a drop-caches interface, it won't just be a one test wonder. :) -Chris
On Wed, 27 Sep 2017 13:50:30 +0100 Chris Wilson <chris@chris-wilson.co.uk> wrote: > Quoting Boris Brezillon (2017-09-27 13:41:41) > > Hi Chris, > > > > On Wed, 27 Sep 2017 13:07:28 +0100 > > Chris Wilson <chris@chris-wilson.co.uk> wrote: > > > > > Quoting Boris Brezillon (2017-09-27 12:51:18) > > > > +static void igt_vc4_trigger_purge(int fd) > > > > +{ > > > > > > May I suggest a /proc/sys/vm/drop_caches-esque interface? > > > For when you want to explicitly control reclaim. > > > > Eric suggested to add a debugfs entry to control the purge, I just > > thought I didn't really need it since I had a way to trigger this > > mechanism without adding yet another userspace -> kernel interface that > > will become part of the ABI and will have to be maintained forever. > > > > If you think this is preferable, I'll go for the debugfs hook. > > I think you will find it useful in future. i915's drop-caches also has > options to make sure the GPU is idle, delayed frees are flushed, etc. > One thing we found useful is that through a debugfs interface, we can > pretend to be the shrinker/in-reclaim, setting > fs_reclaim_acquire(GFP_KERNEL) around the operation. That gives us > better lockdep coverage without having to trigger the shrinker. > > Our experience says that you will make good use of a drop-caches > interface, it won't just be a one test wonder. :) Just had a look at i915_drop_caches_fops [1] and it seems over-complicated given what I can do in the VC4 driver: flush memory of BOs that are marked purgeable. Right now there's no shrinker object registered to the MM layer to help the system release memory. The only one who can trigger a purge is the VC4 BO allocator when it fails to allocate CMA memory. Also note that all VC4 BOs are backed by CMA mem, so I'm not sure plugging the BO purge system to the MM shrinker logic makes a lot of sense (is the MM core expecting shrinkers to release memory coming from the CMA pool?) All this to say I'm not comfortable with designing a generic "drop_caches" debugfs hook that would take various options to delimit the scope of the cache-flush request. I'd prefer to have a simple "purge_purgeable_bos" file that does not care about the input value and flushes everything as soon as someone writes to it. But let's wait for Eric's feedback, maybe he has other plans and a better vision of what will be needed after this simple "purgeable-bo" implementation I'm about to post. Regards, Boris
Boris Brezillon <boris.brezillon@free-electrons.com> writes: > On Wed, 27 Sep 2017 13:50:30 +0100 > Chris Wilson <chris@chris-wilson.co.uk> wrote: > >> Quoting Boris Brezillon (2017-09-27 13:41:41) >> > Hi Chris, >> > >> > On Wed, 27 Sep 2017 13:07:28 +0100 >> > Chris Wilson <chris@chris-wilson.co.uk> wrote: >> > >> > > Quoting Boris Brezillon (2017-09-27 12:51:18) >> > > > +static void igt_vc4_trigger_purge(int fd) >> > > > +{ >> > > >> > > May I suggest a /proc/sys/vm/drop_caches-esque interface? >> > > For when you want to explicitly control reclaim. >> > >> > Eric suggested to add a debugfs entry to control the purge, I just >> > thought I didn't really need it since I had a way to trigger this >> > mechanism without adding yet another userspace -> kernel interface that >> > will become part of the ABI and will have to be maintained forever. >> > >> > If you think this is preferable, I'll go for the debugfs hook. >> >> I think you will find it useful in future. i915's drop-caches also has >> options to make sure the GPU is idle, delayed frees are flushed, etc. >> One thing we found useful is that through a debugfs interface, we can >> pretend to be the shrinker/in-reclaim, setting >> fs_reclaim_acquire(GFP_KERNEL) around the operation. That gives us >> better lockdep coverage without having to trigger the shrinker. >> >> Our experience says that you will make good use of a drop-caches >> interface, it won't just be a one test wonder. :) > > Just had a look at i915_drop_caches_fops [1] and it seems > over-complicated given what I can do in the VC4 driver: flush memory of > BOs that are marked purgeable. > > Right now there's no shrinker object registered to the MM layer to help > the system release memory. The only one who can trigger a purge is the > VC4 BO allocator when it fails to allocate CMA memory. > Also note that all VC4 BOs are backed by CMA mem, so I'm not sure > plugging the BO purge system to the MM shrinker logic makes a lot of > sense (is the MM core expecting shrinkers to release memory coming from > the CMA pool?) Given that general page cache stuff can live in CMA, freeing CMA memory from your shrinker callback should be good for MM. So, yeah, it would be great if (in a later patchset) the mesa BO cache gets purged when the system is busy doing non-graphics tasks and wants the memory back. Also, I just landed the userspace side of BO labeling, so /debug/dri/0/bo_stats will have a lot more useful information in it. We should probably have the mark-purgeable path in the kernel label the BO as purgeable (erasing whatever previous label the BO had). Then, maybe after we can make it so that most allocations label their BOs, not just debug Mesa builds. Need to do some performance testing there. > All this to say I'm not comfortable with designing a generic > "drop_caches" debugfs hook that would take various options to delimit > the scope of the cache-flush request. I'd prefer to have a simple > "purge_purgeable_bos" file that does not care about the input value and > flushes everything as soon as someone writes to it. > But let's wait for Eric's feedback, maybe he has other plans and a > better vision of what will be needed after this simple "purgeable-bo" > implementation I'm about to post. I thought your use of allocations to force purging was pretty elegant. Also, it means that we'll be driving the purging code from the same codepath as Mesa will be (BO allocation) rather than something slightly different. I think we'll want debugfs to test the shrinker path, since I don't know of another good way for userspace to trigger it reliably without also destabilizing the testing environment.
Boris Brezillon <boris.brezillon@free-electrons.com> writes:
> Signed-off-by: Boris Brezillon <boris.brezillon@free-electrons.com>
Another test we should have: Queue up a big rendering job (Copy a
2048x2048@32bpp BO?), mark the source purgeable, force the purge, wait
for rendering, make sure we correctly rendered, and maybe have some
sanity-checking of purgeable state of the BO.
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/vc4_purgeable_bo.c b/tests/vc4_purgeable_bo.c new file mode 100644 index 000000000000..e3eaf0c24563 --- /dev/null +++ b/tests/vc4_purgeable_bo.c @@ -0,0 +1,274 @@ +/* + * 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); +} + +igt_main +{ + struct igt_vc4_bo *bo; + struct igt_list list; + uint32_t *map; + int fd, ret; + + igt_fixture { + fd = drm_open_driver(DRIVER_VC4); + igt_list_init(&list); + + igt_vc4_alloc_mmap_max_bo(fd, &list, 64 * 1024); + igt_assert(!igt_list_empty(&list)); + } + + igt_subtest("mark-purgeable") { + 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") { + 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") { + 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") { + 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") { + struct igt_list tmplist; + + igt_list_init(&tmplist); + + /* 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); + signal(SIGBUS, SIG_DFL); + signal(SIGSEGV, SIG_DFL); + igt_vc4_purgeable_bo(fd, bo->handle, false); + } + + igt_subtest("mark-unpurgeable-not-purged") { + 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; + igt_assert(igt_vc4_purgeable_bo(fd, bo->handle, false)); + igt_assert(*map == 0xdeadbeef); + } + } + + igt_subtest("mark-unpurgeable-purged") { + igt_list_for_each(bo, &list, node) { + map = (uint32_t *)bo->map; + *map = 0xdeadbeef; + igt_vc4_purgeable_bo(fd, bo->handle, true); + } + + /* Trigger a purge. */ + igt_vc4_trigger_purge(fd); + + igt_list_for_each(bo, &list, node) { + map = (uint32_t *)bo->map; + igt_assert(!igt_vc4_purgeable_bo(fd, bo->handle, false)); + igt_assert(*map != 0xdeadbeef); + } + } + + igt_subtest("mark-unpurgeable-nomem") { + struct igt_list tmplist; + + igt_list_for_each(bo, &list, node) + igt_vc4_purgeable_bo(fd, bo->handle, true); + + /* Deplete the CMA pool by allocating as much as we can. This + * should trigger a purge. + */ + igt_list_init(&tmplist); + igt_vc4_alloc_mmap_max_bo(fd, &tmplist, 64 * 1024); + igt_assert(!igt_list_empty(&tmplist)); + + /* Now try to mark all BOs in the intial pool as unpurgeable. + * We shoud see a failure at some point. + */ + igt_list_for_each(bo, &list, node) { + struct drm_vc4_gem_madvise arg = { + .handle = bo->handle, + .madv = VC4_MADV_WILLNEED, + }; + + ret = igt_ioctl(fd, DRM_IOCTL_VC4_GEM_MADVISE, &arg); + if (ret) + break; + } + + igt_assert(ret && errno == ENOMEM); + + /* Release all BOs from the tmplist and try again. This time it + * should succeed. + */ + igt_vc4_unmap_free_bo_pool(fd, &tmplist); + igt_list_for_each(bo, &list, node) + igt_vc4_purgeable_bo(fd, bo->handle, false); + } + + igt_subtest("free-purged-bo") { + 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 { + igt_vc4_unmap_free_bo_pool(fd, &list); + close(fd); + } +}
Signed-off-by: Boris Brezillon <boris.brezillon@free-electrons.com> --- tests/Makefile.sources | 1 + tests/vc4_purgeable_bo.c | 274 +++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 275 insertions(+) create mode 100644 tests/vc4_purgeable_bo.c