tjh.dev / kernel
Linux kernel mirror (for testing) git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
kernel os linux
1
fork

Configure Feed

Select the types of activity you want to include in your feed.

net: ks8851: Reinstate disabling of BHs around IRQ handler

If the driver executes ks8851_irq() AND a TX packet has been sent, then
the driver enables TX queue via netif_wake_queue() which schedules TX
softirq to queue packets for this device.

If CONFIG_PREEMPT_RT=y is set AND a packet has also been received by
the MAC, then ks8851_rx_pkts() calls netdev_alloc_skb_ip_align() to
allocate SKBs for the received packets. If netdev_alloc_skb_ip_align()
is called with BH enabled, then local_bh_enable() at the end of
netdev_alloc_skb_ip_align() will trigger the pending softirq processing,
which may ultimately call the .xmit callback ks8851_start_xmit_par().
The ks8851_start_xmit_par() will try to lock struct ks8851_net_par
.lock spinlock, which is already locked by ks8851_irq() from which
ks8851_start_xmit_par() was called. This leads to a deadlock, which
is reported by the kernel, including a trace listed below.

If CONFIG_PREEMPT_RT is not set, then since commit 0913ec336a6c0
("net: ks8851: Fix deadlock with the SPI chip variant") the deadlock
can also be triggered without received packet in the RX FIFO. The
pending softirqs will be processed on return from
spin_unlock_bh(&ks->statelock) in ks8851_irq(), which triggers the
deadlock as well.

Fix the problem by disabling BH around critical sections, including the
IRQ handler, thus preventing the net_tx_action() softirq from triggering
during these critical sections. The net_tx_action() softirq is triggered
once BH are re-enabled and at the end of the IRQ handler, once all the
other IRQ handler actions have been completed.

__schedule from schedule_rtlock+0x1c/0x34
schedule_rtlock from rtlock_slowlock_locked+0x548/0x904
rtlock_slowlock_locked from rt_spin_lock+0x60/0x9c
rt_spin_lock from ks8851_start_xmit_par+0x74/0x1a8
ks8851_start_xmit_par from netdev_start_xmit+0x20/0x44
netdev_start_xmit from dev_hard_start_xmit+0xd0/0x188
dev_hard_start_xmit from sch_direct_xmit+0xb8/0x25c
sch_direct_xmit from __qdisc_run+0x1f8/0x4ec
__qdisc_run from qdisc_run+0x1c/0x28
qdisc_run from net_tx_action+0x1f0/0x268
net_tx_action from handle_softirqs+0x1a4/0x270
handle_softirqs from __local_bh_enable_ip+0xcc/0xe0
__local_bh_enable_ip from __alloc_skb+0xd8/0x128
__alloc_skb from __netdev_alloc_skb+0x3c/0x19c
__netdev_alloc_skb from ks8851_irq+0x388/0x4d4
ks8851_irq from irq_thread_fn+0x24/0x64
irq_thread_fn from irq_thread+0x178/0x28c
irq_thread from kthread+0x12c/0x138
kthread from ret_from_fork+0x14/0x28

Reviewed-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
Fixes: e0863634bf9f ("net: ks8851: Queue RX packets in IRQ handler instead of disabling BHs")
Cc: stable@vger.kernel.org
Signed-off-by: Marek Vasut <marex@nabladev.com>
Link: https://patch.msgid.link/20260415231020.455298-1-marex@nabladev.com
Signed-off-by: Jakub Kicinski <kuba@kernel.org>

authored by

Marek Vasut and committed by
Jakub Kicinski 5c9fcac3 965dc934

+38 -58
+2 -4
drivers/net/ethernet/micrel/ks8851.h
··· 408 408 struct gpio_desc *gpio; 409 409 struct mii_bus *mii_bus; 410 410 411 - void (*lock)(struct ks8851_net *ks, 412 - unsigned long *flags); 413 - void (*unlock)(struct ks8851_net *ks, 414 - unsigned long *flags); 411 + void (*lock)(struct ks8851_net *ks); 412 + void (*unlock)(struct ks8851_net *ks); 415 413 unsigned int (*rdreg16)(struct ks8851_net *ks, 416 414 unsigned int reg); 417 415 void (*wrreg16)(struct ks8851_net *ks,
+26 -38
drivers/net/ethernet/micrel/ks8851_common.c
··· 28 28 /** 29 29 * ks8851_lock - register access lock 30 30 * @ks: The chip state 31 - * @flags: Spinlock flags 32 31 * 33 32 * Claim chip register access lock 34 33 */ 35 - static void ks8851_lock(struct ks8851_net *ks, unsigned long *flags) 34 + static void ks8851_lock(struct ks8851_net *ks) 36 35 { 37 - ks->lock(ks, flags); 36 + ks->lock(ks); 38 37 } 39 38 40 39 /** 41 40 * ks8851_unlock - register access unlock 42 41 * @ks: The chip state 43 - * @flags: Spinlock flags 44 42 * 45 43 * Release chip register access lock 46 44 */ 47 - static void ks8851_unlock(struct ks8851_net *ks, unsigned long *flags) 45 + static void ks8851_unlock(struct ks8851_net *ks) 48 46 { 49 - ks->unlock(ks, flags); 47 + ks->unlock(ks); 50 48 } 51 49 52 50 /** ··· 127 129 static int ks8851_write_mac_addr(struct net_device *dev) 128 130 { 129 131 struct ks8851_net *ks = netdev_priv(dev); 130 - unsigned long flags; 131 132 u16 val; 132 133 int i; 133 134 134 - ks8851_lock(ks, &flags); 135 + ks8851_lock(ks); 135 136 136 137 /* 137 138 * Wake up chip in case it was powered off when stopped; otherwise, ··· 146 149 if (!netif_running(dev)) 147 150 ks8851_set_powermode(ks, PMECR_PM_SOFTDOWN); 148 151 149 - ks8851_unlock(ks, &flags); 152 + ks8851_unlock(ks); 150 153 151 154 return 0; 152 155 } ··· 160 163 static void ks8851_read_mac_addr(struct net_device *dev) 161 164 { 162 165 struct ks8851_net *ks = netdev_priv(dev); 163 - unsigned long flags; 164 166 u8 addr[ETH_ALEN]; 165 167 u16 reg; 166 168 int i; 167 169 168 - ks8851_lock(ks, &flags); 170 + ks8851_lock(ks); 169 171 170 172 for (i = 0; i < ETH_ALEN; i += 2) { 171 173 reg = ks8851_rdreg16(ks, KS_MAR(i)); ··· 173 177 } 174 178 eth_hw_addr_set(dev, addr); 175 179 176 - ks8851_unlock(ks, &flags); 180 + ks8851_unlock(ks); 177 181 } 178 182 179 183 /** ··· 308 312 { 309 313 struct ks8851_net *ks = _ks; 310 314 struct sk_buff_head rxq; 311 - unsigned long flags; 312 315 unsigned int status; 313 316 struct sk_buff *skb; 314 317 315 - ks8851_lock(ks, &flags); 318 + ks8851_lock(ks); 316 319 317 320 status = ks8851_rdreg16(ks, KS_ISR); 318 321 ks8851_wrreg16(ks, KS_ISR, status); ··· 368 373 ks8851_wrreg16(ks, KS_RXCR1, rxc->rxcr1); 369 374 } 370 375 371 - ks8851_unlock(ks, &flags); 376 + ks8851_unlock(ks); 372 377 373 378 if (status & IRQ_LCI) 374 379 mii_check_link(&ks->mii); ··· 400 405 static int ks8851_net_open(struct net_device *dev) 401 406 { 402 407 struct ks8851_net *ks = netdev_priv(dev); 403 - unsigned long flags; 404 408 int ret; 405 409 406 410 ret = request_threaded_irq(dev->irq, NULL, ks8851_irq, ··· 412 418 413 419 /* lock the card, even if we may not actually be doing anything 414 420 * else at the moment */ 415 - ks8851_lock(ks, &flags); 421 + ks8851_lock(ks); 416 422 417 423 netif_dbg(ks, ifup, ks->netdev, "opening\n"); 418 424 ··· 465 471 466 472 netif_dbg(ks, ifup, ks->netdev, "network device up\n"); 467 473 468 - ks8851_unlock(ks, &flags); 474 + ks8851_unlock(ks); 469 475 mii_check_link(&ks->mii); 470 476 return 0; 471 477 } ··· 481 487 static int ks8851_net_stop(struct net_device *dev) 482 488 { 483 489 struct ks8851_net *ks = netdev_priv(dev); 484 - unsigned long flags; 485 490 486 491 netif_info(ks, ifdown, dev, "shutting down\n"); 487 492 488 493 netif_stop_queue(dev); 489 494 490 - ks8851_lock(ks, &flags); 495 + ks8851_lock(ks); 491 496 /* turn off the IRQs and ack any outstanding */ 492 497 ks8851_wrreg16(ks, KS_IER, 0x0000); 493 498 ks8851_wrreg16(ks, KS_ISR, 0xffff); 494 - ks8851_unlock(ks, &flags); 499 + ks8851_unlock(ks); 495 500 496 501 /* stop any outstanding work */ 497 502 ks8851_flush_tx_work(ks); 498 503 flush_work(&ks->rxctrl_work); 499 504 500 - ks8851_lock(ks, &flags); 505 + ks8851_lock(ks); 501 506 /* shutdown RX process */ 502 507 ks8851_wrreg16(ks, KS_RXCR1, 0x0000); 503 508 ··· 505 512 506 513 /* set powermode to soft power down to save power */ 507 514 ks8851_set_powermode(ks, PMECR_PM_SOFTDOWN); 508 - ks8851_unlock(ks, &flags); 515 + ks8851_unlock(ks); 509 516 510 517 /* ensure any queued tx buffers are dumped */ 511 518 while (!skb_queue_empty(&ks->txq)) { ··· 559 566 static void ks8851_rxctrl_work(struct work_struct *work) 560 567 { 561 568 struct ks8851_net *ks = container_of(work, struct ks8851_net, rxctrl_work); 562 - unsigned long flags; 563 569 564 - ks8851_lock(ks, &flags); 570 + ks8851_lock(ks); 565 571 566 572 /* need to shutdown RXQ before modifying filter parameters */ 567 573 ks8851_wrreg16(ks, KS_RXCR1, 0x00); 568 574 569 - ks8851_unlock(ks, &flags); 575 + ks8851_unlock(ks); 570 576 } 571 577 572 578 static void ks8851_set_rx_mode(struct net_device *dev) ··· 772 780 { 773 781 struct ks8851_net *ks = netdev_priv(dev); 774 782 int offset = ee->offset; 775 - unsigned long flags; 776 783 int len = ee->len; 777 784 u16 tmp; 778 785 ··· 785 794 if (!(ks->rc_ccr & CCR_EEPROM)) 786 795 return -ENOENT; 787 796 788 - ks8851_lock(ks, &flags); 797 + ks8851_lock(ks); 789 798 790 799 ks8851_eeprom_claim(ks); 791 800 ··· 808 817 eeprom_93cx6_wren(&ks->eeprom, false); 809 818 810 819 ks8851_eeprom_release(ks); 811 - ks8851_unlock(ks, &flags); 820 + ks8851_unlock(ks); 812 821 813 822 return 0; 814 823 } ··· 818 827 { 819 828 struct ks8851_net *ks = netdev_priv(dev); 820 829 int offset = ee->offset; 821 - unsigned long flags; 822 830 int len = ee->len; 823 831 824 832 /* must be 2 byte aligned */ ··· 827 837 if (!(ks->rc_ccr & CCR_EEPROM)) 828 838 return -ENOENT; 829 839 830 - ks8851_lock(ks, &flags); 840 + ks8851_lock(ks); 831 841 832 842 ks8851_eeprom_claim(ks); 833 843 ··· 835 845 836 846 eeprom_93cx6_multiread(&ks->eeprom, offset/2, (__le16 *)data, len/2); 837 847 ks8851_eeprom_release(ks); 838 - ks8851_unlock(ks, &flags); 848 + ks8851_unlock(ks); 839 849 840 850 return 0; 841 851 } ··· 894 904 static int ks8851_phy_read_common(struct net_device *dev, int phy_addr, int reg) 895 905 { 896 906 struct ks8851_net *ks = netdev_priv(dev); 897 - unsigned long flags; 898 907 int result; 899 908 int ksreg; 900 909 ··· 901 912 if (ksreg < 0) 902 913 return ksreg; 903 914 904 - ks8851_lock(ks, &flags); 915 + ks8851_lock(ks); 905 916 result = ks8851_rdreg16(ks, ksreg); 906 - ks8851_unlock(ks, &flags); 917 + ks8851_unlock(ks); 907 918 908 919 return result; 909 920 } ··· 938 949 int phy, int reg, int value) 939 950 { 940 951 struct ks8851_net *ks = netdev_priv(dev); 941 - unsigned long flags; 942 952 int ksreg; 943 953 944 954 ksreg = ks8851_phy_reg(reg); 945 955 if (ksreg >= 0) { 946 - ks8851_lock(ks, &flags); 956 + ks8851_lock(ks); 947 957 ks8851_wrreg16(ks, ksreg, value); 948 - ks8851_unlock(ks, &flags); 958 + ks8851_unlock(ks); 949 959 } 950 960 } 951 961
+6 -9
drivers/net/ethernet/micrel/ks8851_par.c
··· 55 55 /** 56 56 * ks8851_lock_par - register access lock 57 57 * @ks: The chip state 58 - * @flags: Spinlock flags 59 58 * 60 59 * Claim chip register access lock 61 60 */ 62 - static void ks8851_lock_par(struct ks8851_net *ks, unsigned long *flags) 61 + static void ks8851_lock_par(struct ks8851_net *ks) 63 62 { 64 63 struct ks8851_net_par *ksp = to_ks8851_par(ks); 65 64 66 - spin_lock_irqsave(&ksp->lock, *flags); 65 + spin_lock_bh(&ksp->lock); 67 66 } 68 67 69 68 /** 70 69 * ks8851_unlock_par - register access unlock 71 70 * @ks: The chip state 72 - * @flags: Spinlock flags 73 71 * 74 72 * Release chip register access lock 75 73 */ 76 - static void ks8851_unlock_par(struct ks8851_net *ks, unsigned long *flags) 74 + static void ks8851_unlock_par(struct ks8851_net *ks) 77 75 { 78 76 struct ks8851_net_par *ksp = to_ks8851_par(ks); 79 77 80 - spin_unlock_irqrestore(&ksp->lock, *flags); 78 + spin_unlock_bh(&ksp->lock); 81 79 } 82 80 83 81 /** ··· 231 233 { 232 234 struct ks8851_net *ks = netdev_priv(dev); 233 235 netdev_tx_t ret = NETDEV_TX_OK; 234 - unsigned long flags; 235 236 unsigned int txqcr; 236 237 u16 txmir; 237 238 int err; ··· 238 241 netif_dbg(ks, tx_queued, ks->netdev, 239 242 "%s: skb %p, %d@%p\n", __func__, skb, skb->len, skb->data); 240 243 241 - ks8851_lock_par(ks, &flags); 244 + ks8851_lock_par(ks); 242 245 243 246 txmir = ks8851_rdreg16_par(ks, KS_TXMIR) & 0x1fff; 244 247 ··· 259 262 ret = NETDEV_TX_BUSY; 260 263 } 261 264 262 - ks8851_unlock_par(ks, &flags); 265 + ks8851_unlock_par(ks); 263 266 264 267 return ret; 265 268 }
+4 -7
drivers/net/ethernet/micrel/ks8851_spi.c
··· 71 71 /** 72 72 * ks8851_lock_spi - register access lock 73 73 * @ks: The chip state 74 - * @flags: Spinlock flags 75 74 * 76 75 * Claim chip register access lock 77 76 */ 78 - static void ks8851_lock_spi(struct ks8851_net *ks, unsigned long *flags) 77 + static void ks8851_lock_spi(struct ks8851_net *ks) 79 78 { 80 79 struct ks8851_net_spi *kss = to_ks8851_spi(ks); 81 80 ··· 84 85 /** 85 86 * ks8851_unlock_spi - register access unlock 86 87 * @ks: The chip state 87 - * @flags: Spinlock flags 88 88 * 89 89 * Release chip register access lock 90 90 */ 91 - static void ks8851_unlock_spi(struct ks8851_net *ks, unsigned long *flags) 91 + static void ks8851_unlock_spi(struct ks8851_net *ks) 92 92 { 93 93 struct ks8851_net_spi *kss = to_ks8851_spi(ks); 94 94 ··· 307 309 struct ks8851_net_spi *kss; 308 310 unsigned short tx_space; 309 311 struct ks8851_net *ks; 310 - unsigned long flags; 311 312 struct sk_buff *txb; 312 313 bool last; 313 314 ··· 314 317 ks = &kss->ks8851; 315 318 last = skb_queue_empty(&ks->txq); 316 319 317 - ks8851_lock_spi(ks, &flags); 320 + ks8851_lock_spi(ks); 318 321 319 322 while (!last) { 320 323 txb = skb_dequeue(&ks->txq); ··· 340 343 ks->tx_space = tx_space; 341 344 spin_unlock_bh(&ks->statelock); 342 345 343 - ks8851_unlock_spi(ks, &flags); 346 + ks8851_unlock_spi(ks); 344 347 } 345 348 346 349 /**