Received-SPF: pass (google.com: domain of linux-kernel+bounces-87198-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.199.223 as permitted sender) client-ip=147.75.199.223;
Content-Type: text/plain; charset=utf-8
Content-Transfer-Encoding: quoted-printable
From: Joel Fernandes <joel@joelfernandes.org>
Precedence: bulk
Mime-Version: 1.0 (1.0)
Subject: Re: [PATCH] net: raise RCU qs after each threaded NAPI poll
Date: Thu, 29 Feb 2024 12:41:55 -0500
Message-Id: <10FC3F5F-AA33-4F81-9EB6-87EB2D41F3EE@joelfernandes.org>
References: <55900c6a-f181-4c5c-8de2-bca640c4af3e@paulmck-laptop>
Cc: Alexei Starovoitov <alexei.starovoitov@gmail.com>,
 Steven Rostedt <rostedt@goodmis.org>, Yan Zhai <yan@cloudflare.com>,
 Eric Dumazet <edumazet@google.com>,
 Network Development <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>,
 LKML <linux-kernel@vger.kernel.org>, rcu@vger.kernel.org,
 bpf <bpf@vger.kernel.org>, kernel-team <kernel-team@cloudflare.com>,
 Mark Rutland <mark.rutland@arm.com>
In-Reply-To: <55900c6a-f181-4c5c-8de2-bca640c4af3e@paulmck-laptop>
To: paulmck@kernel.org


> On Feb 29, 2024, at 11:57=E2=80=AFAM, Paul E. McKenney <paulmck@kernel.org=
> wrote:
>=20
> =EF=BB=BFOn Thu, Feb 29, 2024 at 09:21:48AM -0500, Joel Fernandes wrote:
>>=20
>>=20
>>> On 2/28/2024 5:58 PM, Paul E. McKenney wrote:
>>> On Wed, Feb 28, 2024 at 02:48:44PM -0800, Alexei Starovoitov wrote:
>>>> On Wed, Feb 28, 2024 at 2:31=E2=80=AFPM Steven Rostedt <rostedt@goodmis=
org> wrote:
>>>>>=20
>>>>> On Wed, 28 Feb 2024 14:19:11 -0800
>>>>> "Paul E. McKenney" <paulmck@kernel.org> wrote:
>>>>>=20
>>>>>>>>=20
>>>>>>>> Well, to your initial point, cond_resched() does eventually invoke
>>>>>>>> preempt_schedule_common(), so you are quite correct that as far as
>>>>>>>> Tasks RCU is concerned, cond_resched() is not a quiescent state.
>>>>>>>=20
>>>>>>> Thanks for confirming. :-)
>>>>>>=20
>>>>>> However, given that the current Tasks RCU use cases wait for trampoli=
nes
>>>>>> to be evacuated, Tasks RCU could make the choice that cond_resched()
>>>>>> be a quiescent state, for example, by adjusting rcu_all_qs() and
>>>>>> .rcu_urgent_qs accordingly.
>>>>>>=20
>>>>>> But this seems less pressing given the chance that cond_resched() mig=
ht
>>>>>> go away in favor of lazy preemption.
>>>>>=20
>>>>> Although cond_resched() is technically a "preemption point" and not tr=
uly a
>>>>> voluntary schedule, I would be happy to state that it's not allowed to=
 be
>>>>> called from trampolines, or their callbacks. Now the question is, does=
 BPF
>>>>> programs ever call cond_resched()? I don't think they do.
>>>>>=20
>>>>> [ Added Alexei ]
>>>>=20
>>>> I'm a bit lost in this thread :)
>>>> Just answering the above question.
>>>> bpf progs never call cond_resched() directly.
>>>> But there are sleepable (aka faultable) bpf progs that
>>>> can call some helper or kfunc that may call cond_resched()
>>>> in some path.
>>>> sleepable bpf progs are protected by rcu_tasks_trace.
>>>> That's a very different one vs rcu_tasks.
>>>=20
>>> Suppose that the various cond_resched() invocations scattered throughout=

>>> the kernel acted as RCU Tasks quiescent states, so that as soon as a
>>> given task executed a cond_resched(), synchronize_rcu_tasks() might
>>> return or call_rcu_tasks() might invoke its callback.
>>>=20
>>> Would that cause BPF any trouble?
>>>=20
>>> My guess is "no", because it looks like BPF is using RCU Tasks (as you
>>> say, as opposed to RCU Tasks Trace) only to wait for execution to leave a=

>>> trampoline.  But I trust you much more than I trust myself on this topic=
!
>>=20
>> But it uses RCU Tasks Trace as well (for sleepable bpf programs), not jus=
t
>> Tasks? Looks like that's what Alexei said above as well, and I confirmed i=
t in
>> bpf/trampoline.c
>>=20
>>        /* The trampoline without fexit and fmod_ret progs doesn't call or=
iginal
>>         * function and doesn't use percpu_ref.
>>         * Use call_rcu_tasks_trace() to wait for sleepable progs to finis=
h.
>>         * Then use call_rcu_tasks() to wait for the rest of trampoline as=
m
>>         * and normal progs.
>>         */
>>        call_rcu_tasks_trace(&im->rcu, __bpf_tramp_image_put_rcu_tasks);
>>=20
>> The code comment says it uses both.
>=20
> BPF does quite a few interesting things with these.
>=20
> But would you like to look at the update-side uses of RCU Tasks Rude
> to see if lazy preemption affects them?  I don't believe that there
> are any problems here, but we do need to check.

Sure I will be happy to. I am planning look at it in detail over the 3 day w=
eekend. Too much fun! ;-)

thanks,

- Joel


>=20
>                            Thanx, Paul