From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mails.dpdk.org (mails.dpdk.org [217.70.189.124]) by inbox.dpdk.org (Postfix) with ESMTP id 611E643E95; Wed, 17 Apr 2024 20:04:02 +0200 (CEST) Received: from mails.dpdk.org (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 25BA140A76; Wed, 17 Apr 2024 20:04:02 +0200 (CEST) Received: from dkmailrelay1.smartsharesystems.com (smartserver.smartsharesystems.com [77.243.40.215]) by mails.dpdk.org (Postfix) with ESMTP id 563A4402B5 for ; Wed, 17 Apr 2024 20:04:00 +0200 (CEST) Received: from smartserver.smartsharesystems.com (smartserver.smartsharesys.local [192.168.4.10]) by dkmailrelay1.smartsharesystems.com (Postfix) with ESMTP id 334EA2067B; Wed, 17 Apr 2024 20:04:00 +0200 (CEST) Content-class: urn:content-classes:message MIME-Version: 1.0 Content-Type: text/plain; charset="iso-8859-1" Content-Transfer-Encoding: quoted-printable Subject: RE: [PATCH v3 1/5] latencystats: replace use of VLA Date: Wed, 17 Apr 2024 20:03:55 +0200 X-MimeOLE: Produced By Microsoft Exchange V6.5 Message-ID: <98CBD80474FA8B44BF855DF32C47DC35E9F3B2@smartserver.smartshare.dk> In-Reply-To: <20240417170908.76701-2-stephen@networkplumber.org> X-MS-Has-Attach: X-MS-TNEF-Correlator: Thread-Topic: [PATCH v3 1/5] latencystats: replace use of VLA Thread-Index: AdqQ6f6QLfD/2bWxSMidA5rpgGr9NgABiQtg References: <0240408195036.182545-1-stephen@networkplumber.org> <20240417170908.76701-1-stephen@networkplumber.org> <20240417170908.76701-2-stephen@networkplumber.org> From: =?iso-8859-1?Q?Morten_Br=F8rup?= To: "Stephen Hemminger" , Cc: "Reshma Pattan" 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 > From: Stephen Hemminger [mailto:stephen@networkplumber.org] > Sent: Wednesday, 17 April 2024 19.07 >=20 > The temporary array latencystats is not needed if the algorithm > is converted into one pass. >=20 > Signed-off-by: Stephen Hemminger > --- > lib/latencystats/rte_latencystats.c | 31 = +++++++++++++++-------------- > 1 file changed, 16 insertions(+), 15 deletions(-) >=20 > diff --git a/lib/latencystats/rte_latencystats.c > b/lib/latencystats/rte_latencystats.c > index 4ea9b0d75b..9b345bfb33 100644 > --- a/lib/latencystats/rte_latencystats.c > +++ b/lib/latencystats/rte_latencystats.c > @@ -157,9 +157,9 @@ calc_latency(uint16_t pid __rte_unused, > uint16_t nb_pkts, > void *_ __rte_unused) > { > - unsigned int i, cnt =3D 0; > + unsigned int i; > uint64_t now; > - float latency[nb_pkts]; > + float latency; > static float prev_latency; > /* > * Alpha represents degree of weighting decrease in EWMA, > @@ -169,13 +169,14 @@ calc_latency(uint16_t pid __rte_unused, > const float alpha =3D 0.2; >=20 > now =3D rte_rdtsc(); > - for (i =3D 0; i < nb_pkts; i++) { > - if (pkts[i]->ol_flags & timestamp_dynflag) > - latency[cnt++] =3D now - *timestamp_dynfield(pkts[i]); > - } >=20 > rte_spinlock_lock(&glob_stats->lock); > - for (i =3D 0; i < cnt; i++) { > + for (i =3D 0; i < nb_pkts; i++) { > + if (!(pkts[i]->ol_flags & timestamp_dynflag)) > + continue; > + > + latency =3D now - *timestamp_dynfield(pkts[i]); > + > /* > * The jitter is calculated as statistical mean of > interpacket > * delay variation. The "jitter estimate" is computed by > taking > @@ -187,22 +188,22 @@ calc_latency(uint16_t pid __rte_unused, > * Reference: Calculated as per RFC 5481, sec 4.1, > * RFC 3393 sec 4.5, RFC 1889 sec. > */ > - glob_stats->jitter +=3D (fabsf(prev_latency - latency[i]) > + glob_stats->jitter +=3D (fabsf(prev_latency - latency) > - glob_stats->jitter)/16; > if (glob_stats->min_latency =3D=3D 0) You could add unlikely() to this comparison. It should only happen once = in a lifetime. > - glob_stats->min_latency =3D latency[i]; > - else if (latency[i] < glob_stats->min_latency) > - glob_stats->min_latency =3D latency[i]; > - else if (latency[i] > glob_stats->max_latency) > - glob_stats->max_latency =3D latency[i]; > + glob_stats->min_latency =3D latency; In theory, glob_stats->max_latency should also be initialized with the = first sample value here. > + else if (latency < glob_stats->min_latency) > + glob_stats->min_latency =3D latency; > + else if (latency > glob_stats->max_latency) > + glob_stats->max_latency =3D latency; The min/max comparisons are also unlikely. > /* > * The average latency is measured using exponential moving > * average, i.e. using EWMA > * https://en.wikipedia.org/wiki/Moving_average > */ > glob_stats->avg_latency +=3D > - alpha * (latency[i] - glob_stats->avg_latency); > - prev_latency =3D latency[i]; > + alpha * (latency - glob_stats->avg_latency); > + prev_latency =3D latency; > } > rte_spinlock_unlock(&glob_stats->lock); >=20 > -- > 2.43.0 With or without suggested changes, Acked-by: Morten Br=F8rup