[PATCH] sched: remove redundant update_runtime notifier

[PATCH] sched: remove redundant update_runtime notifier

[Neil Zhang]

migration_call() will do all the things that update_runtime() does.
So it seems update_runtime() is a redundant notifier, remove it.

Furthermore, there is potential risk that the current code will catch
BUG_ON at line 687 of rt.c when do cpu hotplug while there are realtime
threads running because of enable runtime twice.

Cc: bitbucket@online.de
Signed-off-by: Neil Zhang <zhangwm@marvell.com>
---
 kernel/sched/core.c  |    3 ---
 kernel/sched/rt.c    |   40 ----------------------------------------
 kernel/sched/sched.h |    1 -
 3 files changed, 0 insertions(+), 44 deletions(-)

diff --git a/kernel/sched/core.c b/kernel/sched/core.c
index 257002c..7b6a4d8 100644
--- a/kernel/sched/core.c
+++ b/kernel/sched/core.c
@@ -6779,9 +6779,6 @@ void __init sched_init_smp(void)
 	hotcpu_notifier(cpuset_cpu_active, CPU_PRI_CPUSET_ACTIVE);
 	hotcpu_notifier(cpuset_cpu_inactive, CPU_PRI_CPUSET_INACTIVE);
 
-	/* RT runtime code needs to handle some hotplug events */
-	hotcpu_notifier(update_runtime, 0);
-
 	init_hrtick();
 
 	/* Move init over to a non-isolated CPU */
diff --git a/kernel/sched/rt.c b/kernel/sched/rt.c
index 418feb0..1a499d2 100644
--- a/kernel/sched/rt.c
+++ b/kernel/sched/rt.c
@@ -697,15 +697,6 @@ balanced:
 	}
 }
 
-static void disable_runtime(struct rq *rq)
-{
-	unsigned long flags;
-
-	raw_spin_lock_irqsave(&rq->lock, flags);
-	__disable_runtime(rq);
-	raw_spin_unlock_irqrestore(&rq->lock, flags);
-}
-
 static void __enable_runtime(struct rq *rq)
 {
 	rt_rq_iter_t iter;
@@ -730,37 +721,6 @@ static void __enable_runtime(struct rq *rq)
 	}
 }
 
-static void enable_runtime(struct rq *rq)
-{
-	unsigned long flags;
-
-	raw_spin_lock_irqsave(&rq->lock, flags);
-	__enable_runtime(rq);
-	raw_spin_unlock_irqrestore(&rq->lock, flags);
-}
-
-int update_runtime(struct notifier_block *nfb, unsigned long action, void *hcpu)
-{
-	int cpu = (int)(long)hcpu;
-
-	switch (action) {
-	case CPU_DOWN_PREPARE:
-	case CPU_DOWN_PREPARE_FROZEN:
-		disable_runtime(cpu_rq(cpu));
-		return NOTIFY_OK;
-
-	case CPU_DOWN_FAILED:
-	case CPU_DOWN_FAILED_FROZEN:
-	case CPU_ONLINE:
-	case CPU_ONLINE_FROZEN:
-		enable_runtime(cpu_rq(cpu));
-		return NOTIFY_OK;
-
-	default:
-		return NOTIFY_DONE;
-	}
-}
-
 static int balance_runtime(struct rt_rq *rt_rq)
 {
 	int more = 0;
diff --git a/kernel/sched/sched.h b/kernel/sched/sched.h
index fc88644..16d18a1 100644
--- a/kernel/sched/sched.h
+++ b/kernel/sched/sched.h
@@ -890,7 +890,6 @@ extern void sysrq_sched_debug_show(void);
 extern void sched_init_granularity(void);
 extern void update_max_interval(void);
 extern void update_group_power(struct sched_domain *sd, int cpu);
-extern int update_runtime(struct notifier_block *nfb, unsigned long action, void *hcpu);
 extern void init_sched_rt_class(void);
 extern void init_sched_fair_class(void);
 
-- 
1.7.4.1

RE: [PATCH] sched: remove redundant update_runtime notifier

[Neil Zhang]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset="gb2312", Size: 3518 bytes --]

All,

> -----Original Message-----
> From: Neil Zhang [mailto:zhangwm@marvell.com]
> Sent: 2012Äê12ÔÂ28ÈÕ 18:00
> To: mingo@kernel.org; peterz@infradead.org
> Cc: linux-kernel@vger.kernel.org; Neil Zhang; bitbucket@online.de
> Subject: [PATCH] sched: remove redundant update_runtime notifier
> 
> migration_call() will do all the things that update_runtime() does.
> So it seems update_runtime() is a redundant notifier, remove it.
> 
> Furthermore, there is potential risk that the current code will catch BUG_ON
> at line 687 of rt.c when do cpu hotplug while there are realtime threads
> running because of enable runtime twice.
> 
> Cc: bitbucket@online.de
> Signed-off-by: Neil Zhang <zhangwm@marvell.com>
> ---
>  kernel/sched/core.c  |    3 ---
>  kernel/sched/rt.c    |   40 ----------------------------------------
>  kernel/sched/sched.h |    1 -
>  3 files changed, 0 insertions(+), 44 deletions(-)
> 
> diff --git a/kernel/sched/core.c b/kernel/sched/core.c index
> 257002c..7b6a4d8 100644
> --- a/kernel/sched/core.c
> +++ b/kernel/sched/core.c
> @@ -6779,9 +6779,6 @@ void __init sched_init_smp(void)
>  	hotcpu_notifier(cpuset_cpu_active, CPU_PRI_CPUSET_ACTIVE);
>  	hotcpu_notifier(cpuset_cpu_inactive, CPU_PRI_CPUSET_INACTIVE);
> 
> -	/* RT runtime code needs to handle some hotplug events */
> -	hotcpu_notifier(update_runtime, 0);
> -
>  	init_hrtick();
> 
>  	/* Move init over to a non-isolated CPU */ diff --git a/kernel/sched/rt.c
> b/kernel/sched/rt.c index 418feb0..1a499d2 100644
> --- a/kernel/sched/rt.c
> +++ b/kernel/sched/rt.c
> @@ -697,15 +697,6 @@ balanced:
>  	}
>  }
> 
> -static void disable_runtime(struct rq *rq) -{
> -	unsigned long flags;
> -
> -	raw_spin_lock_irqsave(&rq->lock, flags);
> -	__disable_runtime(rq);
> -	raw_spin_unlock_irqrestore(&rq->lock, flags);
> -}
> -
>  static void __enable_runtime(struct rq *rq)  {
>  	rt_rq_iter_t iter;
> @@ -730,37 +721,6 @@ static void __enable_runtime(struct rq *rq)
>  	}
>  }
> 
> -static void enable_runtime(struct rq *rq) -{
> -	unsigned long flags;
> -
> -	raw_spin_lock_irqsave(&rq->lock, flags);
> -	__enable_runtime(rq);
> -	raw_spin_unlock_irqrestore(&rq->lock, flags);
> -}
> -
> -int update_runtime(struct notifier_block *nfb, unsigned long action, void
> *hcpu) -{
> -	int cpu = (int)(long)hcpu;
> -
> -	switch (action) {
> -	case CPU_DOWN_PREPARE:
> -	case CPU_DOWN_PREPARE_FROZEN:
> -		disable_runtime(cpu_rq(cpu));
> -		return NOTIFY_OK;
> -
> -	case CPU_DOWN_FAILED:
> -	case CPU_DOWN_FAILED_FROZEN:
> -	case CPU_ONLINE:
> -	case CPU_ONLINE_FROZEN:
> -		enable_runtime(cpu_rq(cpu));
> -		return NOTIFY_OK;
> -
> -	default:
> -		return NOTIFY_DONE;
> -	}
> -}
> -
>  static int balance_runtime(struct rt_rq *rt_rq)  {
>  	int more = 0;
> diff --git a/kernel/sched/sched.h b/kernel/sched/sched.h index
> fc88644..16d18a1 100644
> --- a/kernel/sched/sched.h
> +++ b/kernel/sched/sched.h
> @@ -890,7 +890,6 @@ extern void sysrq_sched_debug_show(void);
> extern void sched_init_granularity(void);  extern void
> update_max_interval(void);  extern void update_group_power(struct
> sched_domain *sd, int cpu); -extern int update_runtime(struct notifier_block
> *nfb, unsigned long action, void *hcpu);  extern void
> init_sched_rt_class(void);  extern void init_sched_fair_class(void);
> 
> --
> 1.7.4.1

Any comments about this patch?

Best Regards,
Neil Zhang
ÿôèº{.nÇ+‰·Ÿ®‰­†+%ŠËÿ±éݶ\x17¥Šwÿº{.nÇ+‰·¥Š{±þG«éÿŠ{ayº\x1dʇڙë,j\a­¢f£¢·hšïêÿ‘êçz_è®\x03(­éšŽŠÝ¢j"ú\x1a¶^[m§ÿÿ¾\a«þG«éÿ¢¸?™¨è­Ú&£ø§~á¶iO•æ¬z·švØ^\x14\x04\x1a¶^[m§ÿÿÃ\fÿ¶ìÿ¢¸?–I¥

Re: [PATCH] sched: remove redundant update_runtime notifier

[Mike Galbraith]

On Fri, 2012-12-28 at 18:00 +0800, Neil Zhang wrote: 
> migration_call() will do all the things that update_runtime() does.
> So it seems update_runtime() is a redundant notifier, remove it.
> 
> Furthermore, there is potential risk that the current code will catch
> BUG_ON at line 687 of rt.c when do cpu hotplug while there are realtime
> threads running because of enable runtime twice.

Hm, box must be reading lkml behind my back.

PID: 14735  TASK: ffff880e75e86500  CPU: 4   COMMAND: "reboot"
 #0 [ffff880e74b3d890] machine_kexec at ffffffff8102676e
 #1 [ffff880e74b3d8e0] crash_kexec at ffffffff810a3a3a
 #2 [ffff880e74b3d9b0] oops_end at ffffffff81446238
 #3 [ffff880e74b3d9d0] do_invalid_op at ffffffff810035d4
 #4 [ffff880e74b3da70] invalid_op at ffffffff8144d9bb
    [exception RIP: __disable_runtime+494] <== BUG_ON(want);
    RIP: ffffffff81045dae  RSP: ffff880e74b3db28  RFLAGS: 00010082
    RAX: 000000000000046a  RBX: ffff880e7fcd1310  RCX: ffffffff81d88a90
    RDX: 000000000000046a  RSI: 0000000000000050  RDI: ffff880e7fcd19b0
    RBP: ffff880e74b3db98   R8: 0000000000000010   R9: ffff88047f423408
    R10: 0000000000000050  R11: ffffffff81250190  R12: 0000000000000050
    R13: fffffffffde9f140  R14: ffff880e7fcd1310  R15: 000000000000122f
    ORIG_RAX: ffffffffffffffff  CS: 0010  SS: 0018
 #5 [ffff880e74b3dba0] rq_offline_rt at ffffffff8104b36a
 #6 [ffff880e74b3dbc0] set_rq_offline at ffffffff81043c9e
 #7 [ffff880e74b3dbe0] rq_attach_root at ffffffff8104c130
 #8 [ffff880e74b3dc10] cpu_attach_domain at ffffffff81054f44
 #9 [ffff880e74b3dc80] build_sched_domains at ffffffff81055525
#10 [ffff880e74b3dcd0] partition_sched_domains at ffffffff81055835
#11 [ffff880e74b3dd70] cpuset_cpu_inactive at ffffffff81055a0d
#12 [ffff880e74b3dd80] notifier_call_chain at ffffffff81448907
#13 [ffff880e74b3ddb0] _cpu_down at ffffffff8142b57b
#14 [ffff880e74b3de10] disable_nonboot_cpus at ffffffff8105ba23
#15 [ffff880e74b3de40] kernel_restart at ffffffff81071dde
#16 [ffff880e74b3de50] sys_reboot at ffffffff81071fc3
#17 [ffff880e74b3df80] system_call_fastpath at ffffffff8144ca12
    RIP: 00007fed35778316  RSP: 00007fff23f7e198  RFLAGS: 00010202
    RAX: 00000000000000a9  RBX: ffffffff8144ca12  RCX: 00007fed35771130
    RDX: 0000000001234567  RSI: 0000000028121969  RDI: fffffffffee1dead
    RBP: 0000000000000000   R8: 0000000000020fd0   R9: 000000000060d120
    R10: 00007fff23f7df40  R11: 0000000000000202  R12: 0000000000000000
    R13: 0000000000000000  R14: 0000000000000001  R15: 0000000000000000
    ORIG_RAX: 00000000000000a9  CS: 0033  SS: 002b

> Cc: bitbucket@online.de
> Signed-off-by: Neil Zhang <zhangwm@marvell.com>
> ---
>  kernel/sched/core.c  |    3 ---
>  kernel/sched/rt.c    |   40 ----------------------------------------
>  kernel/sched/sched.h |    1 -
>  3 files changed, 0 insertions(+), 44 deletions(-)
> 
> diff --git a/kernel/sched/core.c b/kernel/sched/core.c
> index 257002c..7b6a4d8 100644
> --- a/kernel/sched/core.c
> +++ b/kernel/sched/core.c
> @@ -6779,9 +6779,6 @@ void __init sched_init_smp(void)
>  	hotcpu_notifier(cpuset_cpu_active, CPU_PRI_CPUSET_ACTIVE);
>  	hotcpu_notifier(cpuset_cpu_inactive, CPU_PRI_CPUSET_INACTIVE);
>  
> -	/* RT runtime code needs to handle some hotplug events */
> -	hotcpu_notifier(update_runtime, 0);
> -
>  	init_hrtick();
>  
>  	/* Move init over to a non-isolated CPU */
> diff --git a/kernel/sched/rt.c b/kernel/sched/rt.c
> index 418feb0..1a499d2 100644
> --- a/kernel/sched/rt.c
> +++ b/kernel/sched/rt.c
> @@ -697,15 +697,6 @@ balanced:
>  	}
>  }
>  
> -static void disable_runtime(struct rq *rq)
> -{
> -	unsigned long flags;
> -
> -	raw_spin_lock_irqsave(&rq->lock, flags);
> -	__disable_runtime(rq);
> -	raw_spin_unlock_irqrestore(&rq->lock, flags);
> -}
> -
>  static void __enable_runtime(struct rq *rq)
>  {
>  	rt_rq_iter_t iter;
> @@ -730,37 +721,6 @@ static void __enable_runtime(struct rq *rq)
>  	}
>  }
>  
> -static void enable_runtime(struct rq *rq)
> -{
> -	unsigned long flags;
> -
> -	raw_spin_lock_irqsave(&rq->lock, flags);
> -	__enable_runtime(rq);
> -	raw_spin_unlock_irqrestore(&rq->lock, flags);
> -}
> -
> -int update_runtime(struct notifier_block *nfb, unsigned long action, void *hcpu)
> -{
> -	int cpu = (int)(long)hcpu;
> -
> -	switch (action) {
> -	case CPU_DOWN_PREPARE:
> -	case CPU_DOWN_PREPARE_FROZEN:
> -		disable_runtime(cpu_rq(cpu));
> -		return NOTIFY_OK;
> -
> -	case CPU_DOWN_FAILED:
> -	case CPU_DOWN_FAILED_FROZEN:
> -	case CPU_ONLINE:
> -	case CPU_ONLINE_FROZEN:
> -		enable_runtime(cpu_rq(cpu));
> -		return NOTIFY_OK;
> -
> -	default:
> -		return NOTIFY_DONE;
> -	}
> -}
> -
>  static int balance_runtime(struct rt_rq *rt_rq)
>  {
>  	int more = 0;
> diff --git a/kernel/sched/sched.h b/kernel/sched/sched.h
> index fc88644..16d18a1 100644
> --- a/kernel/sched/sched.h
> +++ b/kernel/sched/sched.h
> @@ -890,7 +890,6 @@ extern void sysrq_sched_debug_show(void);
>  extern void sched_init_granularity(void);
>  extern void update_max_interval(void);
>  extern void update_group_power(struct sched_domain *sd, int cpu);
> -extern int update_runtime(struct notifier_block *nfb, unsigned long action, void *hcpu);
>  extern void init_sched_rt_class(void);
>  extern void init_sched_fair_class(void);
>  

RE: [PATCH] sched: remove redundant update_runtime notifier

[Neil Zhang]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset="utf-8", Size: 6114 bytes --]

Mike,


> -----Original Message-----
> From: Mike Galbraith [mailto:bitbucket@online.de]
> Sent: 2013年1月7日 18:40
> To: Neil Zhang
> Cc: mingo@kernel.org; peterz@infradead.org; linux-kernel@vger.kernel.org
> Subject: Re: [PATCH] sched: remove redundant update_runtime notifier
> 
> On Fri, 2012-12-28 at 18:00 +0800, Neil Zhang wrote:
> > migration_call() will do all the things that update_runtime() does.
> > So it seems update_runtime() is a redundant notifier, remove it.
> >
> > Furthermore, there is potential risk that the current code will catch
> > BUG_ON at line 687 of rt.c when do cpu hotplug while there are
> > realtime threads running because of enable runtime twice.
> 
> Hm, box must be reading lkml behind my back.
> 
> PID: 14735  TASK: ffff880e75e86500  CPU: 4   COMMAND: "reboot"
>  #0 [ffff880e74b3d890] machine_kexec at ffffffff8102676e
>  #1 [ffff880e74b3d8e0] crash_kexec at ffffffff810a3a3a
>  #2 [ffff880e74b3d9b0] oops_end at ffffffff81446238
>  #3 [ffff880e74b3d9d0] do_invalid_op at ffffffff810035d4
>  #4 [ffff880e74b3da70] invalid_op at ffffffff8144d9bb
>     [exception RIP: __disable_runtime+494] <== BUG_ON(want);
>     RIP: ffffffff81045dae  RSP: ffff880e74b3db28  RFLAGS: 00010082
>     RAX: 000000000000046a  RBX: ffff880e7fcd1310  RCX:
> ffffffff81d88a90
>     RDX: 000000000000046a  RSI: 0000000000000050  RDI:
> ffff880e7fcd19b0
>     RBP: ffff880e74b3db98   R8: 0000000000000010   R9:
> ffff88047f423408
>     R10: 0000000000000050  R11: ffffffff81250190  R12:
> 0000000000000050
>     R13: fffffffffde9f140  R14: ffff880e7fcd1310  R15:
> 000000000000122f
>     ORIG_RAX: ffffffffffffffff  CS: 0010  SS: 0018
>  #5 [ffff880e74b3dba0] rq_offline_rt at ffffffff8104b36a
>  #6 [ffff880e74b3dbc0] set_rq_offline at ffffffff81043c9e
>  #7 [ffff880e74b3dbe0] rq_attach_root at ffffffff8104c130
>  #8 [ffff880e74b3dc10] cpu_attach_domain at ffffffff81054f44
>  #9 [ffff880e74b3dc80] build_sched_domains at ffffffff81055525
> #10 [ffff880e74b3dcd0] partition_sched_domains at ffffffff81055835
> #11 [ffff880e74b3dd70] cpuset_cpu_inactive at ffffffff81055a0d
> #12 [ffff880e74b3dd80] notifier_call_chain at ffffffff81448907
> #13 [ffff880e74b3ddb0] _cpu_down at ffffffff8142b57b
> #14 [ffff880e74b3de10] disable_nonboot_cpus at ffffffff8105ba23
> #15 [ffff880e74b3de40] kernel_restart at ffffffff81071dde
> #16 [ffff880e74b3de50] sys_reboot at ffffffff81071fc3
> #17 [ffff880e74b3df80] system_call_fastpath at ffffffff8144ca12
>     RIP: 00007fed35778316  RSP: 00007fff23f7e198  RFLAGS: 00010202
>     RAX: 00000000000000a9  RBX: ffffffff8144ca12  RCX:
> 00007fed35771130
>     RDX: 0000000001234567  RSI: 0000000028121969  RDI:
> fffffffffee1dead
>     RBP: 0000000000000000   R8: 0000000000020fd0   R9:
> 000000000060d120
>     R10: 00007fff23f7df40  R11: 0000000000000202  R12:
> 0000000000000000
>     R13: 0000000000000000  R14: 0000000000000001  R15:
> 0000000000000000
>     ORIG_RAX: 00000000000000a9  CS: 0033  SS: 002b
> 

So, it seems my first patch is still needed, we should avoid enable runtime twice in case the value is changed between the two times.

> > Cc: bitbucket@online.de
> > Signed-off-by: Neil Zhang <zhangwm@marvell.com>
> > ---
> >  kernel/sched/core.c  |    3 ---
> >  kernel/sched/rt.c    |   40 ----------------------------------------
> >  kernel/sched/sched.h |    1 -
> >  3 files changed, 0 insertions(+), 44 deletions(-)
> >
> > diff --git a/kernel/sched/core.c b/kernel/sched/core.c index
> > 257002c..7b6a4d8 100644
> > --- a/kernel/sched/core.c
> > +++ b/kernel/sched/core.c
> > @@ -6779,9 +6779,6 @@ void __init sched_init_smp(void)
> >  	hotcpu_notifier(cpuset_cpu_active, CPU_PRI_CPUSET_ACTIVE);
> >  	hotcpu_notifier(cpuset_cpu_inactive, CPU_PRI_CPUSET_INACTIVE);
> >
> > -	/* RT runtime code needs to handle some hotplug events */
> > -	hotcpu_notifier(update_runtime, 0);
> > -
> >  	init_hrtick();
> >
> >  	/* Move init over to a non-isolated CPU */ diff --git
> > a/kernel/sched/rt.c b/kernel/sched/rt.c index 418feb0..1a499d2 100644
> > --- a/kernel/sched/rt.c
> > +++ b/kernel/sched/rt.c
> > @@ -697,15 +697,6 @@ balanced:
> >  	}
> >  }
> >
> > -static void disable_runtime(struct rq *rq) -{
> > -	unsigned long flags;
> > -
> > -	raw_spin_lock_irqsave(&rq->lock, flags);
> > -	__disable_runtime(rq);
> > -	raw_spin_unlock_irqrestore(&rq->lock, flags);
> > -}
> > -
> >  static void __enable_runtime(struct rq *rq)  {
> >  	rt_rq_iter_t iter;
> > @@ -730,37 +721,6 @@ static void __enable_runtime(struct rq *rq)
> >  	}
> >  }
> >
> > -static void enable_runtime(struct rq *rq) -{
> > -	unsigned long flags;
> > -
> > -	raw_spin_lock_irqsave(&rq->lock, flags);
> > -	__enable_runtime(rq);
> > -	raw_spin_unlock_irqrestore(&rq->lock, flags);
> > -}
> > -
> > -int update_runtime(struct notifier_block *nfb, unsigned long action,
> > void *hcpu) -{
> > -	int cpu = (int)(long)hcpu;
> > -
> > -	switch (action) {
> > -	case CPU_DOWN_PREPARE:
> > -	case CPU_DOWN_PREPARE_FROZEN:
> > -		disable_runtime(cpu_rq(cpu));
> > -		return NOTIFY_OK;
> > -
> > -	case CPU_DOWN_FAILED:
> > -	case CPU_DOWN_FAILED_FROZEN:
> > -	case CPU_ONLINE:
> > -	case CPU_ONLINE_FROZEN:
> > -		enable_runtime(cpu_rq(cpu));
> > -		return NOTIFY_OK;
> > -
> > -	default:
> > -		return NOTIFY_DONE;
> > -	}
> > -}
> > -
> >  static int balance_runtime(struct rt_rq *rt_rq)  {
> >  	int more = 0;
> > diff --git a/kernel/sched/sched.h b/kernel/sched/sched.h index
> > fc88644..16d18a1 100644
> > --- a/kernel/sched/sched.h
> > +++ b/kernel/sched/sched.h
> > @@ -890,7 +890,6 @@ extern void sysrq_sched_debug_show(void);
> extern
> > void sched_init_granularity(void);  extern void
> > update_max_interval(void);  extern void update_group_power(struct
> > sched_domain *sd, int cpu); -extern int update_runtime(struct
> > notifier_block *nfb, unsigned long action, void *hcpu);  extern void
> > init_sched_rt_class(void);  extern void init_sched_fair_class(void);
> >
> 


Best Regards,
Neil Zhang
ÿôèº{.nÇ+‰·Ÿ®‰­†+%ŠËÿ±éݶ\x17¥Šwÿº{.nÇ+‰·¥Š{±þG«éÿŠ{ayº\x1dʇڙë,j\a­¢f£¢·hšïêÿ‘êçz_è®\x03(­éšŽŠÝ¢j"ú\x1a¶^[m§ÿÿ¾\a«þG«éÿ¢¸?™¨è­Ú&£ø§~á¶iO•æ¬z·švØ^\x14\x04\x1a¶^[m§ÿÿÃ\fÿ¶ìÿ¢¸?–I¥

[PATCH] sched: remove redundant update_runtime notifier

[Neil Zhang]

migration_call() will do all the things that update_runtime() does.
So let's remove it.

Furthermore, there is potential risk that the current code will catch
BUG_ON at line 689 of rt.c when do cpu hotplug while there are realtime
threads running because of enabling runtime twice while the rt_runtime
may already changed.

Signed-off-by: Neil Zhang <zhangwm@marvell.com>
---
 kernel/sched/core.c  |    3 ---
 kernel/sched/rt.c    |   40 ----------------------------------------
 kernel/sched/sched.h |    1 -
 3 files changed, 0 insertions(+), 44 deletions(-)

diff --git a/kernel/sched/core.c b/kernel/sched/core.c
index 7f12624..3719c3c 100644
--- a/kernel/sched/core.c
+++ b/kernel/sched/core.c
@@ -6831,9 +6831,6 @@ void __init sched_init_smp(void)
 	hotcpu_notifier(cpuset_cpu_active, CPU_PRI_CPUSET_ACTIVE);
 	hotcpu_notifier(cpuset_cpu_inactive, CPU_PRI_CPUSET_INACTIVE);
 
-	/* RT runtime code needs to handle some hotplug events */
-	hotcpu_notifier(update_runtime, 0);
-
 	init_hrtick();
 
 	/* Move init over to a non-isolated CPU */
diff --git a/kernel/sched/rt.c b/kernel/sched/rt.c
index 127a2c4..111539a 100644
--- a/kernel/sched/rt.c
+++ b/kernel/sched/rt.c
@@ -699,15 +699,6 @@ balanced:
 	}
 }
 
-static void disable_runtime(struct rq *rq)
-{
-	unsigned long flags;
-
-	raw_spin_lock_irqsave(&rq->lock, flags);
-	__disable_runtime(rq);
-	raw_spin_unlock_irqrestore(&rq->lock, flags);
-}
-
 static void __enable_runtime(struct rq *rq)
 {
 	rt_rq_iter_t iter;
@@ -732,37 +723,6 @@ static void __enable_runtime(struct rq *rq)
 	}
 }
 
-static void enable_runtime(struct rq *rq)
-{
-	unsigned long flags;
-
-	raw_spin_lock_irqsave(&rq->lock, flags);
-	__enable_runtime(rq);
-	raw_spin_unlock_irqrestore(&rq->lock, flags);
-}
-
-int update_runtime(struct notifier_block *nfb, unsigned long action, void *hcpu)
-{
-	int cpu = (int)(long)hcpu;
-
-	switch (action) {
-	case CPU_DOWN_PREPARE:
-	case CPU_DOWN_PREPARE_FROZEN:
-		disable_runtime(cpu_rq(cpu));
-		return NOTIFY_OK;
-
-	case CPU_DOWN_FAILED:
-	case CPU_DOWN_FAILED_FROZEN:
-	case CPU_ONLINE:
-	case CPU_ONLINE_FROZEN:
-		enable_runtime(cpu_rq(cpu));
-		return NOTIFY_OK;
-
-	default:
-		return NOTIFY_DONE;
-	}
-}
-
 static int balance_runtime(struct rt_rq *rt_rq)
 {
 	int more = 0;
diff --git a/kernel/sched/sched.h b/kernel/sched/sched.h
index cc03cfd..e73a8bd 100644
--- a/kernel/sched/sched.h
+++ b/kernel/sched/sched.h
@@ -892,7 +892,6 @@ extern void sysrq_sched_debug_show(void);
 extern void sched_init_granularity(void);
 extern void update_max_interval(void);
 extern void update_group_power(struct sched_domain *sd, int cpu);
-extern int update_runtime(struct notifier_block *nfb, unsigned long action, void *hcpu);
 extern void init_sched_rt_class(void);
 extern void init_sched_fair_class(void);
 
-- 
1.7.4.1

Re: [PATCH] sched: remove redundant update_runtime notifier

[Peter Zijlstra]

On Thu, Apr 11, 2013 at 09:04:59PM +0800, Neil Zhang wrote:
> migration_call() will do all the things that update_runtime() does.
> So let's remove it.
> 
> Furthermore, there is potential risk that the current code will catch
> BUG_ON at line 689 of rt.c when do cpu hotplug while there are realtime
> threads running because of enabling runtime twice while the rt_runtime
> may already changed.
> 

Indeed, it looks like the 'new' sched_class::rq_{on,off}line stuff handles all
that just fine.

Thanks for spotting that.

[tip:sched/core] sched: Remove redundant update_runtime notifier

[tip-bot for Neil Zhang]

Commit-ID:  c5405a495e88d93cf9b4f4cc91507c7f4afcb901
Gitweb:     http://git.kernel.org/tip/c5405a495e88d93cf9b4f4cc91507c7f4afcb901
Author:     Neil Zhang <zhangwm@marvell.com>
AuthorDate: Thu, 11 Apr 2013 21:04:59 +0800
Committer:  Ingo Molnar <mingo@kernel.org>
CommitDate: Tue, 28 May 2013 09:40:22 +0200

sched: Remove redundant update_runtime notifier

migration_call() will do all the things that update_runtime() does.
So let's remove it.

Furthermore, there is potential risk that the current code will catch
BUG_ON at line 689 of rt.c when do cpu hotplug while there are realtime
threads running because of enabling runtime twice while the rt_runtime
may already changed.

Signed-off-by: Neil Zhang <zhangwm@marvell.com>
Signed-off-by: Peter Zijlstra <peterz@infradead.org>
Link: http://lkml.kernel.org/r/1365685499-26515-1-git-send-email-zhangwm@marvell.com
Signed-off-by: Ingo Molnar <mingo@kernel.org>
---
 kernel/sched/core.c  |  3 ---
 kernel/sched/rt.c    | 40 ----------------------------------------
 kernel/sched/sched.h |  1 -
 3 files changed, 44 deletions(-)

diff --git a/kernel/sched/core.c b/kernel/sched/core.c
index bfa7e77..79e48e6 100644
--- a/kernel/sched/core.c
+++ b/kernel/sched/core.c
@@ -6285,9 +6285,6 @@ void __init sched_init_smp(void)
 	hotcpu_notifier(cpuset_cpu_active, CPU_PRI_CPUSET_ACTIVE);
 	hotcpu_notifier(cpuset_cpu_inactive, CPU_PRI_CPUSET_INACTIVE);
 
-	/* RT runtime code needs to handle some hotplug events */
-	hotcpu_notifier(update_runtime, 0);
-
 	init_hrtick();
 
 	/* Move init over to a non-isolated CPU */
diff --git a/kernel/sched/rt.c b/kernel/sched/rt.c
index 7aced2e..8853ab1 100644
--- a/kernel/sched/rt.c
+++ b/kernel/sched/rt.c
@@ -699,15 +699,6 @@ balanced:
 	}
 }
 
-static void disable_runtime(struct rq *rq)
-{
-	unsigned long flags;
-
-	raw_spin_lock_irqsave(&rq->lock, flags);
-	__disable_runtime(rq);
-	raw_spin_unlock_irqrestore(&rq->lock, flags);
-}
-
 static void __enable_runtime(struct rq *rq)
 {
 	rt_rq_iter_t iter;
@@ -732,37 +723,6 @@ static void __enable_runtime(struct rq *rq)
 	}
 }
 
-static void enable_runtime(struct rq *rq)
-{
-	unsigned long flags;
-
-	raw_spin_lock_irqsave(&rq->lock, flags);
-	__enable_runtime(rq);
-	raw_spin_unlock_irqrestore(&rq->lock, flags);
-}
-
-int update_runtime(struct notifier_block *nfb, unsigned long action, void *hcpu)
-{
-	int cpu = (int)(long)hcpu;
-
-	switch (action) {
-	case CPU_DOWN_PREPARE:
-	case CPU_DOWN_PREPARE_FROZEN:
-		disable_runtime(cpu_rq(cpu));
-		return NOTIFY_OK;
-
-	case CPU_DOWN_FAILED:
-	case CPU_DOWN_FAILED_FROZEN:
-	case CPU_ONLINE:
-	case CPU_ONLINE_FROZEN:
-		enable_runtime(cpu_rq(cpu));
-		return NOTIFY_OK;
-
-	default:
-		return NOTIFY_DONE;
-	}
-}
-
 static int balance_runtime(struct rt_rq *rt_rq)
 {
 	int more = 0;
diff --git a/kernel/sched/sched.h b/kernel/sched/sched.h
index f1f6256..c806c61 100644
--- a/kernel/sched/sched.h
+++ b/kernel/sched/sched.h
@@ -1041,7 +1041,6 @@ static inline void idle_balance(int cpu, struct rq *rq)
 extern void sysrq_sched_debug_show(void);
 extern void sched_init_granularity(void);
 extern void update_max_interval(void);
-extern int update_runtime(struct notifier_block *nfb, unsigned long action, void *hcpu);
 extern void init_sched_rt_class(void);
 extern void init_sched_fair_class(void);