From patchwork Fri Jul 8 12:56:44 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: "Van Haaren, Harry" X-Patchwork-Id: 113830 X-Patchwork-Delegate: thomas@monjalon.net Return-Path: 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]) by inbox.dpdk.org (Postfix) with ESMTP id C853DA00C5; Fri, 8 Jul 2022 14:57:12 +0200 (CEST) Received: from [217.70.189.124] (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 5EFAA406B4; Fri, 8 Jul 2022 14:57:12 +0200 (CEST) Received: from mga12.intel.com (mga12.intel.com [192.55.52.136]) by mails.dpdk.org (Postfix) with ESMTP id D815E4021E for ; Fri, 8 Jul 2022 14:57:10 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=intel.com; i=@intel.com; q=dns/txt; s=Intel; t=1657285031; x=1688821031; h=from:to:cc:subject:date:message-id:mime-version: content-transfer-encoding; bh=I0fDylEP/1qmu51QeBy0fWN3JmzpYZbGZQrsbj5/TLk=; b=l66AgK0rCl/QP/s14vwOv0oeSeDx5vahSQezuNO9FIOyrW7D0WFWvL2j zixSnuEf4F8wHeXFge3IZxfJGvAtJHX9/VjeNk5Zo9gx72HxFUR1s7hkS T2C7WdOTomQ/T1qnYUfmWrShMszkl2TfKuMmDVoyerggcvkwIPZFtUNfn ynVG5gb1RxKm5MrLKVZN4eDSj/1/Xi4cM+CkNXQkuuY/E1UyFQp/dxOj7 Bo5JYQXCZLk3BnEn8V6AuWmEO2fql7BWMAQRcswtTgTdHPsRi+/RHFlmM wHKGvAlNkhd7xVYLWU18l0HXZP2gkXlP/tdD22hQdZgKp6d1XMO+pfjL9 g==; X-IronPort-AV: E=McAfee;i="6400,9594,10401"; a="264053430" X-IronPort-AV: E=Sophos;i="5.92,255,1650956400"; d="scan'208";a="264053430" Received: from orsmga001.jf.intel.com ([10.7.209.18]) by fmsmga106.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 08 Jul 2022 05:56:54 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.92,255,1650956400"; d="scan'208";a="626696690" Received: from silpixa00401454.ir.intel.com ([10.55.128.122]) by orsmga001.jf.intel.com with ESMTP; 08 Jul 2022 05:56:52 -0700 From: Harry van Haaren To: dev@dpdk.org Cc: Harry van Haaren , =?utf-8?q?Mattias_R=C3=B6?= =?utf-8?q?nnblom?= , Honnappa Nagarahalli , =?utf-8?q?Morten_Br?= =?utf-8?q?=C3=B8rup?= Subject: [PATCH 1/2] test/service: add perf measurements for with stats mode Date: Fri, 8 Jul 2022 12:56:44 +0000 Message-Id: <20220708125645.3141464-1-harry.van.haaren@intel.com> X-Mailer: git-send-email 2.32.0 MIME-Version: 1.0 X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org This commit improves the performance reporting of the service cores polling loop to show both with and without statistics collection modes. Collecting cycle statistics is costly, due to calls to rte_rdtsc() per service iteration. Reported-by: Mattias Rönnblom Suggested-by: Honnappa Nagarahalli Suggested-by: Morten Brørup Signed-off-by: Harry van Haaren --- This is split out as a seperate patch from the fix to allow measuring the before/after of the service stats atomic fixup. --- app/test/test_service_cores.c | 36 ++++++++++++++++++++++++----------- 1 file changed, 25 insertions(+), 11 deletions(-) diff --git a/app/test/test_service_cores.c b/app/test/test_service_cores.c index ced6ed0081..7415b6b686 100644 --- a/app/test/test_service_cores.c +++ b/app/test/test_service_cores.c @@ -777,6 +777,22 @@ service_run_on_app_core_func(void *arg) return rte_service_run_iter_on_app_lcore(*delay_service_id, 1); } +static float +service_app_lcore_perf_measure(uint32_t id) +{ + /* Performance test: call in a loop, and measure tsc() */ + const uint32_t perf_iters = (1 << 12); + uint64_t start = rte_rdtsc(); + uint32_t i; + for (i = 0; i < perf_iters; i++) { + int err = service_run_on_app_core_func(&id); + TEST_ASSERT_EQUAL(0, err, "perf test: returned run failure"); + } + uint64_t end = rte_rdtsc(); + + return (end - start)/(float)perf_iters; +} + static int service_app_lcore_poll_impl(const int mt_safe) { @@ -828,17 +844,15 @@ service_app_lcore_poll_impl(const int mt_safe) "MT Unsafe: App core1 didn't return -EBUSY"); } - /* Performance test: call in a loop, and measure tsc() */ - const uint32_t perf_iters = (1 << 12); - uint64_t start = rte_rdtsc(); - uint32_t i; - for (i = 0; i < perf_iters; i++) { - int err = service_run_on_app_core_func(&id); - TEST_ASSERT_EQUAL(0, err, "perf test: returned run failure"); - } - uint64_t end = rte_rdtsc(); - printf("perf test for %s: %0.1f cycles per call\n", mt_safe ? - "MT Safe" : "MT Unsafe", (end - start)/(float)perf_iters); + /* Measure performance of no-stats and with-stats. */ + float cyc_no_stats = service_app_lcore_perf_measure(id); + + TEST_ASSERT_EQUAL(0, rte_service_set_stats_enable(id, 1), + "failed to enable stats for service."); + float cyc_with_stats = service_app_lcore_perf_measure(id); + + printf("perf test for %s, no stats: %0.1f, with stats %0.1f cycles/call\n", + mt_safe ? "MT Safe" : "MT Unsafe", cyc_no_stats, cyc_with_stats); unregister_all(); return TEST_SUCCESS; From patchwork Fri Jul 8 12:56:45 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: "Van Haaren, Harry" X-Patchwork-Id: 113831 X-Patchwork-Delegate: thomas@monjalon.net Return-Path: 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]) by inbox.dpdk.org (Postfix) with ESMTP id 02AE9A00C5; Fri, 8 Jul 2022 14:57:17 +0200 (CEST) Received: from [217.70.189.124] (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 4C415427F6; Fri, 8 Jul 2022 14:57:13 +0200 (CEST) Received: from mga12.intel.com (mga12.intel.com [192.55.52.136]) by mails.dpdk.org (Postfix) with ESMTP id 7F97F4021E for ; Fri, 8 Jul 2022 14:57:11 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=intel.com; i=@intel.com; q=dns/txt; s=Intel; t=1657285031; x=1688821031; h=from:to:cc:subject:date:message-id:in-reply-to: references:mime-version:content-transfer-encoding; bh=ZDcCKvRR4iJnNFmy7AtBkPKOUJmX/r6dxAcR/AuBRcM=; b=SdEThxaquzZ4Ej64Wo2PcnSLpR5wNNEJyZCPPN8z5NXjvGfb6L/2Wd+X NDSiVY73oIH0x6hpXrpe0D5v3LxFW8KEqWnxqN45yUOXKxCmLDvDzWwlZ 1IE/vyOZnPgHAphJG1wWhPguK36lds308ZkZ7bH6aHMXlql8ad4LuVYj6 W80CQh8Jl+OhT+fgDbClUf6pmIo9N2H4/AkaV+sXQeyrf7kXo2MEvSuUu Bc++41TFOFUm9AdGgTAQei+9m9Whm7m1xWk4BHAANd+R0CB56vljqndnJ fsyzICEEuFn6mZNh2QA0UFDgLO02B92+mT2uvhQAvAqP2Hh7vQm9zfczt g==; X-IronPort-AV: E=McAfee;i="6400,9594,10401"; a="264053439" X-IronPort-AV: E=Sophos;i="5.92,255,1650956400"; d="scan'208";a="264053439" Received: from orsmga001.jf.intel.com ([10.7.209.18]) by fmsmga106.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 08 Jul 2022 05:56:55 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.92,255,1650956400"; d="scan'208";a="626696695" Received: from silpixa00401454.ir.intel.com ([10.55.128.122]) by orsmga001.jf.intel.com with ESMTP; 08 Jul 2022 05:56:54 -0700 From: Harry van Haaren To: dev@dpdk.org Cc: Harry van Haaren , =?utf-8?q?Mattias_R=C3=B6?= =?utf-8?q?nnblom?= , Honnappa Nagarahalli , =?utf-8?q?Morten_Br?= =?utf-8?q?=C3=B8rup?= Subject: [PATCH 2/2] service: fix potential stats race-condition on MT services Date: Fri, 8 Jul 2022 12:56:45 +0000 Message-Id: <20220708125645.3141464-2-harry.van.haaren@intel.com> X-Mailer: git-send-email 2.32.0 In-Reply-To: <20220708125645.3141464-1-harry.van.haaren@intel.com> References: <20220708125645.3141464-1-harry.van.haaren@intel.com> MIME-Version: 1.0 X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org This commit fixes a potential racey-add that could occur if multiple service-lcores were executing the same MT-safe service at the same time, with service statistics collection enabled. Because multiple threads can run and execute the service, the stats values can have multiple writer threads, resulting in the requirement of using atomic addition for correctness. Note that when a MT unsafe service is executed, a spinlock is held, so the stats increments are protected. This fact is used to avoid executing atomic add instructions when not required. This patch causes a 1.25x increase in cycle-cost for polling a MT safe service when statistics are enabled. No change was seen for MT unsafe services, or when statistics are disabled. Reported-by: Mattias Rönnblom Suggested-by: Honnappa Nagarahalli Suggested-by: Morten Brørup Signed-off-by: Harry van Haaren Reviewed-by: Mattias Rönnblom Reviewed-by: Morten Brørup --- --- lib/eal/common/rte_service.c | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/lib/eal/common/rte_service.c b/lib/eal/common/rte_service.c index ef31b1f63c..f045e74ef3 100644 --- a/lib/eal/common/rte_service.c +++ b/lib/eal/common/rte_service.c @@ -363,9 +363,15 @@ service_runner_do_callback(struct rte_service_spec_impl *s, uint64_t start = rte_rdtsc(); s->spec.callback(userdata); uint64_t end = rte_rdtsc(); - s->cycles_spent += end - start; + uint64_t cycles = end - start; cs->calls_per_service[service_idx]++; - s->calls++; + if (service_mt_safe(s)) { + __atomic_fetch_add(&s->cycles_spent, cycles, __ATOMIC_RELAXED); + __atomic_fetch_add(&s->calls, 1, __ATOMIC_RELAXED); + } else { + s->cycles_spent += cycles; + s->calls++; + } } else s->spec.callback(userdata); }