DPDK patches and discussions
 help / color / mirror / Atom feed
From: "Morten Brørup" <mb@smartsharesystems.com>
To: "Mattias Rönnblom" <mattias.ronnblom@ericsson.com>, dev@dpdk.org
Cc: <hofors@lysator.liu.se>, <harry.van.haaren@intel.com>,
	"Stefan Sundkvist" <stefan.sundkvist@ericsson.com>
Subject: RE: [RFC] service: extend service function call statistics
Date: Fri, 26 Jan 2024 00:19:10 +0100	[thread overview]
Message-ID: <98CBD80474FA8B44BF855DF32C47DC35E9F1A4@smartserver.smartshare.dk> (raw)
In-Reply-To: <20240125191529.388570-1-mattias.ronnblom@ericsson.com>

> From: Mattias Rönnblom [mailto:mattias.ronnblom@ericsson.com]
> Sent: Thursday, 25 January 2024 20.15
> 
> Add two new per-service counters.
> 
> RTE_SERVICE_ATTR_IDLE_CALL_COUNT tracks the number of service function
> invocations where no work was performed.
> 
> RTE_SERVICE_ATTR_ERROR_CALL_COUNT tracks the number invocations
> resulting in an error.
> 
> The semantics of RTE_SERVICE_ATTR_CALL_COUNT remains the same (i.e.,
> counting all invocations, regardless of return value).
> 
> The new statistics may be useful for both debugging and profiling
> (e.g., calculate the average per-call processing latency for non-idle
> service calls).
> 
> Service core tests are extended to cover the new counters, and
> coverage for RTE_SERVICE_ATTR_CALL_COUNT is improved.

OK to all of the above. Good stuff.

> 
> The documentation for the CYCLES attributes are updated to reflect
> their actual semantics.

If this is intended behavior, then updating the documentation seems appropriate - I would even go so far as considering it a bug fix.

However, quite a few cycles may be consumed by a service before it can conclude that it had no work to do. Shouldn't that be considered time spent by the service? I.e. should the code be fixed instead of the documentation?

Alternatively, keep the behavior (for backwards compatibility) and fix the documentation, as this patch does, and add an IDLE_CYCLES counter for time spent in idle calls.

PS: We're not using DPDK service cores in our applications, so I'm not familiar with the details. We are using something somewhat similar (but homegrown), also for profiling and power management purposes, and my feedback is based on my experience with our own variant of service cores.

Either way:

Acked-by: Morten Brørup <mb@smartsharesystems.com>


  reply	other threads:[~2024-01-25 23:19 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-01-25 19:15 Mattias Rönnblom
2024-01-25 23:19 ` Morten Brørup [this message]
2024-01-26  8:28   ` Mattias Rönnblom
2024-01-26 10:07     ` Morten Brørup
2024-01-27 19:31       ` Mattias Rönnblom
2024-01-29 12:50         ` Van Haaren, Harry
2024-01-30  7:16           ` Mattias Rönnblom

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=98CBD80474FA8B44BF855DF32C47DC35E9F1A4@smartserver.smartshare.dk \
    --to=mb@smartsharesystems.com \
    --cc=dev@dpdk.org \
    --cc=harry.van.haaren@intel.com \
    --cc=hofors@lysator.liu.se \
    --cc=mattias.ronnblom@ericsson.com \
    --cc=stefan.sundkvist@ericsson.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).