Patch Detail

HTTP 200 OK
Allow: GET, PUT, PATCH, HEAD, OPTIONS
Content-Type: application/json
Vary: Accept

{
    "id": 6625,
    "url": "https://patches.dpdk.org/api/patches/6625/?format=api",
    "web_url": "https://patches.dpdk.org/project/dpdk/patch/1438037168-639-2-git-send-email-rsanford2@gmail.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": "<1438037168-639-2-git-send-email-rsanford2@gmail.com>",
    "list_archive_url": "https://inbox.dpdk.org/dev/1438037168-639-2-git-send-email-rsanford2@gmail.com",
    "date": "2015-07-27T22:46:04",
    "name": "[dpdk-dev,v2,1/3] timer: fix stress test 2 synchronization bug",
    "commit_ref": null,
    "pull_url": null,
    "state": "accepted",
    "archived": true,
    "hash": "d7ca8013a614ed6d3e7dd0f0fdf94728f8fb076a",
    "submitter": {
        "id": 7,
        "url": "https://patches.dpdk.org/api/people/7/?format=api",
        "name": "Robert Sanford",
        "email": "rsanford2@gmail.com"
    },
    "delegate": null,
    "mbox": "https://patches.dpdk.org/project/dpdk/patch/1438037168-639-2-git-send-email-rsanford2@gmail.com/mbox/",
    "series": [],
    "comments": "https://patches.dpdk.org/api/patches/6625/comments/",
    "check": "pending",
    "checks": "https://patches.dpdk.org/api/patches/6625/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 CF243C5E6;\n\tTue, 28 Jul 2015 00:46:27 +0200 (CEST)",
            "from mail-ig0-f169.google.com (mail-ig0-f169.google.com\n\t[209.85.213.169]) by dpdk.org (Postfix) with ESMTP id 9EA57C5A8\n\tfor <dev@dpdk.org>; Tue, 28 Jul 2015 00:46:24 +0200 (CEST)",
            "by igbij6 with SMTP id ij6so92476206igb.1\n\tfor <dev@dpdk.org>; Mon, 27 Jul 2015 15:46:24 -0700 (PDT)",
            "from localhost.localdomain ([23.79.237.14])\n\tby smtp.gmail.com with ESMTPSA id\n\tj18sm7063061igf.2.2015.07.27.15.46.23\n\t(version=TLSv1.2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128);\n\tMon, 27 Jul 2015 15:46:23 -0700 (PDT)"
        ],
        "DKIM-Signature": "v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20120113;\n\th=from:to:cc:subject:date:message-id:in-reply-to:references;\n\tbh=mtddWISqhfEtYyxj8ghBrF7MoEdgVm39TrpuKN1ejkM=;\n\tb=hgZ6kzEnngdN2ykcybV6TzqQFsHTytZFLMUiuUMAvXt3EfuVarOizopmUaJTfMslEj\n\tGSELuZuBRn/y8PJDFywJxX9CX7VQ2F/JDfW391CQIcFlcSaAhcjVTBl6eL5xOYdJXomn\n\ttT1H+R4FYdYb1GfN8pJF8Ajlr3k26Ib9W8Te5MfRBoxebUPN6Zq4M+6QWynyU6+EE47u\n\tcT0aEJGIk5IodgI4WA2L1EI01bw29af6G0b8qgdfS0oOeK8jeMGiuqSra2+m/WxDsEds\n\tz5UQinRZrw8xOrOIM5IUWblXRSbHTZD7lzzICjxDruYMl14kmh51VlwDvrBQJ6giNxdw\n\t1NmA==",
        "X-Received": "by 10.107.137.87 with SMTP id l84mr48327828iod.119.1438037184216;\n\tMon, 27 Jul 2015 15:46:24 -0700 (PDT)",
        "From": "rsanford2@gmail.com",
        "To": "dev@dpdk.org",
        "Date": "Mon, 27 Jul 2015 18:46:04 -0400",
        "Message-Id": "<1438037168-639-2-git-send-email-rsanford2@gmail.com>",
        "X-Mailer": "git-send-email 1.7.1",
        "In-Reply-To": "<1437691347-58708-1-git-send-email-rsanford2@gmail.com>",
        "References": "<1437691347-58708-1-git-send-email-rsanford2@gmail.com>",
        "Subject": "[dpdk-dev] [PATCH v2 1/3] timer: fix stress test 2 synchronization\n\tbug",
        "X-BeenThere": "dev@dpdk.org",
        "X-Mailman-Version": "2.1.15",
        "Precedence": "list",
        "List-Id": "patches and discussions about DPDK <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": "From: Robert Sanford <rsanford@akamai.com>\n\nFix app/test timer stress test 2: Sometimes this test fails and\nseg-faults because the slave lcores get out of phase with the master.\nThe master uses a single int, 'ready', to synchronize multiple slave\nlcores through multiple phases of the test.\n\nTo resolve, we construct simple synchronization primitives that use one\natomic-int state variable per slave. The master tells the slaves when to\nstart, and then waits for all of them to finish. Each slave waits for\nthe master to tell it to start, and then tells the master when it has\nfinished.\n\nSigned-off-by: Robert Sanford <rsanford@akamai.com>\n---\n app/test/test_timer.c |  154 ++++++++++++++++++++++++++++++++++++++-----------\n 1 files changed, 121 insertions(+), 33 deletions(-)",
    "diff": "diff --git a/app/test/test_timer.c b/app/test/test_timer.c\nindex 73da5b6..944e2ad 100644\n--- a/app/test/test_timer.c\n+++ b/app/test/test_timer.c\n@@ -137,13 +137,13 @@\n #include <rte_random.h>\n #include <rte_malloc.h>\n \n-\n #define TEST_DURATION_S 20 /* in seconds */\n #define NB_TIMER 4\n \n #define RTE_LOGTYPE_TESTTIMER RTE_LOGTYPE_USER3\n \n static volatile uint64_t end_time;\n+static volatile int test_failed;\n \n struct mytimerinfo {\n \tstruct rte_timer tim;\n@@ -230,6 +230,64 @@ timer_stress_main_loop(__attribute__((unused)) void *arg)\n \treturn 0;\n }\n \n+/* Need to synchronize slave lcores through multiple steps. */\n+enum { SLAVE_WAITING = 1, SLAVE_RUN_SIGNAL, SLAVE_RUNNING, SLAVE_FINISHED };\n+static rte_atomic16_t slave_state[RTE_MAX_LCORE];\n+\n+static void\n+master_init_slaves(void)\n+{\n+\tunsigned i;\n+\n+\tRTE_LCORE_FOREACH_SLAVE(i) {\n+\t\trte_atomic16_set(&slave_state[i], SLAVE_WAITING);\n+\t}\n+}\n+\n+static void\n+master_start_slaves(void)\n+{\n+\tunsigned i;\n+\n+\tRTE_LCORE_FOREACH_SLAVE(i) {\n+\t\trte_atomic16_set(&slave_state[i], SLAVE_RUN_SIGNAL);\n+\t}\n+\tRTE_LCORE_FOREACH_SLAVE(i) {\n+\t\twhile (rte_atomic16_read(&slave_state[i]) != SLAVE_RUNNING)\n+\t\t\trte_pause();\n+\t}\n+}\n+\n+static void\n+master_wait_for_slaves(void)\n+{\n+\tunsigned i;\n+\n+\tRTE_LCORE_FOREACH_SLAVE(i) {\n+\t\twhile (rte_atomic16_read(&slave_state[i]) != SLAVE_FINISHED)\n+\t\t\trte_pause();\n+\t}\n+}\n+\n+static void\n+slave_wait_to_start(void)\n+{\n+\tunsigned lcore_id = rte_lcore_id();\n+\n+\twhile (rte_atomic16_read(&slave_state[lcore_id]) != SLAVE_RUN_SIGNAL)\n+\t\trte_pause();\n+\trte_atomic16_set(&slave_state[lcore_id], SLAVE_RUNNING);\n+}\n+\n+static void\n+slave_finish(void)\n+{\n+\tunsigned lcore_id = rte_lcore_id();\n+\n+\trte_atomic16_set(&slave_state[lcore_id], SLAVE_FINISHED);\n+}\n+\n+\n static volatile int cb_count = 0;\n \n /* callback for second stress test. will only be called\n@@ -247,31 +305,37 @@ timer_stress2_main_loop(__attribute__((unused)) void *arg)\n {\n \tstatic struct rte_timer *timers;\n \tint i, ret;\n-\tstatic volatile int ready = 0;\n \tuint64_t delay = rte_get_timer_hz() / 4;\n \tunsigned lcore_id = rte_lcore_id();\n+\tunsigned master = rte_get_master_lcore();\n \tint32_t my_collisions = 0;\n-\tstatic rte_atomic32_t collisions = RTE_ATOMIC32_INIT(0);\n+\tstatic rte_atomic32_t collisions;\n \n-\tif (lcore_id == rte_get_master_lcore()) {\n+\tif (lcore_id == master) {\n \t\tcb_count = 0;\n+\t\ttest_failed = 0;\n+\t\trte_atomic32_set(&collisions, 0);\n+\t\tmaster_init_slaves();\n \t\ttimers = rte_malloc(NULL, sizeof(*timers) * NB_STRESS2_TIMERS, 0);\n \t\tif (timers == NULL) {\n \t\t\tprintf(\"Test Failed\\n\");\n \t\t\tprintf(\"- Cannot allocate memory for timers\\n\" );\n-\t\t\treturn -1;\n+\t\t\ttest_failed = 1;\n+\t\t\tmaster_start_slaves();\n+\t\t\tgoto cleanup;\n \t\t}\n \t\tfor (i = 0; i < NB_STRESS2_TIMERS; i++)\n \t\t\trte_timer_init(&timers[i]);\n-\t\tready = 1;\n+\t\tmaster_start_slaves();\n \t} else {\n-\t\twhile (!ready)\n-\t\t\trte_pause();\n+\t\tslave_wait_to_start();\n+\t\tif (test_failed)\n+\t\t\tgoto cleanup;\n \t}\n \n \t/* have all cores schedule all timers on master lcore */\n \tfor (i = 0; i < NB_STRESS2_TIMERS; i++) {\n-\t\tret = rte_timer_reset(&timers[i], delay, SINGLE, rte_get_master_lcore(),\n+\t\tret = rte_timer_reset(&timers[i], delay, SINGLE, master,\n \t\t\t\ttimer_stress2_cb, NULL);\n \t\t/* there will be collisions when multiple cores simultaneously\n \t\t * configure the same timers */\n@@ -281,11 +345,18 @@ timer_stress2_main_loop(__attribute__((unused)) void *arg)\n \tif (my_collisions != 0)\n \t\trte_atomic32_add(&collisions, my_collisions);\n \n-\tready = 0;\n+\t/* wait long enough for timers to expire */\n \trte_delay_ms(500);\n \n+\t/* all cores rendezvous */\n+\tif (lcore_id == master) {\n+\t\tmaster_wait_for_slaves();\n+\t} else {\n+\t\tslave_finish();\n+\t}\n+\n \t/* now check that we get the right number of callbacks */\n-\tif (lcore_id == rte_get_master_lcore()) {\n+\tif (lcore_id == master) {\n \t\tmy_collisions = rte_atomic32_read(&collisions);\n \t\tif (my_collisions != 0)\n \t\t\tprintf(\"- %d timer reset collisions (OK)\\n\", my_collisions);\n@@ -295,49 +366,63 @@ timer_stress2_main_loop(__attribute__((unused)) void *arg)\n \t\t\tprintf(\"- Stress test 2, part 1 failed\\n\");\n \t\t\tprintf(\"- Expected %d callbacks, got %d\\n\", NB_STRESS2_TIMERS,\n \t\t\t\t\tcb_count);\n-\t\t\treturn -1;\n+\t\t\ttest_failed = 1;\n+\t\t\tmaster_start_slaves();\n+\t\t\tgoto cleanup;\n \t\t}\n-\t\tready  = 1;\n+\t\tcb_count = 0;\n+\n+\t\t/* proceed */\n+\t\tmaster_start_slaves();\n \t} else {\n-\t\twhile (!ready)\n-\t\t\trte_pause();\n+\t\t/* proceed */\n+\t\tslave_wait_to_start();\n+\t\tif (test_failed)\n+\t\t\tgoto cleanup;\n \t}\n \n \t/* now test again, just stop and restart timers at random after init*/\n \tfor (i = 0; i < NB_STRESS2_TIMERS; i++)\n-\t\trte_timer_reset(&timers[i], delay, SINGLE, rte_get_master_lcore(),\n+\t\trte_timer_reset(&timers[i], delay, SINGLE, master,\n \t\t\t\ttimer_stress2_cb, NULL);\n-\tcb_count = 0;\n \n \t/* pick random timer to reset, stopping them first half the time */\n \tfor (i = 0; i < 100000; i++) {\n \t\tint r = rand() % NB_STRESS2_TIMERS;\n \t\tif (i % 2)\n \t\t\trte_timer_stop(&timers[r]);\n-\t\trte_timer_reset(&timers[r], delay, SINGLE, rte_get_master_lcore(),\n+\t\trte_timer_reset(&timers[r], delay, SINGLE, master,\n \t\t\t\ttimer_stress2_cb, NULL);\n \t}\n \n+\t/* wait long enough for timers to expire */\n \trte_delay_ms(500);\n \n \t/* now check that we get the right number of callbacks */\n-\tif (lcore_id == rte_get_master_lcore()) {\n-\t\trte_timer_manage();\n-\n-\t\t/* clean up statics, in case we run again */\n-\t\trte_free(timers);\n-\t\ttimers = NULL;\n-\t\tready = 0;\n-\t\trte_atomic32_set(&collisions, 0);\n+\tif (lcore_id == master) {\n+\t\tmaster_wait_for_slaves();\n \n+\t\trte_timer_manage();\n \t\tif (cb_count != NB_STRESS2_TIMERS) {\n \t\t\tprintf(\"Test Failed\\n\");\n \t\t\tprintf(\"- Stress test 2, part 2 failed\\n\");\n \t\t\tprintf(\"- Expected %d callbacks, got %d\\n\", NB_STRESS2_TIMERS,\n \t\t\t\t\tcb_count);\n-\t\t\treturn -1;\n+\t\t\ttest_failed = 1;\n+\t\t} else {\n+\t\t\tprintf(\"Test OK\\n\");\n \t\t}\n-\t\tprintf(\"Test OK\\n\");\n+\t}\n+\n+cleanup:\n+\tif (lcore_id == master) {\n+\t\tmaster_wait_for_slaves();\n+\t\tif (timers != NULL) {\n+\t\t\trte_free(timers);\n+\t\t\ttimers = NULL;\n+\t\t}\n+\t} else {\n+\t\tslave_finish();\n \t}\n \n \treturn 0;\n@@ -485,12 +570,12 @@ test_timer(void)\n \t/* sanity check our timer sources and timer config values */\n \tif (timer_sanity_check() < 0) {\n \t\tprintf(\"Timer sanity checks failed\\n\");\n-\t\treturn -1;\n+\t\treturn TEST_FAILED;\n \t}\n \n \tif (rte_lcore_count() < 2) {\n \t\tprintf(\"not enough lcores for this test\\n\");\n-\t\treturn -1;\n+\t\treturn TEST_FAILED;\n \t}\n \n \t/* init timer */\n@@ -514,9 +599,12 @@ test_timer(void)\n \trte_timer_stop_sync(&mytiminfo[0].tim);\n \n \t/* run a second, slightly different set of stress tests */\n-\tprintf(\"Start timer stress tests 2\\n\");\n+\tprintf(\"\\nStart timer stress tests 2\\n\");\n+\ttest_failed = 0;\n \trte_eal_mp_remote_launch(timer_stress2_main_loop, NULL, CALL_MASTER);\n \trte_eal_mp_wait_lcore();\n+\tif (test_failed)\n+\t\treturn TEST_FAILED;\n \n \t/* calculate the \"end of test\" time */\n \tcur_time = rte_get_timer_cycles();\n@@ -524,7 +612,7 @@ test_timer(void)\n \tend_time = cur_time + (hz * TEST_DURATION_S);\n \n \t/* start other cores */\n-\tprintf(\"Start timer basic tests (%d seconds)\\n\", TEST_DURATION_S);\n+\tprintf(\"\\nStart timer basic tests (%d seconds)\\n\", TEST_DURATION_S);\n \trte_eal_mp_remote_launch(timer_basic_main_loop, NULL, CALL_MASTER);\n \trte_eal_mp_wait_lcore();\n \n@@ -535,7 +623,7 @@ test_timer(void)\n \n \trte_timer_dump_stats(stdout);\n \n-\treturn 0;\n+\treturn TEST_SUCCESS;\n }\n \n static struct test_command timer_cmd = {\n",
    "prefixes": [
        "dpdk-dev",
        "v2",
        "1/3"
    ]
}