Skip to contentSkip to conversation

can: proc: skip stats calculation after reset in can_stat_update()

PATCH New Oliver Hartkopp 1 participant 1 message Oct 19

Subsystems (2)
CAN network drivers CAN network layer
lore
Share & tools
lore

Oliver Hartkopp

author maintainer
#0 lore raw

PATCH 1/1 can: proc: skip stats calculation after reset in can_stat_update()

Commit 80b5f90158d1 ("can: statistics: use atomic access in hot path") introduced local snapshots of the statistics counters at the beginning of can_stat_update(). When can_init_stats() resets the statistics, these stale pre-reset snapshots are still used for the following total/current /max calculations and overwrite the freshly reset values. Additionally a jiffies tick between the snapshot and the reset could trigger a second can_init_stats() call.

Merge the reset conditions into a single check and skip the calculation for the current interval when the statistics have been reset.

Fixes 80b5f90158d1 ("can: statistics: use atomic access in hot path")
Signed-off-by Oliver Hartkopp <[email protected]>
---
 net/can/proc.c | 30 ++++++++++++------------------
 1 file changed, 12 insertions(+), 18 deletions(-)
net/can/proc.c +12 -18
index 0f1fa6564436..b7496d2a37cd 100644
@@ -147,29 +147,22 @@ void can_stat_update(struct timer_list *t)
 	/* fetch and clear values for 'current rate' calculation */
 	long rx_frames_delta = atomic_long_xchg(&pkg_stats->rx_frames_delta, 0);
 	long tx_frames_delta = atomic_long_xchg(&pkg_stats->tx_frames_delta, 0);
 	long matches_delta = atomic_long_xchg(&pkg_stats->matches_delta, 0);
 
-	/* restart counting in timer context on user request */
-	if (user_reset)
-		can_init_stats(net);
-
-	/* restart counting on jiffies overflow */
-	if (j < pkg_stats->jiffies_init)
-		can_init_stats(net);
-
-	/* prevent overflow in calc_rate() */
-	if (rx_frames > (LONG_MAX / HZ))
-		can_init_stats(net);
-
-	/* prevent overflow in calc_rate() */
-	if (tx_frames > (LONG_MAX / HZ))
-		can_init_stats(net);
-
-	/* matches overflow - very improbable */
-	if (matches > (LONG_MAX / 100))
+	/*
+	 * Restart counting in timer context on user request, on jiffies
+	 * overflow or to prevent overflows in calc_rate() and in the
+	 * match ratio calculation. The local snapshots are stale after
+	 * the reset, so skip the calculation for this interval.
+	 */
+	if (user_reset || j < pkg_stats->jiffies_init ||
+	    rx_frames > (LONG_MAX / HZ) || tx_frames > (LONG_MAX / HZ) ||
+	    matches > (LONG_MAX / 100)) {
 		can_init_stats(net);
+		goto out;
+	}
 
 	/* calc total values */
 	if (rx_frames)
 		pkg_stats->total_rx_match_ratio = (matches * 100) / rx_frames;
 

@@ -194,10 +187,11 @@ void can_stat_update(struct timer_list *t)
 		pkg_stats->max_rx_rate = pkg_stats->current_rx_rate;
 
 	if (pkg_stats->max_rx_match_ratio < pkg_stats->current_rx_match_ratio)
 		pkg_stats->max_rx_match_ratio = pkg_stats->current_rx_match_ratio;
 
+out:
 	/* restart timer (one second) */
 	mod_timer(&net->can.stattimer, round_jiffies(jiffies + HZ));
 }
 
 /*
Signature · 1 line
2.53.0