From: "Paul E. McKenney" <paulmck@kernel.org>
To: Yan Zhai <yan@cloudflare.com>
Cc: "Eric Dumazet" <edumazet@google.com>,
netdev@vger.kernel.org, "David S. Miller" <davem@davemloft.net>,
"Jakub Kicinski" <kuba@kernel.org>,
"Paolo Abeni" <pabeni@redhat.com>,
"Jiri Pirko" <jiri@resnulli.us>,
"Simon Horman" <horms@kernel.org>,
"Daniel Borkmann" <daniel@iogearbox.net>,
"Lorenzo Bianconi" <lorenzo@kernel.org>,
"Coco Li" <lixiaoyan@google.com>, "Wei Wang" <weiwan@google.com>,
"Alexander Duyck" <alexanderduyck@fb.com>,
"Hannes Frederic Sowa" <hannes@stressinduktion.org>,
linux-kernel@vger.kernel.org, rcu@vger.kernel.org,
bpf@vger.kernel.org, kernel-team@cloudflare.com,
"Joel Fernandes" <joel@joelfernandes.org>,
"Toke Høiland-Jørgensen" <toke@redhat.com>,
"Alexei Starovoitov" <alexei.starovoitov@gmail.com>,
"Steven Rostedt" <rostedt@goodmis.org>,
mark.rutland@arm.com, "Jesper Brouer" <jesper@cloudflare.com>
Subject: Re: [PATCH v2] net: raise RCU qs after each threaded NAPI poll
Date: Mon, 11 Mar 2024 16:55:37 -0700 [thread overview]
Message-ID: <aa4909dd-98f2-4185-833d-666c640786c6@paulmck-laptop> (raw)
In-Reply-To: <CAO3-Pbp0Pxbbgmjf03wKo6MDrQYE7uiL+mUnheT9UA9Pjj5bUQ@mail.gmail.com>
On Mon, Mar 11, 2024 at 05:58:16PM -0500, Yan Zhai wrote:
> On Fri, Mar 1, 2024 at 4:29 PM Paul E. McKenney <paulmck@kernel.org> wrote:
> >
> > On Fri, Mar 01, 2024 at 11:30:29AM -0600, Yan Zhai wrote:
> > > Hi Eric,
> > >
> > > On Fri, Mar 1, 2024 at 2:30 AM Eric Dumazet <edumazet@google.com> wrote:
> > > >
> > > > I could not see the reason for 1sec (HZ) delays.
> > > >
> > > > Would calling rcu_softirq_qs() every ~10ms instead be a serious issue ?
> > > >
> > > The trouble scenarios are often when we need to detach an ad-hoc BPF
> > > tracing program, or restart a monitoring service. It is fine as long
> > > as they do not block for 10+ seconds or even completely stall under
> > > heavy traffic. Raising a QS every few ms or HZ both work in such
> > > cases.
> > >
> > > > In anycase, if this all about rcu_tasks, I would prefer using a macro
> > > > defined in kernel/rcu/tasks.h
> > > > instead of having a hidden constant in a networking core function.
> > >
> > > Paul E. McKenney was suggesting either current form or
> > >
> > > local_bh_enable();
> > > if (!IS_ENABLED(CONFIG_PREEMPT_RT))
> > > rcu_softirq_qs_enable(local_bh_enable());
> > > else
> > > local_bh_enable();
> > >
> > > With an interval it might have to be
> > > "rcu_softirq_qs_enable(local_bh_enable(), &next_qs);" to avoid an
> > > unnecessary extern/static var. Will it make more sense to you?
> >
> > I was thinking in terms of something like this (untested):
> >
> > #define rcu_softirq_qs_enable(enable_stmt, oldj) \
> > do { \
> > if (!IS_ENABLED(CONFIG_PREEMPT_RT) && \
> > time_after(oldj + HZ / 10, jiffies) { \
> > rcu_softirq_qs(); \
> > (oldj) = jiffies; \
> > } \
> > do { enable_stmt; } while (0) \
> > } while (0)
> >
> > Then the call could be "rcu_softirq_qs_enable(local_bh_enable(), last_qs)",
> > where last_qs is initialized by the caller to jiffies.
> >
> > The reason for putting "enable_stmt;" into anothor do-while loop is
> > in case someone typos an "else" as the first part of the "enable_stmt"
> > argument.
> >
> > Would that work?
> >
> Thanks Paul, just got time to continue this thread as I was
> travelling. I think it is probably better to move
> preempt_disable/enable into the macro to avoid the friction. And also
> since this can affect NAPI thread, NAPI busy loop and XDP cpu map
> thread (+Jesper who reminded me about this), let me send a v3 later to
> cover all of those places.
OK, looking forward to seeing what you come up with.
Thanx, Paul
prev parent reply other threads:[~2024-03-11 23:55 UTC|newest]
Thread overview: 7+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-03-01 3:47 [PATCH v2] net: raise RCU qs after each threaded NAPI poll Yan Zhai
2024-03-01 3:49 ` Yan Zhai
2024-03-01 8:30 ` Eric Dumazet
2024-03-01 17:30 ` Yan Zhai
2024-03-01 22:29 ` Paul E. McKenney
2024-03-11 22:58 ` Yan Zhai
2024-03-11 23:55 ` Paul E. McKenney [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=aa4909dd-98f2-4185-833d-666c640786c6@paulmck-laptop \
--to=paulmck@kernel.org \
--cc=alexanderduyck@fb.com \
--cc=alexei.starovoitov@gmail.com \
--cc=bpf@vger.kernel.org \
--cc=daniel@iogearbox.net \
--cc=davem@davemloft.net \
--cc=edumazet@google.com \
--cc=hannes@stressinduktion.org \
--cc=horms@kernel.org \
--cc=jesper@cloudflare.com \
--cc=jiri@resnulli.us \
--cc=joel@joelfernandes.org \
--cc=kernel-team@cloudflare.com \
--cc=kuba@kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=lixiaoyan@google.com \
--cc=lorenzo@kernel.org \
--cc=mark.rutland@arm.com \
--cc=netdev@vger.kernel.org \
--cc=pabeni@redhat.com \
--cc=rcu@vger.kernel.org \
--cc=rostedt@goodmis.org \
--cc=toke@redhat.com \
--cc=weiwan@google.com \
--cc=yan@cloudflare.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 read-only IMAP folder(s) and NNTP newsgroup(s).