2024-02-17 01:28:53

by Boqun Feng

[permalink] [raw]
Subject: [PATCH v2 4/6] rcu-tasks: Maintain lists to eliminate RCU-tasks/do_exit() deadlocks

From: "Paul E. McKenney" <[email protected]>

This commit continues the elimination of deadlocks involving do_exit()
and RCU tasks by causing exit_tasks_rcu_start() to add the current
task to a per-CPU list and causing exit_tasks_rcu_stop() to remove the
current task from whatever list it is on. These lists will be used to
track tasks that are exiting, while still accounting for any RCU-tasks
quiescent states that these tasks pass though.

[ paulmck: Apply Frederic Weisbecker feedback. ]

Link: https://lore.kernel.org/all/[email protected]/

Reported-by: Chen Zhongjin <[email protected]>
Reported-by: Yang Jihong <[email protected]>
Signed-off-by: Paul E. McKenney <[email protected]>
Tested-by: Yang Jihong <[email protected]>
Tested-by: Chen Zhongjin <[email protected]>
Signed-off-by: Boqun Feng <[email protected]>
---
kernel/rcu/tasks.h | 43 +++++++++++++++++++++++++++++++++----------
1 file changed, 33 insertions(+), 10 deletions(-)

diff --git a/kernel/rcu/tasks.h b/kernel/rcu/tasks.h
index 4a5d562e3189..68a8adf7de8e 100644
--- a/kernel/rcu/tasks.h
+++ b/kernel/rcu/tasks.h
@@ -1151,25 +1151,48 @@ struct task_struct *get_rcu_tasks_gp_kthread(void)
EXPORT_SYMBOL_GPL(get_rcu_tasks_gp_kthread);

/*
- * Contribute to protect against tasklist scan blind spot while the
- * task is exiting and may be removed from the tasklist. See
- * corresponding synchronize_srcu() for further details.
+ * Protect against tasklist scan blind spot while the task is exiting and
+ * may be removed from the tasklist. Do this by adding the task to yet
+ * another list.
+ *
+ * Note that the task will remove itself from this list, so there is no
+ * need for get_task_struct(), except in the case where rcu_tasks_pertask()
+ * adds it to the holdout list, in which case rcu_tasks_pertask() supplies
+ * the needed get_task_struct().
*/
-void exit_tasks_rcu_start(void) __acquires(&tasks_rcu_exit_srcu)
+void exit_tasks_rcu_start(void)
{
- current->rcu_tasks_idx = __srcu_read_lock(&tasks_rcu_exit_srcu);
+ unsigned long flags;
+ struct rcu_tasks_percpu *rtpcp;
+ struct task_struct *t = current;
+
+ WARN_ON_ONCE(!list_empty(&t->rcu_tasks_exit_list));
+ preempt_disable();
+ rtpcp = this_cpu_ptr(rcu_tasks.rtpcpu);
+ t->rcu_tasks_exit_cpu = smp_processor_id();
+ raw_spin_lock_irqsave_rcu_node(rtpcp, flags);
+ if (!rtpcp->rtp_exit_list.next)
+ INIT_LIST_HEAD(&rtpcp->rtp_exit_list);
+ list_add(&t->rcu_tasks_exit_list, &rtpcp->rtp_exit_list);
+ raw_spin_unlock_irqrestore_rcu_node(rtpcp, flags);
+ preempt_enable();
}

/*
- * Contribute to protect against tasklist scan blind spot while the
- * task is exiting and may be removed from the tasklist. See
- * corresponding synchronize_srcu() for further details.
+ * Remove the task from the "yet another list" because do_exit() is now
+ * non-preemptible, allowing synchronize_rcu() to wait beyond this point.
*/
-void exit_tasks_rcu_stop(void) __releases(&tasks_rcu_exit_srcu)
+void exit_tasks_rcu_stop(void)
{
+ unsigned long flags;
+ struct rcu_tasks_percpu *rtpcp;
struct task_struct *t = current;

- __srcu_read_unlock(&tasks_rcu_exit_srcu, t->rcu_tasks_idx);
+ WARN_ON_ONCE(list_empty(&t->rcu_tasks_exit_list));
+ rtpcp = per_cpu_ptr(rcu_tasks.rtpcpu, t->rcu_tasks_exit_cpu);
+ raw_spin_lock_irqsave_rcu_node(rtpcp, flags);
+ list_del_init(&t->rcu_tasks_exit_list);
+ raw_spin_unlock_irqrestore_rcu_node(rtpcp, flags);
}

/*
--
2.43.0



2024-02-22 16:32:34

by Frederic Weisbecker

[permalink] [raw]
Subject: Re: [PATCH v2 4/6] rcu-tasks: Maintain lists to eliminate RCU-tasks/do_exit() deadlocks

Le Fri, Feb 16, 2024 at 05:27:39PM -0800, Boqun Feng a ?crit :
> From: "Paul E. McKenney" <[email protected]>
>
> This commit continues the elimination of deadlocks involving do_exit()
> and RCU tasks by causing exit_tasks_rcu_start() to add the current
> task to a per-CPU list and causing exit_tasks_rcu_stop() to remove the
> current task from whatever list it is on. These lists will be used to
> track tasks that are exiting, while still accounting for any RCU-tasks
> quiescent states that these tasks pass though.
>
> [ paulmck: Apply Frederic Weisbecker feedback. ]
>
> Link: https://lore.kernel.org/all/[email protected]/
>
> Reported-by: Chen Zhongjin <[email protected]>
> Reported-by: Yang Jihong <[email protected]>
> Signed-off-by: Paul E. McKenney <[email protected]>
> Tested-by: Yang Jihong <[email protected]>
> Tested-by: Chen Zhongjin <[email protected]>
> Signed-off-by: Boqun Feng <[email protected]>

Reviewed-by: Frederic Weisbecker <[email protected]>

2024-02-23 12:21:50

by Frederic Weisbecker

[permalink] [raw]
Subject: Re: [PATCH v2 4/6] rcu-tasks: Maintain lists to eliminate RCU-tasks/do_exit() deadlocks

On Fri, Feb 16, 2024 at 05:27:39PM -0800, Boqun Feng wrote:
> -void exit_tasks_rcu_start(void) __acquires(&tasks_rcu_exit_srcu)
> +void exit_tasks_rcu_start(void)
> {
> - current->rcu_tasks_idx = __srcu_read_lock(&tasks_rcu_exit_srcu);
> + unsigned long flags;
> + struct rcu_tasks_percpu *rtpcp;
> + struct task_struct *t = current;
> +
> + WARN_ON_ONCE(!list_empty(&t->rcu_tasks_exit_list));
> + preempt_disable();
> + rtpcp = this_cpu_ptr(rcu_tasks.rtpcpu);
> + t->rcu_tasks_exit_cpu = smp_processor_id();
> + raw_spin_lock_irqsave_rcu_node(rtpcp, flags);
> + if (!rtpcp->rtp_exit_list.next)

And then you might want to turn that into a WARN_ONCE.

Thanks.

> + INIT_LIST_HEAD(&rtpcp->rtp_exit_list);
> + list_add(&t->rcu_tasks_exit_list, &rtpcp->rtp_exit_list);
> + raw_spin_unlock_irqrestore_rcu_node(rtpcp, flags);
> + preempt_enable();
> }

2024-02-24 00:28:29

by Paul E. McKenney

[permalink] [raw]
Subject: Re: [PATCH v2 4/6] rcu-tasks: Maintain lists to eliminate RCU-tasks/do_exit() deadlocks

On Fri, Feb 23, 2024 at 01:19:42PM +0100, Frederic Weisbecker wrote:
> On Fri, Feb 16, 2024 at 05:27:39PM -0800, Boqun Feng wrote:
> > -void exit_tasks_rcu_start(void) __acquires(&tasks_rcu_exit_srcu)
> > +void exit_tasks_rcu_start(void)
> > {
> > - current->rcu_tasks_idx = __srcu_read_lock(&tasks_rcu_exit_srcu);
> > + unsigned long flags;
> > + struct rcu_tasks_percpu *rtpcp;
> > + struct task_struct *t = current;
> > +
> > + WARN_ON_ONCE(!list_empty(&t->rcu_tasks_exit_list));
> > + preempt_disable();
> > + rtpcp = this_cpu_ptr(rcu_tasks.rtpcpu);
> > + t->rcu_tasks_exit_cpu = smp_processor_id();
> > + raw_spin_lock_irqsave_rcu_node(rtpcp, flags);
> > + if (!rtpcp->rtp_exit_list.next)
>
> And then you might want to turn that into a WARN_ONCE.

Excellent point, thank you!

I am queueing a separate patch for this with your Reported-by. (This
change can lag behind, just in case this series needs to go in quickly.)

Thanx, Paul

> Thanks.
>
> > + INIT_LIST_HEAD(&rtpcp->rtp_exit_list);
> > + list_add(&t->rcu_tasks_exit_list, &rtpcp->rtp_exit_list);
> > + raw_spin_unlock_irqrestore_rcu_node(rtpcp, flags);
> > + preempt_enable();
> > }