github-mirror/linux
1
0
Fork 0
mirror of https://github.com/torvalds/linux.git synced 2026-07-31 03:27:03 +02:00
Code Issues Actions Packages Projects Releases Wiki Activity Graph

walt/pause: switch over to mutex and protect call to pause/resume

Browse Source 
Client code can miss the actual paused state of a cpu, if ref-counting
is performed when the pause/resume operation is incomplete, or if
an additional pause/resume operation executes prior to updating ref-counts.
This becomes a problem with the online work handler, which can run
at the same time as any walt_pause/walt_resume call.

Protect the calls to pause/resume CPUs, such that the actual state of the
CPUS is known and stored, prior to beginning a new operation. Ensure
that the work function has accurate information before performing its
resume operation.

Change-Id: Iac279ac731f7c97b289547d36cce7de6e2b91c3a
Signed-off-by: Stephen Dickey <dickey@codeaurora.org>
This commit is contained in:
Stephen Dickey 2021-03-15 11:17:33 -07:00 committed by Rishabh Bhatnagar
parent 17444a20e3
commit 5475717e77
1 changed files with 59 additions and 27 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

86
kernel/sched/walt/walt_pause.c
View File

@ -8,7 +8,7 @@
#ifdef CONFIG_HOTPLUG_CPU
DEFINE_SPINLOCK(pause_lock);
static DEFINE_MUTEX(pause_lock);
struct pause_cpu_state {
int ref_count;
@ -22,14 +22,12 @@ static void inc_ref_counts(struct cpumask *cpus)
int cpu;
struct pause_cpu_state *pause_cpu_state;
spin_lock(&pause_lock);
for_each_cpu(cpu, cpus) {
pause_cpu_state = per_cpu_ptr(&pause_state, cpu);
if (pause_cpu_state->ref_count)
cpumask_clear_cpu(cpu, cpus);
pause_cpu_state->ref_count++;
}
spin_unlock(&pause_lock);
}
/*
@ -41,7 +39,6 @@ static void dec_test_ref_counts(struct cpumask *cpus)
int cpu;
struct pause_cpu_state *pause_cpu_state;
spin_lock(&pause_lock);
for_each_cpu(cpu, cpus) {
pause_cpu_state = per_cpu_ptr(&pause_state, cpu);
WARN_ON_ONCE(pause_cpu_state->ref_count == 0);
@ -49,30 +46,37 @@ static void dec_test_ref_counts(struct cpumask *cpus)
if (pause_cpu_state->ref_count)
cpumask_clear_cpu(cpu, cpus);
}
spin_unlock(&pause_lock);
}
/* cpus will be modified */
int walt_pause_cpus(struct cpumask *cpus)
{
int ret;
cpumask_t saved_cpus;
int ret = 0;
cpumask_t requested_cpus;
cpumask_copy(&saved_cpus, cpus);
/* add ref counts for all cpus in mask */
mutex_lock(&pause_lock);
inc_ref_counts(cpus);
/* only actually pause online CPUs */
/*
* Add ref counts for all cpus in mask, but
* only actually pause online CPUs
*/
cpumask_and(cpus, cpus, cpu_online_mask);
if (cpumask_empty(cpus))
goto unlock;
cpumask_copy(&requested_cpus, cpus);
ret = pause_cpus(cpus);
if (ret < 0) {
dec_test_ref_counts(&saved_cpus);
dec_test_ref_counts(&requested_cpus);
pr_err("pause_cpus failure ret=%d cpus=%*pbl\n", ret,
cpumask_pr_args(&saved_cpus));
cpumask_pr_args(&requested_cpus));
}
unlock:
mutex_unlock(&pause_lock);
return ret;
}
EXPORT_SYMBOL(walt_pause_cpus);
@ -80,35 +84,60 @@ EXPORT_SYMBOL(walt_pause_cpus);
/* cpus will be modified */
int walt_resume_cpus(struct cpumask *cpus)
{
int ret;
cpumask_t saved_cpus;
int ret = 0;
cpumask_t requested_cpus;
cpumask_copy(&saved_cpus, cpus);
mutex_lock(&pause_lock);
/* remove ref counts for all cpus in mask */
dec_test_ref_counts(cpus);
/* only actually resume online CPUs */
cpumask_and(cpus, cpus, cpu_online_mask);
if (cpumask_empty(cpus))
goto unlock;
cpumask_copy(&requested_cpus, cpus);
ret = resume_cpus(cpus);
if (ret < 0) {
inc_ref_counts(&saved_cpus);
inc_ref_counts(&requested_cpus);
pr_err("resume_cpus failure ret=%d cpus=%*pbl\n", ret,
cpumask_pr_args(&saved_cpus));
cpumask_pr_args(&requested_cpus));
}
unlock:
mutex_unlock(&pause_lock);
return ret;
}
EXPORT_SYMBOL(walt_resume_cpus);
struct work_struct walt_pause_online_work;
/* workfn to perform re-pause operation by detecting
* ref-counts and attempting to restore the state.
* It must not adjust the ref-counts for each cpu, or
* the actual paused state will no longer reflect client's
* expectations.
/*
* With refcounting and online/offline operations of the CPU
* a recent and accurate value for the requested CPUs versus
* ref-counted CPUs, must be made.
*
* When a CPU is onlined, this chain of events gets out of order.
* The online workfn can be entered at the same time as the
* walt_resume. If both are resuming the same set of CPUs
* the call to walt_pause will decrement ref-counts and think that
* the CPU is unpaused. If the workfn has already found all the
* ref-counts (and they were still set) it will re-pause
* the CPUs thinking that is what the client intended. This
* leads to a conflict, because the client software is no longer
* tracking these CPUs, and the state doesn't match what the client
* intended.
*
* This case needs protection to maintain a valid state
* of the device (where ref-counts == # of pause requests)
* Use a mutex such that the values read at the start of walt_pause,
* walt_resume, or walt_pause_online_workfn remain valid until the
* operation is complete. A mutex must be used because pause_cpus
* (and resume_cpus) cannot be called with a spinlock held, and
* the operation is not complete
* until those routines return.
*/
static void walt_pause_online_workfn(struct work_struct *work)
{
@ -116,22 +145,25 @@ static void walt_pause_online_workfn(struct work_struct *work)
cpumask_t re_pause_cpus;
int cpu, ret = 0;
mutex_lock(&pause_lock);
cpumask_clear(&re_pause_cpus);
/* search and test all online cpus */
spin_lock(&pause_lock);
for_each_online_cpu(cpu) {
pause_cpu_state = per_cpu_ptr(&pause_state, cpu);
if (pause_cpu_state->ref_count)
cpumask_set_cpu(cpu, &re_pause_cpus);
}
spin_unlock(&pause_lock);
if (cpumask_empty(&re_pause_cpus))
return;
goto unlock;
/* will wait for existing hp operations to complete */
ret = pause_cpus(&re_pause_cpus);
unlock:
mutex_unlock(&pause_lock);
if (ret < 0) {
pr_err("pause_cpus during online failure ret=%d cpus=%*pb1\n", ret,
cpumask_pr_args(&re_pause_cpus));