can: statistics: use atomic access in hot path
Subsystems (2)
Oliver Hartkopp
maintainerPATCH 1/1 can: statistics: use atomic access in hot path
In can_send() and can_receive() CAN messages and CAN filter matches are counted to be visible in the CAN procfs files.
KCSAN detected a data race within can_send() when two CAN frames have been generated by a timer event writing to the same CAN netdevice at the same time. Use atomic operations to access the statistics in the hot path to fix the KCSAN complaint.
--- net/can/af_can.c | 12 ++++++------ net/can/af_can.h | 12 ++++++------ net/can/proc.c | 46 +++++++++++++++++++++++++++------------------- 3 files changed, 39 insertions(+), 31 deletions(-)
net/can/af_can.c +6 -6
@@ -285,12 +285,12 @@ int can_send(struct sk_buff *skb, int loop)if (newskb)netif_rx (newskb);/* update statistics */ - pkg_stats->tx_frames++; - pkg_stats->tx_frames_delta++; +atomic_long_inc (&pkg_stats->tx_frames); +atomic_long_inc (&pkg_stats->tx_frames_delta);return 0 ; inval_skb:kfree_skb (skb);
@@ -645,12 +645,12 @@ static void can_receive(struct sk_buff *skb, struct net_device *dev)struct net *net =dev_net (dev);struct can_pkg_stats *pkg_stats = net->can.pkg_stats;int matches;/* update statistics */ - pkg_stats->rx_frames++; - pkg_stats->rx_frames_delta++; +atomic_long_inc (&pkg_stats->rx_frames); +atomic_long_inc (&pkg_stats->rx_frames_delta);/* create non-zero unique skb identifier together with *skb */ while (!(can_skb_prv (skb)->skbcnt))can_skb_prv (skb)->skbcnt =atomic_inc_return (&skbcounter);
@@ -667,12 +667,12 @@ static void can_receive(struct sk_buff *skb, struct net_device *dev)/* consume the skbuff allocated by the netdevice driver */ consume_skb (skb);if (matches >0 ) { - pkg_stats->matches++; - pkg_stats->matches_delta++; +atomic_long_inc (&pkg_stats->matches); +atomic_long_inc (&pkg_stats->matches_delta); } }static int can_rcv (struct sk_buff *skb,struct net_device *dev,struct packet_type *pt,struct net_device *orig_dev)
net/can/af_can.h +6 -6
@@ -64,13 +64,13 @@ struct receiver {/* can be reset e.g. by can_init_stats() */ struct can_pkg_stats {unsigned long jiffies_init; -unsigned long rx_frames; -unsigned long tx_frames; -unsigned long matches; +atomic_long_t rx_frames; +atomic_long_t tx_frames; +atomic_long_t matches;unsigned long total_rx_rate;unsigned long total_tx_rate;unsigned long total_rx_match_ratio;
@@ -80,13 +80,13 @@ struct can_pkg_stats {unsigned long max_rx_rate;unsigned long max_tx_rate;unsigned long max_rx_match_ratio; -unsigned long rx_frames_delta; -unsigned long tx_frames_delta; -unsigned long matches_delta; +atomic_long_t rx_frames_delta; +atomic_long_t tx_frames_delta; +atomic_long_t matches_delta; };/* persistent statistics */ struct can_rcv_lists_stats {unsigned long stats_reset;
net/can/proc.c +27 -19
101 lines not on this page · raw message
Signature · 1 line
2.47.2
Quoted 9 lines · Oliver Hartkopp
In
can_send()andcan_receive()CAN messages and CAN filter matches are counted to be visible in the CAN procfs files.KCSAN detected a data race within
can_send()when two CAN frames have been generated by a timer event writing to the same CAN netdevice at the same time. Use atomic operations to access the statistics in the hot path to fix the KCSAN complaint.Reported-by [email protected]
Maybe add a Fixes: tag?
Quoted 1 line
Signed-off-by Oliver Hartkopp <[email protected]>
Notwithstanding of above remark:
Yours sincerely,
Vincent Mailhol
Oliver Hartkopp
maintainerQuoted 4 lines · Vincent Mailhol
Quoted 3 lines · Oliver Hartkopp
In
can_send()andcan_receive()CAN messages and CAN filter matches are counted to be visible in the CAN procfs files.
Quoted 8 lines
Quoted 6 lines
KCSAN detected a data race within
can_send()when two CAN frames have been generated by a timer event writing to the same CAN netdevice at the same time. Use atomic operations to access the statistics in the hot path to fix the KCSAN complaint.Reported-by [email protected]Maybe add a Fixes: tag?
I'm not sure about if we should provide a Fixes tag here. The code is from the initial commit in 2008:
But it never created any problems and it is also not that relevant if we have a packet counter which is +/- 1 correct just to be displayed in the procfs for users that accidentally know about this feature.
If we add a Fixes tag the patch is backported to many stable kernels. Not sure if it's worth it.
For future KCSAN tests we are safe then.
Best regards,
Oliver
Quoted 10 lines
Quoted 2 lines
Signed-off-by Oliver Hartkopp <[email protected]>Notwithstanding of above remark:
Reviewed-by Vincent Mailhol <[email protected]>Yours sincerely,
Vincent Mailhol
Quoted 10 lines · Oliver Hartkopp
In
can_send()andcan_receive()CAN messages and CAN filter matches are counted to be visible in the CAN procfs files.KCSAN detected a data race within
can_send()when two CAN frames have been generated by a timer event writing to the same CAN netdevice at the same time. Use atomic operations to access the statistics in the hot path to fix the KCSAN complaint.Reported-by [email protected]Signed-off-by Oliver Hartkopp <[email protected]>
Applied to linux-can.
regards,
Marc
Signature · 4 lines
Pengutronix e.K. | Marc Kleine-Budde | Embedded Linux | https://www.pengutronix.de | Vertretung Nürnberg | Phone: +49-5121-206917-129 | Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-9 |