| 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: Thu, 18 Mar 2010 15:31:40 +0200
From: Sergey Senozhatsky <sergey.senozhatsky@...il.com>
To: Francois Romieu <romieu@...zoreil.com>
Cc: Eric Dumazet <eric.dumazet@...il.com>,
Oleg Nesterov <oleg@...hat.com>,
David Miller <davem@...emloft.net>, netdev@...r.kernel.org
Subject: Re: [PATCH] r8169: Fix rtl8169_rx_interrupt()
Hello,
Patched r8169 (both Eric's and Francois' patches are applied). /*pktgen localhost2localhost*/
[ 498.818640] pktgen 2.72: Packet Generator for packet performance testing.
[ 568.999957] ------------[ cut here ]------------
[ 568.999969] WARNING: at net/sched/sch_generic.c:255 dev_watchdog+0xc1/0x125()
[ 568.999973] Hardware name: F3JC
[ 568.999976] NETDEV WATCHDOG: eth0 (r8169): transmit queue 0 timed out
[ 568.999979] Modules linked in: pktgen snd_hwdep snd_hda_codec_si3054 snd_hda_codec_realtek asus_laptop sparse_keymap sdhci_pci sdhci snd_hda_intel mmc_core led_class
snd_hda_codec snd_pcm snd_timer psmouse snd soundcore snd_page_alloc sg evdev i2c_i801 rng_core serio_raw r8169 mii uhci_hcd sr_mod ehci_hcd cdrom sd_mod usbcore ata_piix
[ 569.000029] Pid: 3350, comm: kpktgend_0 Tainted: G W 2.6.34-rc1-dbg-git6-r8169 #48
[ 569.000033] Call Trace:
[ 569.000041] [<c102e293>] warn_slowpath_common+0x65/0x7c
[ 569.000046] [<c126c024>] ? dev_watchdog+0xc1/0x125
[ 569.000051] [<c102e2de>] warn_slowpath_fmt+0x24/0x27
[ 569.000056] [<c126c024>] dev_watchdog+0xc1/0x125
[ 569.000063] [<c1036afb>] ? run_timer_softirq+0x120/0x1eb
[ 569.000069] [<c1036b51>] run_timer_softirq+0x176/0x1eb
[ 569.000074] [<c1036afb>] ? run_timer_softirq+0x120/0x1eb
[ 569.000079] [<c126bf63>] ? dev_watchdog+0x0/0x125
[ 569.000084] [<c1032d39>] __do_softirq+0x8d/0x117
[ 569.000089] [<c1032dee>] do_softirq+0x2b/0x43
[ 569.000097] [<f809510d>] ? pktgen_xmit+0xdb7/0xe8e [pktgen]
[ 569.000102] [<c1032e9c>] _local_bh_enable_ip+0x88/0xb0
[ 569.000107] [<c1032ecc>] local_bh_enable_ip+0x8/0xa
[ 569.000114] [<c12c83a0>] _raw_spin_unlock_bh+0x2f/0x32
[ 569.000120] [<f809510d>] pktgen_xmit+0xdb7/0xe8e [pktgen]
[ 569.000129] [<f91674a9>] ? rtl8169_start_xmit+0x0/0x304 [r8169]
[ 569.000136] [<c1183940>] ? trace_hardirqs_on_thunk+0xc/0x10
[ 569.000143] [<c105012d>] ? print_lock_contention_bug+0x11/0xb2
[ 569.000150] [<f80935ca>] ? spin_lock+0x8/0xa [pktgen]
[ 569.000156] [<f80954a8>] pktgen_thread_worker+0x18d/0x631 [pktgen]
[ 569.000163] [<c103f931>] ? autoremove_wake_function+0x0/0x2f
[ 569.000169] [<c103f931>] ? autoremove_wake_function+0x0/0x2f
[ 569.000175] [<f809531b>] ? pktgen_thread_worker+0x0/0x631 [pktgen]
[ 569.000180] [<c103f60e>] kthread+0x6a/0x6f
[ 569.000185] [<c103f5a4>] ? kthread+0x0/0x6f
[ 569.000191] [<c1002e42>] kernel_thread_helper+0x6/0x1a
[ 569.000195] ---[ end trace a22d306b065d4a68 ]---
Sergey
On (03/18/10 00:55), Francois Romieu wrote:
> > I suspect lot of work is needed on this driver to make it working, but I
> > dont have a machine with said adapter.
>
> This one should help too if Sergey owns a (MSI) 8168.
>
> diff --git a/drivers/net/r8169.c b/drivers/net/r8169.c
> index dfc3573..721e7f3 100644
> --- a/drivers/net/r8169.c
> +++ b/drivers/net/r8169.c
> @@ -4532,21 +4532,39 @@ static int rtl8169_rx_interrupt(struct net_device *dev,
> return count;
> }
>
> +static void rtl_napi_cond_schedule(struct rtl8169_private *tp, u16 status)
> +{
> + if (status & tp->napi_event) {
> + void __iomem *ioaddr = tp->mmio_addr;
> +
> + RTL_W16(IntrMask, tp->intr_event & ~tp->napi_event);
> + mmiowb();
> + napi_schedule(&tp->napi);
> + }
> +}
> +
> static irqreturn_t rtl8169_interrupt(int irq, void *dev_instance)
> {
> struct net_device *dev = dev_instance;
> struct rtl8169_private *tp = netdev_priv(dev);
> void __iomem *ioaddr = tp->mmio_addr;
> int handled = 0;
> - int status;
> + u16 status;
>
> /* loop handling interrupts until we have no new ones or
> * we hit a invalid/hotplug case.
> */
> status = RTL_R16(IntrStatus);
> while (status && status != 0xffff) {
> + u16 acked;
> +
> handled = 1;
>
> + acked = (status & RxFIFOOver) ? (status | RxOverflow) : status;
> + acked &= ~tp->napi_event;
> +
> + RTL_W16(IntrStatus, acked);
> +
> /* Handle all of the error cases first. These will reset
> * the chip, so just exit the loop.
> */
> @@ -4557,7 +4575,7 @@ static irqreturn_t rtl8169_interrupt(int irq, void *dev_instance)
>
> /* Work around for rx fifo overflow */
> if (unlikely(status & RxFIFOOver) &&
> - (tp->mac_version == RTL_GIGA_MAC_VER_11)) {
> + (tp->mac_version == RTL_GIGA_MAC_VER_11)) {
> netif_stop_queue(dev);
> rtl8169_tx_timeout(dev);
> break;
> @@ -4571,30 +4589,9 @@ static irqreturn_t rtl8169_interrupt(int irq, void *dev_instance)
> if (status & LinkChg)
> rtl8169_check_link_status(dev, tp, ioaddr);
>
> - /* We need to see the lastest version of tp->intr_mask to
> - * avoid ignoring an MSI interrupt and having to wait for
> - * another event which may never come.
> - */
> - smp_rmb();
> - if (status & tp->intr_mask & tp->napi_event) {
> - RTL_W16(IntrMask, tp->intr_event & ~tp->napi_event);
> - tp->intr_mask = ~tp->napi_event;
> -
> - if (likely(napi_schedule_prep(&tp->napi)))
> - __napi_schedule(&tp->napi);
> - else
> - netif_info(tp, intr, dev,
> - "interrupt %04x in poll\n", status);
> - }
> + rtl_napi_cond_schedule(tp, status);
>
> - /* We only get a new MSI interrupt when all active irq
> - * sources on the chip have been acknowledged. So, ack
> - * everything we've seen and check if new sources have become
> - * active to avoid blocking all interrupts from the chip.
> - */
> - RTL_W16(IntrStatus,
> - (status & RxFIFOOver) ? (status | RxOverflow) : status);
> - status = RTL_R16(IntrStatus);
> + break;
> }
>
> return IRQ_RETVAL(handled);
> @@ -4607,22 +4604,19 @@ static int rtl8169_poll(struct napi_struct *napi, int budget)
> void __iomem *ioaddr = tp->mmio_addr;
> int work_done;
>
> +
> + RTL_W16(IntrStatus, tp->napi_event);
> +
> work_done = rtl8169_rx_interrupt(dev, tp, ioaddr, (u32) budget);
> rtl8169_tx_interrupt(dev, tp, ioaddr);
>
> if (work_done < budget) {
> napi_complete(napi);
>
> - /* We need for force the visibility of tp->intr_mask
> - * for other CPUs, as we can loose an MSI interrupt
> - * and potentially wait for a retransmit timeout if we don't.
> - * The posted write to IntrMask is safe, as it will
> - * eventually make it to the chip and we won't loose anything
> - * until it does.
> - */
> - tp->intr_mask = 0xffff;
> - smp_wmb();
> RTL_W16(IntrMask, tp->intr_event);
> + mmiowb();
> +
> + rtl_napi_cond_schedule(tp, RTL_R16(IntrStatus));
> }
>
> return work_done;
>
Content of type "application/pgp-signature" skipped
Powered by blists - more mailing lists