sched/cache: Fix race condition during sched domain rebuild

sched_cache_active_set_unlocked() checks hardware support without
locks:
static void sched_cache_active_set(bool locked)
{
        /* hardware does not support */
        if (!static_branch_likely(&sched_cache_present)) {
                _sched_cache_active_set(false, locked);
                return;
        }
    ...
If build_sched_domains() runs concurrently during CPU hotplug,
it can disable sched_cache_present under sched_domains_mutex
and the CPU hotplug lock. If a debugfs write thread evaluates
sched_cache_present as true right before that, and then blocks
or gets preempted, it might proceed to enable sched_cache_active
after the hardware support has been marked as absent. Make it
safer by acquiring cpus_read_lock() and sched_domains_mutex_lock()
when the user changes sched_cache_active via debugfs.

This bug was reported by sashiko.

Fixes: 067a31358143 ("sched/cache: Allow the user space to turn on and off cache aware scheduling")
Signed-off-by: Chen Yu <yu.c.chen@intel.com>
Co-developed-by: Tim Chen <tim.c.chen@linux.intel.com>
Signed-off-by: Tim Chen <tim.c.chen@linux.intel.com>
Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
Link: https://patch.msgid.link/9afddf439687f04bb56b46625bd9f153eb8abad5.1778703694.git.tim.c.chen@linux.intel.com

diff --git a/kernel/sched/debug.c b/kernel/sched/debug.c
index fe569539e8889bfc6ce557f7b8466459be5c4d1a..ed3a0d65da0ca57476895e30d0809d43e911344b 100644 (file)
--- a/kernel/sched/debug.c
+++ b/kernel/sched/debug.c
@@ -224,7 +224,9 @@ sched_cache_enable_write(struct file *filp, const char __user *ubuf,
 
        sysctl_sched_cache_user = val;
 
-       sched_cache_active_set_unlocked();
+       sched_cache_active_set();
+
+       *ppos += cnt;
 
        return cnt;
 }

diff --git a/kernel/sched/sched.h b/kernel/sched/sched.h
index 27409399137c31748b0e4a2132f7db8e61d62e70..45a3b77f46aa089f00f0b5590387c9949eaa93aa 100644 (file)
--- a/kernel/sched/sched.h
+++ b/kernel/sched/sched.h
@@ -4083,7 +4083,7 @@ static inline bool sched_cache_enabled(void)
        return static_branch_unlikely(&sched_cache_active);
 }
 
-extern void sched_cache_active_set_unlocked(void);
+extern void sched_cache_active_set(void);
 
 #endif
 

diff --git a/kernel/sched/topology.c b/kernel/sched/topology.c
index 7248a7279abeba0924fec0a4a660adb51963c53d..c257134f613d938deb5a3c7135a0ed227cc11e2e 100644 (file)
--- a/kernel/sched/topology.c
+++ b/kernel/sched/topology.c
@@ -917,30 +917,20 @@ err:
        return false;
 }
 
-static void _sched_cache_active_set(bool enable, bool locked)
-{
-       if (enable) {
-               if (locked)
-                       static_branch_enable_cpuslocked(&sched_cache_active);
-               else
-                       static_branch_enable(&sched_cache_active);
-       } else {
-               if (locked)
-                       static_branch_disable_cpuslocked(&sched_cache_active);
-               else
-                       static_branch_disable(&sched_cache_active);
-       }
-}
-
 /*
  * Enable/disable cache aware scheduling according to
  * user input and the presence of hardware support.
  */
-static void sched_cache_active_set(bool locked)
+static void _sched_cache_active_set(void)
 {
+       lockdep_assert_cpus_held();
+       lockdep_assert_held(&sched_domains_mutex);
+
        /* hardware does not support */
        if (!static_branch_likely(&sched_cache_present)) {
-               _sched_cache_active_set(false, locked);
+               static_branch_disable_cpuslocked(&sched_cache_active);
+               if (sched_debug())
+                       pr_info("%s: cache aware scheduling not supported on this platform\n", __func__);
                return;
        }
 
@@ -951,24 +941,23 @@ static void sched_cache_active_set(bool locked)
         * for now.
         */
        if (sysctl_sched_cache_user) {
-               _sched_cache_active_set(true, locked);
+               static_branch_enable_cpuslocked(&sched_cache_active);
                if (sched_debug())
                        pr_info("%s: enabling cache aware scheduling\n", __func__);
        } else {
-               _sched_cache_active_set(false, locked);
+               static_branch_disable_cpuslocked(&sched_cache_active);
                if (sched_debug())
                        pr_info("%s: disabling cache aware scheduling\n", __func__);
        }
 }
 
-static void sched_cache_active_set_locked(void)
+void sched_cache_active_set(void)
 {
-       return sched_cache_active_set(true);
-}
-
-void sched_cache_active_set_unlocked(void)
-{
-       return sched_cache_active_set(false);
+       cpus_read_lock();
+       sched_domains_mutex_lock();
+       _sched_cache_active_set();
+       sched_domains_mutex_unlock();
+       cpus_read_unlock();
 }
 
 /*
@@ -3082,7 +3071,7 @@ error:
        else
                static_branch_disable_cpuslocked(&sched_cache_present);
 
-       sched_cache_active_set_locked();
+       _sched_cache_active_set();
 #endif
        __free_domain_allocs(&d, alloc_state, cpu_map);