[PATCH RT 4/5] allow preemption in mem_cgroup_move_account_page_stat

[PATCH RT 4/5] allow preemption in mem_cgroup_move_account_page_stat

[Nicholas Mc Guire]

allow preemption in mem_cgroup_move_account_page_stat

as the call sites of mem_cgroup_move_account_page_stat are under 
move_lock_mem_cgroup move_unlock_mem_cgroup (spin_lock_irqsave/
spin_unlock_irqrestore respectively) the move operations 
preempt_disable/enable seem to be purely for migration protection so 
a migrate_disable/enable should be fine here.

patch on top of 3.12.10-rt15

Signed-off-by: Nicholas Mc Guire <der.herr@hofr.at>
---
 mm/memcontrol.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/mm/memcontrol.c b/mm/memcontrol.c
index a67e630f..e7cc35a 100644
--- a/mm/memcontrol.c
+++ b/mm/memcontrol.c
@@ -3784,10 +3784,10 @@ void mem_cgroup_move_account_page_stat(struct mem_cgroup *from,
 					enum mem_cgroup_stat_index idx)
 {
 	/* Update stat data for mem_cgroup */
-	preempt_disable();
+	migrate_disable();
 	__this_cpu_sub(from->stat->count[idx], nr_pages);
 	__this_cpu_add(to->stat->count[idx], nr_pages);
-	preempt_enable();
+	migrate_enable();
 }
 
 /**
-- 
1.7.2.5

Re: [PATCH RT 4/5] allow preemption in mem_cgroup_move_account_page_stat

[Sebastian Andrzej Siewior]

* Nicholas Mc Guire | 2014-02-10 16:39:33 [+0100]:

>
>allow preemption in mem_cgroup_move_account_page_stat
>
>as the call sites of mem_cgroup_move_account_page_stat are under 
>move_lock_mem_cgroup move_unlock_mem_cgroup (spin_lock_irqsave/
>spin_unlock_irqrestore respectively) the move operations 
>preempt_disable/enable seem to be purely for migration protection so 
>a migrate_disable/enable should be fine here.
>
>patch on top of 3.12.10-rt15
>
>Signed-off-by: Nicholas Mc Guire <der.herr@hofr.at>

Applied.

Sebastian

Re: [PATCH RT 4/5] allow preemption in mem_cgroup_move_account_page_stat

[Sebastian Andrzej Siewior]

* Nicholas Mc Guire | 2014-02-10 16:39:33 [+0100]:

>index a67e630f..e7cc35a 100644
>--- a/mm/memcontrol.c
>+++ b/mm/memcontrol.c
>@@ -3784,10 +3784,10 @@ void mem_cgroup_move_account_page_stat(struct mem_cgroup *from,
> 					enum mem_cgroup_stat_index idx)
> {
> 	/* Update stat data for mem_cgroup */
>-	preempt_disable();
>+	migrate_disable();
> 	__this_cpu_sub(from->stat->count[idx], nr_pages);
> 	__this_cpu_add(to->stat->count[idx], nr_pages);
>-	preempt_enable();
>+	migrate_enable();
> }

Now, that I look at it again. Isn't it more efficient to keep preemption
disabled for this very short time instead instead of invoking
migrate_disable() which includes preempt_disable()/enable() and a few
more opcodes…
Therefore, I drop it again.

Sebastian

Re: [PATCH RT 4/5] allow preemption in mem_cgroup_move_account_page_stat

[Sebastian Andrzej Siewior]

* Sebastian Andrzej Siewior | 2014-02-14 14:17:05 [+0100]:

>Now, that I look at it again. Isn't it more efficient to keep preemption
>disabled for this very short time instead instead of invoking
>migrate_disable() which includes preempt_disable()/enable() and a few
>more opcodes…
>Therefore, I drop it again.

I think doing the same for the 1/5 of this series since it also disables
migration for a handfull of asm opcodes.

Sebastian

Re: [PATCH RT 4/5] allow preemption in mem_cgroup_move_account_page_stat

[Nicholas Mc Guire]

On Fri, 14 Feb 2014, Sebastian Andrzej Siewior wrote:

> * Sebastian Andrzej Siewior | 2014-02-14 14:17:05 [+0100]:
> 
> >Now, that I look at it again. Isn't it more efficient to keep preemption
> >disabled for this very short time instead instead of invoking
> >migrate_disable() which includes preempt_disable()/enable() and a few
> >more opcodes???
> >Therefore, I drop it again.
> 
> I think doing the same for the 1/5 of this series since it also disables
> migration for a handfull of asm opcodes.
>
yup - nice run in the wrong direction :)
those short pushdowns make no sense - should have looked at that first...
the execution length/complexity (notably worst case on migrate_enable()) is
actually very high compared to a preempt_enable.

thx!
hofrat