Commit db88681c authored by Marc Kleine-Budde's avatar Marc Kleine-Budde

Merge patch series "can: m_can: Optimizations for m_can/tcan part 2"

Markus Schneider-Pargmann <msp@baylibre.com> says:

third version part 2, functionally I had to move from spin_lock to
spin_lock_irqsave because of an interrupt that was calling start_xmit,
see attached stack. This is tested on tcan455x but I don't have the
integrated hardware myself so any testing is appreciated.

The series implements many small and bigger throughput improvements.

Changes in v3:
- Remove parenthesis in error messages
- Use memcpy_and_pad for buffer copy in 'can: m_can: Write transmit
  header and data in one transaction'.
- Replace spin_lock with spin_lock_irqsave. I got a report of a
  interrupt that was calling start_xmit just after the netqueue was
  woken up before the locked region was exited. spin_lock_irqsave should
  fix this. I attached the full stack at the end of the mail if someone
  wants to know.
- Rebased to v6.3-rc1.
- Removed tcan4x5x patches from this series.

Changes in v2: https://lore.kernel.org/all/20230125195059.630377-1-msp@baylibre.com
- Rebased on v6.2-rc5
- Fixed missing/broken accounting for non peripheral m_can devices.

part 1:
v1 - https://lore.kernel.org/lkml/20221116205308.2996556-1-msp@baylibre.com
v2 - https://lore.kernel.org/lkml/20221206115728.1056014-1-msp@baylibre.com

part 2:
v1 - https://lore.kernel.org/lkml/20221221152537.751564-1-msp@baylibre.com
v2 - https://lore.kernel.org/lkml/20230125195059.630377-1-msp@baylibre.com

stack of calling start_xmit within locked region:
	[  308.170171]  dump_backtrace+0x0/0x1a0
	[  308.173841]  show_stack+0x18/0x70
	[  308.177158]  sched_show_task+0x154/0x180
	[  308.181084]  dump_cpu_task+0x44/0x54
	[  308.184664]  rcu_dump_cpu_stacks+0xe8/0x12c
	[  308.188846]  rcu_sched_clock_irq+0x9f4/0xd10
	[  308.193118]  update_process_times+0x9c/0xec
	[  308.197304]  tick_sched_handle+0x34/0x60
	[  308.201231]  tick_sched_timer+0x4c/0xa4
	[  308.205071]  __hrtimer_run_queues+0x138/0x1e0
	[  308.209429]  hrtimer_interrupt+0xe8/0x244
	[  308.213440]  arch_timer_handler_phys+0x38/0x50
	[  308.217890]  handle_percpu_devid_irq+0x84/0x130
	[  308.222422]  handle_domain_irq+0x60/0x90
	[  308.226347]  gic_handle_irq+0x54/0x130
	[  308.230099]  do_interrupt_handler+0x34/0x60
	[  308.234286]  el1_interrupt+0x30/0x80
	[  308.237861]  el1h_64_irq_handler+0x18/0x24
	[  308.241958]  el1h_64_irq+0x78/0x7c
	[  308.245360]  queued_spin_lock_slowpath+0xf4/0x390
	[  308.250067]  m_can_start_tx+0x20/0xb0 [m_can]
	[  308.254431]  m_can_start_xmit+0xd8/0x230 [m_can]
	[  308.259054]  dev_hard_start_xmit+0xd4/0x15c
	[  308.263241]  sch_direct_xmit+0xe8/0x370
	[  308.267080]  __qdisc_run+0x118/0x650
	[  308.270660]  net_tx_action+0x118/0x230
	[  308.274409]  _stext+0x124/0x2a0
	[  308.277549]  __irq_exit_rcu+0xe4/0x100
	[  308.281302]  irq_exit+0x10/0x20
	[  308.284444]  handle_domain_irq+0x64/0x90
	[  308.288367]  gic_handle_irq+0x54/0x130
	[  308.292119]  call_on_irq_stack+0x2c/0x54
	[  308.296043]  do_interrupt_handler+0x54/0x60
	[  308.300228]  el1_interrupt+0x30/0x80
	[  308.303804]  el1h_64_irq_handler+0x18/0x24
	[  308.307901]  el1h_64_irq+0x78/0x7c
	[  308.311303]  __netif_schedule+0x78/0xa0
	[  308.315138]  netif_tx_wake_queue+0x50/0x7c
	[  308.319237]  m_can_isr+0x474/0x1710 [m_can]
	[  308.323425]  irq_thread_fn+0x2c/0x9c
	[  308.327005]  irq_thread+0x178/0x2c0
	[  308.330497]  kthread+0x150/0x160
	[  308.333727]  ret_from_fork+0x10/0x20

Link: https://lore.kernel.org/all/20230315110546.2518305-1-msp@baylibre.com
[mkl: apply patches 1...5 only, adjust message accordingly]
Signed-off-by: default avatarMarc Kleine-Budde <mkl@pengutronix.de>
parents a57915ae 9083e0b0
...@@ -972,8 +972,8 @@ static int m_can_rx_peripheral(struct net_device *dev, u32 irqstatus) ...@@ -972,8 +972,8 @@ static int m_can_rx_peripheral(struct net_device *dev, u32 irqstatus)
/* Don't re-enable interrupts if the driver had a fatal error /* Don't re-enable interrupts if the driver had a fatal error
* (e.g., FIFO read failure). * (e.g., FIFO read failure).
*/ */
if (work_done >= 0) if (work_done < 0)
m_can_enable_all_interrupts(cdev); m_can_disable_all_interrupts(cdev);
return work_done; return work_done;
} }
...@@ -1083,8 +1083,7 @@ static irqreturn_t m_can_isr(int irq, void *dev_id) ...@@ -1083,8 +1083,7 @@ static irqreturn_t m_can_isr(int irq, void *dev_id)
return IRQ_NONE; return IRQ_NONE;
/* ACK all irqs */ /* ACK all irqs */
if (ir & IR_ALL_INT) m_can_write(cdev, M_CAN_IR, ir);
m_can_write(cdev, M_CAN_IR, ir);
if (cdev->ops->clear_interrupts) if (cdev->ops->clear_interrupts)
cdev->ops->clear_interrupts(cdev); cdev->ops->clear_interrupts(cdev);
...@@ -1096,11 +1095,12 @@ static irqreturn_t m_can_isr(int irq, void *dev_id) ...@@ -1096,11 +1095,12 @@ static irqreturn_t m_can_isr(int irq, void *dev_id)
*/ */
if ((ir & IR_RF0N) || (ir & IR_ERR_ALL_30X)) { if ((ir & IR_RF0N) || (ir & IR_ERR_ALL_30X)) {
cdev->irqstatus = ir; cdev->irqstatus = ir;
m_can_disable_all_interrupts(cdev); if (!cdev->is_peripheral) {
if (!cdev->is_peripheral) m_can_disable_all_interrupts(cdev);
napi_schedule(&cdev->napi); napi_schedule(&cdev->napi);
else if (m_can_rx_peripheral(dev, ir) < 0) } else if (m_can_rx_peripheral(dev, ir) < 0) {
goto out_fail; goto out_fail;
}
} }
if (cdev->version == 30) { if (cdev->version == 30) {
...@@ -1262,6 +1262,7 @@ static int m_can_set_bittiming(struct net_device *dev) ...@@ -1262,6 +1262,7 @@ static int m_can_set_bittiming(struct net_device *dev)
static int m_can_chip_config(struct net_device *dev) static int m_can_chip_config(struct net_device *dev)
{ {
struct m_can_classdev *cdev = netdev_priv(dev); struct m_can_classdev *cdev = netdev_priv(dev);
u32 interrupts = IR_ALL_INT;
u32 cccr, test; u32 cccr, test;
int err; int err;
...@@ -1271,6 +1272,11 @@ static int m_can_chip_config(struct net_device *dev) ...@@ -1271,6 +1272,11 @@ static int m_can_chip_config(struct net_device *dev)
return err; return err;
} }
/* Disable unused interrupts */
interrupts &= ~(IR_ARA | IR_ELO | IR_DRX | IR_TEFF | IR_TEFW | IR_TFE |
IR_TCF | IR_HPM | IR_RF1F | IR_RF1W | IR_RF1N |
IR_RF0F | IR_RF0W);
m_can_config_endisable(cdev, true); m_can_config_endisable(cdev, true);
/* RX Buffer/FIFO Element Size 64 bytes data field */ /* RX Buffer/FIFO Element Size 64 bytes data field */
...@@ -1365,16 +1371,13 @@ static int m_can_chip_config(struct net_device *dev) ...@@ -1365,16 +1371,13 @@ static int m_can_chip_config(struct net_device *dev)
m_can_write(cdev, M_CAN_TEST, test); m_can_write(cdev, M_CAN_TEST, test);
/* Enable interrupts */ /* Enable interrupts */
m_can_write(cdev, M_CAN_IR, IR_ALL_INT); if (!(cdev->can.ctrlmode & CAN_CTRLMODE_BERR_REPORTING)) {
if (!(cdev->can.ctrlmode & CAN_CTRLMODE_BERR_REPORTING))
if (cdev->version == 30) if (cdev->version == 30)
m_can_write(cdev, M_CAN_IE, IR_ALL_INT & interrupts &= ~(IR_ERR_LEC_30X);
~(IR_ERR_LEC_30X));
else else
m_can_write(cdev, M_CAN_IE, IR_ALL_INT & interrupts &= ~(IR_ERR_LEC_31X);
~(IR_ERR_LEC_31X)); }
else m_can_write(cdev, M_CAN_IE, interrupts);
m_can_write(cdev, M_CAN_IE, IR_ALL_INT);
/* route all interrupts to INT0 */ /* route all interrupts to INT0 */
m_can_write(cdev, M_CAN_ILS, ILS_ALL_INT0); m_can_write(cdev, M_CAN_ILS, ILS_ALL_INT0);
...@@ -1592,10 +1595,8 @@ static int m_can_close(struct net_device *dev) ...@@ -1592,10 +1595,8 @@ static int m_can_close(struct net_device *dev)
cdev->tx_skb = NULL; cdev->tx_skb = NULL;
destroy_workqueue(cdev->tx_wq); destroy_workqueue(cdev->tx_wq);
cdev->tx_wq = NULL; cdev->tx_wq = NULL;
}
if (cdev->is_peripheral)
can_rx_offload_disable(&cdev->offload); can_rx_offload_disable(&cdev->offload);
}
close_candev(dev); close_candev(dev);
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment