[PATCH] fix dma-buf/udmabuf selftest

[PATCH] fix dma-buf/udmabuf selftest

[Tom Murphy]

This patch fixes the udmabuf selftest. Currently the selftest is broken. 
I fixed the selftest by setting the F_SEAL_SHRINK seal on the memfd 
file descriptor which is required by udmabuf and added the test to 
the selftest Makefile. 

Signed-off-by: Tom Murphy <murphyt7@tcd.ie>
---
 tools/testing/selftests/Makefile                  |  1 +
 tools/testing/selftests/drivers/dma-buf/Makefile  |  2 ++
 tools/testing/selftests/drivers/dma-buf/udmabuf.c | 11 +++++++++--
 3 files changed, 12 insertions(+), 2 deletions(-)

diff --git a/tools/testing/selftests/Makefile b/tools/testing/selftests/Makefile
index f1fe492c8e17..25efcde61d95 100644
--- a/tools/testing/selftests/Makefile
+++ b/tools/testing/selftests/Makefile
@@ -6,6 +6,7 @@ TARGETS += capabilities
 TARGETS += cgroup
 TARGETS += cpufreq
 TARGETS += cpu-hotplug
+TARGETS += drivers/dma-buf
 TARGETS += efivarfs
 TARGETS += exec
 TARGETS += filesystems
diff --git a/tools/testing/selftests/drivers/dma-buf/Makefile b/tools/testing/selftests/drivers/dma-buf/Makefile
index 4154c3d7aa58..f22c3f7cf612 100644
--- a/tools/testing/selftests/drivers/dma-buf/Makefile
+++ b/tools/testing/selftests/drivers/dma-buf/Makefile
@@ -2,4 +2,6 @@ CFLAGS += -I../../../../../usr/include/
 
 TEST_GEN_PROGS := udmabuf
 
+top_srcdir ?=../../../../..
+
 include ../../lib.mk
diff --git a/tools/testing/selftests/drivers/dma-buf/udmabuf.c b/tools/testing/selftests/drivers/dma-buf/udmabuf.c
index 376b1d6730bd..4de902ea14d8 100644
--- a/tools/testing/selftests/drivers/dma-buf/udmabuf.c
+++ b/tools/testing/selftests/drivers/dma-buf/udmabuf.c
@@ -4,7 +4,7 @@
 #include <unistd.h>
 #include <string.h>
 #include <errno.h>
-#include <fcntl.h>
+#include <linux/fcntl.h>
 #include <malloc.h>
 
 #include <sys/ioctl.h>
@@ -33,12 +33,19 @@ int main(int argc, char *argv[])
 		exit(77);
 	}
 
-	memfd = memfd_create("udmabuf-test", MFD_CLOEXEC);
+	memfd = memfd_create("udmabuf-test", MFD_ALLOW_SEALING);
 	if (memfd < 0) {
 		printf("%s: [skip,no-memfd]\n", TEST_PREFIX);
 		exit(77);
 	}
 
+	ret = fcntl(memfd, F_ADD_SEALS, F_SEAL_SHRINK);
+	if (ret < 0) {
+		printf("%s: [skip,fcntl-add-seals]\n", TEST_PREFIX);
+		exit(77);
+	}
+
+
 	size = getpagesize() * NUM_PAGES;
 	ret = ftruncate(memfd, size);
 	if (ret == -1) {
-- 
2.11.0

Re: [PATCH] fix dma-buf/udmabuf selftest

[shuah]

Hi Daniel,

On 11/27/18 3:33 AM, Tom Murphy wrote:
> This patch fixes the udmabuf selftest. Currently the selftest is broken.
> I fixed the selftest by setting the F_SEAL_SHRINK seal on the memfd
> file descriptor which is required by udmabuf and added the test to
> the selftest Makefile.
> 
> Signed-off-by: Tom Murphy <murphyt7@tcd.ie>

Does this change look good you? I plan to get this in for 4.21-rc1. Let 
me know if you have any objections.

thanks,
-- Shuah

Re: [PATCH] fix dma-buf/udmabuf selftest

[Daniel Vetter]

On Wed, Dec 12, 2018 at 10:16 PM shuah <shuah@kernel.org> wrote:
>
> Hi Daniel,
>
> On 11/27/18 3:33 AM, Tom Murphy wrote:
> > This patch fixes the udmabuf selftest. Currently the selftest is broken.
> > I fixed the selftest by setting the F_SEAL_SHRINK seal on the memfd
> > file descriptor which is required by udmabuf and added the test to
> > the selftest Makefile.
> >
> > Signed-off-by: Tom Murphy <murphyt7@tcd.ie>
>
> Does this change look good you? I plan to get this in for 4.21-rc1. Let
> me know if you have any objections.

No idea, would need Gerd's ack I think. And maybe we need to update
MAINTAINERS, to make sure dri-devel gets cc'ed on patches touching the
dma-buf testcases.
-Daniel

> thanks,
> -- Shuah
>


-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch

Re: [PATCH] fix dma-buf/udmabuf selftest

[Gerd Hoffmann]

On Wed, Dec 12, 2018 at 02:15:49PM -0700, shuah wrote:
> Hi Daniel,
> 
> On 11/27/18 3:33 AM, Tom Murphy wrote:
> > This patch fixes the udmabuf selftest. Currently the selftest is broken.
> > I fixed the selftest by setting the F_SEAL_SHRINK seal on the memfd
> > file descriptor which is required by udmabuf and added the test to
> > the selftest Makefile.
> > 
> > Signed-off-by: Tom Murphy <murphyt7@tcd.ie>
> 
> Does this change look good you? I plan to get this in for 4.21-rc1. Let me
> know if you have any objections.

Yes, it's fine.

Reviewed-by: Gerd Hoffmann <kraxel@redhat.com>

Re: [PATCH] fix dma-buf/udmabuf selftest

[Geert Uytterhoeven]

Hi Tom,

On Tue, Nov 27, 2018 at 7:53 PM Tom Murphy <murphyt7@tcd.ie> wrote:
> This patch fixes the udmabuf selftest. Currently the selftest is broken.
> I fixed the selftest by setting the F_SEAL_SHRINK seal on the memfd
> file descriptor which is required by udmabuf and added the test to
> the selftest Makefile.
>
> Signed-off-by: Tom Murphy <murphyt7@tcd.ie>

This is now commit 6edf2e3710f4ef25 ("fix dma-buf/udmabuf selftest").

> --- a/tools/testing/selftests/drivers/dma-buf/udmabuf.c
> +++ b/tools/testing/selftests/drivers/dma-buf/udmabuf.c
> @@ -4,7 +4,7 @@
>  #include <unistd.h>
>  #include <string.h>
>  #include <errno.h>
> -#include <fcntl.h>
> +#include <linux/fcntl.h>

Not including <fcntl.h> means we get

udmabuf.c:30:10: warning: implicit declaration of function ‘open’; did
you mean ‘popen’? [-Wimplicit-function-declaration]
  devfd = open("/dev/udmabuf", O_RDWR);
udmabuf.c:42:8: warning: implicit declaration of function ‘fcntl’; did
you mean ‘fcvt’? [-Wimplicit-function-declaration]
   ret = fcntl(memfd, F_ADD_SEALS, F_SEAL_SHRINK);

However, we need <linux/fcntl.h> for F_ADD_SEALS, F_SEAL_SHRINK.

Including both leads to lots of redefinition warnings.

Can we fix that?

>  #include <malloc.h>
>
>  #include <sys/ioctl.h>
> @@ -33,12 +33,19 @@ int main(int argc, char *argv[])
>                 exit(77);
>         }
>
> -       memfd = memfd_create("udmabuf-test", MFD_CLOEXEC);
> +       memfd = memfd_create("udmabuf-test", MFD_ALLOW_SEALING);
>         if (memfd < 0) {
>                 printf("%s: [skip,no-memfd]\n", TEST_PREFIX);
>                 exit(77);
>         }
>
> +       ret = fcntl(memfd, F_ADD_SEALS, F_SEAL_SHRINK);
> +       if (ret < 0) {
> +               printf("%s: [skip,fcntl-add-seals]\n", TEST_PREFIX);
> +               exit(77);
> +       }
> +
> +
>         size = getpagesize() * NUM_PAGES;
>         ret = ftruncate(memfd, size);
>         if (ret == -1) {

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Re: [PATCH] fix dma-buf/udmabuf selftest

[Tom Murphy]

> However, we need <linux/fcntl.h> for F_ADD_SEALS, F_SEAL_SHRINK.
>
> Including both leads to lots of redefinition warnings.
>
> Can we fix that?

I still haven't looked at this and probably won't get a chance anytime soon.
linux/tools/testing/selftests/memfd/fuse_test.c also suffers from this
(it needs the F_ADD_SEALS too) so that should also be fixed if anyone
gets a chance




On Wed, 9 Jan 2019 at 10:44, Geert Uytterhoeven <geert@linux-m68k.org> wrote:
>
> Hi Tom,
>
> On Tue, Nov 27, 2018 at 7:53 PM Tom Murphy <murphyt7@tcd.ie> wrote:
> > This patch fixes the udmabuf selftest. Currently the selftest is broken.
> > I fixed the selftest by setting the F_SEAL_SHRINK seal on the memfd
> > file descriptor which is required by udmabuf and added the test to
> > the selftest Makefile.
> >
> > Signed-off-by: Tom Murphy <murphyt7@tcd.ie>
>
> This is now commit 6edf2e3710f4ef25 ("fix dma-buf/udmabuf selftest").
>
> > --- a/tools/testing/selftests/drivers/dma-buf/udmabuf.c
> > +++ b/tools/testing/selftests/drivers/dma-buf/udmabuf.c
> > @@ -4,7 +4,7 @@
> >  #include <unistd.h>
> >  #include <string.h>
> >  #include <errno.h>
> > -#include <fcntl.h>
> > +#include <linux/fcntl.h>
>
> Not including <fcntl.h> means we get
>
> udmabuf.c:30:10: warning: implicit declaration of function ‘open’; did
> you mean ‘popen’? [-Wimplicit-function-declaration]
>   devfd = open("/dev/udmabuf", O_RDWR);
> udmabuf.c:42:8: warning: implicit declaration of function ‘fcntl’; did
> you mean ‘fcvt’? [-Wimplicit-function-declaration]
>    ret = fcntl(memfd, F_ADD_SEALS, F_SEAL_SHRINK);
>
> However, we need <linux/fcntl.h> for F_ADD_SEALS, F_SEAL_SHRINK.
>
> Including both leads to lots of redefinition warnings.
>
> Can we fix that?
>
> >  #include <malloc.h>
> >
> >  #include <sys/ioctl.h>
> > @@ -33,12 +33,19 @@ int main(int argc, char *argv[])
> >                 exit(77);
> >         }
> >
> > -       memfd = memfd_create("udmabuf-test", MFD_CLOEXEC);
> > +       memfd = memfd_create("udmabuf-test", MFD_ALLOW_SEALING);
> >         if (memfd < 0) {
> >                 printf("%s: [skip,no-memfd]\n", TEST_PREFIX);
> >                 exit(77);
> >         }
> >
> > +       ret = fcntl(memfd, F_ADD_SEALS, F_SEAL_SHRINK);
> > +       if (ret < 0) {
> > +               printf("%s: [skip,fcntl-add-seals]\n", TEST_PREFIX);
> > +               exit(77);
> > +       }
> > +
> > +
> >         size = getpagesize() * NUM_PAGES;
> >         ret = ftruncate(memfd, size);
> >         if (ret == -1) {
>
> Gr{oetje,eeting}s,
>
>                         Geert
>
> --
> Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
>
> In personal conversations with technical people, I call myself a hacker. But
> when I'm talking to journalists I just say "programmer" or something like that.
>                                 -- Linus Torvalds