Development discussion of WireGuard
 help / color / mirror / Atom feed
From: "Jason A. Donenfeld" <Jason@zx2c4.com>
To: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
Cc: Netdev <netdev@vger.kernel.org>,
	 WireGuard mailing list <wireguard@lists.zx2c4.com>,
	Jakub Kicinski <kuba@kernel.org>,
	 Saeed Mahameed <saeed@kernel.org>,
	Eric Dumazet <edumazet@google.com>,
	 Thomas Gleixner <tglx@linutronix.de>,
	Peter Zijlstra <peterz@infradead.org>
Subject: Re: [PATCH] net: remove lockdep asserts from ____napi_schedule()
Date: Mon, 21 Mar 2022 01:24:35 -0600	[thread overview]
Message-ID: <CAHmME9rjNCM_2xOEUea1_T4aYJY+xqFL=hrEVBO_FK9eVT4cew@mail.gmail.com> (raw)
In-Reply-To: <YjXGALddYuJeRlDk@linutronix.de>

Hi Sebastian,

On Sat, Mar 19, 2022 at 6:01 AM Sebastian Andrzej Siewior
<bigeasy@linutronix.de> wrote:
>
> On 2022-03-18 18:47:38 [-0600], Jason A. Donenfeld wrote:
> > This reverts commit fbd9a2ceba5c ("net: Add lockdep asserts to
> > ____napi_schedule()."). While good in theory, in practice it causes
> > issues with various drivers, and so it can be revisited earlier in the
> > cycle where those drivers can be adjusted if needed.
>
> Do you plan to address to address the wireguard warning?

It seemed to me like you had a lot of interesting ideas regarding
packet batching and performance and whatnot around when bh is enabled
or not. I'm waiting for a patch from you on this, as I mentioned in my
previous email. There is definitely a lot of interesting potential
performance work here. I am curious to play around with it too, of
course, but it sounded to me like you had very specific ideas. I'm not
really sure how to determine how many packets to batch, except for
through empirical observation or some kind of crazy dql thing. Or
maybe there's some optimal quantity due to the way napi works in the
first place. Anyway, there's some research to do here.

>
> > --- a/net/core/dev.c
> > +++ b/net/core/dev.c
> > @@ -4277,9 +4277,6 @@ static inline void ____napi_schedule(struct softnet_data *sd,
> >  {
> >       struct task_struct *thread;
> >
> > -     lockdep_assert_softirq_will_run();
> > -     lockdep_assert_irqs_disabled();
>
> Could you please keep that lockdep_assert_irqs_disabled()? That is
> needed regardless of the upper one.

Feel free to send in a more specific revert if you think it's
justifiable. I just sent in the thing that reverted the patch that
caused the regression - the dumb brute approach.

Jason

      reply	other threads:[~2022-03-21  7:24 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-03-19  0:47 Jason A. Donenfeld
2022-03-19  0:50 ` Jason A. Donenfeld
2022-03-19  4:31   ` Jakub Kicinski
2022-03-19 12:01 ` Sebastian Andrzej Siewior
2022-03-21  7:24   ` Jason A. Donenfeld [this message]

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='CAHmME9rjNCM_2xOEUea1_T4aYJY+xqFL=hrEVBO_FK9eVT4cew@mail.gmail.com' \
    --to=jason@zx2c4.com \
    --cc=bigeasy@linutronix.de \
    --cc=edumazet@google.com \
    --cc=kuba@kernel.org \
    --cc=netdev@vger.kernel.org \
    --cc=peterz@infradead.org \
    --cc=saeed@kernel.org \
    --cc=tglx@linutronix.de \
    --cc=wireguard@lists.zx2c4.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).