[PATCH 0/3] Backport walt_{prepare|finish}_migrate() for kernel v4.4

List overview All Threads
Download

newer

older

DEV, Delivery Notification, ID...

[PATCH RFC] sched/freq: fixing...

Leo Yan

17 Nov 2016 17 Nov '16

6:04 a.m.

This patch series is to backport walt_prepare_migrate() and walt_finish_migrate() functions from Vikram latest WALT patch series [1] to Android common kernel 4.4.

We use these two functions to replace function walt_fixup_busy_time(), as result the scheduler will saperately acquire lock for source rq and destination rq and don't need use function double_lock_balance(). So this will let scheduler flows more safe due we can ensure atomicity by using walt_prepare_migrate() and walt_finish_migrate().

Thanks for Patrick and Vikram's suggestions for this.

Leo Yan (3): sched/walt: port walt_{prepare|finish}_migrate() functions sched/core: fix atomicity broken issue sched/walt: remove walt_fixup_busy_time()

-- 1.9.1

Show replies by date

Leo Yan

17 Nov 17 Nov

6:04 a.m.

New subject: [PATCH 1/3] sched/walt: port walt_{prepare|finish}_migrate() functions

Port walt_prepare_migrate() and walt_finish_migrate() functions, so these two functions can be used separately before and after task migration. As result, we don't need acquire locks both for source and destination rq.

This patch is to directly port these two functions from Vikram latest WALT patches to AOSP Android common kernel 4.4.

Signed-off-by: Vikram Mulukutla markivx@codeaurora.org Signed-off-by: Leo Yan leo.yan@linaro.org --- kernel/sched/walt.c | 90 +++++++++++++++++++++++++++++++++++++++++++++++++++++ kernel/sched/walt.h | 4 +++ 2 files changed, 94 insertions(+)

diff --git a/kernel/sched/walt.c b/kernel/sched/walt.c index d9d0991..b2a22b3 100644 --- a/kernel/sched/walt.c +++ b/kernel/sched/walt.c @@ -820,6 +820,96 @@ void walt_migrate_sync_cpu(int cpu) sync_cpu = smp_processor_id(); }

+void walt_finish_migrate(struct task_struct *p, struct rq *dest_rq, bool locked) +{ + u64 wallclock; + unsigned long flags; + + if (!p->on_rq && p->state != TASK_WAKING) + return; + + if (locked == false) + raw_spin_lock_irqsave(&dest_rq->lock, flags); + + lockdep_assert_held(&dest_rq->lock); + + wallclock = walt_ktime_clock(); + + /* Update counters on destination CPU */ + walt_update_task_ravg(dest_rq->curr, dest_rq, + TASK_UPDATE, wallclock, 0); + + /* We may be in a new window. Update task counters */ + walt_update_task_ravg(p, dest_rq, TASK_MIGRATE, wallclock, 0); + + if (p->ravg.curr_window) { + if (!dest_rq->window_start) { + p->ravg.curr_window = 0; + p->ravg.mark_start = 0; + } + dest_rq->curr_runnable_sum += p->ravg.curr_window; + } + + if (p->ravg.prev_window) { + if (!dest_rq->window_start) + p->ravg.prev_window = 0; + dest_rq->prev_runnable_sum += p->ravg.prev_window; + } + + if (locked == false) + raw_spin_unlock_irqrestore(&dest_rq->lock, flags); + + trace_walt_migration_update_sum(dest_rq, p); +} + +void walt_prepare_migrate(struct task_struct *p, struct rq *src_rq, bool locked) +{ + u64 wallclock; + unsigned long flags; + + if (!p->on_rq && p->state != TASK_WAKING) + return; + + if (exiting_task(p)) + return; + + if (locked == false) + raw_spin_lock_irqsave(&src_rq->lock, flags); + + lockdep_assert_held(&src_rq->lock); + + /* Note that same wallclock reference is used for all 3 events below */ + wallclock = walt_ktime_clock(); + + /* Update counters on source CPU */ + walt_update_task_ravg(task_rq(p)->curr, task_rq(p), + TASK_UPDATE, wallclock, 0); + + /* Update task's counters */ + walt_update_task_ravg(p, task_rq(p), TASK_MIGRATE, wallclock, 0); + + /* Fixup busy time */ + if (p->ravg.curr_window) + src_rq->curr_runnable_sum -= p->ravg.curr_window; + + if (p->ravg.prev_window) + src_rq->prev_runnable_sum -= p->ravg.prev_window; + + if ((s64)src_rq->prev_runnable_sum < 0) { + src_rq->prev_runnable_sum = 0; + WARN_ON(1); + } + if ((s64)src_rq->curr_runnable_sum < 0) { + src_rq->curr_runnable_sum = 0; + WARN_ON(1); + } + + if (locked == false) + raw_spin_unlock_irqrestore(&src_rq->lock, flags); + + trace_walt_migration_update_sum(src_rq, p); +} + void walt_fixup_busy_time(struct task_struct *p, int new_cpu) { struct rq *src_rq = task_rq(p); diff --git a/kernel/sched/walt.h b/kernel/sched/walt.h index e181c87..91bc3cc 100644 --- a/kernel/sched/walt.h +++ b/kernel/sched/walt.h @@ -24,6 +24,8 @@ void walt_inc_cfs_cumulative_runnable_avg(struct cfs_rq *rq, struct task_struct *p); void walt_dec_cfs_cumulative_runnable_avg(struct cfs_rq *rq, struct task_struct *p); +void walt_prepare_migrate(struct task_struct *p, struct rq *rq, bool locked); +void walt_finish_migrate(struct task_struct *p, struct rq *rq, bool locked); void walt_fixup_busy_time(struct task_struct *p, int new_cpu); void walt_init_new_task_load(struct task_struct *p); void walt_mark_task_starting(struct task_struct *p); @@ -47,6 +49,8 @@ static inline void walt_inc_cfs_cumulative_runnable_avg(struct cfs_rq *rq, struct task_struct *p) { } static inline void walt_dec_cfs_cumulative_runnable_avg(struct cfs_rq *rq, struct task_struct *p) { } +static inline void walt_prepare_migrate(struct task_struct *p, struct rq *rq, bool locked) { } +static inline void walt_finish_migrate(struct task_struct *p, struct rq *rq, bool locked) { } static inline void walt_fixup_busy_time(struct task_struct *p, int new_cpu) { } static inline void walt_init_new_task_load(struct task_struct *p) { } static inline void walt_mark_task_starting(struct task_struct *p) { } -- 1.9.1

Leo Yan

6:04 a.m.

New subject: [PATCH 2/3] sched/core: fix atomicity broken issue

The function walt_fixup_busy_time() manipulates source and destination RQ at the same time, so this inheritly requires to acquire two rq locks together for exclusive accessing. The two rq locks are acquired by function double_lock_balance(), but it firstly releases source rq lock and then acquire two locks with specific sequence to avoid recursive deadlock.

This gives chance for other CPU to acquire this lock during double_lock_balance(); it's not safe for below usage for pointer dereference:

raw_spin_lock(&rq->lock); node = get_one_pointer_from_rq(); double_lock_balance(); -> broken atomicity double_unlock_balance(); use_pointer(node); -> may introduce error or panic -> due node has been manipulated -> by other CPU. raw_spin_unlock(&rq->lock);

So this patch is to port Vikram's latest WALT patches to AOSP-4.4, which use functions walt_prepare_migrate() and walt_finish_migrate() to replace walt_fixup_busy_time().

Signed-off-by: Vikram Mulukutla markivx@codeaurora.org Signed-off-by: Leo Yan leo.yan@linaro.org --- kernel/sched/core.c | 10 ++++++---- kernel/sched/deadline.c | 8 ++++++++ kernel/sched/fair.c | 4 ++-- kernel/sched/rt.c | 4 ++++ 4 files changed, 20 insertions(+), 6 deletions(-)

diff --git a/kernel/sched/core.c b/kernel/sched/core.c index 951a4e4..10f36e2 100644 --- a/kernel/sched/core.c +++ b/kernel/sched/core.c @@ -1086,15 +1086,15 @@ static struct rq *move_queued_task(struct rq *rq, struct task_struct *p, int new

dequeue_task(rq, p, 0); p->on_rq = TASK_ON_RQ_MIGRATING; - double_lock_balance(rq, cpu_rq(new_cpu)); + walt_prepare_migrate(p, rq, true); set_task_cpu(p, new_cpu); - double_unlock_balance(rq, cpu_rq(new_cpu)); raw_spin_unlock(&rq->lock);

rq = cpu_rq(new_cpu);

raw_spin_lock(&rq->lock); BUG_ON(task_cpu(p) != new_cpu); + walt_finish_migrate(p, rq, true); p->on_rq = TASK_ON_RQ_QUEUED; enqueue_task(rq, p, 0); check_preempt_curr(rq, p, 0); @@ -1312,8 +1312,6 @@ void set_task_cpu(struct task_struct *p, unsigned int new_cpu) p->sched_class->migrate_task_rq(p); p->se.nr_migrations++; perf_event_task_migrate(p); - - walt_fixup_busy_time(p, new_cpu); }

__set_task_cpu(p, new_cpu); @@ -1328,7 +1326,9 @@ static void __migrate_swap_task(struct task_struct *p, int cpu) dst_rq = cpu_rq(cpu);

deactivate_task(src_rq, p, 0); + walt_prepare_migrate(p, src_rq, true); set_task_cpu(p, cpu); + walt_finish_migrate(p, dst_rq, true); activate_task(dst_rq, p, 0); check_preempt_curr(dst_rq, p, 0); } else { @@ -2021,7 +2021,9 @@ try_to_wake_up(struct task_struct *p, unsigned int state, int wake_flags)

if (task_cpu(p) != cpu) { wake_flags |= WF_MIGRATED; + walt_prepare_migrate(p, task_rq(p), false); set_task_cpu(p, cpu); + walt_finish_migrate(p, cpu_rq(cpu), false); }

#endif /* CONFIG_SMP */ diff --git a/kernel/sched/deadline.c b/kernel/sched/deadline.c index 9d9eb50..ad29d29 100644 --- a/kernel/sched/deadline.c +++ b/kernel/sched/deadline.c @@ -18,6 +18,8 @@

#include <linux/slab.h>

+#include "walt.h" + struct dl_bandwidth def_dl_bandwidth;

static inline struct task_struct *dl_task_of(struct sched_dl_entity *dl_se) @@ -290,7 +292,9 @@ static struct rq *dl_task_offline_migration(struct rq *rq, struct task_struct *p * By now the task is replenished and enqueued; migrate it. */ deactivate_task(rq, p, 0); + walt_prepare_migrate(p, rq, true); set_task_cpu(p, later_rq->cpu); + walt_finish_migrate(p, later_rq, true); activate_task(later_rq, p, 0);

if (!fallback) @@ -1580,7 +1584,9 @@ retry:

deactivate_task(rq, next_task, 0); clear_average_bw(&next_task->dl, &rq->dl); + walt_prepare_migrate(next_task, rq, true); set_task_cpu(next_task, later_rq->cpu); + walt_finish_migrate(next_task, later_rq, true); add_average_bw(&next_task->dl, &later_rq->dl); activate_task(later_rq, next_task, 0); ret = 1; @@ -1670,7 +1676,9 @@ static void pull_dl_task(struct rq *this_rq)

deactivate_task(src_rq, p, 0); clear_average_bw(&p->dl, &src_rq->dl); + walt_prepare_migrate(p, src_rq, true); set_task_cpu(p, this_cpu); + walt_finish_migrate(p, this_rq, true); add_average_bw(&p->dl, &this_rq->dl); activate_task(this_rq, p, 0); dmin = p->dl.deadline; diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c index 8d568b0..df9b531 100644 --- a/kernel/sched/fair.c +++ b/kernel/sched/fair.c @@ -6726,9 +6726,8 @@ static void detach_task(struct task_struct *p, struct lb_env *env)

deactivate_task(env->src_rq, p, 0); p->on_rq = TASK_ON_RQ_MIGRATING; - double_lock_balance(env->src_rq, env->dst_rq); + walt_prepare_migrate(p, env->src_rq, true); set_task_cpu(p, env->dst_cpu); - double_unlock_balance(env->src_rq, env->dst_rq); }

/* @@ -6881,6 +6880,7 @@ static void attach_task(struct rq *rq, struct task_struct *p) lockdep_assert_held(&rq->lock);

BUG_ON(task_rq(p) != rq); + walt_finish_migrate(p, rq, true); p->on_rq = TASK_ON_RQ_QUEUED; activate_task(rq, p, 0); check_preempt_curr(rq, p, 0); diff --git a/kernel/sched/rt.c b/kernel/sched/rt.c index 8a16cba..564b5fd 100644 --- a/kernel/sched/rt.c +++ b/kernel/sched/rt.c @@ -1879,7 +1879,9 @@ retry: }

deactivate_task(rq, next_task, 0); + walt_prepare_migrate(next_task, rq, true); set_task_cpu(next_task, lowest_rq->cpu); + walt_finish_migrate(next_task, lowest_rq, true); activate_task(lowest_rq, next_task, 0); ret = 1;

@@ -2133,7 +2135,9 @@ static void pull_rt_task(struct rq *this_rq) resched = true;

deactivate_task(src_rq, p, 0); + walt_prepare_migrate(p, src_rq, true); set_task_cpu(p, this_cpu); + walt_finish_migrate(p, this_rq, true); activate_task(this_rq, p, 0); /* * We continue with the search, just in -- 1.9.1

Vikram Mulukutla

11:03 p.m.

New subject: [PATCH 2/3] sched/core: fix atomicity broken issue

Hi Leo,

On 11/16/2016 10:04 PM, Leo Yan wrote:

...

The function walt_fixup_busy_time() manipulates source and destination RQ at the same time, so this inheritly requires to acquire two rq locks together for exclusive accessing. The two rq locks are acquired by function double_lock_balance(), but it firstly releases source rq lock and then acquire two locks with specific sequence to avoid recursive deadlock.

This gives chance for other CPU to acquire this lock during double_lock_balance(); it's not safe for below usage for pointer dereference:

raw_spin_lock(&rq->lock); node = get_one_pointer_from_rq(); double_lock_balance(); -> broken atomicity double_unlock_balance(); use_pointer(node); -> may introduce error or panic -> due node has been manipulated -> by other CPU. raw_spin_unlock(&rq->lock);

So this patch is to port Vikram's latest WALT patches to AOSP-4.4, which use functions walt_prepare_migrate() and walt_finish_migrate() to replace walt_fixup_busy_time().

Thanks for doing this! I'm fine with the commit text, but please switch the subject the line to just say 'don't use double lock balance'. We haven't broken anything in the baseline kernel, just that the new code will have a problem.

...

Signed-off-by: Vikram Mulukutla markivx@codeaurora.org Signed-off-by: Leo Yan leo.yan@linaro.org

kernel/sched/core.c | 10 ++++++---- kernel/sched/deadline.c | 8 ++++++++ kernel/sched/fair.c | 4 ++-- kernel/sched/rt.c | 4 ++++ 4 files changed, 20 insertions(+), 6 deletions(-)

diff --git a/kernel/sched/core.c b/kernel/sched/core.c index 951a4e4..10f36e2 100644 --- a/kernel/sched/core.c +++ b/kernel/sched/core.c @@ -1086,15 +1086,15 @@ static struct rq *move_queued_task(struct rq *rq, struct task_struct *p, int new

dequeue_task(rq, p, 0); p->on_rq = TASK_ON_RQ_MIGRATING;

double_lock_balance(rq, cpu_rq(new_cpu));

walt_prepare_migrate(p, rq, true); set_task_cpu(p, new_cpu);

double_unlock_balance(rq, cpu_rq(new_cpu)); raw_spin_unlock(&rq->lock);

rq = cpu_rq(new_cpu);

raw_spin_lock(&rq->lock); BUG_ON(task_cpu(p) != new_cpu);

walt_finish_migrate(p, rq, true); p->on_rq = TASK_ON_RQ_QUEUED; enqueue_task(rq, p, 0); check_preempt_curr(rq, p, 0);

@@ -1312,8 +1312,6 @@ void set_task_cpu(struct task_struct *p, unsigned int new_cpu) p->sched_class->migrate_task_rq(p); p->se.nr_migrations++; perf_event_task_migrate(p);
walt_fixup_busy_time(p, new_cpu);
}

__set_task_cpu(p, new_cpu);
@@ -1328,7 +1326,9 @@ static void __migrate_swap_task(struct task_struct *p, int cpu) dst_rq = cpu_rq(cpu);
deactivate_task(src_rq, p, 0);
walt_prepare_migrate(p, src_rq, true);
set_task_cpu(p, cpu);
walt_finish_migrate(p, dst_rq, true);
activate_task(dst_rq, p, 0); check_preempt_curr(dst_rq, p, 0); } else {
@@ -2021,7 +2021,9 @@ try_to_wake_up(struct task_struct *p, unsigned int state, int wake_flags)

if (task_cpu(p) != cpu) { wake_flags |= WF_MIGRATED;
walt_prepare_migrate(p, task_rq(p), false);
set_task_cpu(p, cpu);
walt_finish_migrate(p, cpu_rq(cpu), false);
}

#endif /* CONFIG_SMP */
diff --git a/kernel/sched/deadline.c b/kernel/sched/deadline.c index 9d9eb50..ad29d29 100644 --- a/kernel/sched/deadline.c +++ b/kernel/sched/deadline.c @@ -18,6 +18,8 @@

#include <linux/slab.h>

+#include "walt.h"

struct dl_bandwidth def_dl_bandwidth;

static inline struct task_struct *dl_task_of(struct sched_dl_entity *dl_se)

@@ -290,7 +292,9 @@ static struct rq *dl_task_offline_migration(struct rq *rq, struct task_struct *p * By now the task is replenished and enqueued; migrate it. */ deactivate_task(rq, p, 0);

walt_prepare_migrate(p, rq, true); set_task_cpu(p, later_rq->cpu);

walt_finish_migrate(p, later_rq, true); activate_task(later_rq, p, 0);

if (!fallback)

@@ -1580,7 +1584,9 @@ retry:

deactivate_task(rq, next_task, 0); clear_average_bw(&next_task->dl, &rq->dl);

walt_prepare_migrate(next_task, rq, true); set_task_cpu(next_task, later_rq->cpu);

walt_finish_migrate(next_task, later_rq, true); add_average_bw(&next_task->dl, &later_rq->dl); activate_task(later_rq, next_task, 0); ret = 1;

@@ -1670,7 +1676,9 @@ static void pull_dl_task(struct rq *this_rq)
	deactivate_task(src_rq, p, 0);
	clear_average_bw(&p->dl, &src_rq->dl);
	walt_prepare_migrate(p, src_rq, true);
set_task_cpu(p, this_cpu);
	walt_finish_migrate(p, this_rq, true);
add_average_bw(&p->dl, &this_rq->dl);
activate_task(this_rq, p, 0);
dmin = p->dl.deadline;
diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c index 8d568b0..df9b531 100644 --- a/kernel/sched/fair.c +++ b/kernel/sched/fair.c @@ -6726,9 +6726,8 @@ static void detach_task(struct task_struct *p, struct lb_env *env)

deactivate_task(env->src_rq, p, 0); p->on_rq = TASK_ON_RQ_MIGRATING;

double_lock_balance(env->src_rq, env->dst_rq);

walt_prepare_migrate(p, env->src_rq, true); set_task_cpu(p, env->dst_cpu);

double_unlock_balance(env->src_rq, env->dst_rq); }

/*

@@ -6881,6 +6880,7 @@ static void attach_task(struct rq *rq, struct task_struct *p) lockdep_assert_held(&rq->lock);

BUG_ON(task_rq(p) != rq);

walt_finish_migrate(p, rq, true); p->on_rq = TASK_ON_RQ_QUEUED; activate_task(rq, p, 0); check_preempt_curr(rq, p, 0);

diff --git a/kernel/sched/rt.c b/kernel/sched/rt.c index 8a16cba..564b5fd 100644 --- a/kernel/sched/rt.c +++ b/kernel/sched/rt.c @@ -1879,7 +1879,9 @@ retry: }

deactivate_task(rq, next_task, 0);

walt_prepare_migrate(next_task, rq, true); set_task_cpu(next_task, lowest_rq->cpu);

walt_finish_migrate(next_task, lowest_rq, true); activate_task(lowest_rq, next_task, 0); ret = 1;

@@ -2133,7 +2135,9 @@ static void pull_rt_task(struct rq *this_rq) resched = true;
	deactivate_task(src_rq, p, 0);
	walt_prepare_migrate(p, src_rq, true);
set_task_cpu(p, this_cpu);
	walt_finish_migrate(p, this_rq, true);
activate_task(this_rq, p, 0);
/*
 * We continue with the search, just in

Provided the subject line is fixed, ACK from me.

Thanks, Vikram

Leo Yan

18 Nov 18 Nov

1 a.m.

New subject: [PATCH 2/3] sched/core: fix atomicity broken issue

Hi Vikram,

On Thu, Nov 17, 2016 at 03:03:02PM -0800, Vikram Mulukutla wrote:

...

On 11/16/2016 10:04 PM, Leo Yan wrote:

...
The function walt_fixup_busy_time() manipulates source and destination RQ at the same time, so this inheritly requires to acquire two rq locks together for exclusive accessing. The two rq locks are acquired by function double_lock_balance(), but it firstly releases source rq lock and then acquire two locks with specific sequence to avoid recursive deadlock.

This gives chance for other CPU to acquire this lock during double_lock_balance(); it's not safe for below usage for pointer dereference:

raw_spin_lock(&rq->lock); node = get_one_pointer_from_rq(); double_lock_balance(); -> broken atomicity double_unlock_balance(); use_pointer(node); -> may introduce error or panic -> due node has been manipulated -> by other CPU. raw_spin_unlock(&rq->lock);

So this patch is to port Vikram's latest WALT patches to AOSP-4.4, which use functions walt_prepare_migrate() and walt_finish_migrate() to replace walt_fixup_busy_time().

Thanks for doing this! I'm fine with the commit text, but please switch the subject the line to just say 'don't use double lock balance'. We haven't broken anything in the baseline kernel, just that the new code will have a problem.

Sure, will do this.

...

...
Signed-off-by: Vikram Mulukutla markivx@codeaurora.org Signed-off-by: Leo Yan leo.yan@linaro.org

kernel/sched/core.c | 10 ++++++---- kernel/sched/deadline.c | 8 ++++++++ kernel/sched/fair.c | 4 ++-- kernel/sched/rt.c | 4 ++++ 4 files changed, 20 insertions(+), 6 deletions(-)

diff --git a/kernel/sched/core.c b/kernel/sched/core.c index 951a4e4..10f36e2 100644 --- a/kernel/sched/core.c +++ b/kernel/sched/core.c @@ -1086,15 +1086,15 @@ static struct rq *move_queued_task(struct rq *rq, struct task_struct *p, int new dequeue_task(rq, p, 0); p->on_rq = TASK_ON_RQ_MIGRATING;

double_lock_balance(rq, cpu_rq(new_cpu));

walt_prepare_migrate(p, rq, true); set_task_cpu(p, new_cpu);

double_unlock_balance(rq, cpu_rq(new_cpu)); raw_spin_unlock(&rq->lock); rq = cpu_rq(new_cpu); raw_spin_lock(&rq->lock); BUG_ON(task_cpu(p) != new_cpu);

walt_finish_migrate(p, rq, true); p->on_rq = TASK_ON_RQ_QUEUED; enqueue_task(rq, p, 0); check_preempt_curr(rq, p, 0);

@@ -1312,8 +1312,6 @@ void set_task_cpu(struct task_struct *p, unsigned int new_cpu) p->sched_class->migrate_task_rq(p); p->se.nr_migrations++; perf_event_task_migrate(p);
walt_fixup_busy_time(p, new_cpu);
} __set_task_cpu(p, new_cpu);
@@ -1328,7 +1326,9 @@ static void __migrate_swap_task(struct task_struct *p, int cpu) dst_rq = cpu_rq(cpu); deactivate_task(src_rq, p, 0);
walt_prepare_migrate(p, src_rq, true);
set_task_cpu(p, cpu);
walt_finish_migrate(p, dst_rq, true);
activate_task(dst_rq, p, 0); check_preempt_curr(dst_rq, p, 0); } else {
@@ -2021,7 +2021,9 @@ try_to_wake_up(struct task_struct *p, unsigned int state, int wake_flags) if (task_cpu(p) != cpu) { wake_flags |= WF_MIGRATED;
walt_prepare_migrate(p, task_rq(p), false);
set_task_cpu(p, cpu);
walt_finish_migrate(p, cpu_rq(cpu), false);
}
#endif /* CONFIG_SMP */ diff --git a/kernel/sched/deadline.c b/kernel/sched/deadline.c index 9d9eb50..ad29d29 100644 --- a/kernel/sched/deadline.c +++ b/kernel/sched/deadline.c @@ -18,6 +18,8 @@ #include <linux/slab.h> +#include "walt.h"

struct dl_bandwidth def_dl_bandwidth; static inline struct task_struct *dl_task_of(struct sched_dl_entity *dl_se) @@ -290,7 +292,9 @@ static struct rq *dl_task_offline_migration(struct rq *rq, struct task_struct *p * By now the task is replenished and enqueued; migrate it. */ deactivate_task(rq, p, 0);

walt_prepare_migrate(p, rq, true); set_task_cpu(p, later_rq->cpu);

walt_finish_migrate(p, later_rq, true); activate_task(later_rq, p, 0); if (!fallback)

@@ -1580,7 +1584,9 @@ retry: deactivate_task(rq, next_task, 0); clear_average_bw(&next_task->dl, &rq->dl);

walt_prepare_migrate(next_task, rq, true); set_task_cpu(next_task, later_rq->cpu);

walt_finish_migrate(next_task, later_rq, true); add_average_bw(&next_task->dl, &later_rq->dl); activate_task(later_rq, next_task, 0); ret = 1;

@@ -1670,7 +1676,9 @@ static void pull_dl_task(struct rq *this_rq) deactivate_task(src_rq, p, 0); clear_average_bw(&p->dl, &src_rq->dl);
	walt_prepare_migrate(p, src_rq, true);
set_task_cpu(p, this_cpu);
	walt_finish_migrate(p, this_rq, true);
add_average_bw(&p->dl, &this_rq->dl);
activate_task(this_rq, p, 0);
dmin = p->dl.deadline;
diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c index 8d568b0..df9b531 100644 --- a/kernel/sched/fair.c +++ b/kernel/sched/fair.c @@ -6726,9 +6726,8 @@ static void detach_task(struct task_struct *p, struct lb_env *env) deactivate_task(env->src_rq, p, 0); p->on_rq = TASK_ON_RQ_MIGRATING;

double_lock_balance(env->src_rq, env->dst_rq);

walt_prepare_migrate(p, env->src_rq, true); set_task_cpu(p, env->dst_cpu);

double_unlock_balance(env->src_rq, env->dst_rq);

} /* @@ -6881,6 +6880,7 @@ static void attach_task(struct rq *rq, struct task_struct *p) lockdep_assert_held(&rq->lock); BUG_ON(task_rq(p) != rq);

walt_finish_migrate(p, rq, true); p->on_rq = TASK_ON_RQ_QUEUED; activate_task(rq, p, 0); check_preempt_curr(rq, p, 0);

diff --git a/kernel/sched/rt.c b/kernel/sched/rt.c index 8a16cba..564b5fd 100644 --- a/kernel/sched/rt.c +++ b/kernel/sched/rt.c @@ -1879,7 +1879,9 @@ retry: } deactivate_task(rq, next_task, 0);

walt_prepare_migrate(next_task, rq, true); set_task_cpu(next_task, lowest_rq->cpu);

walt_finish_migrate(next_task, lowest_rq, true); activate_task(lowest_rq, next_task, 0); ret = 1;

@@ -2133,7 +2135,9 @@ static void pull_rt_task(struct rq *this_rq) resched = true; deactivate_task(src_rq, p, 0);
	walt_prepare_migrate(p, src_rq, true);
set_task_cpu(p, this_cpu);
	walt_finish_migrate(p, this_rq, true);
activate_task(this_rq, p, 0);
/*
 * We continue with the search, just in
Provided the subject line is fixed, ACK from me.

Thanks for ACK :)

...

Thanks, Vikram

Leo Yan

17 Nov 17 Nov

6:04 a.m.

New subject: [PATCH 3/3] sched/walt: remove walt_fixup_busy_time()

Now the walt doesn't use walt_fixup_busy_time(), remove it.

Signed-off-by: Leo Yan leo.yan@linaro.org --- kernel/sched/walt.c | 51 --------------------------------------------------- kernel/sched/walt.h | 2 -- 2 files changed, 53 deletions(-)

diff --git a/kernel/sched/walt.c b/kernel/sched/walt.c index b2a22b3..df72fb4 100644 --- a/kernel/sched/walt.c +++ b/kernel/sched/walt.c @@ -910,57 +910,6 @@ void walt_prepare_migrate(struct task_struct *p, struct rq *src_rq, bool locked) trace_walt_migration_update_sum(src_rq, p); }

-void walt_fixup_busy_time(struct task_struct *p, int new_cpu) -{ - struct rq *src_rq = task_rq(p); - struct rq *dest_rq = cpu_rq(new_cpu); - u64 wallclock; - - if (!p->on_rq && p->state != TASK_WAKING) - return; - - if (exiting_task(p)) { - return; - } - - if (p->state == TASK_WAKING) - double_rq_lock(src_rq, dest_rq); - - wallclock = walt_ktime_clock(); - - walt_update_task_ravg(task_rq(p)->curr, task_rq(p), - TASK_UPDATE, wallclock, 0); - walt_update_task_ravg(dest_rq->curr, dest_rq, - TASK_UPDATE, wallclock, 0); - - walt_update_task_ravg(p, task_rq(p), TASK_MIGRATE, wallclock, 0); - - if (p->ravg.curr_window) { - src_rq->curr_runnable_sum -= p->ravg.curr_window; - dest_rq->curr_runnable_sum += p->ravg.curr_window; - } - - if (p->ravg.prev_window) { - src_rq->prev_runnable_sum -= p->ravg.prev_window; - dest_rq->prev_runnable_sum += p->ravg.prev_window; - } - - if ((s64)src_rq->prev_runnable_sum < 0) { - src_rq->prev_runnable_sum = 0; - WARN_ON(1); - } - if ((s64)src_rq->curr_runnable_sum < 0) { - src_rq->curr_runnable_sum = 0; - WARN_ON(1); - } - - trace_walt_migration_update_sum(src_rq, p); - trace_walt_migration_update_sum(dest_rq, p); - - if (p->state == TASK_WAKING) - double_rq_unlock(src_rq, dest_rq); -} - /* Keep track of max/min capacity possible across CPUs "currently" */ static void __update_min_max_capacity(void) { diff --git a/kernel/sched/walt.h b/kernel/sched/walt.h index 91bc3cc..2e5b99d 100644 --- a/kernel/sched/walt.h +++ b/kernel/sched/walt.h @@ -26,7 +26,6 @@ void walt_dec_cfs_cumulative_runnable_avg(struct cfs_rq *rq, struct task_struct *p); void walt_prepare_migrate(struct task_struct *p, struct rq *rq, bool locked); void walt_finish_migrate(struct task_struct *p, struct rq *rq, bool locked); -void walt_fixup_busy_time(struct task_struct *p, int new_cpu); void walt_init_new_task_load(struct task_struct *p); void walt_mark_task_starting(struct task_struct *p); void walt_set_window_start(struct rq *rq); @@ -51,7 +50,6 @@ static inline void walt_dec_cfs_cumulative_runnable_avg(struct cfs_rq *rq, struct task_struct *p) { } static inline void walt_prepare_migrate(struct task_struct *p, struct rq *rq, bool locked) { } static inline void walt_finish_migrate(struct task_struct *p, struct rq *rq, bool locked) { } -static inline void walt_fixup_busy_time(struct task_struct *p, int new_cpu) { } static inline void walt_init_new_task_load(struct task_struct *p) { } static inline void walt_mark_task_starting(struct task_struct *p) { } static inline void walt_set_window_start(struct rq *rq) { } -- 1.9.1

Vikram Mulukutla

11:06 p.m.

New subject: [PATCH 0/3] Backport walt_{prepare|finish}_migrate() for kernel v4.4

Hi Leo,

On 11/16/2016 10:04 PM, Leo Yan wrote:

...

This patch series is to backport walt_prepare_migrate() and walt_finish_migrate() functions from Vikram latest WALT patch series [1] to Android common kernel 4.4.

We use these two functions to replace function walt_fixup_busy_time(), as result the scheduler will saperately acquire lock for source rq and destination rq and don't need use function double_lock_balance(). So this will let scheduler flows more safe due we can ensure atomicity by using walt_prepare_migrate() and walt_finish_migrate().

Thanks for Patrick and Vikram's suggestions for this.

Thanks for doing this; just one minor comment on the second patch. If you're testing with this series, please test suspend/resume, I may have seen a problem when I did a backport to our own internal 3.18 tree in the resume path, I didn't have time to debug then. I'll be happy to help with any debugging.

...

Leo Yan (3): sched/walt: port walt_{prepare|finish}_migrate() functions sched/core: fix atomicity broken issue sched/walt: remove walt_fixup_busy_time()

kernel/sched/core.c | 10 ++++--- kernel/sched/deadline.c | 8 ++++++ kernel/sched/fair.c | 4 +-- kernel/sched/rt.c | 4 +++ kernel/sched/walt.c | 75 +++++++++++++++++++++++++++++++++++++------------ kernel/sched/walt.h | 6 ++-- 6 files changed, 81 insertions(+), 26 deletions(-)

Thanks, Vikram

Leo Yan

18 Nov 18 Nov

12:57 a.m.

New subject: [PATCH 0/3] Backport walt_{prepare|finish}_migrate() for kernel v4.4

Hi Vikram,

On Thu, Nov 17, 2016 at 03:06:24PM -0800, Vikram Mulukutla wrote:

...

Hi Leo,

On 11/16/2016 10:04 PM, Leo Yan wrote:

...
This patch series is to backport walt_prepare_migrate() and walt_finish_migrate() functions from Vikram latest WALT patch series [1] to Android common kernel 4.4.

We use these two functions to replace function walt_fixup_busy_time(), as result the scheduler will saperately acquire lock for source rq and destination rq and don't need use function double_lock_balance(). So this will let scheduler flows more safe due we can ensure atomicity by using walt_prepare_migrate() and walt_finish_migrate().

Thanks for Patrick and Vikram's suggestions for this.

Thanks for doing this; just one minor comment on the second patch. If you're testing with this series, please test suspend/resume, I may have seen a problem when I did a backport to our own internal 3.18 tree in the resume path, I didn't have time to debug then. I'll be happy to help with any debugging.

Thanks for reminding the suspend/resume testing. I did this testing on Hikey + kernel 4.4 with below command, have not found issue after run 10 times. So if you think other thing I should note, please let me know.

echo +3 > /sys/class/rtc/rtc0/wakealarm; echo mem > /sys/power/state

Thanks, Leo Yan

...

...
Leo Yan (3): sched/walt: port walt_{prepare|finish}_migrate() functions sched/core: fix atomicity broken issue sched/walt: remove walt_fixup_busy_time()

kernel/sched/core.c | 10 ++++--- kernel/sched/deadline.c | 8 ++++++ kernel/sched/fair.c | 4 +-- kernel/sched/rt.c | 4 +++ kernel/sched/walt.c | 75 +++++++++++++++++++++++++++++++++++++------------ kernel/sched/walt.h | 6 ++-- 6 files changed, 81 insertions(+), 26 deletions(-)

Thanks, Vikram _______________________________________________ eas-dev mailing list eas-dev@lists.linaro.org https://lists.linaro.org/mailman/listinfo/eas-dev

Vikram Mulukutla

8:40 p.m.

New subject: [PATCH 0/3] Backport walt_{prepare|finish}_migrate() for kernel v4.4

On 2016-11-17 16:57, Leo Yan wrote:

...

Hi Vikram,

On Thu, Nov 17, 2016 at 03:06:24PM -0800, Vikram Mulukutla wrote:

...
Hi Leo,

[...]

...

...
Thanks for doing this; just one minor comment on the second patch. If you're testing with this series, please test suspend/resume, I may have seen a problem when I did a backport to our own internal 3.18 tree in the resume path, I didn't have time to debug then. I'll be happy to help with any debugging.

Thanks for reminding the suspend/resume testing. I did this testing on Hikey + kernel 4.4 with below command, have not found issue after run 10 times. So if you think other thing I should note, please let me know.

echo +3 > /sys/class/rtc/rtc0/wakealarm; echo mem > /sys/power/state

Thanks, Leo Yan

Cool, thanks for testing that. As long as you don't see any new warnings about overflowing counters in the kernel log, things look good.

[...]

Thanks, Vikram

2912

days inactive

2913

days old

eas-dev@lists.linaro.org

8 comments

participants

tags (0)

participants (2)

Leo Yan
Vikram Mulukutla