Re: [PATCH 3/3] task: Clean house now that tasks on the runqueue are rcu protected
On 09/02, Eric W. Biederman wrote: > > @@ -1644,7 +1644,7 @@ static void task_numa_compare(struct task_numa_env *env, > return; > > rcu_read_lock(); > - cur = task_rcu_dereference(_rq->curr); > + cur = rcu_dereference(dst_rq->curr); > if (cur && ((cur->flags & PF_EXITING) || is_idle_task(cur))) > cur = NULL; afaics rq->curr can't be NULL, so you can also simplify the "if" check cur = task_rcu_dereference(_rq->curr); if ((cur->flags & PF_EXITING) || is_idle_task(cur)) cur = NULL; Same for membarrier_global_expedited/membarrier_private_expedited changed by this patch. Oleg.
Re: [PATCH 3/3] task: Clean house now that tasks on the runqueue are rcu protected
Hi "Eric,
Thank you for the patch! Perhaps something to improve:
[auto build test WARNING on linus/master]
[cannot apply to v5.3-rc7 next-20190902]
[if your patch is applied to the wrong git tree, please drop us a note to help
improve the system]
url:
https://github.com/0day-ci/linux/commits/Eric-W-Biederman/task-Making-tasks-on-the-runqueue-rcu-protected/20190903-130546
reproduce:
# apt-get install sparse
# sparse version: v0.6.1-rc1-7-g2b96cd8-dirty
make ARCH=x86_64 allmodconfig
make C=1 CF='-fdiagnostic-prefix -D__CHECK_ENDIAN__'
If you fix the issue, kindly add following tag
Reported-by: kbuild test robot
sparse warnings: (new ones prefixed by >>)
>> kernel/sched/membarrier.c:74:21: sparse: sparse: incompatible types in
>> comparison expression (different address spaces):
>> kernel/sched/membarrier.c:74:21: sparse:struct task_struct [noderef]
>> *
>> kernel/sched/membarrier.c:74:21: sparse:struct task_struct *
kernel/sched/membarrier.c:153:21: sparse: sparse: incompatible types in
comparison expression (different address spaces):
kernel/sched/membarrier.c:153:21: sparse:struct task_struct [noderef]
*
kernel/sched/membarrier.c:153:21: sparse:struct task_struct *
vim +74 kernel/sched/membarrier.c
32
33 static int membarrier_global_expedited(void)
34 {
35 int cpu;
36 bool fallback = false;
37 cpumask_var_t tmpmask;
38
39 if (num_online_cpus() == 1)
40 return 0;
41
42 /*
43 * Matches memory barriers around rq->curr modification in
44 * scheduler.
45 */
46 smp_mb(); /* system call entry is not a mb. */
47
48 /*
49 * Expedited membarrier commands guarantee that they won't
50 * block, hence the GFP_NOWAIT allocation flag and fallback
51 * implementation.
52 */
53 if (!zalloc_cpumask_var(, GFP_NOWAIT)) {
54 /* Fallback for OOM. */
55 fallback = true;
56 }
57
58 cpus_read_lock();
59 for_each_online_cpu(cpu) {
60 struct task_struct *p;
61
62 /*
63 * Skipping the current CPU is OK even through we can be
64 * migrated at any point. The current CPU, at the point
65 * where we read raw_smp_processor_id(), is ensured to
66 * be in program order with respect to the caller
67 * thread. Therefore, we can skip this CPU from the
68 * iteration.
69 */
70 if (cpu == raw_smp_processor_id())
71 continue;
72
73 rcu_read_lock();
> 74 p = rcu_dereference(cpu_rq(cpu)->curr);
75 if (p && p->mm &&
(atomic_read(>mm->membarrier_state) &
76 MEMBARRIER_STATE_GLOBAL_EXPEDITED)) {
77 if (!fallback)
78 __cpumask_set_cpu(cpu, tmpmask);
79 else
80 smp_call_function_single(cpu, ipi_mb,
NULL, 1);
81 }
82 rcu_read_unlock();
83 }
84 if (!fallback) {
85 preempt_disable();
86 smp_call_function_many(tmpmask, ipi_mb, NULL, 1);
87 preempt_enable();
88 free_cpumask_var(tmpmask);
89 }
90 cpus_read_unlock();
91
92 /*
93 * Memory barrier on the caller thread _after_ we finished
94 * waiting for the last IPI. Matches memory barriers around
95 * rq->curr modification in scheduler.
96 */
97 smp_mb(); /* exit from system call is not a mb */
98 return 0;
99 }
100
---
0-DAY kernel test infrastructureOpen Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
[PATCH 3/3] task: Clean house now that tasks on the runqueue are rcu protected
Use rcu_dereference instead of task_rcu_dereference.
Remove task_rcu_dereference.
Remove the complications of rcuwait that were in place because tasks
on the runqueue were not rcu protected. It is now safe to call
wake_up_process if the target was know to be on the runqueue in the
current rcu interval.
Cc: Davidlohr Bueso
Cc: Peter Zijlstra (Intel)
Cc: Oleg Nesterov
Ref: 8f95c90ceb54 ("sched/wait, RCU: Introduce rcuwait machinery")
Ref: 150593bf8693 ("sched/api: Introduce task_rcu_dereference() and
try_get_task_struct()")
Signed-off-by: "Eric W. Biederman"
---
include/linux/rcuwait.h| 20 +++-
include/linux/sched/task.h | 1 -
kernel/exit.c | 67 --
kernel/sched/fair.c| 2 +-
kernel/sched/membarrier.c | 4 +--
5 files changed, 7 insertions(+), 87 deletions(-)
diff --git a/include/linux/rcuwait.h b/include/linux/rcuwait.h
index 563290fc194f..75c97e4bbc57 100644
--- a/include/linux/rcuwait.h
+++ b/include/linux/rcuwait.h
@@ -6,16 +6,11 @@
/*
* rcuwait provides a way of blocking and waking up a single
- * task in an rcu-safe manner; where it is forbidden to use
- * after exit_notify(). task_struct is not properly rcu protected,
- * unless dealing with rcu-aware lists, ie: find_task_by_*().
+ * task in an rcu-safe manner.
*
- * Alternatively we have task_rcu_dereference(), but the return
- * semantics have different implications which would break the
- * wakeup side. The only time @task is non-nil is when a user is
- * blocked (or checking if it needs to) on a condition, and reset
- * as soon as we know that the condition has succeeded and are
- * awoken.
+ * The only time @task is non-nil is when a user is blocked (or
+ * checking if it needs to) on a condition, and reset as soon as we
+ * know that the condition has succeeded and are awoken.
*/
struct rcuwait {
struct task_struct __rcu *task;
@@ -37,13 +32,6 @@ extern void rcuwait_wake_up(struct rcuwait *w);
*/
#define rcuwait_wait_event(w, condition) \
({ \
- /* \
-* Complain if we are called after do_exit()/exit_notify(), \
-* as we cannot rely on the rcu critical region for the \
-* wakeup side. \
-*/ \
- WARN_ON(current->exit_state); \
- \
rcu_assign_pointer((w)->task, current); \
for (;;) { \
/* \
diff --git a/include/linux/sched/task.h b/include/linux/sched/task.h
index 4c44c37236b2..8bd51af44bf8 100644
--- a/include/linux/sched/task.h
+++ b/include/linux/sched/task.h
@@ -115,7 +115,6 @@ static inline void put_task_struct(struct task_struct *t)
__put_task_struct(t);
}
-struct task_struct *task_rcu_dereference(struct task_struct **ptask);
void put_task_struct_rcu_user(struct task_struct *task);
#ifdef CONFIG_ARCH_WANTS_DYNAMIC_TASK_STRUCT
diff --git a/kernel/exit.c b/kernel/exit.c
index 2e259286f4e7..f943773622fc 100644
--- a/kernel/exit.c
+++ b/kernel/exit.c
@@ -234,69 +234,6 @@ void release_task(struct task_struct *p)
goto repeat;
}
-/*
- * Note that if this function returns a valid task_struct pointer (!NULL)
- * task->usage must remain >0 for the duration of the RCU critical section.
- */
-struct task_struct *task_rcu_dereference(struct task_struct **ptask)
-{
- struct sighand_struct *sighand;
- struct task_struct *task;
-
- /*
-* We need to verify that release_task() was not called and thus
-* delayed_put_task_struct() can't run and drop the last reference
-* before rcu_read_unlock(). We check task->sighand != NULL,
-* but we can read the already freed and reused memory.
-*/
-retry:
- task = rcu_dereference(*ptask);
- if (!task)
- return NULL;
-
- probe_kernel_address(>sighand, sighand);
-
- /*
-* Pairs with atomic_dec_and_test() in put_task_struct(). If this task
-* was already freed we can not miss the preceding update of this
-* pointer.
-*/
- smp_rmb();
- if (unlikely(task != READ_ONCE(*ptask)))
- goto retry;
-
- /*
-* We've re-checked that "task == *ptask", now we have two different
-* cases:
-*
-* 1. This is actually the same task/task_struct. In this case
-*sighand != NULL tells us it is still alive.
-*
-* 2. This is another task which got the same memory for task_struct.
-*We can't know
