From 24982cc1a94c22b563f102ab4b476c10412ecbc3 Mon Sep 17 00:00:00 2001 From: Shaleen Agrawal Date: Sat, 30 Apr 2022 08:55:59 -0700 Subject: [PATCH] sched/walt: window rollover missing fix Currently, the code has a flow where a window rollover is not issued during migration. This could lead to a situation where no future window rollovers are handled. For example in a 4 + 3 + 1 topology, A B C indicate the window boundaries while X indicates the rq->window_start of each cpu and "GLB" indicates the timestamp of the global window tracker (walt_irq_work_lastq_ws) 1. A B C CPU0----X----------------------------------------------------------------- CPU1----X----------------------------------------------------------------- CPU2----X----------------------------------------------------------------- CPU3----X----------------------------------------------------------------- CPU4----X----------------------------------------------------------------- CPU5----X----------------------------------------------------------------- CPU6----X----------------------------------------------------------------- CPU7----X----------------------------------------------------------------- GLB X Now CPU1 gets an event(e) right before C, in which case it advances his rq1->window_start to B, also advances GLB to B and issues a rollover irq work, which will advance window_starts for all other cpus. 2. A B C CPU0----X----------------------------------------------------------------- CPU1----------------------------X---------------------e------------------- CPU2----X----------------------------------------------------------------- CPU3----X----------------------------------------------------------------- CPU4----X----------------------------------------------------------------- CPU5----X----------------------------------------------------------------- CPU6----X----------------------------------------------------------------- CPU7----X----------------------------------------------------------------- GLB X Since the event was very close to C, the irq_work(i) could run right after C, which advances all the window_starts to C, including that of cpu1. 3. A B C CPU0----------------------------------------------------X----------------- CPU1--------------------------------------------------e-X-i--------------- CPU2----------------------------------------------------X----------------- CPU3----------------------------------------------------X----------------- CPU4----------------------------------------------------X----------------- CPU5----------------------------------------------------X----------------- CPU6----------------------------------------------------X----------------- CPU7----------------------------------------------------X----------------- GLB X This advancement of window_start on cpu1 causes it to advance G (and reissue another irq_work, which is not relevant to this discussion) like below. CPU1 will advance the global because it matches its window_start (atomic_cmpxchg) 4. A B C CPU0----------------------------------------------------X----------------- CPU1--------------------------------------------------e-X-i--------------- CPU2----------------------------------------------------X----------------- CPU3----------------------------------------------------X----------------- CPU4----------------------------------------------------X----------------- CPU5----------------------------------------------------X----------------- CPU6----------------------------------------------------X----------------- CPU7----------------------------------------------------X----------------- GLB X However we could run in to a situation where the irq work (i) is held up because cpu0's rq lock isn't available (BTW a window rollover holds all the rqlocks) for some other reason. And while the irq work is held there, the rq locks of cpus1-7 are available, an inter cluster migration between cpu1 and cpu7 could happen causing cpu1,7 to advance their window_start. Redrawing the 3rd picture 3a. A B C CPU0----X----------------------------------------------------------------- CPU1--------------------------------------------------e-X---m-----i------- CPU2----X----------------------------------------------------------------- CPU3----X----------------------------------------------------------------- CPU4----X----------------------------------------------------------------- CPU5----X----------------------------------------------------------------- CPU6----X----------------------------------------------------------------- CPU7----------------------------------------------------X---m------------- GLB X Eventually when the irq work gets around to hold the locks on all the cpus, it will move the rest of the cpu's window_start to C. However the Global is left at B and cpu1 that should have advanced the Global, won't do it because of an unchecked advancement by the migration with cpu7. After this point the global is old and will never match any cpu's window_start and WALT will never do a window_rollover 4a. A B C CPU0----------------------------------------------------X----------------- CPU1--------------------------------------------------e-X---m-----i------- CPU2----------------------------------------------------X----------------- CPU3----------------------------------------------------X----------------- CPU4----------------------------------------------------X----------------- CPU5----------------------------------------------------X----------------- CPU6----------------------------------------------------X----------------- CPU7----------------------------------------------------X---m------------- GLB X Fix this by issuing migration walt_irq_work when an intercluster migration ends up rolling over the window in the dest rq. Change-Id: Ib3985a1cdf72342305398a9c648159195ca99658 Signed-off-by: Shaleen Agrawal --- kernel/sched/walt/walt.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/kernel/sched/walt/walt.c b/kernel/sched/walt/walt.c index 95cf1ed00be2..d138ddc8e61d 100644 --- a/kernel/sched/walt/walt.c +++ b/kernel/sched/walt/walt.c @@ -993,6 +993,7 @@ static inline bool is_new_task(struct task_struct *p) return wts->active_time < NEW_TASK_ACTIVE_TIME; } +static inline void run_walt_irq_work_rollover(u64 old_window_start, struct rq *rq); static void fixup_busy_time(struct task_struct *p, int new_cpu) { @@ -1009,6 +1010,7 @@ static void fixup_busy_time(struct task_struct *p, int new_cpu) struct walt_rq *dest_wrq = (struct walt_rq *) dest_rq->android_vendor_data1; struct walt_rq *src_wrq = (struct walt_rq *) src_rq->android_vendor_data1; struct walt_task_struct *wts = (struct walt_task_struct *) p->android_vendor_data1; + u64 old_window_start; if (!p->on_rq && READ_ONCE(p->__state) != TASK_WAKING) return; @@ -1040,7 +1042,8 @@ static void fixup_busy_time(struct task_struct *p, int new_cpu) * window boundary or if the counters will be accessed * or not. */ - update_window_start(dest_rq, wallclock, TASK_UPDATE); + old_window_start = update_window_start(dest_rq, wallclock, TASK_UPDATE); + run_walt_irq_work_rollover(old_window_start, dest_rq); update_task_cpu_cycles(p, new_cpu, wallclock);