Patch Detail
get:
Show a patch.
patch:
Update a patch.
put:
Update a patch.
GET /api/patches/27691/?format=api
{ "id": 27691, "url": "https://patches.dpdk.org/api/patches/27691/?format=api", "web_url": "https://patches.dpdk.org/project/dpdk/patch/1503320296-51122-9-git-send-email-harry.van.haaren@intel.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": "<1503320296-51122-9-git-send-email-harry.van.haaren@intel.com>", "list_archive_url": "https://inbox.dpdk.org/dev/1503320296-51122-9-git-send-email-harry.van.haaren@intel.com", "date": "2017-08-21T12:58:09", "name": "[dpdk-dev,v2,08/15] service: fix and refactor atomic service accesses", "commit_ref": null, "pull_url": null, "state": "accepted", "archived": true, "hash": "888e5e198ffd4f4a48539dbdd0ea5b6de7e7ed4b", "submitter": { "id": 317, "url": "https://patches.dpdk.org/api/people/317/?format=api", "name": "Van Haaren, Harry", "email": "harry.van.haaren@intel.com" }, "delegate": { "id": 1, "url": "https://patches.dpdk.org/api/users/1/?format=api", "username": "tmonjalo", "first_name": "Thomas", "last_name": "Monjalon", "email": "thomas@monjalon.net" }, "mbox": "https://patches.dpdk.org/project/dpdk/patch/1503320296-51122-9-git-send-email-harry.van.haaren@intel.com/mbox/", "series": [], "comments": "https://patches.dpdk.org/api/patches/27691/comments/", "check": "success", "checks": "https://patches.dpdk.org/api/patches/27691/checks/", "tags": {}, "related": [], "headers": { "Return-Path": "<dev-bounces@dpdk.org>", "X-Original-To": "patchwork@dpdk.org", "Delivered-To": "patchwork@dpdk.org", "Received": [ "from [92.243.14.124] (localhost [IPv6:::1])\n\tby dpdk.org (Postfix) with ESMTP id 07E587D86;\n\tMon, 21 Aug 2017 14:58:41 +0200 (CEST)", "from mga05.intel.com (mga05.intel.com [192.55.52.43])\n\tby dpdk.org (Postfix) with ESMTP id E289B7D0F\n\tfor <dev@dpdk.org>; Mon, 21 Aug 2017 14:58:34 +0200 (CEST)", "from fmsmga002.fm.intel.com ([10.253.24.26])\n\tby fmsmga105.fm.intel.com with ESMTP; 21 Aug 2017 05:58:34 -0700", "from silpixa00398672.ir.intel.com ([10.237.223.128])\n\tby fmsmga002.fm.intel.com with ESMTP; 21 Aug 2017 05:58:33 -0700" ], "X-ExtLoop1": "1", "X-IronPort-AV": "E=Sophos; i=\"5.41,409,1498546800\"; d=\"scan'208\";\n\ta=\"1208383314\"", "From": "Harry van Haaren <harry.van.haaren@intel.com>", "To": "dev@dpdk.org", "Cc": "Harry van Haaren <harry.van.haaren@intel.com>", "Date": "Mon, 21 Aug 2017 13:58:09 +0100", "Message-Id": "<1503320296-51122-9-git-send-email-harry.van.haaren@intel.com>", "X-Mailer": "git-send-email 2.7.4", "In-Reply-To": "<1503320296-51122-1-git-send-email-harry.van.haaren@intel.com>", "References": "<1502800360-15782-1-git-send-email-harry.van.haaren@intel.com>\n\t<1503320296-51122-1-git-send-email-harry.van.haaren@intel.com>", "Subject": "[dpdk-dev] [PATCH v2 08/15] service: fix and refactor atomic\n\tservice accesses", "X-BeenThere": "dev@dpdk.org", "X-Mailman-Version": "2.1.15", "Precedence": "list", "List-Id": "DPDK patches and discussions <dev.dpdk.org>", "List-Unsubscribe": "<http://dpdk.org/ml/options/dev>,\n\t<mailto:dev-request@dpdk.org?subject=unsubscribe>", "List-Archive": "<http://dpdk.org/ml/archives/dev/>", "List-Post": "<mailto:dev@dpdk.org>", "List-Help": "<mailto:dev-request@dpdk.org?subject=help>", "List-Subscribe": "<http://dpdk.org/ml/listinfo/dev>,\n\t<mailto:dev-request@dpdk.org?subject=subscribe>", "Errors-To": "dev-bounces@dpdk.org", "Sender": "\"dev\" <dev-bounces@dpdk.org>" }, "content": "This commit fixes an issue in the service runner function,\nwhere the atomic value was not cleared on exiting the service\nfunction. This resulted in future attempts to run the service\nto appear like the function was running, however it was in\nreality deadlocked.\n\nThis commit refactors the atomic handling to be more readable,\nby splitting the implementation code into a new static inline\nfunction. The remaining flow control of atomics in the existing\nfunction is refactored for readability.\n\nFixes: 21698354c832 (\"service: introduce service cores concept\")\n\nSigned-off-by: Harry van Haaren <harry.van.haaren@intel.com>\n---\n doc/guides/rel_notes/release_17_11.rst | 7 ++++++\n lib/librte_eal/common/rte_service.c | 45 ++++++++++++++++++++--------------\n 2 files changed, 33 insertions(+), 19 deletions(-)", "diff": "diff --git a/doc/guides/rel_notes/release_17_11.rst b/doc/guides/rel_notes/release_17_11.rst\nindex 170f4f9..6e6ba1c 100644\n--- a/doc/guides/rel_notes/release_17_11.rst\n+++ b/doc/guides/rel_notes/release_17_11.rst\n@@ -65,6 +65,13 @@ Resolved Issues\n EAL\n ~~~\n \n+* **Service core fails to call service callback due to atomic lock**\n+\n+ In a specific configuration of multi-thread unsafe services and service\n+ cores, a service core previously did not correctly release the atomic lock\n+ on the service. This would result in the cores polling the service, but it\n+ looked like another thread was executing the service callback. The logic for\n+ atomic locking of the services has been fixed and refactored for readability.\n \n Drivers\n ~~~~~~~\ndiff --git a/lib/librte_eal/common/rte_service.c b/lib/librte_eal/common/rte_service.c\nindex 1bfd149..6291c0c 100644\n--- a/lib/librte_eal/common/rte_service.c\n+++ b/lib/librte_eal/common/rte_service.c\n@@ -296,6 +296,23 @@ rte_service_runstate_get(uint32_t id)\n \treturn (s->runstate == RUNSTATE_RUNNING) && (s->num_mapped_cores > 0);\n }\n \n+static inline void\n+rte_service_runner_do_callback(struct rte_service_spec_impl *s,\n+\t\t\t struct core_state *cs, uint32_t service_idx)\n+{\n+\tvoid *userdata = s->spec.callback_userdata;\n+\n+\tif (service_stats_enabled(s)) {\n+\t\tuint64_t start = rte_rdtsc();\n+\t\ts->spec.callback(userdata);\n+\t\tuint64_t end = rte_rdtsc();\n+\t\ts->cycles_spent += end - start;\n+\t\tcs->calls_per_service[service_idx]++;\n+\t\ts->calls++;\n+\t} else\n+\t\ts->spec.callback(userdata);\n+}\n+\n static int32_t\n rte_service_runner_func(void *arg)\n {\n@@ -315,26 +332,16 @@ rte_service_runner_func(void *arg)\n \t\t\t/* check do we need cmpset, if MT safe or <= 1 core\n \t\t\t * mapped, atomic ops are not required.\n \t\t\t */\n-\t\t\tconst int need_cmpset = !((service_mt_safe(s) == 0) &&\n-\t\t\t\t\t\t(s->num_mapped_cores > 1));\n-\t\t\tuint32_t *lock = (uint32_t *)&s->execute_lock;\n-\n-\t\t\tif (need_cmpset || rte_atomic32_cmpset(lock, 0, 1)) {\n-\t\t\t\tvoid *userdata = s->spec.callback_userdata;\n-\n-\t\t\t\tif (service_stats_enabled(s)) {\n-\t\t\t\t\tuint64_t start = rte_rdtsc();\n-\t\t\t\t\ts->spec.callback(userdata);\n-\t\t\t\t\tuint64_t end = rte_rdtsc();\n-\t\t\t\t\ts->cycles_spent += end - start;\n-\t\t\t\t\tcs->calls_per_service[i]++;\n-\t\t\t\t\ts->calls++;\n-\t\t\t\t} else\n-\t\t\t\t\ts->spec.callback(userdata);\n-\n-\t\t\t\tif (need_cmpset)\n+\t\t\tconst int use_atomics = (service_mt_safe(s) == 0) &&\n+\t\t\t\t\t\t(s->num_mapped_cores > 1);\n+\t\t\tif (use_atomics) {\n+\t\t\t\tuint32_t *lock = (uint32_t *)&s->execute_lock;\n+\t\t\t\tif (rte_atomic32_cmpset(lock, 0, 1)) {\n+\t\t\t\t\trte_service_runner_do_callback(s, cs, i);\n \t\t\t\t\trte_atomic32_clear(&s->execute_lock);\n-\t\t\t}\n+\t\t\t\t}\n+\t\t\t} else\n+\t\t\t\trte_service_runner_do_callback(s, cs, i);\n \t\t}\n \n \t\trte_smp_rmb();\n", "prefixes": [ "dpdk-dev", "v2", "08/15" ] }