[v5,1/1] examples/l2fwd-jobstats: fix lock availability

Message ID 20240811155957.576645-1-rkudurumalla@marvell.com (mailing list archive)
State New
Delegated to: Thomas Monjalon
Headers
Series [v5,1/1] examples/l2fwd-jobstats: fix lock availability |

Checks

Context Check Description
ci/checkpatch success coding style OK
ci/loongarch-compilation success Compilation OK
ci/Intel-compilation success Compilation OK
ci/loongarch-unit-testing success Unit Testing PASS
ci/intel-Testing success Testing PASS
ci/iol-broadcom-Performance success Performance Testing PASS
ci/iol-mellanox-Performance success Performance Testing PASS
ci/iol-intel-Performance success Performance Testing PASS
ci/intel-Functional success Functional PASS
ci/iol-marvell-Functional success Functional Testing PASS
ci/iol-intel-Functional success Functional Testing PASS
ci/github-robot: build success github build: passed
ci/iol-sample-apps-testing success Testing PASS
ci/iol-broadcom-Functional success Functional Testing PASS
ci/iol-compile-amd64-testing success Testing PASS
ci/iol-unit-amd64-testing success Testing PASS
ci/iol-unit-arm64-testing success Testing PASS
ci/iol-compile-arm64-testing success Testing PASS

Commit Message

Rakesh Kudurumalla Aug. 11, 2024, 3:59 p.m. UTC
Race condition between jobstats and time metrics
for forwarding and flushing is maintained using spinlock.
Timer metrics are not displayed properly due to the
frequent unavailability of the lock.This patch fixes
the issue by introducing a delay before acquiring
the lock in the loop. This delay allows for betteravailability
of the lock, ensuring that show_lcore_stats() can
periodically update the statistics even when forwarding
jobs are running.

Fixes: 204896f8d66c ("examples/l2fwd-jobstats: add new example")
Cc: stable@dpdk.org

Signed-off-by: Rakesh Kudurumalla <rkudurumalla@marvell.com>
---

v5: updated cause of issue in commit message

 examples/l2fwd-jobstats/main.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
  

Comments

Stephen Hemminger Aug. 11, 2024, 4:17 p.m. UTC | #1
On Sun, 11 Aug 2024 21:29:57 +0530
Rakesh Kudurumalla <rkudurumalla@marvell.com> wrote:

> Race condition between jobstats and time metrics
> for forwarding and flushing is maintained using spinlock.
> Timer metrics are not displayed properly due to the
> frequent unavailability of the lock.This patch fixes
> the issue by introducing a delay before acquiring
> the lock in the loop. This delay allows for betteravailability
> of the lock, ensuring that show_lcore_stats() can
> periodically update the statistics even when forwarding
> jobs are running.
> 
> Fixes: 204896f8d66c ("examples/l2fwd-jobstats: add new example")
> Cc: stable@dpdk.org
> 
> Signed-off-by: Rakesh Kudurumalla <rkudurumalla@marvell.com>

Would be better if this code used RCU and not a lock
  
Rakesh Kudurumalla Aug. 16, 2024, 5:25 a.m. UTC | #2
> -----Original Message-----
> From: Stephen Hemminger <stephen@networkplumber.org>
> Sent: Sunday, August 11, 2024 9:47 PM
> To: Rakesh Kudurumalla <rkudurumalla@marvell.com>
> Cc: ferruh.yigit@amd.com; andrew.rybchenko@oktetlabs.ru;
> orika@nvidia.com; thomas@monjalon.net; dev@dpdk.org; Jerin Jacob
> <jerinj@marvell.com>; Nithin Kumar Dabilpuram
> <ndabilpuram@marvell.com>; stable@dpdk.org
> Subject: [EXTERNAL] Re: [PATCH v5 1/1] examples/l2fwd-jobstats: fix lock
> availability
> 
> On Sun, 11 Aug 2024 21: 29: 57 +0530 Rakesh Kudurumalla
> <rkudurumalla@ marvell. com> wrote: > Race condition between jobstats
> and time metrics > for forwarding and flushing is maintained using spinlock.
> > Timer metrics are not displayed 
> On Sun, 11 Aug 2024 21:29:57 +0530
> Rakesh Kudurumalla <rkudurumalla@marvell.com> wrote:
> 
> > Race condition between jobstats and time metrics for forwarding and
> > flushing is maintained using spinlock.
> > Timer metrics are not displayed properly due to the frequent
> > unavailability of the lock.This patch fixes the issue by introducing a
> > delay before acquiring the lock in the loop. This delay allows for
> > betteravailability of the lock, ensuring that show_lcore_stats() can
> > periodically update the statistics even when forwarding jobs are
> > running.
> >
> > Fixes: 204896f8d66c ("examples/l2fwd-jobstats: add new example")
> > Cc: stable@dpdk.org
> >
> > Signed-off-by: Rakesh Kudurumalla <rkudurumalla@marvell.com>
> 
> Would be better if this code used RCU and not a lock

Currently the jobstats app uses the lock only for collecting single snapshot of different statistics and printing the same from main core. With RCU since we cannot pause the worker core to collect such a single snapshot, integrating RCU would need a full redesign of the application and would take lot of effort.
  

Patch

diff --git a/examples/l2fwd-jobstats/main.c b/examples/l2fwd-jobstats/main.c
index 308b8edd20..7bb38b290f 100644
--- a/examples/l2fwd-jobstats/main.c
+++ b/examples/l2fwd-jobstats/main.c
@@ -542,7 +542,7 @@  l2fwd_main_loop(void)
 		} while (likely(stats_read_pending == 0));
 
 		rte_spinlock_unlock(&qconf->lock);
-		rte_pause();
+		rte_delay_us(10);
 	}
 	/* >8 End of minimize impact of stats reading. */
 }