From mboxrd@z Thu Jan 1 00:00:00 1970 From: Sebastian Andrzej Siewior Subject: Re: [PATCH] allow preemption in recursive migrate_disable call Date: Fri, 22 Nov 2013 16:51:20 +0100 Message-ID: <20131122155120.GF8698@linutronix.de> References: <20131120003333.GA13545@opentech.at> <20131121101913.GZ10022@twins.programming.kicks-ass.net> Mime-Version: 1.0 Content-Type: text/plain; charset=utf-8 Cc: Nicholas Mc Guire , linux-rt-users@vger.kernel.org, Thomas Gleixner , Andreas Platschek To: Peter Zijlstra Return-path: Received: from www.linutronix.de ([62.245.132.108]:45452 "EHLO Galois.linutronix.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1755926Ab3KVPvX (ORCPT ); Fri, 22 Nov 2013 10:51:23 -0500 Content-Disposition: inline In-Reply-To: <20131121101913.GZ10022@twins.programming.kicks-ass.net> Sender: linux-rt-users-owner@vger.kernel.org List-ID: * Peter Zijlstra | 2013-11-21 11:19:13 [+0100]: >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 >> 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 >> --- >> 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) ? After staring at code I would say migrate_disable() needs to ignore MIGRATE_DISABLE_SET_AFFIN in order to complete. In the migrate_disable() case it simply doesn't matter if it is set and it can't be set for the 0 case. I was a little worried because the preempt_disable() also protects counter during add/substract operations but since there can only be one task at a time doing this, there should be no problem. >Other than that, > >Acked-by: Peter Zijlstra Sebastian