Re: [PATCH] can: kvaser_pciefd: refine error prone echo_skb_max handling logic

[Axel Forsman <axfo@kvaser.com>]

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