* [PATCH] allow preemption in recursive migrate_disable call
@ 2013-11-20 0:33 Nicholas Mc Guire
2013-11-21 10:19 ` Peter Zijlstra
2013-11-22 17:37 ` Sebastian Andrzej Siewior
0 siblings, 2 replies; 5+ messages in thread
From: Nicholas Mc Guire @ 2013-11-20 0:33 UTC (permalink / raw)
To: linux-rt-users; +Cc: Peter Zijlstra, Thomas Gleixner, Andreas Platschek
>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
--
1.7.3.4
^ permalink raw reply related [flat|nested] 5+ messages in thread* Re: [PATCH] allow preemption in recursive migrate_disable call
2013-11-20 0:33 [PATCH] allow preemption in recursive migrate_disable call Nicholas Mc Guire
@ 2013-11-21 10:19 ` Peter Zijlstra
2013-11-22 15:51 ` Sebastian Andrzej Siewior
2013-11-23 5:19 ` Nicholas Mc Guire
2013-11-22 17:37 ` Sebastian Andrzej Siewior
1 sibling, 2 replies; 5+ messages in thread
From: Peter Zijlstra @ 2013-11-21 10:19 UTC (permalink / raw)
To: Nicholas Mc Guire; +Cc: linux-rt-users, Thomas Gleixner, Andreas Platschek
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) ?
Other than that,
Acked-by: Peter Zijlstra <peterz@infradead.org>
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH] allow preemption in recursive migrate_disable call
2013-11-21 10:19 ` Peter Zijlstra
@ 2013-11-22 15:51 ` Sebastian Andrzej Siewior
2013-11-23 5:19 ` Nicholas Mc Guire
1 sibling, 0 replies; 5+ messages in thread
From: Sebastian Andrzej Siewior @ 2013-11-22 15:51 UTC (permalink / raw)
To: Peter Zijlstra
Cc: Nicholas Mc Guire, linux-rt-users, Thomas Gleixner,
Andreas Platschek
* 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 <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) ?
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 <peterz@infradead.org>
Sebastian
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH] allow preemption in recursive migrate_disable call
2013-11-21 10:19 ` Peter Zijlstra
2013-11-22 15:51 ` Sebastian Andrzej Siewior
@ 2013-11-23 5:19 ` Nicholas Mc Guire
1 sibling, 0 replies; 5+ messages in thread
From: Nicholas Mc Guire @ 2013-11-23 5:19 UTC (permalink / raw)
To: Peter Zijlstra; +Cc: linux-rt-users, Thomas Gleixner, Andreas Platschek
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
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] allow preemption in recursive migrate_disable call
2013-11-20 0:33 [PATCH] allow preemption in recursive migrate_disable call Nicholas Mc Guire
2013-11-21 10:19 ` Peter Zijlstra
@ 2013-11-22 17:37 ` Sebastian Andrzej Siewior
1 sibling, 0 replies; 5+ messages in thread
From: Sebastian Andrzej Siewior @ 2013-11-22 17:37 UTC (permalink / raw)
To: Nicholas Mc Guire
Cc: linux-rt-users, Peter Zijlstra, Thomas Gleixner,
Andreas Platschek
* Nicholas Mc Guire | 2013-11-20 01:33:33 [+0100]:
> 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
applied but I dropped this line^
>
>Signed-off-by: Nicholas Mc Guire <der.herr@hofr.at>
Sebastian
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2013-11-23 5:19 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-11-20 0:33 [PATCH] allow preemption in recursive migrate_disable call Nicholas Mc Guire
2013-11-21 10:19 ` Peter Zijlstra
2013-11-22 15:51 ` Sebastian Andrzej Siewior
2013-11-23 5:19 ` Nicholas Mc Guire
2013-11-22 17:37 ` Sebastian Andrzej Siewior
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).