lkml.org 
[lkml]   [2010]   [Mar]   [25]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
Patch in this message
/
Date
From
SubjectRe: [PATCH 0/6] sched/cpusets fixes, more changes are needed
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);


\
 
 \ /
  Last update: 2010-03-25 17:07    [W:0.074 / U:0.012 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site