Received-SPF: pass (google.com: domain of linux-kernel+bounces-86826-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:4601:e00::3 as permitted sender) client-ip=2604:1380:4601:e00::3;
Message-ID: <888d2f90-6d2f-4d4f-a9f6-fbf2f2611821@joelfernandes.org>
Date: Thu, 29 Feb 2024 09:21:48 -0500
Precedence: bulk
MIME-Version: 1.0
User-Agent: Mozilla Thunderbird
Subject: Re: [PATCH] net: raise RCU qs after each threaded NAPI poll
Content-Language: en-US
To: paulmck@kernel.org, Alexei Starovoitov <alexei.starovoitov@gmail.com>
Cc: 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>
References: <02913b40-7b74-48b3-b15d-53133afd3ba6@paulmck-laptop>
 <3D27EFEF-0452-4555-8277-9159486B41BF@joelfernandes.org>
 <ba95955d-b63d-4670-b947-e77b740b1a49@paulmck-laptop>
 <20240228173307.529d11ee@gandalf.local.home>
 <CAADnVQ+szRDGaDJPoBFR9KyeMjwpuxOCNys=yxDaCLYZkSkyYw@mail.gmail.com>
 <8ae889cb-ee1d-4c72-9414-e21258118ce3@paulmck-laptop>
From: Joel Fernandes <joel@joelfernandes.org>
In-Reply-To: <8ae889cb-ee1d-4c72-9414-e21258118ce3@paulmck-laptop>
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit


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 PM Steven Rostedt <rostedt@goodmis.org> wrote:
>>>
>>> On Wed, 28 Feb 2024 14:19:11 -0800
>>> "Paul E. McKenney" <paulmck@kernel.org> wrote:
>>>
>>>>>>
>>>>>> 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.
>>>>>
>>>>>  Thanks for confirming. :-)
>>>>
>>>> However, given that the current Tasks RCU use cases wait for trampolines
>>>> 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.
>>>>
>>>> But this seems less pressing given the chance that cond_resched() might
>>>> go away in favor of lazy preemption.
>>>
>>> Although cond_resched() is technically a "preemption point" and not truly 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.
>>>
>>> [ Added Alexei ]
>>
>> 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.
> 
> 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.
> 
> Would that cause BPF any trouble?
> 
> 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!

But it uses RCU Tasks Trace as well (for sleepable bpf programs), not just
Tasks? Looks like that's what Alexei said above as well, and I confirmed it in
bpf/trampoline.c

        /* The trampoline without fexit and fmod_ret progs doesn't call original
         * function and doesn't use percpu_ref.
         * Use call_rcu_tasks_trace() to wait for sleepable progs to finish.
         * Then use call_rcu_tasks() to wait for the rest of trampoline asm
         * and normal progs.
         */
        call_rcu_tasks_trace(&im->rcu, __bpf_tramp_image_put_rcu_tasks);

The code comment says it uses both.

Thanks,

 - Joel