* [PATCH] sched/deadline: No need to check NULL later_mask @ 2016-04-01 11:13 Xunlei Pang 2016-04-01 11:29 ` Peter Zijlstra 0 siblings, 1 reply; 7+ messages in thread From: Xunlei Pang @ 2016-04-01 11:13 UTC (permalink / raw) To: linux-kernel Cc: Peter Zijlstra, Juri Lelli, Ingo Molnar, Steven Rostedt, Xunlei Pang Unlike rt tasks, we (should) have no deadline tasks in booting phase before the mask is allocated, so we can safely remove the check. Signed-off-by: Xunlei Pang <xlpang@redhat.com> --- kernel/sched/deadline.c | 4 ---- 1 file changed, 4 deletions(-) diff --git a/kernel/sched/deadline.c b/kernel/sched/deadline.c index fb98160..7b8aa93 100644 --- a/kernel/sched/deadline.c +++ b/kernel/sched/deadline.c @@ -1295,10 +1295,6 @@ static int find_later_rq(struct task_struct *task) int this_cpu = smp_processor_id(); int best_cpu, cpu = task_cpu(task); - /* Make sure the mask is initialized first */ - if (unlikely(!later_mask)) - return -1; - if (task->nr_cpus_allowed == 1) return -1; -- 1.8.3.1 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH] sched/deadline: No need to check NULL later_mask 2016-04-01 11:13 [PATCH] sched/deadline: No need to check NULL later_mask Xunlei Pang @ 2016-04-01 11:29 ` Peter Zijlstra 2016-04-01 12:16 ` Xunlei Pang 0 siblings, 1 reply; 7+ messages in thread From: Peter Zijlstra @ 2016-04-01 11:29 UTC (permalink / raw) To: Xunlei Pang; +Cc: linux-kernel, Juri Lelli, Ingo Molnar, Steven Rostedt On Fri, Apr 01, 2016 at 07:13:18PM +0800, Xunlei Pang wrote: > Unlike rt tasks, we (should) have no deadline tasks in > booting phase before the mask is allocated, so we can > safely remove the check. Why? And have the kernel explode once it grows an early deadline task? Is there _any_ benefit to this? ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] sched/deadline: No need to check NULL later_mask 2016-04-01 11:29 ` Peter Zijlstra @ 2016-04-01 12:16 ` Xunlei Pang 2016-04-01 16:21 ` Peter Zijlstra 0 siblings, 1 reply; 7+ messages in thread From: Xunlei Pang @ 2016-04-01 12:16 UTC (permalink / raw) To: Peter Zijlstra; +Cc: linux-kernel, Juri Lelli, Ingo Molnar, Steven Rostedt On 2016/04/01 at 19:29, Peter Zijlstra wrote: > On Fri, Apr 01, 2016 at 07:13:18PM +0800, Xunlei Pang wrote: >> Unlike rt tasks, we (should) have no deadline tasks in >> booting phase before the mask is allocated, so we can >> safely remove the check. > Why? And have the kernel explode once it grows an early deadline task? > > Is there _any_ benefit to this? This is a performance-critical path, it'd be better to avoid such a check. I think in the early boot stage before sched_init_smp(), it's weird to use a deadline task, relying on rt tasks should be enough for us. Please ignore it if you don't think so :-) Regards, Xunlei ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] sched/deadline: No need to check NULL later_mask 2016-04-01 12:16 ` Xunlei Pang @ 2016-04-01 16:21 ` Peter Zijlstra 2016-04-02 10:14 ` Xunlei Pang 0 siblings, 1 reply; 7+ messages in thread From: Peter Zijlstra @ 2016-04-01 16:21 UTC (permalink / raw) To: xlpang; +Cc: linux-kernel, Juri Lelli, Ingo Molnar, Steven Rostedt On Fri, Apr 01, 2016 at 08:16:37PM +0800, Xunlei Pang wrote: > On 2016/04/01 at 19:29, Peter Zijlstra wrote: > > On Fri, Apr 01, 2016 at 07:13:18PM +0800, Xunlei Pang wrote: > >> Unlike rt tasks, we (should) have no deadline tasks in > >> booting phase before the mask is allocated, so we can > >> safely remove the check. > > Why? And have the kernel explode once it grows an early deadline task? > > > > Is there _any_ benefit to this? > > This is a performance-critical path, it'd be better to avoid such a check. And the changelog didn't say.. and if its an optimization you should at least attempt numbers or instruction counts or whatnot. > I think in the early boot stage before sched_init_smp(), it's weird to > use a deadline task, relying on rt tasks should be enough for us. You never know. Something like the below should completely avoid the problem though. It uses __initdata storage when coming up and switched to allocated data before we bring up smp. A similar thing could be done to RT.. In fact, both could share the mask, its a temporary mask anyhow, and I'm pretty sure there's more such cpumasks lying about that we can merge. Completely untested... --- diff --git a/kernel/sched/core.c b/kernel/sched/core.c index 11594230ef4d..acdc291577a0 100644 --- a/kernel/sched/core.c +++ b/kernel/sched/core.c @@ -7243,8 +7244,6 @@ void __init sched_init_smp(void) sched_init_granularity(); free_cpumask_var(non_isolated_cpus); - init_sched_rt_class(); - init_sched_dl_class(); } #else void __init sched_init_smp(void) @@ -7444,6 +7443,9 @@ void __init sched_init(void) zalloc_cpumask_var(&cpu_isolated_map, GFP_NOWAIT); idle_thread_set_boot_cpu(); set_cpu_rq_start_time(); + + init_sched_rt_class(); + init_sched_dl_class(); #endif init_sched_fair_class(); diff --git a/kernel/sched/deadline.c b/kernel/sched/deadline.c index affd97ec9f65..24d7dbede99e 100644 --- a/kernel/sched/deadline.c +++ b/kernel/sched/deadline.c @@ -1273,7 +1273,8 @@ static struct task_struct *pick_earliest_pushable_dl_task(struct rq *rq, int cpu return NULL; } -static DEFINE_PER_CPU(cpumask_var_t, local_cpu_mask_dl); +static __initdata struct cpumask __local_mask; +static DEFINE_PER_CPU(cpumask_var_t, local_cpu_mask_dl) = &__local_mask; static int find_later_rq(struct task_struct *task) { @@ -1282,10 +1283,6 @@ static int find_later_rq(struct task_struct *task) int this_cpu = smp_processor_id(); int best_cpu, cpu = task_cpu(task); - /* Make sure the mask is initialized first */ - if (unlikely(!later_mask)) - return -1; - if (task->nr_cpus_allowed == 1) return -1; ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH] sched/deadline: No need to check NULL later_mask 2016-04-01 16:21 ` Peter Zijlstra @ 2016-04-02 10:14 ` Xunlei Pang 2016-04-06 9:30 ` Peter Zijlstra 0 siblings, 1 reply; 7+ messages in thread From: Xunlei Pang @ 2016-04-02 10:14 UTC (permalink / raw) To: Peter Zijlstra, xlpang Cc: linux-kernel, Juri Lelli, Ingo Molnar, Steven Rostedt On 2016/04/02 at 00:21, Peter Zijlstra wrote: > On Fri, Apr 01, 2016 at 08:16:37PM +0800, Xunlei Pang wrote: >> On 2016/04/01 at 19:29, Peter Zijlstra wrote: >>> On Fri, Apr 01, 2016 at 07:13:18PM +0800, Xunlei Pang wrote: >>>> Unlike rt tasks, we (should) have no deadline tasks in >>>> booting phase before the mask is allocated, so we can >>>> safely remove the check. >>> Why? And have the kernel explode once it grows an early deadline task? >>> >>> Is there _any_ benefit to this? >> This is a performance-critical path, it'd be better to avoid such a check. > And the changelog didn't say.. and if its an optimization you should at > least attempt numbers or instruction counts or whatnot. > >> I think in the early boot stage before sched_init_smp(), it's weird to >> use a deadline task, relying on rt tasks should be enough for us. > You never know. > > Something like the below should completely avoid the problem though. > > It uses __initdata storage when coming up and switched to allocated data > before we bring up smp. > > A similar thing could be done to RT.. > > In fact, both could share the mask, its a temporary mask anyhow, and I'm > pretty sure there's more such cpumasks lying about that we can merge. > > Completely untested... > > --- > diff --git a/kernel/sched/core.c b/kernel/sched/core.c > index 11594230ef4d..acdc291577a0 100644 > --- a/kernel/sched/core.c > +++ b/kernel/sched/core.c > @@ -7243,8 +7244,6 @@ void __init sched_init_smp(void) > sched_init_granularity(); > free_cpumask_var(non_isolated_cpus); > > - init_sched_rt_class(); > - init_sched_dl_class(); > } > #else > void __init sched_init_smp(void) > @@ -7444,6 +7443,9 @@ void __init sched_init(void) > zalloc_cpumask_var(&cpu_isolated_map, GFP_NOWAIT); > idle_thread_set_boot_cpu(); > set_cpu_rq_start_time(); > + > + init_sched_rt_class(); > + init_sched_dl_class(); > #endif > init_sched_fair_class(); > > diff --git a/kernel/sched/deadline.c b/kernel/sched/deadline.c > index affd97ec9f65..24d7dbede99e 100644 > --- a/kernel/sched/deadline.c > +++ b/kernel/sched/deadline.c > @@ -1273,7 +1273,8 @@ static struct task_struct *pick_earliest_pushable_dl_task(struct rq *rq, int cpu > return NULL; > } > > -static DEFINE_PER_CPU(cpumask_var_t, local_cpu_mask_dl); > +static __initdata struct cpumask __local_mask; > +static DEFINE_PER_CPU(cpumask_var_t, local_cpu_mask_dl) = &__local_mask; > > static int find_later_rq(struct task_struct *task) > { > @@ -1282,10 +1283,6 @@ static int find_later_rq(struct task_struct *task) > int this_cpu = smp_processor_id(); > int best_cpu, cpu = task_cpu(task); > > - /* Make sure the mask is initialized first */ > - if (unlikely(!later_mask)) > - return -1; > - > if (task->nr_cpus_allowed == 1) > return -1; > Your proposal is very nice! At the sched_init() stage we only have one (to be "idle") task and with irq disabled, no scheduling will happen, and the cpu_possible_mask was already initiated, so it's safe to simply move them there. Also, how about rt&deadline sharing a percpu mask? Because only one of them can use the mask at a moment, operations are always under some spin_lock_irqsave(). I made a new patch below, slightly tested by running tens of rt&dl tasks for a while, are you fine with it? --- diff --git a/kernel/sched/core.c b/kernel/sched/core.c index 0b21e7a..94ae69a 100644 --- a/kernel/sched/core.c +++ b/kernel/sched/core.c @@ -93,6 +93,11 @@ DEFINE_MUTEX(sched_domains_mutex); DEFINE_PER_CPU_SHARED_ALIGNED(struct rq, runqueues); +#ifdef CONFIG_SMP +/* Used by Push/Pull scheduling shared by rt and deadline */ +DEFINE_PER_CPU_ALIGNED(cpumask_var_t, sched_pp_mask); +#endif + static void update_rq_clock_task(struct rq *rq, s64 delta); void update_rq_clock(struct rq *rq) @@ -7172,9 +7177,6 @@ void __init sched_init_smp(void) BUG(); sched_init_granularity(); free_cpumask_var(non_isolated_cpus); - - init_sched_rt_class(); - init_sched_dl_class(); } #else void __init sched_init_smp(void) @@ -7374,6 +7376,11 @@ void __init sched_init(void) zalloc_cpumask_var(&cpu_isolated_map, GFP_NOWAIT); idle_thread_set_boot_cpu(); set_cpu_rq_start_time(); + + for_each_possible_cpu(i) { + zalloc_cpumask_var_node(&per_cpu(sched_pp_mask, i), + GFP_KERNEL, cpu_to_node(i)); + } #endif init_sched_fair_class(); diff --git a/kernel/sched/deadline.c b/kernel/sched/deadline.c index aeca716..a3557f8 100644 --- a/kernel/sched/deadline.c +++ b/kernel/sched/deadline.c @@ -1288,19 +1288,13 @@ next_node: return NULL; } -static DEFINE_PER_CPU(cpumask_var_t, local_cpu_mask_dl); - static int find_later_rq(struct task_struct *task) { struct sched_domain *sd; - struct cpumask *later_mask = this_cpu_cpumask_var_ptr(local_cpu_mask_dl); + struct cpumask *later_mask; int this_cpu = smp_processor_id(); int best_cpu, cpu = task_cpu(task); - /* Make sure the mask is initialized first */ - if (unlikely(!later_mask)) - return -1; - if (task->nr_cpus_allowed == 1) return -1; @@ -1308,6 +1302,7 @@ static int find_later_rq(struct task_struct *task) * We have to consider system topology and task affinity * first, then we can look for a suitable cpu. */ + later_mask = this_cpu_cpumask_var_ptr(sched_pp_mask); best_cpu = cpudl_find(&task_rq(task)->rd->cpudl, task, later_mask); if (best_cpu == -1) @@ -1694,15 +1689,6 @@ static void rq_offline_dl(struct rq *rq) cpudl_clear_freecpu(&rq->rd->cpudl, rq->cpu); } -void __init init_sched_dl_class(void) -{ - unsigned int i; - - for_each_possible_cpu(i) - zalloc_cpumask_var_node(&per_cpu(local_cpu_mask_dl, i), - GFP_KERNEL, cpu_to_node(i)); -} - #endif /* CONFIG_SMP */ static void switched_from_dl(struct rq *rq, struct task_struct *p) diff --git a/kernel/sched/rt.c b/kernel/sched/rt.c index 5624713..3491809 100644 --- a/kernel/sched/rt.c +++ b/kernel/sched/rt.c @@ -1612,22 +1612,17 @@ static struct task_struct *pick_highest_pushable_task(struct rq *rq, int cpu) return NULL; } -static DEFINE_PER_CPU(cpumask_var_t, local_cpu_mask); - static int find_lowest_rq(struct task_struct *task) { struct sched_domain *sd; - struct cpumask *lowest_mask = this_cpu_cpumask_var_ptr(local_cpu_mask); + struct cpumask *lowest_mask; int this_cpu = smp_processor_id(); int cpu = task_cpu(task); - /* Make sure the mask is initialized first */ - if (unlikely(!lowest_mask)) - return -1; - if (task->nr_cpus_allowed == 1) return -1; /* No other targets possible */ + lowest_mask = this_cpu_cpumask_var_ptr(sched_pp_mask); if (!cpupri_find(&task_rq(task)->rd->cpupri, task, lowest_mask)) return -1; /* No targets found */ @@ -2164,16 +2159,6 @@ static void switched_from_rt(struct rq *rq, struct task_struct *p) queue_pull_task(rq); } - -void __init init_sched_rt_class(void) -{ - unsigned int i; - - for_each_possible_cpu(i) { - zalloc_cpumask_var_node(&per_cpu(local_cpu_mask, i), - GFP_KERNEL, cpu_to_node(i)); - } -} #endif /* CONFIG_SMP */ /* diff --git a/kernel/sched/sched.h b/kernel/sched/sched.h index e6d4a3f..b892954 100644 --- a/kernel/sched/sched.h +++ b/kernel/sched/sched.h @@ -715,6 +715,10 @@ static inline int cpu_of(struct rq *rq) DECLARE_PER_CPU_SHARED_ALIGNED(struct rq, runqueues); +#ifdef CONFIG_SMP +DECLARE_PER_CPU_ALIGNED(cpumask_var_t, sched_pp_mask); +#endif + #define cpu_rq(cpu) (&per_cpu(runqueues, (cpu))) #define this_rq() this_cpu_ptr(&runqueues) #define task_rq(p) cpu_rq(task_cpu(p)) @@ -1296,8 +1300,6 @@ extern void sysrq_sched_debug_show(void); extern void sched_init_granularity(void); extern void update_max_interval(void); -extern void init_sched_dl_class(void); -extern void init_sched_rt_class(void); extern void init_sched_fair_class(void); extern void resched_curr(struct rq *rq); -- 1.8.3.1 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH] sched/deadline: No need to check NULL later_mask 2016-04-02 10:14 ` Xunlei Pang @ 2016-04-06 9:30 ` Peter Zijlstra 2016-04-06 13:08 ` Xunlei Pang 0 siblings, 1 reply; 7+ messages in thread From: Peter Zijlstra @ 2016-04-06 9:30 UTC (permalink / raw) To: xlpang; +Cc: linux-kernel, Juri Lelli, Ingo Molnar, Steven Rostedt On Sat, Apr 02, 2016 at 06:14:28PM +0800, Xunlei Pang wrote: > Your proposal is very nice! > > At the sched_init() stage we only have one (to be "idle") task and with irq disabled, > no scheduling will happen, and the cpu_possible_mask was already initiated, so it's > safe to simply move them there. > > Also, how about rt&deadline sharing a percpu mask? Because only one of them can > use the mask at a moment, operations are always under some spin_lock_irqsave(). > > I made a new patch below, slightly tested by running tens of rt&dl tasks for a while, > are you fine with it? Yep, looks fine. Please submit as a proper patch. Thanks! ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] sched/deadline: No need to check NULL later_mask 2016-04-06 9:30 ` Peter Zijlstra @ 2016-04-06 13:08 ` Xunlei Pang 0 siblings, 0 replies; 7+ messages in thread From: Xunlei Pang @ 2016-04-06 13:08 UTC (permalink / raw) To: Peter Zijlstra; +Cc: linux-kernel, Juri Lelli, Ingo Molnar, Steven Rostedt On 2016/04/06 at 17:30, Peter Zijlstra wrote: > On Sat, Apr 02, 2016 at 06:14:28PM +0800, Xunlei Pang wrote: >> Your proposal is very nice! >> >> At the sched_init() stage we only have one (to be "idle") task and with irq disabled, >> no scheduling will happen, and the cpu_possible_mask was already initiated, so it's >> safe to simply move them there. >> >> Also, how about rt&deadline sharing a percpu mask? Because only one of them can >> use the mask at a moment, operations are always under some spin_lock_irqsave(). >> >> I made a new patch below, slightly tested by running tens of rt&dl tasks for a while, >> are you fine with it? > Yep, looks fine. Please submit as a proper patch. Will do, thanks! Regards, Xunlei > > Thanks! ^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2016-04-06 13:08 UTC | newest] Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2016-04-01 11:13 [PATCH] sched/deadline: No need to check NULL later_mask Xunlei Pang 2016-04-01 11:29 ` Peter Zijlstra 2016-04-01 12:16 ` Xunlei Pang 2016-04-01 16:21 ` Peter Zijlstra 2016-04-02 10:14 ` Xunlei Pang 2016-04-06 9:30 ` Peter Zijlstra 2016-04-06 13:08 ` Xunlei Pang
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).