github-mirror/linux
1
0
Fork 0
mirror of https://github.com/torvalds/linux.git synced 2026-05-24 15:12:13 +02:00
Code Issues Actions Packages Projects Releases Wiki Activity Graph

can: kvaser_pciefd: Fix echo_skb race

Browse Source 
The functions kvaser_pciefd_start_xmit() and
kvaser_pciefd_handle_ack_packet() raced to stop/wake TX queues and
get/put echo skbs, as kvaser_pciefd_can->echo_lock was only ever taken
when transmitting and KCAN_TX_NR_PACKETS_CURRENT gets decremented
prior to handling of ACKs. E.g., this caused the following error:

    can_put_echo_skb: BUG! echo_skb 5 is occupied!

Instead, use the synchronization helpers in netdev_queues.h. As those
piggyback on BQL barriers, start updating in-flight packets and bytes
counts as well.

Cc: stable@vger.kernel.org
Signed-off-by: Axel Forsman <axfo@kvaser.com>
Tested-by: Jimmy Assarsson <extja@kvaser.com>
Reviewed-by: Jimmy Assarsson <extja@kvaser.com>
Link: https://patch.msgid.link/20250520114332.8961-3-axfo@kvaser.com
Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
This commit is contained in:
Axel Forsman 2025-05-20 13:43:31 +02:00 committed by Marc Kleine-Budde
parent 9176bd205e
commit 8256e0ca60
1 changed files with 59 additions and 34 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

91
drivers/net/can/kvaser_pciefd.c
View File

@ -16,6 +16,7 @@
#include <linux/netdevice.h> #include <linux/netdevice.h>
#include <linux/pci.h> #include <linux/pci.h>
#include <linux/timer.h> #include <linux/timer.h>
#include <net/netdev_queues.h>
MODULE_LICENSE("Dual BSD/GPL"); MODULE_LICENSE("Dual BSD/GPL");
MODULE_AUTHOR("Kvaser AB <support@kvaser.com>"); MODULE_AUTHOR("Kvaser AB <support@kvaser.com>");
@ -410,10 +411,13 @@ struct kvaser_pciefd_can {
void __iomem *reg_base; void __iomem *reg_base;
struct can_berr_counter bec; struct can_berr_counter bec;
u8 cmd_seq; u8 cmd_seq;
u8 tx_max_count;
u8 tx_idx;
u8 ack_idx;
int err_rep_cnt; int err_rep_cnt;
int echo_idx; unsigned int completed_tx_pkts;
unsigned int completed_tx_bytes;
spinlock_t lock; /* Locks sensitive registers (e.g. MODE) */ spinlock_t lock; /* Locks sensitive registers (e.g. MODE) */
spinlock_t echo_lock; /* Locks the message echo buffer */
struct timer_list bec_poll_timer; struct timer_list bec_poll_timer;
struct completion start_comp, flush_comp; struct completion start_comp, flush_comp;
}; };
@ -714,6 +718,9 @@ static int kvaser_pciefd_open(struct net_device *netdev)
int ret; int ret;
struct kvaser_pciefd_can *can = netdev_priv(netdev); struct kvaser_pciefd_can *can = netdev_priv(netdev);
can->tx_idx = 0;
can->ack_idx = 0;
ret = open_candev(netdev); ret = open_candev(netdev);
if (ret) if (ret)
return ret; return ret;
@ -745,21 +752,26 @@ static int kvaser_pciefd_stop(struct net_device *netdev)
timer_delete(&can->bec_poll_timer); timer_delete(&can->bec_poll_timer);
} }
can->can.state = CAN_STATE_STOPPED; can->can.state = CAN_STATE_STOPPED;
netdev_reset_queue(netdev);
close_candev(netdev); close_candev(netdev);
return ret; return ret;
} }
static unsigned int kvaser_pciefd_tx_avail(const struct kvaser_pciefd_can *can)
{
return can->tx_max_count - (READ_ONCE(can->tx_idx) - READ_ONCE(can->ack_idx));
}
static int kvaser_pciefd_prepare_tx_packet(struct kvaser_pciefd_tx_packet *p, static int kvaser_pciefd_prepare_tx_packet(struct kvaser_pciefd_tx_packet *p,
struct kvaser_pciefd_can *can, struct can_priv *can, u8 seq,
struct sk_buff *skb) struct sk_buff *skb)
{ {
struct canfd_frame *cf = (struct canfd_frame *)skb->data; struct canfd_frame *cf = (struct canfd_frame *)skb->data;
int packet_size; int packet_size;
int seq = can->echo_idx;
memset(p, 0, sizeof(*p)); memset(p, 0, sizeof(*p));
if (can->can.ctrlmode & CAN_CTRLMODE_ONE_SHOT) if (can->ctrlmode & CAN_CTRLMODE_ONE_SHOT)
p->header[1] |= KVASER_PCIEFD_TPACKET_SMS; p->header[1] |= KVASER_PCIEFD_TPACKET_SMS;
if (cf->can_id & CAN_RTR_FLAG) if (cf->can_id & CAN_RTR_FLAG)
@ -782,7 +794,7 @@ static int kvaser_pciefd_prepare_tx_packet(struct kvaser_pciefd_tx_packet *p,
} else { } else {
p->header[1] |= p->header[1] |=
FIELD_PREP(KVASER_PCIEFD_RPACKET_DLC_MASK, FIELD_PREP(KVASER_PCIEFD_RPACKET_DLC_MASK,
can_get_cc_dlc((struct can_frame *)cf, can->can.ctrlmode)); can_get_cc_dlc((struct can_frame *)cf, can->ctrlmode));
} }
p->header[1] |= FIELD_PREP(KVASER_PCIEFD_PACKET_SEQ_MASK, seq); p->header[1] |= FIELD_PREP(KVASER_PCIEFD_PACKET_SEQ_MASK, seq);
@ -797,22 +809,24 @@ static netdev_tx_t kvaser_pciefd_start_xmit(struct sk_buff *skb,
struct net_device *netdev) struct net_device *netdev)
{ {
struct kvaser_pciefd_can *can = netdev_priv(netdev); struct kvaser_pciefd_can *can = netdev_priv(netdev);
unsigned long irq_flags;
struct kvaser_pciefd_tx_packet packet; struct kvaser_pciefd_tx_packet packet;
unsigned int seq = can->tx_idx & (can->can.echo_skb_max - 1);
unsigned int frame_len;
int nr_words; int nr_words;
u8 count;
if (can_dev_dropped_skb(netdev, skb)) if (can_dev_dropped_skb(netdev, skb))
return NETDEV_TX_OK; return NETDEV_TX_OK;
if (!netif_subqueue_maybe_stop(netdev, 0, kvaser_pciefd_tx_avail(can), 1, 1))
return NETDEV_TX_BUSY;
nr_words = kvaser_pciefd_prepare_tx_packet(&packet, can, skb); nr_words = kvaser_pciefd_prepare_tx_packet(&packet, &can->can, seq, skb);
spin_lock_irqsave(&can->echo_lock, irq_flags);
/* Prepare and save echo skb in internal slot */ /* Prepare and save echo skb in internal slot */
can_put_echo_skb(skb, netdev, can->echo_idx, 0); WRITE_ONCE(can->can.echo_skb[seq], NULL);
frame_len = can_skb_get_frame_len(skb);
/* Move echo index to the next slot */ can_put_echo_skb(skb, netdev, seq, frame_len);
can->echo_idx = (can->echo_idx + 1) % can->can.echo_skb_max; netdev_sent_queue(netdev, frame_len);
WRITE_ONCE(can->tx_idx, can->tx_idx + 1);
/* Write header to fifo */ /* Write header to fifo */
iowrite32(packet.header[0], iowrite32(packet.header[0],
@ -836,14 +850,7 @@ static netdev_tx_t kvaser_pciefd_start_xmit(struct sk_buff *skb,
KVASER_PCIEFD_KCAN_FIFO_LAST_REG); KVASER_PCIEFD_KCAN_FIFO_LAST_REG);
} }
count = FIELD_GET(KVASER_PCIEFD_KCAN_TX_NR_PACKETS_CURRENT_MASK, netif_subqueue_maybe_stop(netdev, 0, kvaser_pciefd_tx_avail(can), 1, 1);
ioread32(can->reg_base + KVASER_PCIEFD_KCAN_TX_NR_PACKETS_REG));
/* No room for a new message, stop the queue until at least one
* successful transmit
*/
if (count >= can->can.echo_skb_max || can->can.echo_skb[can->echo_idx])
netif_stop_queue(netdev);
spin_unlock_irqrestore(&can->echo_lock, irq_flags);
return NETDEV_TX_OK; return NETDEV_TX_OK;
} }
@ -970,6 +977,8 @@ static int kvaser_pciefd_setup_can_ctrls(struct kvaser_pciefd *pcie)
can->kv_pcie = pcie; can->kv_pcie = pcie;
can->cmd_seq = 0; can->cmd_seq = 0;
can->err_rep_cnt = 0; can->err_rep_cnt = 0;
can->completed_tx_pkts = 0;
can->completed_tx_bytes = 0;
can->bec.txerr = 0; can->bec.txerr = 0;
can->bec.rxerr = 0; can->bec.rxerr = 0;
@ -983,11 +992,10 @@ static int kvaser_pciefd_setup_can_ctrls(struct kvaser_pciefd *pcie)
tx_nr_packets_max = tx_nr_packets_max =
FIELD_GET(KVASER_PCIEFD_KCAN_TX_NR_PACKETS_MAX_MASK, FIELD_GET(KVASER_PCIEFD_KCAN_TX_NR_PACKETS_MAX_MASK,
ioread32(can->reg_base + KVASER_PCIEFD_KCAN_TX_NR_PACKETS_REG)); ioread32(can->reg_base + KVASER_PCIEFD_KCAN_TX_NR_PACKETS_REG));
can->tx_max_count = min(KVASER_PCIEFD_CAN_TX_MAX_COUNT, tx_nr_packets_max - 1);
can->can.clock.freq = pcie->freq; can->can.clock.freq = pcie->freq;
can->can.echo_skb_max = min(KVASER_PCIEFD_CAN_TX_MAX_COUNT, tx_nr_packets_max - 1); can->can.echo_skb_max = roundup_pow_of_two(can->tx_max_count);
can->echo_idx = 0;
spin_lock_init(&can->echo_lock);
spin_lock_init(&can->lock); spin_lock_init(&can->lock);
can->can.bittiming_const = &kvaser_pciefd_bittiming_const; can->can.bittiming_const = &kvaser_pciefd_bittiming_const;
@ -1510,19 +1518,21 @@ static int kvaser_pciefd_handle_ack_packet(struct kvaser_pciefd *pcie,
netdev_dbg(can->can.dev, "Packet was flushed\n"); netdev_dbg(can->can.dev, "Packet was flushed\n");
} else { } else {
int echo_idx = FIELD_GET(KVASER_PCIEFD_PACKET_SEQ_MASK, p->header[0]); int echo_idx = FIELD_GET(KVASER_PCIEFD_PACKET_SEQ_MASK, p->header[0]);
int len; unsigned int len, frame_len = 0;
u8 count;
struct sk_buff *skb; struct sk_buff *skb;
if (echo_idx != (can->ack_idx & (can->can.echo_skb_max - 1)))
return 0;
skb = can->can.echo_skb[echo_idx]; skb = can->can.echo_skb[echo_idx];
if (skb) if (!skb)
return 0;
kvaser_pciefd_set_skb_timestamp(pcie, skb, p->timestamp); kvaser_pciefd_set_skb_timestamp(pcie, skb, p->timestamp);
len = can_get_echo_skb(can->can.dev, echo_idx, NULL); len = can_get_echo_skb(can->can.dev, echo_idx, &frame_len);
count = FIELD_GET(KVASER_PCIEFD_KCAN_TX_NR_PACKETS_CURRENT_MASK,
ioread32(can->reg_base + KVASER_PCIEFD_KCAN_TX_NR_PACKETS_REG));
if (count < can->can.echo_skb_max && netif_queue_stopped(can->can.dev)) /* Pairs with barrier in kvaser_pciefd_start_xmit() */
netif_wake_queue(can->can.dev); smp_store_release(&can->ack_idx, can->ack_idx + 1);
can->completed_tx_pkts++;
can->completed_tx_bytes += frame_len;
if (!one_shot_fail) { if (!one_shot_fail) {
can->can.dev->stats.tx_bytes += len; can->can.dev->stats.tx_bytes += len;
@ -1638,11 +1648,26 @@ static int kvaser_pciefd_read_buffer(struct kvaser_pciefd *pcie, int dma_buf)
{ {
int pos = 0; int pos = 0;
int res = 0; int res = 0;
unsigned int i;
do { do {
res = kvaser_pciefd_read_packet(pcie, &pos, dma_buf); res = kvaser_pciefd_read_packet(pcie, &pos, dma_buf);
} while (!res && pos > 0 && pos < KVASER_PCIEFD_DMA_SIZE); } while (!res && pos > 0 && pos < KVASER_PCIEFD_DMA_SIZE);
/* Report ACKs in this buffer to BQL en masse for correct periods */
for (i = 0; i < pcie->nr_channels; ++i) {
struct kvaser_pciefd_can *can = pcie->can[i];
if (!can->completed_tx_pkts)
continue;
netif_subqueue_completed_wake(can->can.dev, 0,
can->completed_tx_pkts,
can->completed_tx_bytes,
kvaser_pciefd_tx_avail(can), 1);
can->completed_tx_pkts = 0;
can->completed_tx_bytes = 0;
}
return res; return res;
} }