Messages in this thread Patch in this message | | | Subject | Re: [PATCH] sched/core: Create new task with twice disabled preemption | From | Kirill Tkhai <> | Date | Fri, 14 Feb 2014 16:44:01 +0400 |
| |
В Птн, 14/02/2014 в 12:35 +0000, Catalin Marinas пишет: > On Thu, Feb 13, 2014 at 07:51:56PM +0400, Kirill Tkhai wrote: > > Preemption state on enter in finish_task_switch() is different > > in cases of context_switch() and schedule_tail(). > > > > In the first case we have it twice disabled: at the start of > > schedule() and during spin locking. In the second it is only > > once: the value which was set in init_task_preempt_count(). > > > > For archs without __ARCH_WANT_UNLOCKED_CTXSW set this means > > that all newly created tasks execute finish_arch_post_lock_switch() > > and post_schedule() with preemption enabled. > > > > It seems there is possible a problem in rare situations on arm64, > > when one freshly created thread preempts another before > > finish_arch_post_lock_switch() has finished. If mm is the same, > > then TIF_SWITCH_MM on the second won't be set. > > > > The second rare but possible issue is zeroing of post_schedule() > > on a wrong cpu. > > > > So, lets fix this and unify preempt_count state. > > An alternative to your patch:
It looks better, than the initial.
You may add my Acked-by if you want.
> diff --git a/kernel/sched/core.c b/kernel/sched/core.c > index b46131ef6aab..b932b6a4c716 100644 > --- a/kernel/sched/core.c > +++ b/kernel/sched/core.c > @@ -2210,6 +2210,10 @@ asmlinkage void schedule_tail(struct task_struct *prev) > { > struct rq *rq = this_rq(); > > +#ifndef __ARCH_WANT_UNLOCKED_CTXSW > + /* In this case, finish_task_switch reenables preemption */ > + preempt_disable(); > +#endif > finish_task_switch(rq, prev); > > /* > @@ -2218,10 +2222,7 @@ asmlinkage void schedule_tail(struct task_struct *prev) > */ > post_schedule(rq); > > -#ifdef __ARCH_WANT_UNLOCKED_CTXSW > - /* In this case, finish_task_switch does not reenable preemption */ > preempt_enable(); > -#endif > if (current->set_child_tid) > put_user(task_pid_vnr(current), current->set_child_tid); > } >
-- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
| |