lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
Open Source and information security mailing list archives
| ||
|
Date: Tue, 24 Jul 2007 11:48:16 -0400 From: "Beauchemin, Mark" <Mark.Beauchemin@...amorenet.com> To: "Thomas Gleixner" <tglx@...utronix.de> Cc: <linux-kernel@...r.kernel.org>, <mingo@...e.hu>, "David Miller" <davem@...emloft.net> Subject: RE: [PATCH -rt] Preemption problem in kernel RT Patch Thomas, I think I've gotten to the heart of the problem. Here's an excerpt from the latest -rt patch: net/core/dev.c in the function dev_queue_xmit @@ -1588,11 +1588,17 @@ gso: Either shot noqueue qdisc, it is even simpler 8) */ if (dev->flags & IFF_UP) { - int cpu = smp_processor_id(); /* ok because BHs are off */ + /* + * No need to check for recursion with threaded interrupts: + */ +#ifdef CONFIG_PREEMPT_RT + if (1) { +#else + int cpu = raw_smp_processor_id(); /* ok because BHs are off */ if (dev->xmit_lock_owner != cpu) { - - HARD_TX_LOCK(dev, cpu); +#endif + HARD_TX_LOCK(dev); if (!netif_queue_stopped(dev) && !netif_subqueue_stopped(dev, skb->queue_mapping)) { I'm not sure why the check for recursion has been removed. In the backtrace below, I think it would be caught by this check and not recursively call the spinlock code. > Call Trace: > [D010BB40] [C01C7BA8] rt_spin_lock_slowlock+0x44/0x1f8 (unreliable) > [D010BB90] [C0153464] dev_queue_xmit+0x298/0x2a0 Tunnel2 > [D010BBB0] [C0176398] ip_output+0x288/0x2dc > [D010BBE0] [C01AC078] ipip_tunnel_xmit+0x508/0x698 > [D010BC60] [C0150DF4] dev_hard_start_xmit+0x1b4/0x2a4 > [D010BC80] [C0153430] dev_queue_xmit+0x264/0x2a0 Tunnel4 > [D010BCA0] [C0176398] ip_output+0x288/0x2dc > [D010BCD0] [C01AC078] ipip_tunnel_xmit+0x508/0x698 > [D010BD50] [C0150DF4] dev_hard_start_xmit+0x1b4/0x2a4 > [D010BD70] [C0153430] dev_queue_xmit+0x264/0x2a0 Tunnel2 > [D010BD90] [C0176398] ip_output+0x288/0x2dc > [D010BDC0] [C017685C] ip_queue_xmit+0x1ac/0x4e4 > [D010BE30] [C018762C] tcp_transmit_skb+0x390/0x810 > [D010BE70] [C018882C] tcp_retransmit_skb+0x160/0x638 > [D010BEA0] [C018BA5C] tcp_write_timer+0x274/0x6c0 > [D010BED0] [C0024314] run_timer_softirq+0x2d0/0xedc > [D010BF80] [C001F1C4] ksoftirqd+0xf8/0x1b0 > [D010BFC0] [C0031588] kthread+0xc0/0xfc > [D010BFF0] [C000471C] kernel_thread+0x44/0x60 I found one other place in the code which appears to do the same thing. Although it is written to handle smp collisions, I think it should also handle the error case above. Index: linux-rt-rebase.q/net/sched/sch_generic.c =================================================================== --- linux-rt-rebase.q.orig/net/sched/sch_generic.c +++ linux-rt-rebase.q/net/sched/sch_generic.c @@ -12,6 +12,7 @@ */ #include <linux/bitops.h> +#include <linux/kallsyms.h> #include <linux/module.h> #include <linux/types.h> #include <linux/kernel.h> @@ -150,16 +151,28 @@ static inline int qdisc_restart(struct n */ lockless = (dev->features & NETIF_F_LLTX); - if (!lockless && !netif_tx_trylock(dev)) { - /* Another CPU grabbed the driver tx lock */ - return handle_dev_cpu_collision(skb, dev, q); + if (!lockless) { +#ifdef CONFIG_PREEMPT_RT + netif_tx_lock(dev); +#else + if (netif_tx_trylock(dev)) + /* Another CPU grabbed the driver tx lock */ + return handle_dev_cpu_collision(skb, dev, q); +#endif What do you think? Thanks, Mark - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@...r.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists