archive mirror
 help / color / mirror / Atom feed
From: Thomas Gleixner <>
To: Jakub Kicinski <>,
	Sebastian Andrzej Siewior <>
Cc:,,,,, Juri Lelli <>,
	Clark Williams <>
Subject: Re: [PATCH net-next 1/2] net: add a napi variant for RT-well-behaved drivers
Date: Sat, 15 May 2021 22:53:28 +0200	[thread overview]
Message-ID: <> (raw)
In-Reply-To: <20210515133104.491fc691@kicinski-fedora-PC1C0HJN>

On Sat, May 15 2021 at 13:31, Jakub Kicinski wrote:
> On Sat, 15 May 2021 13:07:40 +0200 Sebastian Andrzej Siewior wrote:
>> Now assume another interrupt comes in which wakes a force-threaded
>> handler (while ksoftirqd is preempted). Before the forced-threaded
>> handler is invoked, BH is disabled via local_bh_disable(). Since
>> ksoftirqd is preempted with BH disabled, disabling BH forces the
>> ksoftirqd thread to the priority of the interrupt thread (SCHED_FIFO,
>> prio 50 by default) due to the priority inheritance protocol. The
>> threaded handler will run once ksoftirqd is done which has now been
>> accelerated.
> Thanks for the explanation. I'm not married to the patch, if you prefer
> we can keep the status quo.
> I'd think, however, that always deferring to ksoftirqd is conceptually
> easier to comprehend. For power users who need networking there is
> prefer-busy-poll (which allows application to ask the kernel to service
> queues when it wants to, with some minimal poll frequency guarantees)
> and threaded NAPI - which some RT users already started to adapt.
> Your call.
>> Part of the problem from RT perspective is the heavy use of softirq and
>> the BH disabled regions which act as a BKL. I *think* having the network
>> driver running in a thread would be better (in terms of prioritisation).
>> I know, davem explained the benefits of NAPI/softirq when it comes to
>> routing/forwarding (incl. NET_RX/TX priority) and part where NAPI kicks
>> in during a heavy load (say a packet flood) and system is still
>> responsible.
> Right, although with modern multi-core systems where only a subset 
> of cores process network Rx things look different.

Bah, I completely forgot about that aspect. Thanks Sebastian for
bringing it up. I was too focussed on the other questions and there is
obviously the onset of alzheimer.

Anyway it's a touch choice to make. There are too many options to chose
from nowadays. 10 years ago running the softirq at the back of the
threaded irq handler which just scheduled NAPI was definitely a win, but
with threaded NAPI, zero copy and other things it's not that important
anymore IMO. But I might be missing something obviously.

I've cc'ed a few RT folks @RHT who might give some insight.



  reply	other threads:[~2021-05-15 20:53 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-05-14 22:24 Jakub Kicinski
2021-05-14 22:24 ` [PATCH net-next 2/2] nfp: use napi_schedule_irq() Jakub Kicinski
2021-05-17  9:48   ` Simon Horman
2021-05-15  0:17 ` [PATCH net-next 1/2] net: add a napi variant for RT-well-behaved drivers Thomas Gleixner
2021-05-15  0:21   ` Jakub Kicinski
2021-05-15  0:29     ` Thomas Gleixner
2021-05-15  9:49 ` Thomas Gleixner
2021-05-15 20:38   ` Jakub Kicinski
2021-05-15 11:07 ` Sebastian Andrzej Siewior
2021-05-15 20:31   ` Jakub Kicinski
2021-05-15 20:53     ` Thomas Gleixner [this message]
2021-05-21 14:11       ` Juri Lelli

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:

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

  git send-email \ \ \ \ \ \ \ \ \ \ \ \
    --subject='Re: [PATCH net-next 1/2] net: add a napi variant for RT-well-behaved drivers' \

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).