Re: [PATCH] allow preemption in recursive migrate_disable call

[Nicholas Mc Guire <der.herr@hofr.at>]

On Thu, 21 Nov 2013, Peter Zijlstra wrote:

> On Wed, Nov 20, 2013 at 01:33:33AM +0100, Nicholas Mc Guire wrote:
> > From a7259c360b6c8b873f5fcf6d5eed0ae78534a6c5 Mon Sep 17 00:00:00 2001
> > From: Nicholas Mc Guire <der.herr@hofr.at>
> > Date: Wed, 20 Nov 2013 07:22:09 +0800
> > Subject: [PATCH] allow preemption in recursive migrate_disable call
> > 
> >  Minor cleanup in migrate_disable/migrate_enable. The recursive case
> >  does not need to disable preemption as it is "pinned" to the current
> >  cpu any way so it is safe to preempt it.
> > 
> >  No functional change to migrate_disable/enable
> > 
> > Signed-off-by: Nicholas Mc Guire <der.herr@hofr.at>
> > ---
> >  kernel/sched/core.c |    6 ++----
> >  1 files changed, 2 insertions(+), 4 deletions(-)
> > 
> > diff --git a/kernel/sched/core.c b/kernel/sched/core.c
> > index a7fafc28..22fa2e2 100644
> > --- a/kernel/sched/core.c
> > +++ b/kernel/sched/core.c
> > @@ -2418,13 +2418,12 @@ void migrate_disable(void)
> >  	}
> >  #endif
> >  
> > -	preempt_disable();
> >  	if (p->migrate_disable) {
> >  		p->migrate_disable++;
> > -		preempt_enable();
> >  		return;
> >  	}
> >  
> > +	preempt_disable();
> >  	preempt_lazy_disable();
> >  	pin_current_cpu();
> >  	p->migrate_disable = 1;
> > @@ -2454,13 +2453,12 @@ void migrate_enable(void)
> >  #endif
> >  	WARN_ON_ONCE(p->migrate_disable <= 0);
> >  
> > -	preempt_disable();
> >  	if (migrate_disable_count(p) > 1) {
> >  		p->migrate_disable--;
> > -		preempt_enable();
> >  		return;
> >  	}
> >  
> > +	preempt_disable();
> >  	if (unlikely(migrate_disabled_updated(p))) {
> >  		/*
> >  		 * Undo whatever update_migrate_disable() did, also see there
> 
> Is there a reason one uses p->migrate_disable and the other uses
> migrate_disable_count(p) ? 
>
in the update case incrementting is fine with or without 
MIGRATE_DISABLE_SET_AFFIN set, in the migrate_enable case though
if the nesting level were actuall 1 and MIGRATE_DISABLE_SET_AFFIN were set
we would end up in the wrong branch (1<<30)+1 > 1.

thx!
hofrat