netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Heiner Kallweit <hkallweit1@gmail.com>
To: Mike Galbraith <efault@gmx.de>, Vladimir Oltean <olteanv@gmail.com>
Cc: netdev <netdev@vger.kernel.org>,
	Realtek linux nic maintainers <nic_swsd@realtek.com>
Subject: Re: [patchlet] r8169: fix napi_schedule_irqoff() called with irqs enabled warning
Date: Fri, 16 Oct 2020 21:15:32 +0200	[thread overview]
Message-ID: <e65ea1a0-fa97-5d07-fbbf-4071f91e2429@gmail.com> (raw)
In-Reply-To: <42ff951039f3c92def8490d3842e3f7eaf6900ff.camel@gmx.de>

On 16.10.2020 19:11, Mike Galbraith wrote:
> On Fri, 2020-10-16 at 17:26 +0300, Vladimir Oltean wrote:
>> On Fri, Oct 16, 2020 at 01:34:55PM +0200, Heiner Kallweit wrote:
>>> I'm aware of the topic, but missing the benefits of the irqoff version
>>> unconditionally doesn't seem to be the best option.
>>
>> What are the benefits of the irqoff version? As far as I see it, the
>> only use case for that function is when the caller has _explicitly_
>> disabled interrupts.
> 
> Yeah, it's a straight up correctness issue as it sits.  There is a
> dinky bit of overhead added to the general case when using the correct
> function though, at least on x86.  I personally don't see why we should
> care deeply enough to want to add more code to avoid it given there are
> about a zillions places where we do the same for the same reason, but
> that's a maintainer call.
> 
Of course switching back to napi_schedule() is the easiest solution,
and also for r8169 we may come to the conclusion that it's the best one.
(or, considering full RT, we may even remove the irqoff version completely)
But we should spend at least a few thoughts on whether and how the
irqoff version could be improved. This would have two benefits:
- avoid the local_irq_save/local_irq_restore overhead (architecture-dependent)
- automatically fix all drivers using the irqoff version
If others go the easy way, then this doesn't mean that we must not think
about a better way.

  parent reply	other threads:[~2020-10-16 19:15 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-10-16 11:26 [patchlet] r8169: fix napi_schedule_irqoff() called with irqs enabled warning Mike Galbraith
2020-10-16 11:34 ` Heiner Kallweit
2020-10-16 11:57   ` Mike Galbraith
2020-10-16 14:26   ` Vladimir Oltean
2020-10-16 14:41     ` Heiner Kallweit
2020-10-16 15:40       ` Vladimir Oltean
2020-10-16 17:11     ` Mike Galbraith
2020-10-16 17:19       ` Vladimir Oltean
2020-10-16 19:15       ` Heiner Kallweit [this message]
2020-10-17  2:26         ` Mike Galbraith

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=e65ea1a0-fa97-5d07-fbbf-4071f91e2429@gmail.com \
    --to=hkallweit1@gmail.com \
    --cc=efault@gmx.de \
    --cc=netdev@vger.kernel.org \
    --cc=nic_swsd@realtek.com \
    --cc=olteanv@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 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).