2018-11-27 18:46:42

by Tom Murphy

[permalink] [raw]
Subject: [PATCH] fix dma-buf/udmabuf selftest

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 <[email protected]>
---
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



2018-12-12 21:18:49

by Shuah Khan

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

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 <[email protected]>

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


2018-12-12 21:58:19

by Daniel Vetter

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

On Wed, Dec 12, 2018 at 10:16 PM shuah <[email protected]> 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 <[email protected]>
>
> 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

2018-12-13 10:53:05

by Gerd Hoffmann

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

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 <[email protected]>
>
> 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 <[email protected]>


2019-01-09 11:50:08

by Geert Uytterhoeven

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

Hi Tom,

On Tue, Nov 27, 2018 at 7:53 PM Tom Murphy <[email protected]> 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 <[email protected]>

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 -- [email protected]

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

2019-02-08 14:33:13

by Tom Murphy

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

> 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 <[email protected]> wrote:
>
> Hi Tom,
>
> On Tue, Nov 27, 2018 at 7:53 PM Tom Murphy <[email protected]> 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 <[email protected]>
>
> 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 -- [email protected]
>
> 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