Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S934562AbcLMQr5 (ORCPT ); Tue, 13 Dec 2016 11:47:57 -0500 Received: from resqmta-po-08v.sys.comcast.net ([96.114.154.167]:44097 "EHLO resqmta-po-08v.sys.comcast.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S934156AbcLMQqp (ORCPT ); Tue, 13 Dec 2016 11:46:45 -0500 Reply-To: shuah@kernel.org Subject: Re: [PATCH v2 1/7] selftest: sync: basic tests for sw_sync framework References: <20160921211205.22657-1-emilio.lopez@collabora.co.uk> <20161019124954.29232-1-emilio.lopez@collabora.co.uk> <20161019124954.29232-2-emilio.lopez@collabora.co.uk> <20161101161847.GB3334@joana> To: Gustavo Padovan , =?UTF-8?Q?Emilio_L=c3=b3pez?= Cc: devel@driverdev.osuosl.org, gustavo.padovan@collabora.co.uk, riandrews@android.com, daniel.vetter@ffwll.ch, John.C.Harrison@Intel.com, linux-kernel@vger.kernel.org, dri-devel@lists.freedesktop.org, ghackmann@google.com, robdclark@gmail.com, linux-kselftest@vger.kernel.org, maarten.lankhorst@linux.intel.com, daniels@collabora.com, arve@android.com, emil.l.velikov@gmail.com, mpe@ellerman.id.au, Shuah Khan , Shuah Khan From: Shuah Khan Message-ID: <6e8a65a6-e72d-beca-1c68-32bd3287c674@kernel.org> Date: Tue, 13 Dec 2016 09:46:40 -0700 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.4.0 MIME-Version: 1.0 In-Reply-To: <20161101161847.GB3334@joana> Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 8bit X-CMAE-Envelope: MS4wfF5iP3Mj7F2E7/1BWR26xsXjzW+YH5AxPqEaK8rhWYIau7/kt5uJPGVD9t8Xe1mWT+vA4/7e+0hDhWCxdI+9iVI6w2zW7lYypXOUG/HKTIpITQ+FoTNL g2YTQLGEdX7bFzFfneBqfsWdQBjcZSdWcaW+gUGVVi7wuhCxIc+u/2i+de5S3F7JYFE8hqjPowaQQr/jyvNLky0Igx523llPLNbLhnGUfGZWKnPgginTFgBx 0gY8qqRYVPMUOEvr9Egv0QonqizM6V3H/cC1DInP6xkxuxhxFx3ur8qGLcnRZWNqtsr2NfiGPTPBTMta8/cpxOh/VwrWcwmA7D7kNcDfutbcBSy0bhwlhDWh d1CVoi1XbkbRkYZY2OsZP2bKQjP11FSMA8RIziw7fQNHEQFcVuBQ+xu8Y9jW4echAEjNBq5be8zDcqYsWRE5Nu+2AdIHOkvXCqwna1X7xeh3trWuoPWEqVb1 Yv2i1Y/5e09rP3dxuER3m5w9BBnKfoG8ezN7kBzI93LYkw08NhaGFJHGvMmVducq5GH6UZ5G0h+Ue9jSZ0mGLlV8O0jGd03dH5/NdRaN3y/d0/qSEzGzYXl5 CWapPQN6F+7sAcvSmmTrVv7ArYpfY6FvoH4NO4TtyPn0oyEo9/LsLqVE7NxG+kKy1XSQ5VVvnzHzjoZFdKTNWl9nOgr+yLXpc/ZeuKrDNuK2NdEkH9gce28k GRymfXJyofsOfnhslLTtIfukux1vaUWVz68jsZSNeThCt9c/OJX0SEyKCP701mep7fDmIldo3YW1DmPZrxkw2JnMZxbX4naRPYD6iqyMBlsX5I0/VpxrVeYZ i1er8zpLvqBRqz+7Gu+v2YZb1X93IbNorNCdkW2/ Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 9682 Lines: 240 Hi Gustavo, On 11/01/2016 10:18 AM, Gustavo Padovan wrote: > Hi Emilio, > > 2016-10-19 Emilio L?pez : > >> 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 >> --- >> 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 >> +#include >> +#include >> +#include >> +#include >> +#include >> + >> +#include >> +#include >> +#include >> + >> +#include "sync.h" >> +#include "sw_sync.h" >> + >> +#include >> + >> + >> +/* 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