Patch Detail
get:
Show a patch.
patch:
Update a patch.
put:
Update a patch.
GET /api/patches/73580/?format=api
{ "id": 73580, "url": "https://patches.dpdk.org/api/patches/73580/?format=api", "web_url": "https://patches.dpdk.org/project/dpdk/patch/1594277162-15021-1-git-send-email-phil.yang@arm.com/", "project": { "id": 1, "url": "https://patches.dpdk.org/api/projects/1/?format=api", "name": "DPDK", "link_name": "dpdk", "list_id": "dev.dpdk.org", "list_email": "dev@dpdk.org", "web_url": "http://core.dpdk.org", "scm_url": "git://dpdk.org/dpdk", "webscm_url": "http://git.dpdk.org/dpdk", "list_archive_url": "https://inbox.dpdk.org/dev", "list_archive_url_format": "https://inbox.dpdk.org/dev/{}", "commit_url_format": "" }, "msgid": "<1594277162-15021-1-git-send-email-phil.yang@arm.com>", "list_archive_url": "https://inbox.dpdk.org/dev/1594277162-15021-1-git-send-email-phil.yang@arm.com", "date": "2020-07-09T06:46:02", "name": "[v2] eal: use c11 atomic built-ins for interrupt status", "commit_ref": null, "pull_url": null, "state": "superseded", "archived": true, "hash": "24dc5ed28d156fded03bf0cf5e74818c4ed9c3dc", "submitter": { "id": 833, "url": "https://patches.dpdk.org/api/people/833/?format=api", "name": "Phil Yang", "email": "phil.yang@arm.com" }, "delegate": { "id": 24651, "url": "https://patches.dpdk.org/api/users/24651/?format=api", "username": "dmarchand", "first_name": "David", "last_name": "Marchand", "email": "david.marchand@redhat.com" }, "mbox": "https://patches.dpdk.org/project/dpdk/patch/1594277162-15021-1-git-send-email-phil.yang@arm.com/mbox/", "series": [ { "id": 10910, "url": "https://patches.dpdk.org/api/series/10910/?format=api", "web_url": "https://patches.dpdk.org/project/dpdk/list/?series=10910", "date": "2020-07-09T06:46:02", "name": "[v2] eal: use c11 atomic built-ins for interrupt status", "version": 2, "mbox": "https://patches.dpdk.org/series/10910/mbox/" } ], "comments": "https://patches.dpdk.org/api/patches/73580/comments/", "check": "success", "checks": "https://patches.dpdk.org/api/patches/73580/checks/", "tags": {}, "related": [], "headers": { "Return-Path": "<dev-bounces@dpdk.org>", "X-Original-To": "patchwork@inbox.dpdk.org", "Delivered-To": "patchwork@inbox.dpdk.org", "Received": [ "from dpdk.org (dpdk.org [92.243.14.124])\n\tby inbox.dpdk.org (Postfix) with ESMTP id C7296A0526;\n\tThu, 9 Jul 2020 08:46:55 +0200 (CEST)", "from [92.243.14.124] (localhost [127.0.0.1])\n\tby dpdk.org (Postfix) with ESMTP id E051F1DBB4;\n\tThu, 9 Jul 2020 08:46:54 +0200 (CEST)", "from foss.arm.com (foss.arm.com [217.140.110.172])\n by dpdk.org (Postfix) with ESMTP id 3A6901DA96\n for <dev@dpdk.org>; Thu, 9 Jul 2020 08:46:53 +0200 (CEST)", "from usa-sjc-imap-foss1.foss.arm.com (unknown [10.121.207.14])\n by usa-sjc-mx-foss1.foss.arm.com (Postfix) with ESMTP id A77781FB;\n Wed, 8 Jul 2020 23:46:52 -0700 (PDT)", "from phil-VirtualBox.arm.com (A010647.Arm.com [10.170.226.139])\n by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPA id 6C65C3F71E;\n Wed, 8 Jul 2020 23:46:49 -0700 (PDT)" ], "From": "Phil Yang <phil.yang@arm.com>", "To": "david.marchand@redhat.com,\n\tdev@dpdk.org", "Cc": "mdr@ashroe.eu, aconole@redhat.com, drc@linux.vnet.ibm.com,\n Honnappa.Nagarahalli@arm.com, Ruifeng.Wang@arm.com, nd@arm.com,\n dodji@redhat.com, nhorman@tuxdriver.com, hkalra@marvell.com", "Date": "Thu, 9 Jul 2020 14:46:02 +0800", "Message-Id": "<1594277162-15021-1-git-send-email-phil.yang@arm.com>", "X-Mailer": "git-send-email 2.7.4", "In-Reply-To": "<1591871065-12461-1-git-send-email-phil.yang@arm.com>", "References": "<1591871065-12461-1-git-send-email-phil.yang@arm.com>", "Subject": "[dpdk-dev] [PATCH v2] eal: use c11 atomic built-ins for interrupt\n\tstatus", "X-BeenThere": "dev@dpdk.org", "X-Mailman-Version": "2.1.15", "Precedence": "list", "List-Id": "DPDK patches and discussions <dev.dpdk.org>", "List-Unsubscribe": "<https://mails.dpdk.org/options/dev>,\n <mailto:dev-request@dpdk.org?subject=unsubscribe>", "List-Archive": "<http://mails.dpdk.org/archives/dev/>", "List-Post": "<mailto:dev@dpdk.org>", "List-Help": "<mailto:dev-request@dpdk.org?subject=help>", "List-Subscribe": "<https://mails.dpdk.org/listinfo/dev>,\n <mailto:dev-request@dpdk.org?subject=subscribe>", "Errors-To": "dev-bounces@dpdk.org", "Sender": "\"dev\" <dev-bounces@dpdk.org>" }, "content": "The event status is defined as a volatile variable and shared between\nthreads. Use c11 atomic built-ins with explicit ordering instead of\nrte_atomic ops which enforce unnecessary barriers on aarch64.\n\nThe event status has been cleaned up by the compare-and-swap operation\nwhen we free the event data, so there is no need to set it to invalid\nafter that.\n\nSigned-off-by: Phil Yang <phil.yang@arm.com>\nReviewed-by: Ruifeng Wang <ruifeng.wang@arm.com>\nReviewed-by: Honnappa Nagarahalli <honnappa.nagarahalli@arm.com>\nReviewed-by: Harman Kalra <hkalra@marvell.com>\n---\nv2:\n1. Fixed typo.\n2. Updated libabigail.abignore to pass ABI check.\n3. Merged v1 two patches into one patch.\n\n devtools/libabigail.abignore | 4 +++\n lib/librte_eal/include/rte_eal_interrupts.h | 2 +-\n lib/librte_eal/linux/eal_interrupts.c | 48 ++++++++++++++++++++---------\n 3 files changed, 38 insertions(+), 16 deletions(-)", "diff": "diff --git a/devtools/libabigail.abignore b/devtools/libabigail.abignore\nindex 0133f75..daa4631 100644\n--- a/devtools/libabigail.abignore\n+++ b/devtools/libabigail.abignore\n@@ -48,6 +48,10 @@\n changed_enumerators = RTE_CRYPTO_AEAD_LIST_END\n [suppress_variable]\n name = rte_crypto_aead_algorithm_strings\n+; Ignore updates of epoll event\n+[suppress_type]\n+ type_kind = struct\n+ name = rte_epoll_event\n \n ;;;;;;;;;;;;;;;;;;;;;;\n ; Temporary exceptions till DPDK 20.11\ndiff --git a/lib/librte_eal/include/rte_eal_interrupts.h b/lib/librte_eal/include/rte_eal_interrupts.h\nindex 773a34a..b1e8a29 100644\n--- a/lib/librte_eal/include/rte_eal_interrupts.h\n+++ b/lib/librte_eal/include/rte_eal_interrupts.h\n@@ -59,7 +59,7 @@ enum {\n \n /** interrupt epoll event obj, taken by epoll_event.ptr */\n struct rte_epoll_event {\n-\tvolatile uint32_t status; /**< OUT: event status */\n+\tuint32_t status; /**< OUT: event status */\n \tint fd; /**< OUT: event fd */\n \tint epfd; /**< OUT: epoll instance the ev associated with */\n \tstruct rte_epoll_data epdata;\ndiff --git a/lib/librte_eal/linux/eal_interrupts.c b/lib/librte_eal/linux/eal_interrupts.c\nindex 84eeaa1..7a50869 100644\n--- a/lib/librte_eal/linux/eal_interrupts.c\n+++ b/lib/librte_eal/linux/eal_interrupts.c\n@@ -26,7 +26,6 @@\n #include <rte_eal.h>\n #include <rte_per_lcore.h>\n #include <rte_lcore.h>\n-#include <rte_atomic.h>\n #include <rte_branch_prediction.h>\n #include <rte_debug.h>\n #include <rte_log.h>\n@@ -1221,11 +1220,18 @@ eal_epoll_process_event(struct epoll_event *evs, unsigned int n,\n {\n \tunsigned int i, count = 0;\n \tstruct rte_epoll_event *rev;\n+\tuint32_t valid_status;\n \n \tfor (i = 0; i < n; i++) {\n \t\trev = evs[i].data.ptr;\n-\t\tif (!rev || !rte_atomic32_cmpset(&rev->status, RTE_EPOLL_VALID,\n-\t\t\t\t\t\t RTE_EPOLL_EXEC))\n+\t\tvalid_status = RTE_EPOLL_VALID;\n+\t\t/* ACQUIRE memory ordering here pairs with RELEASE\n+\t\t * ordering bellow acting as a lock to synchronize\n+\t\t * the event data updating.\n+\t\t */\n+\t\tif (!rev || !__atomic_compare_exchange_n(&rev->status,\n+\t\t\t\t &valid_status, RTE_EPOLL_EXEC, 0,\n+\t\t\t\t __ATOMIC_ACQUIRE, __ATOMIC_RELAXED))\n \t\t\tcontinue;\n \n \t\tevents[count].status = RTE_EPOLL_VALID;\n@@ -1237,8 +1243,11 @@ eal_epoll_process_event(struct epoll_event *evs, unsigned int n,\n \t\t\trev->epdata.cb_fun(rev->fd,\n \t\t\t\t\t rev->epdata.cb_arg);\n \n-\t\trte_compiler_barrier();\n-\t\trev->status = RTE_EPOLL_VALID;\n+\t\t/* the status update should be observed after\n+\t\t * the other fields changes.\n+\t\t */\n+\t\t__atomic_store_n(&rev->status, RTE_EPOLL_VALID,\n+\t\t\t\t__ATOMIC_RELEASE);\n \t\tcount++;\n \t}\n \treturn count;\n@@ -1308,10 +1317,14 @@ rte_epoll_wait(int epfd, struct rte_epoll_event *events,\n static inline void\n eal_epoll_data_safe_free(struct rte_epoll_event *ev)\n {\n-\twhile (!rte_atomic32_cmpset(&ev->status, RTE_EPOLL_VALID,\n-\t\t\t\t RTE_EPOLL_INVALID))\n-\t\twhile (ev->status != RTE_EPOLL_VALID)\n+\tuint32_t valid_status = RTE_EPOLL_VALID;\n+\twhile (!__atomic_compare_exchange_n(&ev->status, &valid_status,\n+\t\t RTE_EPOLL_INVALID, 0, __ATOMIC_ACQUIRE, __ATOMIC_RELAXED)) {\n+\t\twhile (__atomic_load_n(&ev->status,\n+\t\t\t\t__ATOMIC_RELAXED) != RTE_EPOLL_VALID)\n \t\t\trte_pause();\n+\t\tvalid_status = RTE_EPOLL_VALID;\n+\t}\n \tmemset(&ev->epdata, 0, sizeof(ev->epdata));\n \tev->fd = -1;\n \tev->epfd = -1;\n@@ -1333,7 +1346,8 @@ rte_epoll_ctl(int epfd, int op, int fd,\n \t\tepfd = rte_intr_tls_epfd();\n \n \tif (op == EPOLL_CTL_ADD) {\n-\t\tevent->status = RTE_EPOLL_VALID;\n+\t\t__atomic_store_n(&event->status, RTE_EPOLL_VALID,\n+\t\t\t\t__ATOMIC_RELAXED);\n \t\tevent->fd = fd; /* ignore fd in event */\n \t\tevent->epfd = epfd;\n \t\tev.data.ptr = (void *)event;\n@@ -1345,11 +1359,13 @@ rte_epoll_ctl(int epfd, int op, int fd,\n \t\t\top, fd, strerror(errno));\n \t\tif (op == EPOLL_CTL_ADD)\n \t\t\t/* rollback status when CTL_ADD fail */\n-\t\t\tevent->status = RTE_EPOLL_INVALID;\n+\t\t\t__atomic_store_n(&event->status, RTE_EPOLL_INVALID,\n+\t\t\t\t\t__ATOMIC_RELAXED);\n \t\treturn -1;\n \t}\n \n-\tif (op == EPOLL_CTL_DEL && event->status != RTE_EPOLL_INVALID)\n+\tif (op == EPOLL_CTL_DEL && __atomic_load_n(&event->status,\n+\t\t\t__ATOMIC_RELAXED) != RTE_EPOLL_INVALID)\n \t\teal_epoll_data_safe_free(event);\n \n \treturn 0;\n@@ -1378,7 +1394,8 @@ rte_intr_rx_ctl(struct rte_intr_handle *intr_handle, int epfd,\n \tcase RTE_INTR_EVENT_ADD:\n \t\tepfd_op = EPOLL_CTL_ADD;\n \t\trev = &intr_handle->elist[efd_idx];\n-\t\tif (rev->status != RTE_EPOLL_INVALID) {\n+\t\tif (__atomic_load_n(&rev->status,\n+\t\t\t\t__ATOMIC_RELAXED) != RTE_EPOLL_INVALID) {\n \t\t\tRTE_LOG(INFO, EAL, \"Event already been added.\\n\");\n \t\t\treturn -EEXIST;\n \t\t}\n@@ -1401,7 +1418,8 @@ rte_intr_rx_ctl(struct rte_intr_handle *intr_handle, int epfd,\n \tcase RTE_INTR_EVENT_DEL:\n \t\tepfd_op = EPOLL_CTL_DEL;\n \t\trev = &intr_handle->elist[efd_idx];\n-\t\tif (rev->status == RTE_EPOLL_INVALID) {\n+\t\tif (__atomic_load_n(&rev->status,\n+\t\t\t\t__ATOMIC_RELAXED) == RTE_EPOLL_INVALID) {\n \t\t\tRTE_LOG(INFO, EAL, \"Event does not exist.\\n\");\n \t\t\treturn -EPERM;\n \t\t}\n@@ -1426,12 +1444,12 @@ rte_intr_free_epoll_fd(struct rte_intr_handle *intr_handle)\n \n \tfor (i = 0; i < intr_handle->nb_efd; i++) {\n \t\trev = &intr_handle->elist[i];\n-\t\tif (rev->status == RTE_EPOLL_INVALID)\n+\t\tif (__atomic_load_n(&rev->status,\n+\t\t\t\t__ATOMIC_RELAXED) == RTE_EPOLL_INVALID)\n \t\t\tcontinue;\n \t\tif (rte_epoll_ctl(rev->epfd, EPOLL_CTL_DEL, rev->fd, rev)) {\n \t\t\t/* force free if the entry valid */\n \t\t\teal_epoll_data_safe_free(rev);\n-\t\t\trev->status = RTE_EPOLL_INVALID;\n \t\t}\n \t}\n }\n", "prefixes": [ "v2" ] }