Re: [PATCH v2 1/7] selftest: sync: basic tests for sw_sync framework

Hi Gustavo,

On 11/01/2016 10:18 AM, Gustavo Padovan wrote:

Hi Emilio,

2016-10-19 Emilio López emilio.lopez@collabora.co.uk:

...

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@collabora.co.uk

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:

I didn't notice that this comment wasn't addressed. I will go ahead and continue with the plan to get these into 4.10-rc1. If you want to send me patch for this behavior, please do. We can address that as a fix in 4.10-rc2

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

thanks, -- Shuah