Patch Detail
get:
Show a patch.
patch:
Update a patch.
put:
Update a patch.
GET /api/patches/74493/?format=api
{ "id": 74493, "url": "https://patches.dpdk.org/api/patches/74493/?format=api", "web_url": "https://patches.dpdk.org/project/dpdk/patch/20200720120938.34660-1-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": "<20200720120938.34660-1-harry.van.haaren@intel.com>", "list_archive_url": "https://inbox.dpdk.org/dev/20200720120938.34660-1-harry.van.haaren@intel.com", "date": "2020-07-20T12:09:38", "name": "service: fix stop API to wait for service thread", "commit_ref": null, "pull_url": null, "state": "superseded", "archived": true, "hash": "654a740cadf395fbd137b8c2d6f88acbe9dec486", "submitter": { "id": 317, "url": "https://patches.dpdk.org/api/people/317/?format=api", "name": "Van Haaren, Harry", "email": "harry.van.haaren@intel.com" }, "delegate": null, "mbox": "https://patches.dpdk.org/project/dpdk/patch/20200720120938.34660-1-harry.van.haaren@intel.com/mbox/", "series": [ { "id": 11170, "url": "https://patches.dpdk.org/api/series/11170/?format=api", "web_url": "https://patches.dpdk.org/project/dpdk/list/?series=11170", "date": "2020-07-20T12:09:38", "name": "service: fix stop API to wait for service thread", "version": 1, "mbox": "https://patches.dpdk.org/series/11170/mbox/" } ], "comments": "https://patches.dpdk.org/api/patches/74493/comments/", "check": "fail", "checks": "https://patches.dpdk.org/api/patches/74493/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 68C22A0540;\n\tMon, 20 Jul 2020 14:08:49 +0200 (CEST)", "from [92.243.14.124] (localhost [127.0.0.1])\n\tby dpdk.org (Postfix) with ESMTP id 9F48229CB;\n\tMon, 20 Jul 2020 14:08:48 +0200 (CEST)", "from mga05.intel.com (mga05.intel.com [192.55.52.43])\n by dpdk.org (Postfix) with ESMTP id 4117325B3\n for <dev@dpdk.org>; Mon, 20 Jul 2020 14:08:46 +0200 (CEST)", "from fmsmga005.fm.intel.com ([10.253.24.32])\n by fmsmga105.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384;\n 20 Jul 2020 05:08:45 -0700", "from silpixa00399779.ir.intel.com (HELO\n silpixa00399779.ger.corp.intel.com) ([10.237.222.209])\n by fmsmga005.fm.intel.com with ESMTP; 20 Jul 2020 05:08:43 -0700" ], "IronPort-SDR": [ "\n DNvr1Yz2Iv5O9a+SBCXrLjP+B/ffDAL+94g7h0tGKTJnV6thRaoNKt44INISkdeLt7SA0j4oYe\n tiJKxM6/XPuA==", "\n ybks7pDfn5c57GDM30InwQYQaL5VPFZG8NJRag2fpy0biwt+4iAs1Bec5Ikbs8ZzccYxZRpiNy\n 5llmuRN8unzg==" ], "X-IronPort-AV": [ "E=McAfee;i=\"6000,8403,9687\"; a=\"234747612\"", "E=Sophos;i=\"5.75,374,1589266800\"; d=\"scan'208\";a=\"234747612\"", "E=Sophos;i=\"5.75,374,1589266800\"; d=\"scan'208\";a=\"487703130\"" ], "X-Amp-Result": "SKIPPED(no attachment in message)", "X-Amp-File-Uploaded": "False", "X-ExtLoop1": "1", "From": "Harry van Haaren <harry.van.haaren@intel.com>", "To": "dev@dpdk.org", "Cc": "david.marchand@redhat.com, igor.romanov@oktetlabs.ru,\n honnappa.nagarahalli@arm.com, ferruh.yigit@intel.com, nd@arm.com,\n aconole@redhat.com, l.wojciechow@partner.samsung.com,\n Harry van Haaren <harry.van.haaren@intel.com>", "Date": "Mon, 20 Jul 2020 13:09:38 +0100", "Message-Id": "<20200720120938.34660-1-harry.van.haaren@intel.com>", "X-Mailer": "git-send-email 2.17.1", "Subject": "[dpdk-dev] [PATCH] service: fix stop API to wait for service thread", "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": "This commit improves the service_lcore_stop() implementation,\nwaiting for the service core in question to return. The service\nthread itself now has a variable to indicate if its thread is\nactive. When zero the service thread has completed its service,\nand has returned from the service_runner_func() function.\n\nThis fixes a race condition observed in the DPDK CI, where the\nstatistics of the service were not consistent with the expectation\ndue to the service thread still running, and incrementing a stat\nafter stop was called.\n\nSigned-off-by: Harry van Haaren <harry.van.haaren@intel.com>\n\n---\n\nThis is one possible solution, that avoids a class of race-conditions\nbased on stop() api and following behaviours. Without a change in\nimplementation of the service core thread, we could not detect when\nthe thread was actually finished. This is now possible, and the stop\napi makes use of it to wait for 1000x one millisecond, or log a warning\nthat a service core didn't return quickly.\n\nThanks for the discussion/debug on list - I'm not sure how to add\nreported-by/suggested-by etc tags: but I'll resend a V2 (or can add\non apply).\n\n---\n lib/librte_eal/common/rte_service.c | 24 ++++++++++++++++++++++++\n 1 file changed, 24 insertions(+)", "diff": "diff --git a/lib/librte_eal/common/rte_service.c b/lib/librte_eal/common/rte_service.c\nindex 6a0e0ff65..d2255587d 100644\n--- a/lib/librte_eal/common/rte_service.c\n+++ b/lib/librte_eal/common/rte_service.c\n@@ -65,6 +65,7 @@ struct core_state {\n \t/* map of services IDs are run on this core */\n \tuint64_t service_mask;\n \tuint8_t runstate; /* running or stopped */\n+\tuint8_t thread_active; /* indicates when the thread is in service_run() */\n \tuint8_t is_service_core; /* set if core is currently a service core */\n \tuint8_t service_active_on_lcore[RTE_SERVICE_NUM_MAX];\n \tuint64_t loops;\n@@ -457,6 +458,8 @@ service_runner_func(void *arg)\n \tconst int lcore = rte_lcore_id();\n \tstruct core_state *cs = &lcore_states[lcore];\n \n+\t__atomic_store_n(&cs->thread_active, 1, __ATOMIC_RELAXED);\n+\n \t/* runstate act as the guard variable. Use load-acquire\n \t * memory order here to synchronize with store-release\n \t * in runstate update functions.\n@@ -475,6 +478,7 @@ service_runner_func(void *arg)\n \t\tcs->loops++;\n \t}\n \n+\t__atomic_store_n(&cs->thread_active, 0, __ATOMIC_RELAXED);\n \treturn 0;\n }\n \n@@ -765,6 +769,26 @@ rte_service_lcore_stop(uint32_t lcore)\n \t__atomic_store_n(&lcore_states[lcore].runstate, RUNSTATE_STOPPED,\n \t\t__ATOMIC_RELEASE);\n \n+\t/* wait for service lcore to return */\n+\ti = 0;\n+\tuint8_t active;\n+\tuint64_t start = rte_rdtsc();\n+\tdo {\n+\t\tactive = __atomic_load_n(&lcore_states[lcore].thread_active,\n+\t\t\t\t\t __ATOMIC_RELAXED);\n+\t\tif (active == 0)\n+\t\t\tbreak;\n+\t\trte_delay_ms(1);\n+\t\ti++;\n+\t} while (i < 1000);\n+\n+\tif (active != 0) {\n+\t\tuint64_t end = rte_rdtsc();\n+\t\tRTE_LOG(WARNING, EAL,\n+\t\t\t\"service lcore stop() failed, waited for %ld cycles\\n\",\n+\t\t\tend - start);\n+\t}\n+\n \treturn 0;\n }\n \n", "prefixes": [] }