DPDK patches and discussions
 help / color / mirror / Atom feed
From: "Mattias Rönnblom" <mattias.ronnblom@ericsson.com>
To: Van@dpdk.org, Haaren@dpdk.org, Harry <harry.van.haaren@intel.com>
Cc: dev@dpdk.org,
	"Honnappa Nagarahalli" <Honnappa.Nagarahalli@arm.com>,
	"Morten Brørup" <mb@smartsharesystems.com>, nd <nd@arm.com>,
	"Mattias Rönnblom" <mattias.ronnblom@ericsson.com>
Subject: [PATCH 4/6] service: tweak cycle statistics semantics
Date: Tue, 6 Sep 2022 18:13:50 +0200	[thread overview]
Message-ID: <20220906161352.296110-4-mattias.ronnblom@ericsson.com> (raw)
In-Reply-To: <20220906161352.296110-1-mattias.ronnblom@ericsson.com>

As a part of its service function, a service usually polls some kind
of source (e.g., an RX queue, a ring, an eventdev port, or a timer
wheel) to retrieve one or more items of work.

In low-load situations, the service framework reports a significant
amount of cycles spent for all running services, despite the fact they
have performed little or no actual work.

The per-call cycle expenditure for an idle service (i.e., a service
currently without pending jobs) is typically very low. Polling an
empty ring or RX queue is inexpensive. However, since the service
function call frequency on an idle or lightly loaded lcore is going to
be very high indeed, the service function calls' cycles adds up to a
significant amount. The only thing preventing the idle services'
cycles counters to make up 100% of the available CPU cycles is the
overhead of the service framework itself.

If the RTE_SERVICE_ATTR_CYCLES or RTE_SERVICE_LCORE_ATTR_CYCLES are
used to estimate service core load, the cores may look very busy when
the system is mostly doing nothing useful at all.

This patch allows for an idle service to indicate that no actual work
was performed during a particular service function call (by returning
-EAGAIN). In such cases the RTE_SERVICE_ATTR_CYCLES and
RTE_SERVICE_LCORE_ATTR_CYCLES values are not incremented.

The convention of returning -EAGAIN for idle services may in the
future also be used to have the lcore enter a short sleep, or reduce
its operating frequency, in case all services are currently idle.

This change is backward-compatible.

Signed-off-by: Mattias Rönnblom <mattias.ronnblom@ericsson.com>
---
 lib/eal/common/rte_service.c            | 22 ++++++++++++++--------
 lib/eal/include/rte_service_component.h |  5 +++++
 2 files changed, 19 insertions(+), 8 deletions(-)

diff --git a/lib/eal/common/rte_service.c b/lib/eal/common/rte_service.c
index 4cac866792..123610688c 100644
--- a/lib/eal/common/rte_service.c
+++ b/lib/eal/common/rte_service.c
@@ -10,6 +10,7 @@
 #include <rte_service_component.h>
 
 #include <rte_lcore.h>
+#include <rte_branch_prediction.h>
 #include <rte_common.h>
 #include <rte_cycles.h>
 #include <rte_atomic.h>
@@ -364,24 +365,29 @@ service_runner_do_callback(struct rte_service_spec_impl *s,
 
 	if (service_stats_enabled(s)) {
 		uint64_t start = rte_rdtsc();
-		s->spec.callback(userdata);
-		uint64_t end = rte_rdtsc();
-		uint64_t cycles = end - start;
+		int rc = s->spec.callback(userdata);
 
 		/* The lcore service worker thread is the only writer,
 		 * and thus only a non-atomic load and an atomic store
 		 * is needed, and not the more expensive atomic
 		 * add.
 		 */
-		__atomic_store_n(&cs->cycles, cs->cycles + cycles,
-				 __ATOMIC_RELAXED);
+
+		if (likely(rc != -EAGAIN)) {
+			uint64_t end = rte_rdtsc();
+			uint64_t cycles = end - start;
+
+			__atomic_store_n(&cs->cycles, cs->cycles + cycles,
+					 __ATOMIC_RELAXED);
+			__atomic_store_n(&cs->cycles_per_service[service_idx],
+					 cs->cycles_per_service[service_idx] +
+					 cycles, __ATOMIC_RELAXED);
+		}
+
 		__atomic_store_n(&cs->calls_per_service[service_idx],
 				 cs->calls_per_service[service_idx] + 1,
 				 __ATOMIC_RELAXED);
 
-		__atomic_store_n(&cs->cycles_per_service[service_idx],
-				 cs->cycles_per_service[service_idx] + cycles,
-				 __ATOMIC_RELAXED);
 	} else
 		s->spec.callback(userdata);
 }
diff --git a/lib/eal/include/rte_service_component.h b/lib/eal/include/rte_service_component.h
index 9e66ee7e29..9be49d698a 100644
--- a/lib/eal/include/rte_service_component.h
+++ b/lib/eal/include/rte_service_component.h
@@ -19,6 +19,11 @@ extern "C" {
 
 /**
  * Signature of callback function to run a service.
+ *
+ * A service function call resulting in no actual work being
+ * performed, should return -EAGAIN. In that case, the (presumbly few)
+ * cycles spent will not be counted toward the lcore or service-level
+ * cycles attributes.
  */
 typedef int32_t (*rte_service_func)(void *args);
 
-- 
2.34.1


  parent reply	other threads:[~2022-09-06 16:16 UTC|newest]

Thread overview: 43+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-07-08 12:56 [PATCH 1/2] test/service: add perf measurements for with stats mode Harry van Haaren
2022-07-08 12:56 ` [PATCH 2/2] service: fix potential stats race-condition on MT services Harry van Haaren
2022-07-08 13:23   ` Morten Brørup
2022-07-08 13:44     ` Van Haaren, Harry
2022-07-08 14:14       ` Morten Brørup
2022-07-08 13:48   ` Mattias Rönnblom
2022-07-08 15:16   ` Honnappa Nagarahalli
2022-07-08 15:31     ` Van Haaren, Harry
2022-07-08 16:21       ` Bruce Richardson
2022-07-08 16:33         ` Honnappa Nagarahalli
2022-07-08 20:02         ` Mattias Rönnblom
2022-07-08 16:29     ` Morten Brørup
2022-07-08 16:45       ` Honnappa Nagarahalli
2022-07-08 17:22         ` Morten Brørup
2022-07-08 17:39           ` Honnappa Nagarahalli
2022-07-08 18:08             ` Morten Brørup
2022-09-06 16:13   ` [PATCH 1/6] service: reduce statistics overhead for parallel services Mattias Rönnblom
2022-09-06 16:13     ` [PATCH 2/6] service: introduce per-lcore cycles counter Mattias Rönnblom
2022-09-06 16:13     ` [PATCH 3/6] service: reduce average case service core overhead Mattias Rönnblom
2022-10-03 13:33       ` Van Haaren, Harry
2022-10-03 14:32         ` Mattias Rönnblom
2022-09-06 16:13     ` Mattias Rönnblom [this message]
2022-09-07  8:41       ` [PATCH 4/6] service: tweak cycle statistics semantics Morten Brørup
2022-10-03 13:45         ` Van Haaren, Harry
2022-09-06 16:13     ` [PATCH 5/6] event/sw: report idle when no work is performed Mattias Rönnblom
2022-09-06 16:13     ` [PATCH 6/6] service: provide links to functions in documentation Mattias Rönnblom
2022-10-03  8:06     ` [PATCH 1/6] service: reduce statistics overhead for parallel services David Marchand
2022-10-03  8:40       ` Mattias Rönnblom
2022-10-03  9:53         ` David Marchand
2022-10-03 11:37           ` Mattias Rönnblom
2022-10-03 13:03             ` Van Haaren, Harry
2022-10-03 13:33     ` Van Haaren, Harry
2022-10-03 14:37       ` Mattias Rönnblom
2022-10-05  9:16     ` [PATCH v2 0/6] Service cores performance and statistics improvements Mattias Rönnblom
2022-10-05  9:16       ` [PATCH v2 1/6] service: reduce statistics overhead for parallel services Mattias Rönnblom
2022-10-05  9:16       ` [PATCH v2 2/6] service: introduce per-lcore cycles counter Mattias Rönnblom
2022-10-05  9:16       ` [PATCH v2 3/6] service: reduce average case service core overhead Mattias Rönnblom
2022-10-05  9:16       ` [PATCH v2 4/6] service: tweak cycle statistics semantics Mattias Rönnblom
2022-10-05  9:16       ` [PATCH v2 5/6] event/sw: report idle when no work is performed Mattias Rönnblom
2022-10-05  9:16       ` [PATCH v2 6/6] service: provide links to functions in documentation Mattias Rönnblom
2022-10-05  9:49       ` [PATCH v2 0/6] Service cores performance and statistics improvements Morten Brørup
2022-10-05 10:14         ` Mattias Rönnblom
2022-10-05 13:39       ` David Marchand

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20220906161352.296110-4-mattias.ronnblom@ericsson.com \
    --to=mattias.ronnblom@ericsson.com \
    --cc=Haaren@dpdk.org \
    --cc=Honnappa.Nagarahalli@arm.com \
    --cc=Van@dpdk.org \
    --cc=dev@dpdk.org \
    --cc=harry.van.haaren@intel.com \
    --cc=mb@smartsharesystems.com \
    --cc=nd@arm.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).