> > 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# > > 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..
Actually, I don't mind you send this fix directly, due to you use an incorrect IPI mechanism to implement it, I would like to know wether you truly understand the tasks-rude and how schedule_on_each_cpu() ensures memory order. > > > > > > > > > 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# > > 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 >
