Thanks for finding and fixing this bug.
Fedor Pchelkin pchelkin@ispras.ru writes:
Actually the trick with rounding up allows to calculate seq numbers efficiently, avoiding a more consuming 'mod' operation used in the current patch.
Indeed, that was the intention.
So another approach to fix the problem would be to precompute the rounded up value of echo_skb_max and pass it to alloc_candev() making the size of the underlying echo_skb[] sufficient.
I believe that is preferable---if memory usage is a concern KVASER_PCIEFD_CAN_TX_MAX_COUNT could be lowered by one. Something like the following:
diff --git a/drivers/net/can/kvaser_pciefd.c b/drivers/net/can/kvaser_pciefd.c index f6921368cd14..0071a51ce2c1 100644 --- a/drivers/net/can/kvaser_pciefd.c +++ b/drivers/net/can/kvaser_pciefd.c @@ -966,7 +966,7 @@ static int kvaser_pciefd_setup_can_ctrls(struct kvaser_pciefd *pcie) u32 status, tx_nr_packets_max;
netdev = alloc_candev(sizeof(struct kvaser_pciefd_can), - KVASER_PCIEFD_CAN_TX_MAX_COUNT); + roundup_pow_of_two(KVASER_PCIEFD_CAN_TX_MAX_COUNT)); if (!netdev) return -ENOMEM;
@@ -995,7 +995,6 @@ static int kvaser_pciefd_setup_can_ctrls(struct kvaser_pciefd *pcie) can->tx_max_count = min(KVASER_PCIEFD_CAN_TX_MAX_COUNT, tx_nr_packets_max - 1);
can->can.clock.freq = pcie->freq; - can->can.echo_skb_max = roundup_pow_of_two(can->tx_max_count); spin_lock_init(&can->lock);
can->can.bittiming_const = &kvaser_pciefd_bittiming_const;
/Axel Forsman