On 17 October 2026 13:20:50 BST, Bradley Morgan <[email protected]> wrote: >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]>
Gimme a sec I'll submit the patch.. > >> >>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 --- Thanks! "I'm not a very positive person" - Linus torvalds
