Re: [PATCH v2 1/7] selftest: sync: basic tests for sw_sync framework
From: Gustavo Padovan
Date: Tue Nov 01 2016 - 12:18:58 EST
Hi Emilio,
2016-10-19 Emilio López <emilio.lopez@xxxxxxxxxxxxxxx>:
> These tests are based on the libsync test suite from Android.
> This commit lays the ground for future tests, as well as includes
> tests for a variety of basic allocation commands.
>
> Signed-off-by: Emilio López <emilio.lopez@xxxxxxxxxxxxxxx>
> ---
> tools/testing/selftests/Makefile | 1 +
> tools/testing/selftests/sync/.gitignore | 1 +
> tools/testing/selftests/sync/Makefile | 18 +++
> tools/testing/selftests/sync/sw_sync.h | 46 +++++++
> tools/testing/selftests/sync/sync.c | 221 ++++++++++++++++++++++++++++++
> tools/testing/selftests/sync/sync.h | 40 ++++++
> tools/testing/selftests/sync/sync_alloc.c | 74 ++++++++++
> tools/testing/selftests/sync/sync_test.c | 71 ++++++++++
> tools/testing/selftests/sync/synctest.h | 47 +++++++
> 9 files changed, 519 insertions(+)
> create mode 100644 tools/testing/selftests/sync/.gitignore
> create mode 100644 tools/testing/selftests/sync/Makefile
> create mode 100644 tools/testing/selftests/sync/sw_sync.h
> create mode 100644 tools/testing/selftests/sync/sync.c
> create mode 100644 tools/testing/selftests/sync/sync.h
> create mode 100644 tools/testing/selftests/sync/sync_alloc.c
> create mode 100644 tools/testing/selftests/sync/sync_test.c
> create mode 100644 tools/testing/selftests/sync/synctest.h
>
> diff --git a/tools/testing/selftests/Makefile b/tools/testing/selftests/Makefile
> index f770dba..69cf1a6 100644
> --- a/tools/testing/selftests/Makefile
> +++ b/tools/testing/selftests/Makefile
> @@ -23,6 +23,7 @@ TARGETS += seccomp
> TARGETS += sigaltstack
> TARGETS += size
> TARGETS += static_keys
> +TARGETS += sync
> TARGETS += sysctl
> ifneq (1, $(quicktest))
> TARGETS += timers
> diff --git a/tools/testing/selftests/sync/.gitignore b/tools/testing/selftests/sync/.gitignore
> new file mode 100644
> index 0000000..f5091e7
> --- /dev/null
> +++ b/tools/testing/selftests/sync/.gitignore
> @@ -0,0 +1 @@
> +sync_test
> diff --git a/tools/testing/selftests/sync/Makefile b/tools/testing/selftests/sync/Makefile
> new file mode 100644
> index 0000000..620a59a
> --- /dev/null
> +++ b/tools/testing/selftests/sync/Makefile
> @@ -0,0 +1,18 @@
> +CFLAGS += -O2 -g -std=gnu89 -pthread -Wall -Wextra
> +CFLAGS += -I../../../../usr/include/
> +LDFLAGS += -pthread
> +
> +TEST_PROGS = sync_test
> +
> +all: $(TEST_PROGS)
> +
> +include ../lib.mk
> +
> +OBJS = sync_test.o sync.o
> +
> +TESTS += sync_alloc.o
> +
> +sync_test: $(OBJS) $(TESTS)
> +
> +clean:
> + $(RM) sync_test $(OBJS) $(TESTS)
> diff --git a/tools/testing/selftests/sync/sw_sync.h b/tools/testing/selftests/sync/sw_sync.h
> new file mode 100644
> index 0000000..e2cfc6ba
> --- /dev/null
> +++ b/tools/testing/selftests/sync/sw_sync.h
> @@ -0,0 +1,46 @@
> +/*
> + * sw_sync abstraction
> + *
> + * Copyright 2015-2016 Collabora Ltd.
> + *
> + * Based on the implementation from the Android Open Source Project,
> + *
> + * Copyright 2013 Google, Inc
> + *
> + * 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 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.
> + */
> +
> +#ifndef SELFTESTS_SW_SYNC_H
> +#define SELFTESTS_SW_SYNC_H
> +
> +/*
> + * sw_sync is mainly intended for testing and should not be compiled into
> + * production kernels
> + */
> +
> +int sw_sync_timeline_create(void);
> +int sw_sync_timeline_is_valid(int fd);
> +int sw_sync_timeline_inc(int fd, unsigned int count);
> +void sw_sync_timeline_destroy(int fd);
> +
> +int sw_sync_fence_create(int fd, const char *name, unsigned int value);
> +int sw_sync_fence_is_valid(int fd);
> +void sw_sync_fence_destroy(int fd);
> +
> +#endif
> diff --git a/tools/testing/selftests/sync/sync.c b/tools/testing/selftests/sync/sync.c
> new file mode 100644
> index 0000000..f3d599f
> --- /dev/null
> +++ b/tools/testing/selftests/sync/sync.c
> @@ -0,0 +1,221 @@
> +/*
> + * sync / sw_sync abstraction
> + * Copyright 2015-2016 Collabora Ltd.
> + *
> + * Based on the implementation from the Android Open Source Project,
> + *
> + * Copyright 2012 Google, Inc
> + *
> + * 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 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 <fcntl.h>
> +#include <malloc.h>
> +#include <poll.h>
> +#include <stdint.h>
> +#include <string.h>
> +#include <unistd.h>
> +
> +#include <sys/ioctl.h>
> +#include <sys/stat.h>
> +#include <sys/types.h>
> +
> +#include "sync.h"
> +#include "sw_sync.h"
> +
> +#include <linux/sync_file.h>
> +
> +
> +/* SW_SYNC ioctls */
> +struct sw_sync_create_fence_data {
> + __u32 value;
> + char name[32];
> + __s32 fence;
> +};
> +
> +#define SW_SYNC_IOC_MAGIC 'W'
> +#define SW_SYNC_IOC_CREATE_FENCE _IOWR(SW_SYNC_IOC_MAGIC, 0,\
> + struct sw_sync_create_fence_data)
> +#define SW_SYNC_IOC_INC _IOW(SW_SYNC_IOC_MAGIC, 1, __u32)
> +
> +
> +int sync_wait(int fd, int timeout)
> +{
> + struct pollfd fds;
> +
> + fds.fd = fd;
> + fds.events = POLLIN | POLLERR;
> +
> + return poll(&fds, 1, timeout);
> +}
I think it is a good idea to keep the sync_wait behaviour similar to
what it was on android libsync. It should be something like this:
int sw_sync_wait(int fd, int timeout)
{
struct pollfd fds = {0};
int ret;
fds.fd = fd;
fds.events = POLLIN;
do {
ret = poll(&fds, 1, timeout);
if (ret > 0) {
if (fds.revents & (POLLERR | POLLNVAL)) {
errno = EINVAL;
return -1;
}
return 0;
} else if (ret == 0) {
errno = ETIME;
return -1;
}
} while (ret == -1 && (errno == EINTR || errno == EAGAIN));
return ret;
}
Gustavo