Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]
Groups > linux.kernel > #1607133 > unrolled thread
| Started by | Byungchul Park <byungchul.park@lge.com> |
|---|---|
| First post | 2017-03-23 03:40 +0100 |
| Last post | 2017-03-28 02:10 +0200 |
| Articles | 3 — 2 participants |
Back to article view | Back to linux.kernel
[PATCH v3 1/3] sched/deadline: Make find_later_rq() choose a closer cpu in topology Byungchul Park <byungchul.park@lge.com> - 2017-03-23 03:40 +0100
Re: [PATCH v3 1/3] sched/deadline: Make find_later_rq() choose a closer cpu in topology Juri Lelli <juri.lelli@arm.com> - 2017-03-27 16:40 +0200
Re: [PATCH v3 1/3] sched/deadline: Make find_later_rq() choose a closer cpu in topology Byungchul Park <byungchul.park@lge.com> - 2017-03-28 02:10 +0200
| From | Byungchul Park <byungchul.park@lge.com> |
|---|---|
| Date | 2017-03-23 03:40 +0100 |
| Subject | [PATCH v3 1/3] sched/deadline: Make find_later_rq() choose a closer cpu in topology |
| Message-ID | <to0w1-3Qh-11@gated-at.bofh.it> |
When cpudl_find() returns any among free_cpus, the cpu might not be
closer than others, considering sched domain. For example:
this_cpu: 15
free_cpus: 0, 1,..., 14 (== later_mask)
best_cpu: 0
topology:
0 --+
+--+
1 --+ |
+-- ... --+
2 --+ | |
+--+ |
3 --+ |
... ...
12 --+ |
+--+ |
13 --+ | |
+-- ... -+
14 --+ |
+--+
15 --+
In this case, it would be best to select 14 since it's a free cpu and
closest to 15(this_cpu). However, currently the code select 0(best_cpu)
even though that's just any among free_cpus. Fix it.
Signed-off-by: Byungchul Park <byungchul.park@lge.com>
---
kernel/sched/deadline.c | 29 +++++++++++++++--------------
1 file changed, 15 insertions(+), 14 deletions(-)
diff --git a/kernel/sched/deadline.c b/kernel/sched/deadline.c
index a2ce590..49c93b9 100644
--- a/kernel/sched/deadline.c
+++ b/kernel/sched/deadline.c
@@ -1324,7 +1324,7 @@ 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);
int this_cpu = smp_processor_id();
- int best_cpu, cpu = task_cpu(task);
+ int cpu = task_cpu(task);
/* Make sure the mask is initialized first */
if (unlikely(!later_mask))
@@ -1337,17 +1337,14 @@ 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.
*/
- best_cpu = cpudl_find(&task_rq(task)->rd->cpudl,
- task, later_mask);
- if (best_cpu == -1)
+ if (cpudl_find(&task_rq(task)->rd->cpudl, task, later_mask) == -1)
return -1;
/*
- * If we are here, some target has been found,
- * the most suitable of which is cached in best_cpu.
- * This is, among the runqueues where the current tasks
- * have later deadlines than the task's one, the rq
- * with the latest possible one.
+ * If we are here, some targets have been found, including
+ * the most suitable which is, among the runqueues where the
+ * current tasks have later deadlines than the task's one, the
+ * rq with the latest possible one.
*
* Now we check how well this matches with task's
* affinity and system topology.
@@ -1367,6 +1364,7 @@ static int find_later_rq(struct task_struct *task)
rcu_read_lock();
for_each_domain(cpu, sd) {
if (sd->flags & SD_WAKE_AFFINE) {
+ int closest_cpu;
/*
* If possible, preempting this_cpu is
@@ -1378,14 +1376,17 @@ static int find_later_rq(struct task_struct *task)
return this_cpu;
}
+ closest_cpu = cpumask_first_and(later_mask,
+ sched_domain_span(sd));
/*
- * Last chance: if best_cpu is valid and is
- * in the mask, that becomes our choice.
+ * Last chance: if a cpu being in both later_mask
+ * and current sd span is valid, that becomes our
+ * choice. Of course, the latest possible cpu is
+ * already under consideration through later_mask.
*/
- if (best_cpu < nr_cpu_ids &&
- cpumask_test_cpu(best_cpu, sched_domain_span(sd))) {
+ if (closest_cpu < nr_cpu_ids) {
rcu_read_unlock();
- return best_cpu;
+ return closest_cpu;
}
}
}
--
1.9.1
[toc] | [next] | [standalone]
| From | Juri Lelli <juri.lelli@arm.com> |
|---|---|
| Date | 2017-03-27 16:40 +0200 |
| Subject | Re: [PATCH v3 1/3] sched/deadline: Make find_later_rq() choose a closer cpu in topology |
| Message-ID | <tpDYm-XG-29@gated-at.bofh.it> |
| In reply to | #1607133 |
Hi,
On 23/03/17 11:12, Byungchul Park wrote:
> When cpudl_find() returns any among free_cpus, the cpu might not be
> closer than others, considering sched domain. For example:
>
> this_cpu: 15
> free_cpus: 0, 1,..., 14 (== later_mask)
> best_cpu: 0
>
> topology:
>
> 0 --+
> +--+
> 1 --+ |
> +-- ... --+
> 2 --+ | |
> +--+ |
> 3 --+ |
>
> ... ...
>
> 12 --+ |
> +--+ |
> 13 --+ | |
> +-- ... -+
> 14 --+ |
> +--+
> 15 --+
>
> In this case, it would be best to select 14 since it's a free cpu and
> closest to 15(this_cpu). However, currently the code select 0(best_cpu)
> even though that's just any among free_cpus. Fix it.
>
> Signed-off-by: Byungchul Park <byungchul.park@lge.com>
> ---
> kernel/sched/deadline.c | 29 +++++++++++++++--------------
> 1 file changed, 15 insertions(+), 14 deletions(-)
>
> diff --git a/kernel/sched/deadline.c b/kernel/sched/deadline.c
> index a2ce590..49c93b9 100644
> --- a/kernel/sched/deadline.c
> +++ b/kernel/sched/deadline.c
> @@ -1324,7 +1324,7 @@ 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);
> int this_cpu = smp_processor_id();
> - int best_cpu, cpu = task_cpu(task);
> + int cpu = task_cpu(task);
>
> /* Make sure the mask is initialized first */
> if (unlikely(!later_mask))
> @@ -1337,17 +1337,14 @@ 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.
> */
> - best_cpu = cpudl_find(&task_rq(task)->rd->cpudl,
> - task, later_mask);
> - if (best_cpu == -1)
> + if (cpudl_find(&task_rq(task)->rd->cpudl, task, later_mask) == -1)
It seems that with this we loose the last user of the current return
value of cpudl_find() (heap maximum). I guess we want to change the
return value to be (int)bool, as in rt, so that we can simplify this and
the conditions in check_preempt_equal_dl.
> return -1;
>
> /*
> - * If we are here, some target has been found,
> - * the most suitable of which is cached in best_cpu.
> - * This is, among the runqueues where the current tasks
> - * have later deadlines than the task's one, the rq
> - * with the latest possible one.
> + * If we are here, some targets have been found, including
> + * the most suitable which is, among the runqueues where the
> + * current tasks have later deadlines than the task's one, the
> + * rq with the latest possible one.
> *
> * Now we check how well this matches with task's
> * affinity and system topology.
> @@ -1367,6 +1364,7 @@ static int find_later_rq(struct task_struct *task)
> rcu_read_lock();
> for_each_domain(cpu, sd) {
> if (sd->flags & SD_WAKE_AFFINE) {
> + int closest_cpu;
Can we still call this best_cpu, so that we are aligned with rt?
Thanks,
- Juri
[toc] | [prev] | [next] | [standalone]
| From | Byungchul Park <byungchul.park@lge.com> |
|---|---|
| Date | 2017-03-28 02:10 +0200 |
| Subject | Re: [PATCH v3 1/3] sched/deadline: Make find_later_rq() choose a closer cpu in topology |
| Message-ID | <tpMRX-7JX-7@gated-at.bofh.it> |
| In reply to | #1609895 |
On Mon, Mar 27, 2017 at 03:33:43PM +0100, Juri Lelli wrote:
> > diff --git a/kernel/sched/deadline.c b/kernel/sched/deadline.c
> > index a2ce590..49c93b9 100644
> > --- a/kernel/sched/deadline.c
> > +++ b/kernel/sched/deadline.c
> > @@ -1324,7 +1324,7 @@ 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);
> > int this_cpu = smp_processor_id();
> > - int best_cpu, cpu = task_cpu(task);
> > + int cpu = task_cpu(task);
> >
> > /* Make sure the mask is initialized first */
> > if (unlikely(!later_mask))
> > @@ -1337,17 +1337,14 @@ 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.
> > */
> > - best_cpu = cpudl_find(&task_rq(task)->rd->cpudl,
> > - task, later_mask);
> > - if (best_cpu == -1)
> > + if (cpudl_find(&task_rq(task)->rd->cpudl, task, later_mask) == -1)
>
> It seems that with this we loose the last user of the current return
> value of cpudl_find() (heap maximum). I guess we want to change the
> return value to be (int)bool, as in rt, so that we can simplify this and
> the conditions in check_preempt_equal_dl.
Hi Juri,
Actually I changed the return value to be bool, but didn't include the
patch since it looks not that valuable. But I will add it if you also
think so. ;)
>
> > return -1;
> >
> > /*
> > - * If we are here, some target has been found,
> > - * the most suitable of which is cached in best_cpu.
> > - * This is, among the runqueues where the current tasks
> > - * have later deadlines than the task's one, the rq
> > - * with the latest possible one.
> > + * If we are here, some targets have been found, including
> > + * the most suitable which is, among the runqueues where the
> > + * current tasks have later deadlines than the task's one, the
> > + * rq with the latest possible one.
> > *
> > * Now we check how well this matches with task's
> > * affinity and system topology.
> > @@ -1367,6 +1364,7 @@ static int find_later_rq(struct task_struct *task)
> > rcu_read_lock();
> > for_each_domain(cpu, sd) {
> > if (sd->flags & SD_WAKE_AFFINE) {
> > + int closest_cpu;
>
> Can we still call this best_cpu, so that we are aligned with rt?
OK. I will rename it to best_cpu.
Thanks,
Byungchul
[toc] | [prev] | [standalone]
Back to top | Article view | linux.kernel
csiph-web