From d6bbc5de66fc7ac51773bc92d6a48a94e622f9b1 Mon Sep 17 00:00:00 2001 From: Gustavo Padovan Date: Fri, 10 Jun 2016 16:09:36 -0300 Subject: [PATCH 1/6] libsync: move kernel headers for sync ioctls to sync.h This patch moves the legacy API to the internal sync.h header and add documentation to it. Test: Sync unit tests still passes. Change-Id: I9b17eb23af30043b3df5fb9e857affad68ba8521 --- libsync/include/sync/sync.h | 51 ++++++++++++++++++++++++++++++++++++- libsync/sync.c | 20 +++------------ 2 files changed, 54 insertions(+), 17 deletions(-) diff --git a/libsync/include/sync/sync.h b/libsync/include/sync/sync.h index 2e5d82f15..60989f2e3 100644 --- a/libsync/include/sync/sync.h +++ b/libsync/include/sync/sync.h @@ -22,9 +22,16 @@ #include #include +#include + __BEGIN_DECLS -// XXX: These structs are copied from the header "linux/sync.h". +struct sync_legacy_merge_data { + int32_t fd2; + char name[32]; + int32_t fence; +}; + struct sync_fence_info_data { uint32_t len; char name[32]; @@ -41,6 +48,48 @@ struct sync_pt_info { uint8_t driver_data[0]; }; +#define SYNC_IOC_MAGIC '>' + +/** + * DOC: SYNC_IOC_LEGACY_WAIT - wait for a fence to signal + * + * pass timeout in milliseconds. Waits indefinitely timeout < 0. + * + * This is the legacy version of the Sync API before the de-stage that happened + * on Linux kernel 4.7. + */ +#define SYNC_IOC_LEGACY_WAIT _IOW(SYNC_IOC_MAGIC, 0, __s32) + +/** + * DOC: SYNC_IOC_MERGE - merge two fences + * + * Takes a struct sync_merge_data. Creates a new fence containing copies of + * the sync_pts in both the calling fd and sync_merge_data.fd2. Returns the + * new fence's fd in sync_merge_data.fence + * + * This is the legacy version of the Sync API before the de-stage that happened + * on Linux kernel 4.7. + */ +#define SYNC_IOC_LEGACY_MERGE _IOWR(SYNC_IOC_MAGIC, 1, \ + struct sync_legacy_merge_data) + +/** + * DOC: SYNC_IOC_LEGACY_FENCE_INFO - get detailed information on a fence + * + * Takes a struct sync_fence_info_data with extra space allocated for pt_info. + * Caller should write the size of the buffer into len. On return, len is + * updated to reflect the total size of the sync_fence_info_data including + * pt_info. + * + * pt_info is a buffer containing sync_pt_infos for every sync_pt in the fence. + * To iterate over the sync_pt_infos, use the sync_pt_info.len field. + * + * This is the legacy version of the Sync API before the de-stage that happened + * on Linux kernel 4.7. + */ +#define SYNC_IOC_LEGACY_FENCE_INFO _IOWR(SYNC_IOC_MAGIC, 2,\ + struct sync_fence_info_data) + /* timeout in msecs */ int sync_wait(int fd, int timeout); int sync_merge(const char *name, int fd1, int fd2); diff --git a/libsync/sync.c b/libsync/sync.c index 6281b205c..fff62e77c 100644 --- a/libsync/sync.c +++ b/libsync/sync.c @@ -27,18 +27,6 @@ #include -// The sync code is undergoing a major change. Add enough in to get -// everything to compile wih the latest uapi headers. -struct sync_merge_data { - int32_t fd2; - char name[32]; - int32_t fence; -}; - -#define SYNC_IOC_MAGIC '>' -#define SYNC_IOC_WAIT _IOW(SYNC_IOC_MAGIC, 0, __s32) -#define SYNC_IOC_MERGE _IOWR(SYNC_IOC_MAGIC, 1, struct sync_merge_data) -#define SYNC_IOC_FENCE_INFO _IOWR(SYNC_IOC_MAGIC, 2, struct sync_fence_info_data) struct sw_sync_create_fence_data { __u32 value; @@ -54,18 +42,18 @@ int sync_wait(int fd, int timeout) { __s32 to = timeout; - return ioctl(fd, SYNC_IOC_WAIT, &to); + return ioctl(fd, SYNC_IOC_LEGACY_WAIT, &to); } int sync_merge(const char *name, int fd1, int fd2) { - struct sync_merge_data data; + struct sync_legacy_merge_data data; int err; data.fd2 = fd2; strlcpy(data.name, name, sizeof(data.name)); - err = ioctl(fd1, SYNC_IOC_MERGE, &data); + err = ioctl(fd1, SYNC_IOC_LEGACY_MERGE, &data); if (err < 0) return err; @@ -82,7 +70,7 @@ struct sync_fence_info_data *sync_fence_info(int fd) return NULL; info->len = 4096; - err = ioctl(fd, SYNC_IOC_FENCE_INFO, info); + err = ioctl(fd, SYNC_IOC_LEGACY_FENCE_INFO, info); if (err < 0) { free(info); return NULL; From 6786575d42e78c8faa51393871eb958d87d57e08 Mon Sep 17 00:00:00 2001 From: Gustavo Padovan Date: Fri, 10 Jun 2016 16:24:49 -0300 Subject: [PATCH 2/6] libsync: add new Sync kernel API Add the new API to the internal sync.h file. As there is two different APIs we will need to discovery dynamically which one to use. v2: Fix Documentation Test: Sync unit tests still passes. Change-Id: I2ab3cd46e48ba5d9c73d54f9583b1a8141566581 --- libsync/include/sync/sync.h | 60 +++++++++++++++++++++++++++++++++++++ 1 file changed, 60 insertions(+) diff --git a/libsync/include/sync/sync.h b/libsync/include/sync/sync.h index 60989f2e3..50ed0ac57 100644 --- a/libsync/include/sync/sync.h +++ b/libsync/include/sync/sync.h @@ -90,6 +90,66 @@ struct sync_pt_info { #define SYNC_IOC_LEGACY_FENCE_INFO _IOWR(SYNC_IOC_MAGIC, 2,\ struct sync_fence_info_data) +struct sync_merge_data { + char name[32]; + int32_t fd2; + int32_t fence; + uint32_t flags; + uint32_t pad; +}; + +struct sync_file_info { + char name[32]; + int32_t status; + uint32_t flags; + uint32_t num_fences; + uint32_t pad; + + uint64_t sync_fence_info; +}; + +struct sync_fence_info { + char obj_name[32]; + char driver_name[32]; + int32_t status; + uint32_t flags; + uint64_t timestamp_ns; +}; + +/** + * Mainline API: + * + * Opcodes 0, 1 and 2 were burned during a API change to avoid users of the + * old API to get weird errors when trying to handling sync_files. The API + * change happened during the de-stage of the Sync Framework when there was + * no upstream users available. + */ + +/** + * DOC: SYNC_IOC_MERGE - merge two fences + * + * Takes a struct sync_merge_data. Creates a new fence containing copies of + * the sync_pts in both the calling fd and sync_merge_data.fd2. Returns the + * new fence's fd in sync_merge_data.fence + * + * This is the new version of the Sync API after the de-stage that happened + * on Linux kernel 4.7. + */ +#define SYNC_IOC_MERGE _IOWR(SYNC_IOC_MAGIC, 3, struct sync_merge_data) + +/** + * DOC: SYNC_IOC_FILE_INFO - get detailed information on a sync_file + * + * Takes a struct sync_file_info. If num_fences is 0, the field is updated + * with the actual number of fences. If num_fences is > 0, the system will + * use the pointer provided on sync_fence_info to return up to num_fences of + * struct sync_fence_info, with detailed fence information. + * + * This is the new version of the Sync API after the de-stage that happened + * on Linux kernel 4.7. + */ +#define SYNC_IOC_FILE_INFO _IOWR(SYNC_IOC_MAGIC, 4, struct sync_file_info) + /* timeout in msecs */ int sync_wait(int fd, int timeout); int sync_merge(const char *name, int fd1, int fd2); From ffc687baad033ecc96f6c560b205fea61afe9e41 Mon Sep 17 00:00:00 2001 From: Gustavo Padovan Date: Fri, 10 Jun 2016 16:51:29 -0300 Subject: [PATCH 3/6] libsync: open new location of sw_sync file sw_sync file for debug was moved to debugfs. Try to open it and if it fails try to open /dev/sw_sync. Test: Sync unit tests still passes. Change-Id: Ie078fbc2eb5294f28b916a9e65b7fcd3a18a8580 hange-Id: I216874964368d939bed2779d98cd89e527a57d45 --- libsync/sync.c | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/libsync/sync.c b/libsync/sync.c index fff62e77c..5946096b0 100644 --- a/libsync/sync.c +++ b/libsync/sync.c @@ -101,7 +101,13 @@ void sync_fence_info_free(struct sync_fence_info_data *info) int sw_sync_timeline_create(void) { - return open("/dev/sw_sync", O_RDWR); + int ret; + + ret = open("/sys/kernel/debug/sync/sw_sync", O_RDWR); + if (ret < 0) + ret = open("/dev/sw_sync", O_RDWR); + + return ret; } int sw_sync_timeline_inc(int fd, unsigned count) From 61ab0d74d218d0be32d0cab1c7ee22c5e12216fe Mon Sep 17 00:00:00 2001 From: Gustavo Padovan Date: Sat, 11 Jun 2016 11:11:19 -0300 Subject: [PATCH 4/6] libsync: add support to new Sync API Change libsync functions in a way that it can run dynamically on both APIs. v2: fix whitespace changes and poll return handling v3: handle error cases on sync_wait() Test: Sync unit tests still passes. Change-Id: I743ab92ce39cbfa75dca41dd0a435efa9f2aab66 hange-Id: Ib56f2c6441b41028bc9f66998676790b7713988a --- libsync/sync.c | 124 ++++++++++++++++++++++++++++++++++++++++++------- 1 file changed, 108 insertions(+), 16 deletions(-) diff --git a/libsync/sync.c b/libsync/sync.c index 5946096b0..9ed03dba8 100644 --- a/libsync/sync.c +++ b/libsync/sync.c @@ -20,6 +20,8 @@ #include #include #include +#include +#include #include #include @@ -40,43 +42,133 @@ struct sw_sync_create_fence_data { int sync_wait(int fd, int timeout) { - __s32 to = timeout; + struct pollfd fds; + int ret; - return ioctl(fd, SYNC_IOC_LEGACY_WAIT, &to); + if (fd < 0) { + errno = EINVAL; + return -1; + } + + 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; } int sync_merge(const char *name, int fd1, int fd2) { - struct sync_legacy_merge_data data; - int err; + struct sync_legacy_merge_data legacy_data; + struct sync_merge_data data; + int ret; data.fd2 = fd2; strlcpy(data.name, name, sizeof(data.name)); + data.flags = 0; + data.pad = 0; - err = ioctl(fd1, SYNC_IOC_LEGACY_MERGE, &data); - if (err < 0) - return err; + ret = ioctl(fd1, SYNC_IOC_MERGE, &data); + if (ret < 0 && errno == ENOTTY) { + legacy_data.fd2 = fd2; + strlcpy(legacy_data.name, name, sizeof(legacy_data.name)); + + ret = ioctl(fd1, SYNC_IOC_LEGACY_MERGE, &legacy_data); + if (ret < 0) + return ret; + + return legacy_data.fence; + } else if (ret < 0) { + return ret; + } return data.fence; } struct sync_fence_info_data *sync_fence_info(int fd) { - struct sync_fence_info_data *info; - int err; + struct sync_fence_info_data *legacy_info; + struct sync_pt_info *legacy_pt_info; + struct sync_file_info *info; + struct sync_fence_info *fence_info; + int err, num_fences, i; - info = malloc(4096); - if (info == NULL) + legacy_info = malloc(4096); + if (legacy_info == NULL) return NULL; - info->len = 4096; - err = ioctl(fd, SYNC_IOC_LEGACY_FENCE_INFO, info); - if (err < 0) { - free(info); + legacy_info->len = 4096; + err = ioctl(fd, SYNC_IOC_LEGACY_FENCE_INFO, legacy_info); + if (err < 0 && errno != ENOTTY) { + free(legacy_info); return NULL; + } else if (err == 0) { + return legacy_info; } - return info; + info = calloc(1, sizeof(*info)); + if (info == NULL) + goto free; + + err = ioctl(fd, SYNC_IOC_FILE_INFO, info); + if (err < 0) + goto free; + + num_fences = info->num_fences; + + if (num_fences) { + info->flags = 0; + info->num_fences = num_fences; + info->sync_fence_info = (uint64_t) calloc(num_fences, + sizeof(struct sync_fence_info)); + if ((void *)info->sync_fence_info == NULL) + goto free; + + err = ioctl(fd, SYNC_IOC_FILE_INFO, info); + if (err < 0) { + free((void *)info->sync_fence_info); + goto free; + } + } + + legacy_info->len = sizeof(*legacy_info) + + num_fences * sizeof(struct sync_fence_info); + strlcpy(legacy_info->name, info->name, sizeof(legacy_info->name)); + legacy_info->status = info->status; + + legacy_pt_info = (struct sync_pt_info *)legacy_info->pt_info; + fence_info = (struct sync_fence_info *)info->sync_fence_info; + for (i = 0 ; i < num_fences ; i++) { + legacy_pt_info[i].len = sizeof(*legacy_pt_info); + strlcpy(legacy_pt_info[i].obj_name, fence_info[i].obj_name, + sizeof(legacy_pt_info->obj_name)); + strlcpy(legacy_pt_info[i].driver_name, fence_info[i].driver_name, + sizeof(legacy_pt_info->driver_name)); + legacy_pt_info[i].status = fence_info[i].status; + legacy_pt_info[i].timestamp_ns = fence_info[i].timestamp_ns; + } + + free((void *)info->sync_fence_info); + free(info); + return legacy_info; + +free: + free(legacy_info); + free(info); + return NULL; } struct sync_pt_info *sync_pt_info(struct sync_fence_info_data *info, From e4682802cbeeefe8e870e5b7b110515659a6a14f Mon Sep 17 00:00:00 2001 From: Gustavo Padovan Date: Tue, 6 Dec 2016 16:09:51 -0200 Subject: [PATCH 5/6] libsync: tests: remove WaitOnDestroyedTimeline test The mainline Sync File implementation doesn't have wait ioctl anymore. Only poll is supported now, and we already have a test for that. Test: Sync unit tests still passes. Change-Id: Iadde7b2173024af9b8d20316e640297cf214c645 --- libsync/tests/sync_test.cpp | 27 --------------------------- 1 file changed, 27 deletions(-) diff --git a/libsync/tests/sync_test.cpp b/libsync/tests/sync_test.cpp index 2c409dc27..7c65756f0 100644 --- a/libsync/tests/sync_test.cpp +++ b/libsync/tests/sync_test.cpp @@ -348,33 +348,6 @@ TEST(FenceTest, MergeSameFence) { ASSERT_EQ(selfMergeFence.getSignaledCount(), 1); } -TEST(FenceTest, WaitOnDestroyedTimeline) { - SyncTimeline timeline; - ASSERT_TRUE(timeline.isValid()); - - SyncFence fenceSig(timeline, 100); - SyncFence fenceKill(timeline, 200); - - // Spawn a thread to wait on a fence when the timeline is killed. - thread waitThread{ - [&]() { - ASSERT_EQ(timeline.inc(100), 0); - - ASSERT_EQ(fenceKill.wait(-1), -1); - ASSERT_EQ(errno, ENOENT); - } - }; - - // Wait for the thread to spool up. - fenceSig.wait(); - - // Kill the timeline. - timeline.destroy(); - - // wait for the thread to clean up. - waitThread.join(); -} - TEST(FenceTest, PollOnDestroyedTimeline) { SyncTimeline timeline; ASSERT_TRUE(timeline.isValid()); From 801492b8a66a8f08f99667a4123ce07aa4c0a4f7 Mon Sep 17 00:00:00 2001 From: Gustavo Padovan Date: Tue, 6 Dec 2016 16:13:17 -0200 Subject: [PATCH 6/6] libsync: tests: redefine PollOnDestroyedTimeline() On mainline if the sw_sync timeline is destroyed the fences doesn't not signal or error. So change the test to check if the fence is still there by polling the fence with timeout zero and asserting if it is not signalled. Test: Sync unit tests still passes. Change-Id: Icb8e629018eef35074ae91d0f29ed1f12e90492b --- libsync/tests/sync_test.cpp | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/libsync/tests/sync_test.cpp b/libsync/tests/sync_test.cpp index 7c65756f0..ff8a300f1 100644 --- a/libsync/tests/sync_test.cpp +++ b/libsync/tests/sync_test.cpp @@ -364,8 +364,7 @@ TEST(FenceTest, PollOnDestroyedTimeline) { struct pollfd fds; fds.fd = fenceKill.getFd(); fds.events = POLLIN | POLLERR; - ASSERT_EQ(poll(&fds, 1, -1), 1); - ASSERT_TRUE(fds.revents & POLLERR); + ASSERT_EQ(poll(&fds, 1, 0), 0); } };