linux-kernel - Re: [PATCH] net: raise RCU qs after each threaded NAPI poll

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <55900c6a-f181-4c5c-8de2-bca640c4af3e@paulmck-laptop>
Date: Thu, 29 Feb 2024 08:57:07 -0800
From: "Paul E. McKenney" <paulmck@...nel.org>
To: Joel Fernandes <joel@...lfernandes.org>
Cc: Alexei Starovoitov <alexei.starovoitov@...il.com>,
	Steven Rostedt <rostedt@...dmis.org>, Yan Zhai <yan@...udflare.com>,
	Eric Dumazet <edumazet@...gle.com>,
	Network Development <netdev@...r.kernel.org>,
	"David S. Miller" <davem@...emloft.net>,
	Jakub Kicinski <kuba@...nel.org>, Paolo Abeni <pabeni@...hat.com>,
	Jiri Pirko <jiri@...nulli.us>, Simon Horman <horms@...nel.org>,
	Daniel Borkmann <daniel@...earbox.net>,
	Lorenzo Bianconi <lorenzo@...nel.org>,
	Coco Li <lixiaoyan@...gle.com>, Wei Wang <weiwan@...gle.com>,
	Alexander Duyck <alexanderduyck@...com>,
	Hannes Frederic Sowa <hannes@...essinduktion.org>,
	LKML <linux-kernel@...r.kernel.org>, rcu@...r.kernel.org,
	bpf <bpf@...r.kernel.org>, kernel-team <kernel-team@...udflare.com>,
	Mark Rutland <mark.rutland@....com>
Subject: Re: [PATCH] net: raise RCU qs after each threaded NAPI poll

On Thu, Feb 29, 2024 at 09:21:48AM -0500, Joel Fernandes wrote:
> 
> 
> 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@...dmis.org> wrote:
> >>>
> >>> On Wed, 28 Feb 2024 14:19:11 -0800
> >>> "Paul E. McKenney" <paulmck@...nel.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.

BPF does quite a few interesting things with these.

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.

							Thanx, Paul