[PATCH 1/5] kselftests: dmabuf-heaps: Fix Makefile's inclusion of the kernel's usr/include dir
Copied in from somewhere else, the makefile was including the kerne's usr/include dir, which caused the asm/ioctl.h file to be used.
Unfortunately, that file has different values for _IOC_SIZEBITS and _IOC_WRITE than include/uapi/asm-generic/ioctl.h which then causes the _IOCW macros to give the wrong ioctl numbers, specifically for DMA_BUF_IOCTL_SYNC.
This patch simply removes the extra include from the Makefile
Cc: Shuah Khan shuah@kernel.org Cc: Brian Starkey brian.starkey@arm.com Cc: Sumit Semwal sumit.semwal@linaro.org Cc: Laura Abbott labbott@kernel.org Cc: Hridya Valsaraju hridya@google.com Cc: Suren Baghdasaryan surenb@google.com Cc: Sandeep Patil sspatil@google.com Cc: Daniel Mentz danielmentz@google.com Cc: linux-media@vger.kernel.org Cc: dri-devel@lists.freedesktop.org Cc: linux-kselftest@vger.kernel.org Fixes: a8779927fd86c ("kselftests: Add dma-heap test") Signed-off-by: John Stultz john.stultz@linaro.org --- tools/testing/selftests/dmabuf-heaps/Makefile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/tools/testing/selftests/dmabuf-heaps/Makefile b/tools/testing/selftests/dmabuf-heaps/Makefile index 607c2acd2082..604b43ece15f 100644 --- a/tools/testing/selftests/dmabuf-heaps/Makefile +++ b/tools/testing/selftests/dmabuf-heaps/Makefile @@ -1,5 +1,5 @@ # SPDX-License-Identifier: GPL-2.0 -CFLAGS += -static -O3 -Wl,-no-as-needed -Wall -I../../../../usr/include +CFLAGS += -static -O3 -Wl,-no-as-needed -Wall
TEST_GEN_PROGS = dmabuf-heap
Add logic to check the dmabuf sync calls succeed.
Cc: Shuah Khan shuah@kernel.org Cc: Brian Starkey brian.starkey@arm.com Cc: Sumit Semwal sumit.semwal@linaro.org Cc: Laura Abbott labbott@kernel.org Cc: Hridya Valsaraju hridya@google.com Cc: Suren Baghdasaryan surenb@google.com Cc: Sandeep Patil sspatil@google.com Cc: Daniel Mentz danielmentz@google.com Cc: linux-media@vger.kernel.org Cc: dri-devel@lists.freedesktop.org Cc: linux-kselftest@vger.kernel.org Signed-off-by: John Stultz john.stultz@linaro.org --- .../selftests/dmabuf-heaps/dmabuf-heap.c | 20 ++++++++++++------- 1 file changed, 13 insertions(+), 7 deletions(-)
diff --git a/tools/testing/selftests/dmabuf-heaps/dmabuf-heap.c b/tools/testing/selftests/dmabuf-heaps/dmabuf-heap.c index 909da9cdda97..46f6759a8acc 100644 --- a/tools/testing/selftests/dmabuf-heaps/dmabuf-heap.c +++ b/tools/testing/selftests/dmabuf-heaps/dmabuf-heap.c @@ -130,16 +130,13 @@ static int dmabuf_heap_alloc(int fd, size_t len, unsigned int flags, dmabuf_fd); }
-static void dmabuf_sync(int fd, int start_stop) +static int dmabuf_sync(int fd, int start_stop) { struct dma_buf_sync sync = { .flags = start_stop | DMA_BUF_SYNC_RW, }; - int ret;
- ret = ioctl(fd, DMA_BUF_IOCTL_SYNC, &sync); - if (ret) - printf("sync failed %d\n", errno); + return ioctl(fd, DMA_BUF_IOCTL_SYNC, &sync); }
#define ONE_MEG (1024 * 1024) @@ -197,9 +194,18 @@ static int test_alloc_and_import(char *heap_name) } printf("import passed\n");
- dmabuf_sync(dmabuf_fd, DMA_BUF_SYNC_START); + ret = dmabuf_sync(dmabuf_fd, DMA_BUF_SYNC_START); + if (ret < 0) { + printf("Sync start failed!\n"); + goto out; + } + memset(p, 0xff, ONE_MEG); - dmabuf_sync(dmabuf_fd, DMA_BUF_SYNC_END); + ret = dmabuf_sync(dmabuf_fd, DMA_BUF_SYNC_END); + if (ret < 0) { + printf("Sync end failed!\n"); + goto out; + } printf("syncs passed\n");
close_handle(importer_fd, handle);
While testing against a vgem device is helpful for testing importing they aren't always configured in, so don't make it a fatal failure.
Cc: Shuah Khan shuah@kernel.org Cc: Brian Starkey brian.starkey@arm.com Cc: Sumit Semwal sumit.semwal@linaro.org Cc: Laura Abbott labbott@kernel.org Cc: Hridya Valsaraju hridya@google.com Cc: Suren Baghdasaryan surenb@google.com Cc: Sandeep Patil sspatil@google.com Cc: Daniel Mentz danielmentz@google.com Cc: linux-media@vger.kernel.org Cc: dri-devel@lists.freedesktop.org Cc: linux-kselftest@vger.kernel.org Signed-off-by: John Stultz john.stultz@linaro.org --- .../testing/selftests/dmabuf-heaps/dmabuf-heap.c | 15 +++++++-------- 1 file changed, 7 insertions(+), 8 deletions(-)
diff --git a/tools/testing/selftests/dmabuf-heaps/dmabuf-heap.c b/tools/testing/selftests/dmabuf-heaps/dmabuf-heap.c index 46f6759a8acc..8cedd539c7fb 100644 --- a/tools/testing/selftests/dmabuf-heaps/dmabuf-heap.c +++ b/tools/testing/selftests/dmabuf-heaps/dmabuf-heap.c @@ -184,15 +184,14 @@ static int test_alloc_and_import(char *heap_name) if (importer_fd < 0) { ret = importer_fd; printf("Failed to open vgem\n"); - goto out; - } - - ret = import_vgem_fd(importer_fd, dmabuf_fd, &handle); - if (ret < 0) { - printf("Failed to import buffer\n"); - goto out; + } else { + ret = import_vgem_fd(importer_fd, dmabuf_fd, &handle); + if (ret < 0) { + printf("Failed to import buffer\n"); + goto out; + } + printf("import passed\n"); } - printf("import passed\n");
ret = dmabuf_sync(dmabuf_fd, DMA_BUF_SYNC_START); if (ret < 0) {
Cleanup the test output so it is a bit easier to read
Cc: Shuah Khan shuah@kernel.org Cc: Brian Starkey brian.starkey@arm.com Cc: Sumit Semwal sumit.semwal@linaro.org Cc: Laura Abbott labbott@kernel.org Cc: Hridya Valsaraju hridya@google.com Cc: Suren Baghdasaryan surenb@google.com Cc: Sandeep Patil sspatil@google.com Cc: Daniel Mentz danielmentz@google.com Cc: linux-media@vger.kernel.org Cc: dri-devel@lists.freedesktop.org Cc: linux-kselftest@vger.kernel.org Signed-off-by: John Stultz john.stultz@linaro.org --- .../selftests/dmabuf-heaps/dmabuf-heap.c | 44 +++++++++---------- 1 file changed, 21 insertions(+), 23 deletions(-)
diff --git a/tools/testing/selftests/dmabuf-heaps/dmabuf-heap.c b/tools/testing/selftests/dmabuf-heaps/dmabuf-heap.c index 8cedd539c7fb..d179d81e2355 100644 --- a/tools/testing/selftests/dmabuf-heaps/dmabuf-heap.c +++ b/tools/testing/selftests/dmabuf-heaps/dmabuf-heap.c @@ -148,16 +148,14 @@ static int test_alloc_and_import(char *heap_name) void *p = NULL; int ret;
- printf("Testing heap: %s\n", heap_name); - heap_fd = dmabuf_heap_open(heap_name); if (heap_fd < 0) return -1;
- printf("Allocating 1 MEG\n"); + printf(" Testing allocation and importing: "); ret = dmabuf_heap_alloc(heap_fd, ONE_MEG, 0, &dmabuf_fd); if (ret) { - printf("Allocation Failed!\n"); + printf("FAIL (Allocation Failed!)\n"); ret = -1; goto out; } @@ -169,11 +167,10 @@ static int test_alloc_and_import(char *heap_name) dmabuf_fd, 0); if (p == MAP_FAILED) { - printf("mmap() failed: %m\n"); + printf("FAIL (mmap() failed)\n"); ret = -1; goto out; } - printf("mmap passed\n");
dmabuf_sync(dmabuf_fd, DMA_BUF_SYNC_START); memset(p, 1, ONE_MEG / 2); @@ -183,33 +180,31 @@ static int test_alloc_and_import(char *heap_name) importer_fd = open_vgem(); if (importer_fd < 0) { ret = importer_fd; - printf("Failed to open vgem\n"); + printf("(Could not open vgem - skipping): "); } else { ret = import_vgem_fd(importer_fd, dmabuf_fd, &handle); if (ret < 0) { - printf("Failed to import buffer\n"); + printf("FAIL (Failed to import buffer)\n"); goto out; } - printf("import passed\n"); }
ret = dmabuf_sync(dmabuf_fd, DMA_BUF_SYNC_START); if (ret < 0) { - printf("Sync start failed!\n"); + printf("FAIL (DMA_BUF_SYNC_START failed!)\n"); goto out; }
memset(p, 0xff, ONE_MEG); ret = dmabuf_sync(dmabuf_fd, DMA_BUF_SYNC_END); if (ret < 0) { - printf("Sync end failed!\n"); + printf("FAIL (DMA_BUF_SYNC_END failed!)\n"); goto out; } - printf("syncs passed\n");
close_handle(importer_fd, handle); ret = 0; - + printf(" OK\n"); out: if (p) munmap(p, ONE_MEG); @@ -297,23 +292,24 @@ static int test_alloc_compat(char *heap_name) if (heap_fd < 0) return -1;
- printf("Testing (theoretical)older alloc compat\n"); + printf(" Testing (theoretical)older alloc compat: "); ret = dmabuf_heap_alloc_older(heap_fd, ONE_MEG, 0, &dmabuf_fd); if (ret) { - printf("Older compat allocation failed!\n"); + printf("FAIL (Older compat allocation failed!)\n"); ret = -1; goto out; } close(dmabuf_fd); + printf("OK\n");
- printf("Testing (theoretical)newer alloc compat\n"); + printf(" Testing (theoretical)newer alloc compat: "); ret = dmabuf_heap_alloc_newer(heap_fd, ONE_MEG, 0, &dmabuf_fd); if (ret) { - printf("Newer compat allocation failed!\n"); + printf("FAIL (Newer compat allocation failed!)\n"); ret = -1; goto out; } - printf("Ioctl compatibility tests passed\n"); + printf("OK\n"); out: if (dmabuf_fd >= 0) close(dmabuf_fd); @@ -332,17 +328,17 @@ static int test_alloc_errors(char *heap_name) if (heap_fd < 0) return -1;
- printf("Testing expected error cases\n"); + printf(" Testing expected error cases: "); ret = dmabuf_heap_alloc(0, ONE_MEG, 0x111111, &dmabuf_fd); if (!ret) { - printf("Did not see expected error (invalid fd)!\n"); + printf("FAIL (Did not see expected error (invalid fd)!)\n"); ret = -1; goto out; }
ret = dmabuf_heap_alloc(heap_fd, ONE_MEG, 0x111111, &dmabuf_fd); if (!ret) { - printf("Did not see expected error (invalid heap flags)!\n"); + printf("FAIL (Did not see expected error (invalid heap flags)!)\n"); ret = -1; goto out; } @@ -350,12 +346,12 @@ static int test_alloc_errors(char *heap_name) ret = dmabuf_heap_alloc_fdflags(heap_fd, ONE_MEG, ~(O_RDWR | O_CLOEXEC), 0, &dmabuf_fd); if (!ret) { - printf("Did not see expected error (invalid fd flags)!\n"); + printf("FAIL (Did not see expected error (invalid fd flags)!)\n"); ret = -1; goto out; }
- printf("Expected error checking passed\n"); + printf("OK\n"); ret = 0; out: if (dmabuf_fd >= 0) @@ -384,6 +380,8 @@ int main(void) if (!strncmp(dir->d_name, "..", 3)) continue;
+ printf("Testing heap: %s\n", dir->d_name); + printf("=======================================\n"); ret = test_alloc_and_import(dir->d_name); if (ret) break;
Add a check to validate that buffers allocated from the heaps are properly zeroed before being given to userland.
It is done by allocating a number of buffers, and filling them with a nonzero pattern, then closing and reallocating more buffers and checking that they are all properly zeroed.
This is helpful to validate any cached buffers are zeroed before being given back out.
Cc: Shuah Khan shuah@kernel.org Cc: Brian Starkey brian.starkey@arm.com Cc: Sumit Semwal sumit.semwal@linaro.org Cc: Laura Abbott labbott@kernel.org Cc: Hridya Valsaraju hridya@google.com Cc: Suren Baghdasaryan surenb@google.com Cc: Sandeep Patil sspatil@google.com Cc: Daniel Mentz danielmentz@google.com Cc: linux-media@vger.kernel.org Cc: dri-devel@lists.freedesktop.org Cc: linux-kselftest@vger.kernel.org Signed-off-by: John Stultz john.stultz@linaro.org --- .../selftests/dmabuf-heaps/dmabuf-heap.c | 86 +++++++++++++++++++ 1 file changed, 86 insertions(+)
diff --git a/tools/testing/selftests/dmabuf-heaps/dmabuf-heap.c b/tools/testing/selftests/dmabuf-heaps/dmabuf-heap.c index d179d81e2355..29af27acd40e 100644 --- a/tools/testing/selftests/dmabuf-heaps/dmabuf-heap.c +++ b/tools/testing/selftests/dmabuf-heaps/dmabuf-heap.c @@ -218,6 +218,84 @@ static int test_alloc_and_import(char *heap_name) return ret; }
+static int test_alloc_zeroed(char *heap_name, size_t size) +{ + int heap_fd = -1, dmabuf_fd[32]; + int i, j, ret; + void *p = NULL; + char *c; + + printf(" Testing alloced %ldk buffers are zeroed: ", size / 1024); + heap_fd = dmabuf_heap_open(heap_name); + if (heap_fd < 0) + return -1; + + /* Allocate and fill a bunch of buffers */ + for (i = 0; i < 32; i++) { + ret = dmabuf_heap_alloc(heap_fd, size, 0, &dmabuf_fd[i]); + if (ret < 0) { + printf("FAIL (Allocation (%i) failed)\n", i); + goto out; + } + /* mmap and fill with simple pattern */ + p = mmap(NULL, size, PROT_READ | PROT_WRITE, MAP_SHARED, dmabuf_fd[i], 0); + if (p == MAP_FAILED) { + printf("FAIL (mmap() failed!)\n"); + ret = -1; + goto out; + } + dmabuf_sync(dmabuf_fd[i], DMA_BUF_SYNC_START); + memset(p, 0xff, size); + dmabuf_sync(dmabuf_fd[i], DMA_BUF_SYNC_END); + munmap(p, size); + } + /* close them all */ + for (i = 0; i < 32; i++) + close(dmabuf_fd[i]); + + /* Allocate and validate all buffers are zeroed */ + for (i = 0; i < 32; i++) { + ret = dmabuf_heap_alloc(heap_fd, size, 0, &dmabuf_fd[i]); + if (ret < 0) { + printf("FAIL (Allocation (%i) failed)\n", i); + goto out; + } + + /* mmap and validate everything is zero */ + p = mmap(NULL, size, PROT_READ | PROT_WRITE, MAP_SHARED, dmabuf_fd[i], 0); + if (p == MAP_FAILED) { + printf("FAIL (mmap() failed!)\n"); + ret = -1; + goto out; + } + dmabuf_sync(dmabuf_fd[i], DMA_BUF_SYNC_START); + c = (char *)p; + for (j = 0; j < size; j++) { + if (c[j] != 0) { + printf("FAIL (Allocated buffer not zeroed @ %i)\n", j); + break; + } + } + dmabuf_sync(dmabuf_fd[i], DMA_BUF_SYNC_END); + munmap(p, size); + } + /* close them all */ + for (i = 0; i < 32; i++) + close(dmabuf_fd[i]); + + close(heap_fd); + printf("OK\n"); + return 0; + +out: + while (i > 0) { + close(dmabuf_fd[i]); + i--; + } + close(heap_fd); + return ret; +} + /* Test the ioctl version compatibility w/ a smaller structure then expected */ static int dmabuf_heap_alloc_older(int fd, size_t len, unsigned int flags, int *dmabuf_fd) @@ -386,6 +464,14 @@ int main(void) if (ret) break;
+ ret = test_alloc_zeroed(dir->d_name, 4 * 1024); + if (ret) + break; + + ret = test_alloc_zeroed(dir->d_name, ONE_MEG); + if (ret) + break; + ret = test_alloc_compat(dir->d_name); if (ret) break;
On 1/28/21 8:05 PM, John Stultz wrote:
Copied in from somewhere else, the makefile was including the kerne's usr/include dir, which caused the asm/ioctl.h file to be used.
Unfortunately, that file has different values for _IOC_SIZEBITS and _IOC_WRITE than include/uapi/asm-generic/ioctl.h which then causes the _IOCW macros to give the wrong ioctl numbers, specifically for DMA_BUF_IOCTL_SYNC.
This patch simply removes the extra include from the Makefile
Cc: Shuah Khan shuah@kernel.org Cc: Brian Starkey brian.starkey@arm.com Cc: Sumit Semwal sumit.semwal@linaro.org Cc: Laura Abbott labbott@kernel.org Cc: Hridya Valsaraju hridya@google.com Cc: Suren Baghdasaryan surenb@google.com Cc: Sandeep Patil sspatil@google.com Cc: Daniel Mentz danielmentz@google.com Cc: linux-media@vger.kernel.org Cc: dri-devel@lists.freedesktop.org Cc: linux-kselftest@vger.kernel.org Fixes: a8779927fd86c ("kselftests: Add dma-heap test") Signed-off-by: John Stultz john.stultz@linaro.org
tools/testing/selftests/dmabuf-heaps/Makefile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/tools/testing/selftests/dmabuf-heaps/Makefile b/tools/testing/selftests/dmabuf-heaps/Makefile index 607c2acd2082..604b43ece15f 100644 --- a/tools/testing/selftests/dmabuf-heaps/Makefile +++ b/tools/testing/selftests/dmabuf-heaps/Makefile @@ -1,5 +1,5 @@ # SPDX-License-Identifier: GPL-2.0 -CFLAGS += -static -O3 -Wl,-no-as-needed -Wall -I../../../../usr/include +CFLAGS += -static -O3 -Wl,-no-as-needed -Wall
TEST_GEN_PROGS = dmabuf-heap
Thanks John for all these 5 fix and cleanup patches.
Applied to linux-kselftest next for 5.12-rc1
thanks, -- Shuah
On Mon, Feb 8, 2021 at 3:23 PM Shuah Khan skhan@linuxfoundation.org wrote:
On 1/28/21 8:05 PM, John Stultz wrote: Thanks John for all these 5 fix and cleanup patches.
Applied to linux-kselftest next for 5.12-rc1
Great! I was just prepping to resend them :) Thanks so much! -john
dri-devel@lists.freedesktop.org
-
John Stultz -
Shuah Khan