[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <5568ff6b-6d5c-ea63-3c4a-c0714103aa9d@huawei.com>
Date: Thu, 12 Jun 2025 11:06:07 +0800
From: Xiongfeng Wang <wangxiongfeng2@...wei.com>
To: Frederic Weisbecker <frederic@...nel.org>, Joel Fernandes
<joelagnelf@...dia.com>
CC: <linux-kernel@...r.kernel.org>, "Paul E. McKenney" <paulmck@...nel.org>,
Neeraj Upadhyay <neeraj.upadhyay@...nel.org>, Joel Fernandes
<joel@...lfernandes.org>, Josh Triplett <josh@...htriplett.org>, Boqun Feng
<boqun.feng@...il.com>, Uladzislau Rezki <urezki@...il.com>, Steven Rostedt
<rostedt@...dmis.org>, Mathieu Desnoyers <mathieu.desnoyers@...icios.com>,
Lai Jiangshan <jiangshanlai@...il.com>, Zqiang <qiang.zhang1211@...il.com>,
<rcu@...r.kernel.org>, <bpf@...r.kernel.org>, <xiqi2@...wei.com>
Subject: Re: [PATCH 2/2] rcu: Fix lockup when RCU reader used while IRQ
exiting
+cc (Qi, my colleague who helps testing the modification)
On 2025/6/10 20:23, Frederic Weisbecker wrote:
> Le Mon, Jun 09, 2025 at 02:01:24PM -0400, Joel Fernandes a écrit :
>> During rcu_read_unlock_special(), if this happens during irq_exit(), we
...skipped...
We have tested the below modification without the modification written by Joel
using the previous syzkaller benchmark. The kernel still panic.
The dmesg log is attached.
Thanks,
Xiongfeng
>
> diff --git a/include/linux/irq_work.h b/include/linux/irq_work.h
> index 136f2980cba3..4149ed516524 100644
> --- a/include/linux/irq_work.h
> +++ b/include/linux/irq_work.h
> @@ -57,6 +57,9 @@ static inline bool irq_work_is_hard(struct irq_work *work)
> bool irq_work_queue(struct irq_work *work);
> bool irq_work_queue_on(struct irq_work *work, int cpu);
>
> +bool irq_work_kick(void);
> +bool irq_work_kick_on(int cpu);
> +
> void irq_work_tick(void);
> void irq_work_sync(struct irq_work *work);
>
> diff --git a/kernel/irq_work.c b/kernel/irq_work.c
> index 73f7e1fd4ab4..383a3e9050d9 100644
> --- a/kernel/irq_work.c
> +++ b/kernel/irq_work.c
> @@ -181,6 +181,22 @@ bool irq_work_queue_on(struct irq_work *work, int cpu)
> #endif /* CONFIG_SMP */
> }
>
> +static void kick_func(struct irq_work *work)
> +{
> +}
> +
> +static DEFINE_PER_CPU(struct irq_work, kick_work) = IRQ_WORK_INIT_HARD(kick_func);
> +
> +bool irq_work_kick(void)
> +{
> + return irq_work_queue(this_cpu_ptr(&kick_work));
> +}
> +
> +bool irq_work_kick_on(int cpu)
> +{
> + return irq_work_queue_on(per_cpu_ptr(&kick_work, cpu), cpu);
> +}
> +
> bool irq_work_needs_cpu(void)
> {
> struct llist_head *raised, *lazy;
> diff --git a/kernel/rcu/tree.h b/kernel/rcu/tree.h
> index a9a811d9d7a3..b33888071e41 100644
> --- a/kernel/rcu/tree.h
> +++ b/kernel/rcu/tree.h
> @@ -191,7 +191,6 @@ struct rcu_data {
> /* during and after the last grace */
> /* period it is aware of. */
> struct irq_work defer_qs_iw; /* Obtain later scheduler attention. */
> - bool defer_qs_iw_pending; /* Scheduler attention pending? */
> struct work_struct strict_work; /* Schedule readers for strict GPs. */
>
> /* 2) batch handling */
> diff --git a/kernel/rcu/tree_plugin.h b/kernel/rcu/tree_plugin.h
> index 3c0bbbbb686f..0c7b7c220b46 100644
> --- a/kernel/rcu/tree_plugin.h
> +++ b/kernel/rcu/tree_plugin.h
> @@ -619,17 +619,6 @@ notrace void rcu_preempt_deferred_qs(struct task_struct *t)
> rcu_preempt_deferred_qs_irqrestore(t, flags);
> }
>
> -/*
> - * Minimal handler to give the scheduler a chance to re-evaluate.
> - */
> -static void rcu_preempt_deferred_qs_handler(struct irq_work *iwp)
> -{
> - struct rcu_data *rdp;
> -
> - rdp = container_of(iwp, struct rcu_data, defer_qs_iw);
> - rdp->defer_qs_iw_pending = false;
> -}
> -
> /*
> * Handle special cases during rcu_read_unlock(), such as needing to
> * notify RCU core processing or task having blocked during the RCU
> @@ -673,18 +662,10 @@ static void rcu_read_unlock_special(struct task_struct *t)
> set_tsk_need_resched(current);
> set_preempt_need_resched();
> if (IS_ENABLED(CONFIG_IRQ_WORK) && irqs_were_disabled &&
> - expboost && !rdp->defer_qs_iw_pending && cpu_online(rdp->cpu)) {
> + expboost && cpu_online(rdp->cpu)) {
> // Get scheduler to re-evaluate and call hooks.
> // If !IRQ_WORK, FQS scan will eventually IPI.
> - if (IS_ENABLED(CONFIG_RCU_STRICT_GRACE_PERIOD) &&
> - IS_ENABLED(CONFIG_PREEMPT_RT))
> - rdp->defer_qs_iw = IRQ_WORK_INIT_HARD(
> - rcu_preempt_deferred_qs_handler);
> - else
> - init_irq_work(&rdp->defer_qs_iw,
> - rcu_preempt_deferred_qs_handler);
> - rdp->defer_qs_iw_pending = true;
> - irq_work_queue_on(&rdp->defer_qs_iw, rdp->cpu);
> + irq_work_kick();
> }
> }
> local_irq_restore(flags);
> diff --git a/kernel/time/tick-sched.c b/kernel/time/tick-sched.c
> index c527b421c865..84170656334d 100644
> --- a/kernel/time/tick-sched.c
> +++ b/kernel/time/tick-sched.c
> @@ -377,14 +377,6 @@ static bool can_stop_full_tick(int cpu, struct tick_sched *ts)
> return true;
> }
>
> -static void nohz_full_kick_func(struct irq_work *work)
> -{
> - /* Empty, the tick restart happens on tick_nohz_irq_exit() */
> -}
> -
> -static DEFINE_PER_CPU(struct irq_work, nohz_full_kick_work) =
> - IRQ_WORK_INIT_HARD(nohz_full_kick_func);
> -
> /*
> * Kick this CPU if it's full dynticks in order to force it to
> * re-evaluate its dependency on the tick and restart it if necessary.
> @@ -396,7 +388,7 @@ static void tick_nohz_full_kick(void)
> if (!tick_nohz_full_cpu(smp_processor_id()))
> return;
>
> - irq_work_queue(this_cpu_ptr(&nohz_full_kick_work));
> + irq_work_kick();
> }
>
> /*
> @@ -408,7 +400,7 @@ void tick_nohz_full_kick_cpu(int cpu)
> if (!tick_nohz_full_cpu(cpu))
> return;
>
> - irq_work_queue_on(&per_cpu(nohz_full_kick_work, cpu), cpu);
> + irq_work_kick_on(cpu);
> }
>
> static void tick_nohz_kick_task(struct task_struct *tsk)
>
>
>
>
View attachment "irq_panic.txt" of type "text/plain" (24044 bytes)
Powered by blists - more mailing lists