All the mail mirrored from lore.kernel.org
 help / color / mirror / Atom feed
From: Heiner Kallweit <hkallweit1@gmail.com>
To: Nikolai Zhubr <zhubr.2@gmail.com>, netdev <netdev@vger.kernel.org>
Cc: Jeff Garzik <jgarzik@pobox.com>
Subject: Re: Realtek 8139 problem on 486.
Date: Mon, 31 May 2021 21:05:14 +0200	[thread overview]
Message-ID: <bf4e6e02-06ce-3001-29c8-9f0bd3f58cd8@gmail.com> (raw)
In-Reply-To: <60B514A0.1020701@gmail.com>

On 31.05.2021 18:53, Nikolai Zhubr wrote:
> Hi all,
> 
> 31.05.2021 2:17, Nikolai Zhubr:
> [...]
>> However indeed, it seems a problem was introduced with a rework of
>> interrupt handling (rtl8139_interrupt) in 2.6.3, because I have already
>> pushed all other differences from 2.6.3 to 2.6.2 and it still keeps
>> working fine.
>> My resulting minimized diff is still ~300 lines, it is too big and
>> complicated to be usefull to post here as is.
> 
> Some more input.
> 
> I was able to minimize the problematic diff to basically one screenfull, it is quite comprehencable now, and I'm including it below. It is the change in status/event handling due to a switch to NAPI that intruduced the problem.
> Now, in some more detailed tests, I observe that _receiving_ still works fine. It is _sending_ that suffers, and apparently, only when trying to send a lot at a time. In such case I see these warnings:
> 
> NETDEV WATCHDOG: eth0: transmit timed out
> eth0: link up, 100Mbps, full-duplex, lpa 0xC5E1
> 
> It looks like the queue of tx frames somehow gets messed up.
> 
> The essential diff fragment:
> ================================================
>      dev->open = rtl8139_open;
>      dev->hard_start_xmit = rtl8139_start_xmit;
> +    dev->poll = rtl8139_poll;
>      dev->weight = 64;
>      dev->stop = rtl8139_close;
>      dev->get_stats = rtl8139_get_stats;
> @@ -2015,7 +2010,7 @@
>              tp->stats.rx_bytes += pkt_size;
>              tp->stats.rx_packets++;
> 
> -            netif_rx (skb);
> +            netif_receive_skb (skb);
>          } else {
>              if (net_ratelimit())
>                  printk (KERN_WARNING
> @@ -2138,10 +2133,8 @@
>      u16 status, ackstat;
>      int link_changed = 0; /* avoid bogus "uninit" warning */
>      int handled = 0;
> -    int boguscnt = max_interrupt_work;
> 
>      spin_lock (&tp->lock);
> -    do {
>      status = RTL_R16 (IntrStatus);
> 
>      /* shared irq? */
> @@ -2169,8 +2162,14 @@
>      if (ackstat)
>          RTL_W16 (IntrStatus, ackstat);
> 
> -    if (netif_running (dev) && (status & RxAckBits))
> -        rtl8139_rx (dev, tp, 1000000000);
> +    /* Receive packets are processed by poll routine.
> +       If not running start it now. */
> +    if (status & RxAckBits){
> +        if (netif_rx_schedule_prep(dev)) {
> +            RTL_W16_F (IntrMask, rtl8139_norx_intr_mask);
> +            __netif_rx_schedule (dev);
> +        }
> +    }
> 
>      /* Check uncommon events with one test. */
>      if (unlikely(status & (PCIErr | PCSTimeout | RxUnderrun | RxErr)))
> @@ -2182,16 +2181,6 @@
>          if (status & TxErr)
>              RTL_W16 (IntrStatus, TxErr);
>      }
> -    boguscnt--;
> -    } while (boguscnt > 0);
> -
> ================================================
> 
> 
> Thank you,
> 
> Regards,
> Nikolai
> 
> 
> 
>>
>>
Issue could be related to rx and tx processing now potentially running in parallel.
I only have access to the current 8139too source code, hopefully the following
works on the old version:

In the end of rtl8139_start_xmit() there's
if ((tp->cur_tx - NUM_TX_DESC) == tp->dirty_tx)
		netif_stop_queue (dev);

Try changing this to

if (tp->cur_tx - NUM_TX_DESC == tp->dirty_tx) {
	smp_wmb();
	netif_stop_queue (dev);
	smp_mb__after_atomic();       /* if this doesn't exist yet, use mb() */
	if (tp->cur_tx - NUM_TX_DESC != tp->dirty_tx)
		netif_start_queue(dev);
}


And at the end of rtl8139_tx_interrupt() change

	if (tp->dirty_tx != dirty_tx) {
		tp->dirty_tx = dirty_tx;
		mb();
		netif_wake_queue (dev);
	}

to

	if (tp->dirty_tx != dirty_tx) {
		tp->dirty_tx = dirty_tx;
		mb();
		if (netif_queue_stopped(dev) && tp->cur_tx - NUM_TX_DESC != tp->dirty_tx)
			netif_wake_queue (dev);
	}

  parent reply	other threads:[~2021-05-31 19:05 UTC|newest]

Thread overview: 61+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-05-29 14:08 Realtek 8139 problem on 486 Nikolai Zhubr
2021-05-29 18:42 ` Heiner Kallweit
2021-05-29 21:44   ` tedheadster
2021-05-30  0:49     ` Nikolai Zhubr
2021-05-30 10:36       ` Nikolai Zhubr
2021-05-30 17:27         ` Nikolai Zhubr
2021-05-30 20:54           ` Arnd Bergmann
2021-05-30 23:17             ` Nikolai Zhubr
2021-05-31 16:53               ` Nikolai Zhubr
2021-05-31 18:39                 ` Arnd Bergmann
2021-05-31 22:18                   ` Nikolai Zhubr
2021-05-31 22:30                     ` Heiner Kallweit
2021-06-01  7:20                       ` Arnd Bergmann
2021-06-01 10:53                         ` Nikolai Zhubr
2021-06-01 11:42                           ` Heiner Kallweit
2021-06-01 16:09                             ` Nikolai Zhubr
2021-06-01 21:48                               ` Heiner Kallweit
2021-06-01 23:37                                 ` Nikolai Zhubr
2021-06-02  9:12                                   ` Arnd Bergmann
2021-06-07 23:07                                     ` Nikolai Zhubr
2021-06-08  7:44                                       ` Arnd Bergmann
2021-06-08 20:32                                         ` Nikolai Zhubr
2021-06-08 20:45                                           ` Arnd Bergmann
2021-06-08 22:07                                             ` Nikolai Zhubr
2021-06-09  7:09                                               ` Arnd Bergmann
2021-06-12 17:40                                                 ` Nikolai Zhubr
2021-06-12 22:41                                                   ` Arnd Bergmann
2021-06-13 14:10                                                     ` Nikolai Zhubr
2021-06-13 21:52                                                       ` Arnd Bergmann
2021-06-03 18:32                                 ` Maciej W. Rozycki
2021-06-04  7:36                                   ` Arnd Bergmann
2021-06-20  0:34                                     ` Thomas Gleixner
2021-06-20 10:19                                       ` Arnd Bergmann
2021-06-21  4:10                                       ` Maciej W. Rozycki
2021-06-21 11:22                                         ` Arnd Bergmann
2021-06-21 14:42                                           ` Maciej W. Rozycki
2021-06-21 15:20                                             ` Arnd Bergmann
2021-06-22 11:12                                             ` David Laight
2021-06-22 12:42                                           ` Nikolai Zhubr
2021-06-22 13:22                                             ` Arnd Bergmann
2021-06-22 18:42                                               ` Nikolai Zhubr
2021-06-22 19:26                                                 ` Arnd Bergmann
2021-06-23  1:04                                                   ` Maciej W. Rozycki
2021-06-24 17:56                                                     ` Nikolai Zhubr
2021-06-24 18:25                                                       ` Maciej W. Rozycki
2021-07-14 23:32                                                         ` Maciej W. Rozycki
2021-07-15  7:32                                                           ` Nikolai Zhubr
2021-07-16 23:48                                                             ` Maciej W. Rozycki
2021-06-23 16:31                                                   ` Nikolai Zhubr
2021-06-23 23:39                                                     ` Maciej W. Rozycki
2021-06-24  8:28                                                       ` Arnd Bergmann
2021-07-02 19:02                                                         ` Nikolai Zhubr
2021-07-03  9:10                                                           ` Arnd Bergmann
2021-07-08 19:21                                                             ` Nikolai Zhubr
2021-07-09  7:31                                                               ` Arnd Bergmann
2021-07-09 12:43                                                               ` David Laight
2021-06-01 17:44                             ` Maciej W. Rozycki
2021-06-02 15:14                               ` Nikolai Zhubr
2021-06-02 15:28                                 ` Arnd Bergmann
2021-05-31 19:05                 ` Heiner Kallweit [this message]
2021-05-31 18:29 ` Denis Kirjanov

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=bf4e6e02-06ce-3001-29c8-9f0bd3f58cd8@gmail.com \
    --to=hkallweit1@gmail.com \
    --cc=jgarzik@pobox.com \
    --cc=netdev@vger.kernel.org \
    --cc=zhubr.2@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.