Patch Detail
get:
Show a patch.
patch:
Update a patch.
put:
Update a patch.
GET /api/patches/118063/?format=api
{ "id": 118063, "url": "http://patches.dpdk.org/api/patches/118063/?format=api", "web_url": "http://patches.dpdk.org/project/dpdk/patch/20221012123112.2951802-6-david.marchand@redhat.com/", "project": { "id": 1, "url": "http://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": "<20221012123112.2951802-6-david.marchand@redhat.com>", "list_archive_url": "https://inbox.dpdk.org/dev/20221012123112.2951802-6-david.marchand@redhat.com", "date": "2022-10-12T12:31:08", "name": "[v3,5/9] trace: fix dynamically enabling trace points", "commit_ref": null, "pull_url": null, "state": "superseded", "archived": true, "hash": "09c48f32677762138e85ffc1a355e8c1e9853fd8", "submitter": { "id": 1173, "url": "http://patches.dpdk.org/api/people/1173/?format=api", "name": "David Marchand", "email": "david.marchand@redhat.com" }, "delegate": { "id": 1, "url": "http://patches.dpdk.org/api/users/1/?format=api", "username": "tmonjalo", "first_name": "Thomas", "last_name": "Monjalon", "email": "thomas@monjalon.net" }, "mbox": "http://patches.dpdk.org/project/dpdk/patch/20221012123112.2951802-6-david.marchand@redhat.com/mbox/", "series": [ { "id": 25183, "url": "http://patches.dpdk.org/api/series/25183/?format=api", "web_url": "http://patches.dpdk.org/project/dpdk/list/?series=25183", "date": "2022-10-12T12:31:03", "name": "Trace subsystem fixes", "version": 3, "mbox": "http://patches.dpdk.org/series/25183/mbox/" } ], "comments": "http://patches.dpdk.org/api/patches/118063/comments/", "check": "success", "checks": "http://patches.dpdk.org/api/patches/118063/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 mails.dpdk.org (mails.dpdk.org [217.70.189.124])\n\tby inbox.dpdk.org (Postfix) with ESMTP id 2822EA00C4;\n\tWed, 12 Oct 2022 14:32:04 +0200 (CEST)", "from [217.70.189.124] (localhost [127.0.0.1])\n\tby mails.dpdk.org (Postfix) with ESMTP id 6886343082;\n\tWed, 12 Oct 2022 14:31:48 +0200 (CEST)", "from us-smtp-delivery-124.mimecast.com\n (us-smtp-delivery-124.mimecast.com [170.10.133.124])\n by mails.dpdk.org (Postfix) with ESMTP id 667AA4307E\n for <dev@dpdk.org>; Wed, 12 Oct 2022 14:31:47 +0200 (CEST)", "from mimecast-mx02.redhat.com (mx3-rdu2.redhat.com\n [66.187.233.73]) by relay.mimecast.com with ESMTP with STARTTLS\n (version=TLSv1.2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id\n us-mta-256-3J3eh5FjO821ZD-r0u4IhA-1; Wed, 12 Oct 2022 08:31:43 -0400", "from smtp.corp.redhat.com (int-mx06.intmail.prod.int.rdu2.redhat.com\n [10.11.54.6])\n (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits))\n (No client certificate requested)\n by mimecast-mx02.redhat.com (Postfix) with ESMTPS id 538D81C08969;\n Wed, 12 Oct 2022 12:31:43 +0000 (UTC)", "from localhost.localdomain (ovpn-193-115.brq.redhat.com\n [10.40.193.115])\n by smtp.corp.redhat.com (Postfix) with ESMTP id 350D92157F56;\n Wed, 12 Oct 2022 12:31:42 +0000 (UTC)" ], "DKIM-Signature": "v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com;\n s=mimecast20190719; t=1665577906;\n h=from:from:reply-to:subject:subject:date:date:message-id:message-id:\n to:to:cc:cc:mime-version:mime-version:content-type:content-type:\n content-transfer-encoding:content-transfer-encoding:\n in-reply-to:in-reply-to:references:references;\n bh=e4Tygx5p9+Fa4LrCghuMiU/eNiplWQc4UBewimcT8+Y=;\n b=JJ7cuYUDakGlwEJI+1QrtaUH+zoAYi3FgS42pkPfnQPMjrEyKS3nReBDqh/2leuGKcI/nZ\n gTI/FEk3FttcLltkqwM1BnrKDuI5Rd4klKkgZHgO559g16aSsOoPGX/MxoBTvNEgWIePyg\n aVIoyVCvFCd9G/wwuqbgVUvsUCOJUhw=", "X-MC-Unique": "3J3eh5FjO821ZD-r0u4IhA-1", "From": "David Marchand <david.marchand@redhat.com>", "To": "dev@dpdk.org", "Cc": "jerinj@marvell.com,\n\tskori@marvell.com,\n\tstable@dpdk.org", "Subject": "[PATCH v3 5/9] trace: fix dynamically enabling trace points", "Date": "Wed, 12 Oct 2022 14:31:08 +0200", "Message-Id": "<20221012123112.2951802-6-david.marchand@redhat.com>", "In-Reply-To": "<20221012123112.2951802-1-david.marchand@redhat.com>", "References": "<20220921120359.2201131-1-david.marchand@redhat.com>\n <20221012123112.2951802-1-david.marchand@redhat.com>", "MIME-Version": "1.0", "X-Scanned-By": "MIMEDefang 3.1 on 10.11.54.6", "X-Mimecast-Spam-Score": "0", "X-Mimecast-Originator": "redhat.com", "Content-Transfer-Encoding": "8bit", "Content-Type": "text/plain; charset=\"US-ASCII\"; x-default=true", "X-BeenThere": "dev@dpdk.org", "X-Mailman-Version": "2.1.29", "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" }, "content": "Enabling trace points at runtime was not working if no trace point had\nbeen enabled first at rte_eal_init() time. The reason was that\ntrace.args reflected the arguments passed to --trace= EAL option.\n\nTo fix this:\n- the trace subsystem initialisation is updated: trace directory\n creation is deferred to when traces are dumped (to avoid creating\n directories that may not be used),\n- per lcore memory allocation still relies on rte_trace_is_enabled() but\n this helper now tracks if any trace point is enabled. The\n documentation is updated accordingly,\n- cleanup helpers must always be called in rte_eal_cleanup() since some\n trace points might have been enabled and disabled in the lifetime of\n the DPDK application,\n\nWith this fix, we can update the unit test and check that a trace point\ncallback is invoked when expected.\n\nNote:\n- the 'trace' global variable might be shadowed with the argument\n passed to the functions dealing with trace point handles.\n 'tp' has been used for referring to trace_point object.\n Prefer 't' for referring to handles,\n\nFixes: 84c4fae4628f (\"trace: implement operation APIs\")\nCc: stable@dpdk.org\n\nSigned-off-by: David Marchand <david.marchand@redhat.com>\nAcked-by: Sunil Kumar Kori <skori@marvell.com>\n---\nChanges since v1:\n- restored level to INFO for trace directory log message,\n- moved trace_mkdir() to rte_trace_save,\n\n---\n app/test/test_trace.c | 20 ++++++++++\n app/test/test_trace.h | 2 +\n doc/guides/prog_guide/trace_lib.rst | 14 +++++--\n lib/eal/common/eal_common_trace.c | 53 ++++++++++---------------\n lib/eal/common/eal_common_trace_utils.c | 11 ++++-\n lib/eal/common/eal_trace.h | 3 +-\n 6 files changed, 65 insertions(+), 38 deletions(-)", "diff": "diff --git a/app/test/test_trace.c b/app/test/test_trace.c\nindex 44ac38a4fa..2660f52f1d 100644\n--- a/app/test/test_trace.c\n+++ b/app/test/test_trace.c\n@@ -9,6 +9,8 @@\n #include \"test.h\"\n #include \"test_trace.h\"\n \n+int app_dpdk_test_tp_count;\n+\n #ifdef RTE_EXEC_ENV_WINDOWS\n \n static int\n@@ -95,8 +97,15 @@ test_trace_point_regex(void)\n static int32_t\n test_trace_point_disable_enable(void)\n {\n+\tint expected;\n \tint rc;\n \n+\t/* At tp registration, the associated counter increases once. */\n+\texpected = 1;\n+\tTEST_ASSERT_EQUAL(app_dpdk_test_tp_count, expected,\n+\t\t\"Expecting %d, but got %d for app_dpdk_test_tp_count\",\n+\t\texpected, app_dpdk_test_tp_count);\n+\n \trc = rte_trace_point_disable(&__app_dpdk_test_tp);\n \tif (rc < 0)\n \t\tgoto failed;\n@@ -104,6 +113,12 @@ test_trace_point_disable_enable(void)\n \tif (rte_trace_point_is_enabled(&__app_dpdk_test_tp))\n \t\tgoto failed;\n \n+\t/* No emission expected */\n+\tapp_dpdk_test_tp(\"app.dpdk.test.tp\");\n+\tTEST_ASSERT_EQUAL(app_dpdk_test_tp_count, expected,\n+\t\t\"Expecting %d, but got %d for app_dpdk_test_tp_count\",\n+\t\texpected, app_dpdk_test_tp_count);\n+\n \trc = rte_trace_point_enable(&__app_dpdk_test_tp);\n \tif (rc < 0)\n \t\tgoto failed;\n@@ -113,6 +128,11 @@ test_trace_point_disable_enable(void)\n \n \t/* Emit the trace */\n \tapp_dpdk_test_tp(\"app.dpdk.test.tp\");\n+\texpected++;\n+\tTEST_ASSERT_EQUAL(app_dpdk_test_tp_count, expected,\n+\t\t\"Expecting %d, but got %d for app_dpdk_test_tp_count\",\n+\t\texpected, app_dpdk_test_tp_count);\n+\n \treturn TEST_SUCCESS;\n \n failed:\ndiff --git a/app/test/test_trace.h b/app/test/test_trace.h\nindex 413842f60d..4ad44e2bea 100644\n--- a/app/test/test_trace.h\n+++ b/app/test/test_trace.h\n@@ -3,10 +3,12 @@\n */\n #include <rte_trace_point.h>\n \n+extern int app_dpdk_test_tp_count;\n RTE_TRACE_POINT(\n \tapp_dpdk_test_tp,\n \tRTE_TRACE_POINT_ARGS(const char *str),\n \trte_trace_point_emit_string(str);\n+\tapp_dpdk_test_tp_count++;\n )\n \n RTE_TRACE_POINT_FP(\ndiff --git a/doc/guides/prog_guide/trace_lib.rst b/doc/guides/prog_guide/trace_lib.rst\nindex fbadf9fde9..9a8f38073d 100644\n--- a/doc/guides/prog_guide/trace_lib.rst\n+++ b/doc/guides/prog_guide/trace_lib.rst\n@@ -271,10 +271,16 @@ Trace memory\n The trace memory will be allocated through an internal function\n ``__rte_trace_mem_per_thread_alloc()``. The trace memory will be allocated\n per thread to enable lock less trace-emit function.\n-The memory for the trace memory for DPDK lcores will be allocated on\n-``rte_eal_init()`` if the trace is enabled through a EAL option.\n-For non DPDK threads, on the first trace emission, the memory will be\n-allocated.\n+\n+For non lcore threads, the trace memory is allocated on the first trace\n+emission.\n+\n+For lcore threads, if trace points are enabled through a EAL option, the trace\n+memory is allocated when the threads are known of DPDK\n+(``rte_eal_init`` for EAL lcores, ``rte_thread_register`` for non-EAL lcores).\n+Otherwise, when trace points are enabled later in the life of the application,\n+the behavior is the same as non lcore threads and the trace memory is allocated\n+on the first trace emission.\n \n Trace memory layout\n ~~~~~~~~~~~~~~~~~~~\ndiff --git a/lib/eal/common/eal_common_trace.c b/lib/eal/common/eal_common_trace.c\nindex 6b8660c318..6aa11a3b50 100644\n--- a/lib/eal/common/eal_common_trace.c\n+++ b/lib/eal/common/eal_common_trace.c\n@@ -48,12 +48,6 @@ eal_trace_init(void)\n \t\tgoto fail;\n \t}\n \n-\tif (!STAILQ_EMPTY(&trace.args))\n-\t\ttrace.status = true;\n-\n-\tif (!rte_trace_is_enabled())\n-\t\treturn 0;\n-\n \trte_spinlock_init(&trace.lock);\n \n \t/* Is duplicate trace name registered */\n@@ -72,13 +66,9 @@ eal_trace_init(void)\n \tif (trace_metadata_create() < 0)\n \t\tgoto fail;\n \n-\t/* Create trace directory */\n-\tif (trace_mkdir())\n-\t\tgoto free_meta;\n-\n \t/* Save current epoch timestamp for future use */\n \tif (trace_epoch_time_save() < 0)\n-\t\tgoto fail;\n+\t\tgoto free_meta;\n \n \t/* Apply global configurations */\n \tSTAILQ_FOREACH(arg, &trace.args, next)\n@@ -98,8 +88,6 @@ eal_trace_init(void)\n void\n eal_trace_fini(void)\n {\n-\tif (!rte_trace_is_enabled())\n-\t\treturn;\n \ttrace_mem_free();\n \ttrace_metadata_destroy();\n \teal_trace_args_free();\n@@ -108,17 +96,17 @@ eal_trace_fini(void)\n bool\n rte_trace_is_enabled(void)\n {\n-\treturn trace.status;\n+\treturn __atomic_load_n(&trace.status, __ATOMIC_ACQUIRE) != 0;\n }\n \n static void\n-trace_mode_set(rte_trace_point_t *trace, enum rte_trace_mode mode)\n+trace_mode_set(rte_trace_point_t *t, enum rte_trace_mode mode)\n {\n \tif (mode == RTE_TRACE_MODE_OVERWRITE)\n-\t\t__atomic_and_fetch(trace, ~__RTE_TRACE_FIELD_ENABLE_DISCARD,\n+\t\t__atomic_and_fetch(t, ~__RTE_TRACE_FIELD_ENABLE_DISCARD,\n \t\t\t__ATOMIC_RELEASE);\n \telse\n-\t\t__atomic_or_fetch(trace, __RTE_TRACE_FIELD_ENABLE_DISCARD,\n+\t\t__atomic_or_fetch(t, __RTE_TRACE_FIELD_ENABLE_DISCARD,\n \t\t\t__ATOMIC_RELEASE);\n }\n \n@@ -146,36 +134,42 @@ trace_point_is_invalid(rte_trace_point_t *t)\n }\n \n bool\n-rte_trace_point_is_enabled(rte_trace_point_t *trace)\n+rte_trace_point_is_enabled(rte_trace_point_t *t)\n {\n \tuint64_t val;\n \n-\tif (trace_point_is_invalid(trace))\n+\tif (trace_point_is_invalid(t))\n \t\treturn false;\n \n-\tval = __atomic_load_n(trace, __ATOMIC_ACQUIRE);\n+\tval = __atomic_load_n(t, __ATOMIC_ACQUIRE);\n \treturn (val & __RTE_TRACE_FIELD_ENABLE_MASK) != 0;\n }\n \n int\n-rte_trace_point_enable(rte_trace_point_t *trace)\n+rte_trace_point_enable(rte_trace_point_t *t)\n {\n-\tif (trace_point_is_invalid(trace))\n+\tuint64_t prev;\n+\n+\tif (trace_point_is_invalid(t))\n \t\treturn -ERANGE;\n \n-\t__atomic_or_fetch(trace, __RTE_TRACE_FIELD_ENABLE_MASK,\n-\t\t__ATOMIC_RELEASE);\n+\tprev = __atomic_fetch_or(t, __RTE_TRACE_FIELD_ENABLE_MASK, __ATOMIC_RELEASE);\n+\tif ((prev & __RTE_TRACE_FIELD_ENABLE_MASK) == 0)\n+\t\t__atomic_add_fetch(&trace.status, 1, __ATOMIC_RELEASE);\n \treturn 0;\n }\n \n int\n-rte_trace_point_disable(rte_trace_point_t *trace)\n+rte_trace_point_disable(rte_trace_point_t *t)\n {\n-\tif (trace_point_is_invalid(trace))\n+\tuint64_t prev;\n+\n+\tif (trace_point_is_invalid(t))\n \t\treturn -ERANGE;\n \n-\t__atomic_and_fetch(trace, ~__RTE_TRACE_FIELD_ENABLE_MASK,\n-\t\t__ATOMIC_RELEASE);\n+\tprev = __atomic_fetch_and(t, ~__RTE_TRACE_FIELD_ENABLE_MASK, __ATOMIC_RELEASE);\n+\tif ((prev & __RTE_TRACE_FIELD_ENABLE_MASK) != 0)\n+\t\t__atomic_sub_fetch(&trace.status, 1, __ATOMIC_RELEASE);\n \treturn 0;\n }\n \n@@ -417,9 +411,6 @@ trace_mem_free(void)\n \tstruct trace *trace = trace_obj_get();\n \tuint32_t count;\n \n-\tif (!rte_trace_is_enabled())\n-\t\treturn;\n-\n \trte_spinlock_lock(&trace->lock);\n \tfor (count = 0; count < trace->nb_trace_mem_list; count++) {\n \t\ttrace_mem_per_thread_free_unlocked(&trace->lcore_meta[count]);\ndiff --git a/lib/eal/common/eal_common_trace_utils.c b/lib/eal/common/eal_common_trace_utils.c\nindex 2b55dbec65..7bf1c05e12 100644\n--- a/lib/eal/common/eal_common_trace_utils.c\n+++ b/lib/eal/common/eal_common_trace_utils.c\n@@ -314,14 +314,18 @@ trace_dir_default_path_get(char *dir_path)\n \treturn 0;\n }\n \n-int\n+static int\n trace_mkdir(void)\n {\n \tstruct trace *trace = trace_obj_get();\n \tchar session[TRACE_DIR_STR_LEN];\n+\tstatic bool already_done;\n \tchar *dir_path;\n \tint rc;\n \n+\tif (already_done)\n+\t\treturn 0;\n+\n \tif (!trace->dir_offset) {\n \t\tdir_path = calloc(1, sizeof(trace->dir));\n \t\tif (dir_path == NULL) {\n@@ -365,6 +369,7 @@ trace_mkdir(void)\n \t}\n \n \tRTE_LOG(INFO, EAL, \"Trace dir: %s\\n\", trace->dir);\n+\talready_done = true;\n \treturn 0;\n }\n \n@@ -434,6 +439,10 @@ rte_trace_save(void)\n \tif (trace->nb_trace_mem_list == 0)\n \t\treturn rc;\n \n+\trc = trace_mkdir();\n+\tif (rc < 0)\n+\t\treturn rc;\n+\n \trc = trace_meta_save(trace);\n \tif (rc)\n \t\treturn rc;\ndiff --git a/lib/eal/common/eal_trace.h b/lib/eal/common/eal_trace.h\nindex 06751eb23a..72a5a461ae 100644\n--- a/lib/eal/common/eal_trace.h\n+++ b/lib/eal/common/eal_trace.h\n@@ -54,7 +54,7 @@ struct trace {\n \tchar dir[PATH_MAX];\n \tint dir_offset;\n \tint register_errno;\n-\tbool status;\n+\tuint32_t status;\n \tenum rte_trace_mode mode;\n \trte_uuid_t uuid;\n \tuint32_t buff_len;\n@@ -104,7 +104,6 @@ void trace_uuid_generate(void);\n int trace_metadata_create(void);\n void trace_metadata_destroy(void);\n char *trace_metadata_fixup_field(const char *field);\n-int trace_mkdir(void);\n int trace_epoch_time_save(void);\n void trace_mem_free(void);\n void trace_mem_per_thread_free(void);\n", "prefixes": [ "v3", "5/9" ] }