Re: [PATCH 0/6] sched/cpusets fixes, more changes are needed

[Oleg Nesterov <oleg@redhat.com>]

On 03/25, Oleg Nesterov wrote:
>
> I like the current idea to call select_task_rq() without rq->lock, but
> of course this is up to you.
>
> However, once again, can't we make a simpler patch?
>
> 	- remove PF_STARTING from task_waking()
> 	
> 	- change sched_fork() to set RUNNING instead of WAKING
>
> 	- change wake_up_new_task() to set WAKING under rq->lock
>
> This looks simpler to me, and allows to drop rq->lock in ttwu() right
> after it sets WAKING.

IOW, something like the (unchecked, uncompiled) patch below.

What do you think?

Oleg.

--- x/kernel/sched.c
+++ x/kernel/sched.c
@@ -912,7 +912,7 @@ static inline void finish_lock_switch(st
  */
 static inline int task_is_waking(struct task_struct *p)
 {
-	return unlikely((p->state == TASK_WAKING) && !(p->flags & PF_STARTING));
+	return unlikely(p->state == TASK_WAKING);
 }
 
 /*
@@ -2568,11 +2568,10 @@ void sched_fork(struct task_struct *p, i
 
 	__sched_fork(p);
 	/*
-	 * We mark the process as waking here. This guarantees that
-	 * nobody will actually run it, and a signal or other external
-	 * event cannot wake it up and insert it on the runqueue either.
+	 * We mark the process as running here. This guarantees that
+	 * nobody will actually wake it up until wake_up_new_task().
 	 */
-	p->state = TASK_WAKING;
+	p->state = TASK_RUNNING;
 
 	/*
 	 * Revert to default priority/policy on fork if requested.
@@ -2638,15 +2637,18 @@ void wake_up_new_task(struct task_struct
 	struct rq *rq;
 	int cpu = get_cpu();
 
+	p->state = TASK_WAKING;
+	smp_mb();
+	raw_spin_unlock_wait(&rq->lock);
+
 #ifdef CONFIG_SMP
 	/*
 	 * Fork balancing, do it here and not earlier because:
 	 *  - cpus_allowed can change in the fork path
 	 *  - any previously selected cpu might disappear through hotplug
 	 *
-	 * We still have TASK_WAKING but PF_STARTING is gone now, meaning
-	 * ->cpus_allowed is stable, we have preemption disabled, meaning
-	 * cpu_online_mask is stable.
+	 * TASK_WAKING means ->cpus_allowed is stable, we have preemption
+	 * disabled, meaning cpu_online_mask is stable.
 	 */
 	cpu = select_task_rq(p, SD_BALANCE_FORK, 0);
 	set_task_cpu(p, cpu);