All the mail mirrored from lore.kernel.org
 help / color / mirror / Atom feed
From: Arnd Bergmann <arnd@kernel.org>
To: "Maciej W. Rozycki" <macro@orcam.me.uk>
Cc: Nikolai Zhubr <zhubr.2@gmail.com>,
	Thomas Gleixner <tglx@linutronix.de>,
	Heiner Kallweit <hkallweit1@gmail.com>,
	netdev <netdev@vger.kernel.org>,
	"the arch/x86 maintainers" <x86@kernel.org>,
	Ingo Molnar <mingo@redhat.com>, Borislav Petkov <bp@alien8.de>,
	"H. Peter Anvin" <hpa@zytor.com>
Subject: Re: Realtek 8139 problem on 486.
Date: Thu, 24 Jun 2021 10:28:21 +0200	[thread overview]
Message-ID: <CAK8P3a0u+usoPon7aNOAB_g+Jzkhbz9Q7-vyYci1ReHB6c-JMQ@mail.gmail.com> (raw)
In-Reply-To: <alpine.DEB.2.21.2106240044080.37803@angie.orcam.me.uk>

On Thu, Jun 24, 2021 at 1:39 AM Maciej W. Rozycki <macro@orcam.me.uk> wrote:
>
> On Wed, 23 Jun 2021, Nikolai Zhubr wrote:
>
> > > As I said before, I still think we should also merge the 8139 driver patch,
> > > probably without that loop. It's not great, but I'm much more confident
> > > I understand what that does and that the patched version is better than
> > > the current code.
> >
> > Yes, the 'poll' approach apparently works stable and does not cause any
> > measurable performance decrease. But it would need some carefull
> > cleanup/review, especially WRT locking.

Right, I forgot you saw that one WARN_ON trigger. If you enable KALLSYMS
and BUGVERBOSE, it should become obvious what that one is about.

> >  Now that all real event handling work
> > is happening in the poll function, it still has to be protected against the
> > (potentially also long-running) reset function which in current design can be
> > called e.g. from a different thread due to tx timeout, and this does not look
> > good, but it is a bit beyond my capability to arrange it better. Besides, the
> > idea was to keep the fix simple and avoid a massive rework...

Since the calls are just moved from hardirq to softirq context, it should
be possible to do this without changing the locking state of any bit, by
just making sure that irqs are disabled during the code that was part of
the hardirq handler.

If you remove the loop, you can move the spin_lock_irqsave(&tp->lock)
back down  after the rx handler.

Anything on top of that can be done as a cleanup or simplification.

>  The simplest fix is not always the right one.

Sure, but in this case, a fairly simple change can help make this driver
work on any machine that for whatever reason treats the IRQ as
edge triggered.

>  I've now posted a small patch series that adds a PCI IRQ router for the
> SiS85C497 device.

Thanks a lot, that should help avoid the same problem on
this chipset with other drivers.

        Arnd

  reply	other threads:[~2021-06-24  8:30 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 [this message]
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
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=CAK8P3a0u+usoPon7aNOAB_g+Jzkhbz9Q7-vyYci1ReHB6c-JMQ@mail.gmail.com \
    --to=arnd@kernel.org \
    --cc=bp@alien8.de \
    --cc=hkallweit1@gmail.com \
    --cc=hpa@zytor.com \
    --cc=macro@orcam.me.uk \
    --cc=mingo@redhat.com \
    --cc=netdev@vger.kernel.org \
    --cc=tglx@linutronix.de \
    --cc=x86@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.