Re: [PATCH v3 tip/core/rcu 1/9] rcu: Add call_rcu_tasks()

["Paul E. McKenney" <paulmck@linux.vnet.ibm.com>]

On Tue, Aug 05, 2014 at 08:47:55AM +0800, Lai Jiangshan wrote:
> On 08/04/2014 10:56 PM, Peter Zijlstra wrote:
> > On Mon, Aug 04, 2014 at 02:25:15PM +0200, Peter Zijlstra wrote:
> >> On Mon, Aug 04, 2014 at 04:50:44AM -0700, Paul E. McKenney wrote:
> >>> OK, I will bite...
> >>>
> >>> What kinds of tasks are on a runqueue, but neither ->on_cpu nor
> >>> PREEMPT_ACTIVE?
> >>
> >> Userspace tasks, they don't necessarily get PREEMPT_ACTIVE when
> >> preempted. Now obviously you're not _that_ interested in userspace tasks
> >> for this, so that might be ok.
> >>
> >> But the main point was, you cannot use ->on_cpu or PREEMPT_ACTIVE
> >> without holding rq->lock.
> > 
> > Hmm, maybe you can, we have the context switch in between setting
> > ->on_cpu and clearing PREEMPT_ACTIVE and vice-versa.
> > 
> > The context switch (obviously) provides a full barrier, so we might be
> > able to -- with careful consideration -- read these two separate values
> > and construct something usable from them.
> > 
> > Something like:
> > 
> > 	task_preempt_count(tsk) & PREEMPT_ACTIVE
> 	the @tsk is running on_cpu, the above result is false.
> > 	smp_rmb();
> > 	tsk->on_cpu
> 	now the @tsk is preempted, the above result also is false.
> 
> 	so it is useless if we fetch the preempt_count and on_cpu in two separated
> instructions.  Maybe it would work if we also take tsk->nivcsw in consideration.
> (I just noticed that tsk->n[i]vcsw are the version numbers for the tsk->on_cpu)
> 
> bool task_on_cpu_or_preempted(tsk)
> {
> 	unsigned long saved_nivcsw;
> 
> 	saved_nivcsw = task->nivcsw;
> 	if (tsk->on_cpu)
> 		return true;
> 
> 	smp_rmb();
> 
> 	if (task_preempt_count(tsk) & PREEMPT_ACTIVE)
> 		return true;
> 
> 	smp_rmb();
> 
> 	if (tsk->on_cpu || task->nivcsw != saved_nivcsw)
> 		return true;
> 
> 	return false;
> }
> 
> > 
> > And because we set PREEMPT_ACTIVE before clearing on_cpu, this should
> > race the right way (err towards the inclusive side).
> > 
> > Obviously that wants a big fat comment...

How about the following?  Non-nohz_full userspace tasks are already covered
courtesy of scheduling-clock interrupts, and this handles nohz_full usermode
tasks.

Thoughts?

							Thanx, Paul

------------------------------------------------------------------------

rcu: Make TASKS_RCU handle nohx_full= CPUs

Currently TASKS_RCU would ignore a CPU running a task in nohz_full=
usermode execution.  There would be neither a context switch nor a
scheduling-clock interrupt to tell TASKS_RCU that the task in question
had passed through a quiescent state.  The grace period would therefore
extend indefinitely.  This commit therefore makes RCU's dyntick-idle
subsystem record the task_struct structure of the task that is running
in dyntick-idle mode on each CPU.  The TASKS_RCU grace period can
then access this information and record a quiescent state on
behalf of any CPU running in dyntick-idle usermode.

Signed-off-by: Paul E. McKenney <paulmck@linux.vnet.ibm.com>

diff --git a/include/linux/rcupdate.h b/include/linux/rcupdate.h
index f504f797c9c8..777aac3a34c0 100644
--- a/include/linux/rcupdate.h
+++ b/include/linux/rcupdate.h
@@ -1140,5 +1140,14 @@ static inline void rcu_sysidle_force_exit(void)
 
 #endif /* #else #ifdef CONFIG_NO_HZ_FULL_SYSIDLE */
 
+#if defined(CONFIG_TASKS_RCU) && defined(CONFIG_NO_HZ_FULL)
+struct task_struct *rcu_dynticks_task_cur(int cpu);
+#else /* #if defined(CONFIG_TASKS_RCU) && defined(CONFIG_NO_HZ_FULL) */
+static inline struct task_struct *rcu_dynticks_task_cur(int cpu)
+{
+	return NULL;
+}
+#endif /* #else #if defined(CONFIG_TASKS_RCU) && defined(CONFIG_NO_HZ_FULL) */
+
 
 #endif /* __LINUX_RCUPDATE_H */
diff --git a/kernel/rcu/tree.c b/kernel/rcu/tree.c
index 645a33efc0d4..86a0a7d5bbbd 100644
--- a/kernel/rcu/tree.c
+++ b/kernel/rcu/tree.c
@@ -526,6 +526,7 @@ static void rcu_eqs_enter_common(struct rcu_dynticks *rdtp, long long oldval,
 	atomic_inc(&rdtp->dynticks);
 	smp_mb__after_atomic();  /* Force ordering with next sojourn. */
 	WARN_ON_ONCE(atomic_read(&rdtp->dynticks) & 0x1);
+	rcu_dynticks_task_enter(rdtp, current);
 
 	/*
 	 * It is illegal to enter an extended quiescent state while
@@ -642,6 +643,7 @@ void rcu_irq_exit(void)
 static void rcu_eqs_exit_common(struct rcu_dynticks *rdtp, long long oldval,
 			       int user)
 {
+	rcu_dynticks_task_exit(rdtp);
 	smp_mb__before_atomic();  /* Force ordering w/previous sojourn. */
 	atomic_inc(&rdtp->dynticks);
 	/* CPUs seeing atomic_inc() must see later RCU read-side crit sects */
diff --git a/kernel/rcu/tree.h b/kernel/rcu/tree.h
index 0f69a79c5b7d..1e79fa1b7cbf 100644
--- a/kernel/rcu/tree.h
+++ b/kernel/rcu/tree.h
@@ -88,6 +88,9 @@ struct rcu_dynticks {
 				    /* Process level is worth LLONG_MAX/2. */
 	int dynticks_nmi_nesting;   /* Track NMI nesting level. */
 	atomic_t dynticks;	    /* Even value for idle, else odd. */
+#if defined(CONFIG_TASKS_RCU) && defined(CONFIG_NO_HZ_FULL)
+	struct task_struct *dynticks_tsk;
+#endif /* #if defined(CONFIG_TASKS_RCU) && defined(CONFIG_NO_HZ_FULL) */
 #ifdef CONFIG_NO_HZ_FULL_SYSIDLE
 	long long dynticks_idle_nesting;
 				    /* irq/process nesting level from idle. */
@@ -579,6 +582,9 @@ static void rcu_sysidle_report_gp(struct rcu_state *rsp, int isidle,
 static void rcu_bind_gp_kthread(void);
 static void rcu_sysidle_init_percpu_data(struct rcu_dynticks *rdtp);
 static bool rcu_nohz_full_cpu(struct rcu_state *rsp);
+static void rcu_dynticks_task_enter(struct rcu_dynticks *rdtp,
+				    struct task_struct *t);
+static void rcu_dynticks_task_exit(struct rcu_dynticks *rdtp);
 
 #endif /* #ifndef RCU_TREE_NONCORE */
 
diff --git a/kernel/rcu/tree_plugin.h b/kernel/rcu/tree_plugin.h
index a86a363ea453..442d62edc564 100644
--- a/kernel/rcu/tree_plugin.h
+++ b/kernel/rcu/tree_plugin.h
@@ -2852,3 +2852,29 @@ static void rcu_bind_gp_kthread(void)
 		set_cpus_allowed_ptr(current, cpumask_of(cpu));
 #endif /* #ifdef CONFIG_NO_HZ_FULL */
 }
+
+/* Record the current task on dyntick-idle entry. */
+static void rcu_dynticks_task_enter(struct rcu_dynticks *rdtp,
+				    struct task_struct *t)
+{
+#if defined(CONFIG_TASKS_RCU) && defined(CONFIG_NO_HZ_FULL)
+	ACCESS_ONCE(rdtp->dynticks_tsk) = t;
+#endif /* #if defined(CONFIG_TASKS_RCU) && defined(CONFIG_NO_HZ_FULL) */
+}
+
+/* Record no current task on dyntick-idle exit. */
+static void rcu_dynticks_task_exit(struct rcu_dynticks *rdtp)
+{
+	rcu_dynticks_task_enter(rdtp, NULL);
+}
+
+#if defined(CONFIG_TASKS_RCU) && defined(CONFIG_NO_HZ_FULL)
+struct task_struct *rcu_dynticks_task_cur(int cpu)
+{
+	struct rcu_dynticks *rdtp = &per_cpu(rcu_dynticks, cpu);
+	struct task_struct *t = ACCESS_ONCE(rdtp->dynticks_tsk);
+
+	smp_read_barrier_depends(); /* Dereferences after fetch of "t". */
+	return t;
+}
+#endif /* #if defined(CONFIG_TASKS_RCU) && defined(CONFIG_NO_HZ_FULL) */
diff --git a/kernel/rcu/update.c b/kernel/rcu/update.c
index ad2a8df43757..6ad6af2ab028 100644
--- a/kernel/rcu/update.c
+++ b/kernel/rcu/update.c
@@ -481,6 +481,28 @@ static void check_holdout_task(struct task_struct *t,
 	sched_show_task(current);
 }
 
+/* Check for nohz_full CPUs executing in userspace. */
+static void check_no_hz_full_tasks(void)
+{
+#ifdef CONFIG_NO_HZ_FULL
+	int cpu;
+	struct task_struct *t;
+
+	for_each_online_cpu(cpu) {
+		cond_resched();
+		rcu_read_lock();
+		t = rcu_dynticks_task_cur(cpu);
+		if (t == NULL || is_idle_task(t)) {
+			rcu_read_unlock();
+			continue;
+		}
+		if (ACCESS_ONCE(t->rcu_tasks_holdout))
+			ACCESS_ONCE(t->rcu_tasks_holdout) = 0;
+		rcu_read_unlock();
+	}
+#endif /* #ifdef CONFIG_NO_HZ_FULL */
+}
+
 /* RCU-tasks kthread that detects grace periods and invokes callbacks. */
 static int __noreturn rcu_tasks_kthread(void *arg)
 {
@@ -584,6 +606,7 @@ static int __noreturn rcu_tasks_kthread(void *arg)
 				lastreport = jiffies;
 			firstreport = true;
 			WARN_ON(signal_pending(current));
+			check_no_hz_full_tasks();
 			rcu_read_lock();
 			list_for_each_entry_rcu(t, &rcu_tasks_holdouts,
 						rcu_tasks_holdout_list)