Re: [PATCH RFC v3 13/13] x86, arm64: Build Tasks RCU on Tasks Trace readers in trampolines

[bot+bpf-ci]

> diff --git a/Documentation/RCU/Design/Requirements/Requirements.rst b/Documentation/RCU/Design/Requirements/Requirements.rst
> index 8101fe6229d57..34b81512cc5f3 100644
> --- a/Documentation/RCU/Design/Requirements/Requirements.rst
> +++ b/Documentation/RCU/Design/Requirements/Requirements.rst
> @@ -2756,6 +2756,26 @@ synchronize_rcu(), and rcu_barrier(), respectively. In
>  three APIs are therefore implemented by separate functions that check
>  for voluntary context switches.
>
> +Architectures that select ``CONFIG_HAVE_RCU_TRAMPOLINE_READERS`` keep the
> +same three APIs but implement the grace period differently
> +(``CONFIG_TASKS_RCU_TRAMPOLINE_READERS``).  There, every trampoline whose
> +lifetime Tasks RCU guards enters a Tasks Trace RCU read-side critical
> +section (rcu_read_lock_trace() or its assembly equivalent) before calling
> +out and leaves it before returning, so a task anywhere inside such a
> +call-out is an ordinary Tasks Trace reader whether or not it is
> +preempted.  The few trampoline instructions outside that reader can only
> +be occupied by a task that was interrupted there, so the grace period
> +additionally waits for each CPU to pass through a context switch, and the
> +irq-exit preemption path, the only switch that can catch a task inside
> +such text (rcu_tasks_trampoline_text()), briefly makes such a task a
> +holdout until it is next seen elsewhere.  On such kernels an involuntary
> +context switch outside trampoline text *is* a Tasks-RCU quiescent state,
> +a Tasks RCU grace period no longer depends on how long any task runs
> +without sleeping, cond_resched_tasks_rcu_qs() is unnecessary, and the
> +obligation moves to the trampolines: anything that relies on
> +synchronize_rcu_tasks() to protect code a task may be preempted in must
> +take the Tasks Trace reader (see register_ftrace_direct()).
                                                               ^^^^^

Does this paragraph describe the complete set of options? The series
itself includes kernel/kprobes.c:kprobe_optimizer(), which relies on
synchronize_rcu_tasks() to protect instructions a task may be preempted
in, but instead of taking a Tasks Trace reader it registers an address
predicate and calls rcu_tasks_wait_irq_preempted(kprobe_in_optimized_region)
before synchronize_rcu_tasks().

kernel/kprobes.c:526-539 documents that alternative: "kprobe_optimizer()
relies on synchronize_rcu_tasks() to wait for tasks that ... the
quiescent-state check asks this instead."

Since this paragraph is the single place in Documentation/RCU that
describes the new contract for HAVE_RCU_TRAMPOLINE_READERS architectures,
a subsystem author reading it would conclude there is one way to comply
and would get no pointer to rcu_tasks_wait_irq_preempted() or
rcu_tasks_trampoline_text() for code that cannot take a reader.

Could the paragraph mention the rcu_tasks_wait_irq_preempted() escape
hatch, or soften to "must take the Tasks Trace reader or otherwise make
its text known to the quiescent-state check"?

> diff --git a/Documentation/RCU/checklist.rst b/Documentation/RCU/checklist.rst
> index 4b30f701225fd..7082686cbd66d 100644
> --- a/Documentation/RCU/checklist.rst
> +++ b/Documentation/RCU/checklist.rst
> @@ -252,7 +252,12 @@ over a rather long period of time, but improvements are always welcome!
>  	a.	If the updater uses synchronize_rcu_tasks() or
>  		call_rcu_tasks(), then the readers must refrain from
>  		executing voluntary context switches, that is, from
> -		blocking.
> +		blocking.  On architectures that select
> +		CONFIG_HAVE_RCU_TRAMPOLINE_READERS a reader must in
> +		addition be a Tasks Trace RCU reader (that is what the
> +		trampolines there do around their call-outs); an
> +		arbitrary stretch of preemptible kernel code is not
> +		protected.
                        ^^^^^^^^^

Same question here. This text also states the obligation in absolute
terms ("a reader must in addition be a Tasks Trace RCU reader"), with
no mention of the rcu_tasks_wait_irq_preempted() alternative that
kprobe_optimizer() uses.

[ ... ]


---
AI reviewed your patch. Please fix the bug or email reply why it's not a bug.
See: https://github.com/kernel-patches/vmtest/blob/master/ci/claude/README.md

CI run summary: https://github.com/kernel-patches/bpf/actions/runs/34975311706