Prev: [PATCH 5/6] perf symbols: Avoid unnecessary symbol loading when dso list is specified
Next: proc: turn signal_struct->count into "int nr_threads"
From: Peter Zijlstra on 25 Mar 2010 13:30 On Thu, 2010-03-25 at 17:10 +0100, Oleg Nesterov wrote: > Argh, sorry for noise... > > On 03/25, Oleg Nesterov wrote: > > > > 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 > > When I reread this thread, suddenly finally I noticed you mentioned > _twice_ your patch does this too ;) Not to mention the patch itself > which I misread. Sorry. > > > IOW, something like the (unchecked, uncompiled) patch below. > > Still, what do you think? Yeah, such a smaller patch might work too, but I was trying to remove some more of the complexity we grown. Being able to fully remove that TASK_WAKING check from task_rq_lock() and only have it in set_cpus_allowed_ptr() would reduce some fast-path logic. You patch add a memory barrier and an unlock_wait(), which, while seemingly correct, are harder to parse than the modified locking. (Ideally we'd protect ->cpus_allowed using a per-task lock, but adding more atomics ops to ttwu() is to be avoided) (Now if I could manage to remove that lock-drop for the cgroup muck we'd be able to remove TASK_WAKING... but that looks like a long term goal) -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo(a)vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
From: Oleg Nesterov on 25 Mar 2010 15:20
On 03/25, Peter Zijlstra wrote: > > Yeah, such a smaller patch might work too, but I was trying to remove > some more of the complexity we grown. > > Being able to fully remove that TASK_WAKING check from task_rq_lock() > and only have it in set_cpus_allowed_ptr() would reduce some fast-path > logic. OK. Agreed. > You patch add a memory barrier and an unlock_wait(), which, while > seemingly correct, are harder to parse than the modified locking. Yes, lock + set WAKING + unlock is simpler and cleaner, but this doesn't matter. I think your patch should address all problems. Oleg. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo(a)vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/ |