On 17 October 2026 13:14:29 BST, Zqiang <[email protected]> wrote: >> >> On 17 October 2026 00:41:00 BST, Zqiang <[email protected]> wrote: >> >> > >> > > >> > > On 15 October 2026 10:46:19 BST, Zqiang <[email protected]> >wrote: >> > > >> > > > >> > > > The RCU Tasks Rude grace period is implemented by invoking the >> > > > schedule_on_each_cpu() with an empty work function, which forces >> > > > a context switch on every online CPU and guarantees that all >> > > > rude-tasks read-side critical sections have completed. However, >> > > > schedule_on_each_cpu() can fail (via alloc_percpu() returning >> > > > -ENOMEM) and returns immediately, in which case those context >> > > > switches never happen, too-short grace period happen. >> > > > >> > > > This commit therefore wraps the schedule_on_each_cpu() call in >> > > > WARN_ONCE() so that this failure is reported as a "Too-short >> > > > grace period" warning. >> > > > >> > > NAK, this seems like a edge case, and potentially panicking the >system >> > > with panic on oops doesn't seem right to me, because we would >reserve >> > > >> > that >> > >> > > >> > > for when something definitely goes really wrong and we need to >notify >> > > the user. >> > > >> > > I don't believe this patch should go in for this reason, it's a >heavy >> > > >> > edge >> > >> > > >> > > case, that doesn't *have* nessesarily need to be accounted for. >> > > >> > Indeed, this is a rare event, so I use WARN_ONCE() to mark it, the >> > WARN_ONCE() >> > not panicking system unless you set warn_on_oops true, this can help >us >> > troubleshoot >> > UAF issues caused by a too short tasks-rude grace period. >> > >> > or maybe we can use DEFINE_PER_CPU(struct work_struct, rude_work) to >> > replace >> > alloc_percpu(struct work_struct) queue to all online CPUs. >> > >> > Anyway, Thank you for your reply. I would also like to hear the >opinions >> > of others. >> > >> Fair enough, but me and Paul done some discussions: >> >> Here is the TL;DR >> >> My ^W^Wai slop^W^W tool helped me with this: if the too short grace >> happens, >> it falls back. >> >> root@p10-linux-brads:~/linux# mailto:root@p10-linux-brads:~/linux# git >diff kernel/rcu >> diff --git a/kernel/rcu/tasks.h b/kernel/rcu/tasks.h >> index 627295396cd9..65bb7d4ed632 100644 >> --- a/kernel/rcu/tasks.h >> +++ b/kernel/rcu/tasks.h >> @@ -1344,15 +1344,23 @@ void exit_tasks_rcu_finish(void) { } >> // Ordering is provided by the scheduler's context-switch code. >> >> // Empty function to allow workqueues to force a context switch. >> -static void rcu_tasks_be_rude(struct work_struct *work) >> +static void __maybe_unused rcu_tasks_be_rude(struct work_struct *work) >> { >> } >> >> // Wait for one rude RCU-tasks grace period. >> +static void rcu_tasks_rude_ipi(void *info) >> +{ >> +} >> + >> static void rcu_tasks_rude_wait_gp(struct rcu_tasks *rtp) >> { >> + int cpu; >> rtp->n_ipis += cpumask_weight(cpu_online_mask); >> - schedule_on_each_cpu(rcu_tasks_be_rude); >> + cpus_read_lock(); >> + for_each_online_cpu(cpu) >> + smp_call_function_single(cpu, rcu_tasks_rude_ipi, NULL, 1); >> + cpus_read_unlock(); >> } > >RCU Tasks Rude use preempt_disable/enable as the read critical section, >you use smp_call_function_single() to send ipi to each online CPUs, >it's not forces a context sched switch, and not wait the >preempt_disable/enable >read critical section to exit, this is the wrong mechanism. ;-( > >Obviously, we only need to not apply alloc_percpu dynamic allocation: > >diff --git a/kernel/rcu/tasks.h b/kernel/rcu/tasks.h >index fcac7361ec51..49b629b0010b 100644 >--- a/kernel/rcu/tasks.h >+++ b/kernel/rcu/tasks.h >@@ -1357,11 +1357,25 @@ static void rcu_tasks_be_rude(struct work_struct *work) > { > } > >+static DEFINE_PER_CPU(struct work_struct, tasks_rude_work); >+ > // Wait for one rude RCU-tasks grace period. > static void rcu_tasks_rude_wait_gp(struct rcu_tasks *rtp) > { >+ int cpu; >+ >+ guard(cpus_read_lock)(); > rtp->n_ipis += cpumask_weight(cpu_online_mask); >- schedule_on_each_cpu(rcu_tasks_be_rude); >+ >+ for_each_online_cpu(cpu) { >+ struct work_struct *work = &per_cpu(tasks_rude_work, cpu); >+ >+ INIT_WORK(work, rcu_tasks_be_rude); >+ schedule_work_on(cpu, work); >+ } >+ >+ for_each_online_cpu(cpu) >+ flush_work(&per_cpu(tasks_rude_work, cpu)); > } > > static void call_rcu_tasks_rude(struct rcu_head *rhp, rcu_callback_t > func); >
Wow! Reviewed-by: Bradley Morgan <[email protected]> > >Thanks >Zqiang > > > >> >> static void call_rcu_tasks_rude(struct rcu_head *rhp, rcu_callback_t >func); >> root@p10-linux-brads:~/linux# mailto:root@p10-linux-brads:~/linux# >> >> On POWERPC, this passes 12 hours of rcutorture on the RUDE01 config. >> >> If you plan on submitting this into a patch, please add my authorship, >or >> my c-d-b. >> >> Feel free to rcutorture test it ;d >> >> BTW: have you reproduced this on your own system? >> >> > >> > Thanks >> > Zqiang >> > >> > > >> > > Zqiang, have you reproduced this OOM on your own system, if so, >could >> > > >> > you >> > >> > > >> > > tell me more details, you may turn my frown into a smiley face ;) >> > > >> > > > >> > > > Signed-off-by: Zqiang <[email protected]> >> > > > --- >> > > > kernel/rcu/tasks.h | 3 ++- >> > > > 1 file changed, 2 insertions(+), 1 deletion(-) >> > > > >> > > > diff --git a/kernel/rcu/tasks.h b/kernel/rcu/tasks.h >> > > > index fcac7361ec51..91c25717402e 100644 >> > > > --- a/kernel/rcu/tasks.h >> > > > +++ b/kernel/rcu/tasks.h >> > > > @@ -1361,7 +1361,8 @@ static void rcu_tasks_be_rude(struct >work_struct >> > > >> > *work) >> > >> > > >> > > > static void rcu_tasks_rude_wait_gp(struct rcu_tasks *rtp) >> > > > { >> > > > rtp->n_ipis += cpumask_weight(cpu_online_mask); >> > > > - schedule_on_each_cpu(rcu_tasks_be_rude); >> > > > + WARN_ONCE(schedule_on_each_cpu(rcu_tasks_be_rude), >> > > > + "%s: Too-short grace period, OOM is now expected >behavior!!!\n", >> > > >> > rtp->name); >> > >> > > >> > > > >> > > Good message, makes sense. But I don't think this is right, see >above. >> > > >> > > > >> > > > } >> > > > >> > > > static void call_rcu_tasks_rude(struct rcu_head *rhp, >rcu_callback_t >> > > > func); >> > > > >> > > --- Thanks! >> > > "I'm not a very positive person" - Linus torvalds >> > > >> > >> --- Thanks! >> "I'm not a very positive person" - Linus torvalds >> --- Thanks! "I'm not a very positive person" - Linus torvalds
