Kernel: Don't use the profile timer if we don't have a timer to assign

Author: https://github.com/supercomputer7
Commit: https://github.com/SerenityOS/serenity/commit/8a4cc735b91
Pull-request: https://github.com/SerenityOS/serenity/pull/7140
Reviewed-by: https://github.com/IdanHo ✅

Kernel/Syscalls/profiling.cpp

@@ -30,12 +30,13 @@ KResultOr<int> Process::sys$profiling_enable(pid_t pid)
             g_global_perf_events = PerformanceEventBuffer::try_create_with_size(32 * MiB).leak_ptr();
 
         ScopedSpinLock lock(g_processes_lock);
-        g_profiling_all_threads = true;
+        if (!TimeManagement::the().enable_profile_timer())
+            return ENOTSUP;
         Process::for_each([](auto& process) {
             PerformanceManager::add_process_created_event(process);
             return IterationDecision::Continue;
         });
-        TimeManagement::the().enable_profile_timer();
+        g_profiling_all_threads = true;
         return 0;
     }
 
@@ -49,8 +50,9 @@ KResultOr<int> Process::sys$profiling_enable(pid_t pid)
         return EPERM;
     if (!process->create_perf_events_buffer_if_needed())
         return ENOMEM;
+    if (!TimeManagement::the().enable_profile_timer())
+        return ENOTSUP;
     process->set_profiling(true);
-    TimeManagement::the().enable_profile_timer();
     return 0;
 }
 
@@ -62,8 +64,9 @@ KResultOr<int> Process::sys$profiling_disable(pid_t pid)
         if (!is_superuser())
             return EPERM;
         ScopedCritical critical;
+        if (!TimeManagement::the().disable_profile_timer())
+            return ENOTSUP;
         g_profiling_all_threads = false;
-        TimeManagement::the().disable_profile_timer();
         return 0;
     }
 
@@ -75,7 +78,9 @@ KResultOr<int> Process::sys$profiling_disable(pid_t pid)
         return EPERM;
     if (!process->is_profiling())
         return EINVAL;
-    TimeManagement::the().disable_profile_timer();
+    // FIXME: If we enabled the profile timer and it's not supported, how do we disable it now?
+    if (!TimeManagement::the().disable_profile_timer())
+        return ENOTSUP;
     process->set_profiling(false);
     return 0;
 }

Kernel/Time/TimeManagement.cpp

@@ -262,10 +262,16 @@ UNMAP_AFTER_INIT bool TimeManagement::probe_and_set_non_legacy_hardware_timers()
 
     VERIFY(periodic_timers.size() + non_periodic_timers.size() > 0);
 
-    if (periodic_timers.size() > 0)
-        m_system_timer = periodic_timers[0];
-    else
-        m_system_timer = non_periodic_timers[0];
+    size_t taken_periodic_timers_count = 0;
+    size_t taken_non_periodic_timers_count = 0;
+
+    if (periodic_timers.size() > taken_periodic_timers_count) {
+        m_system_timer = periodic_timers[taken_periodic_timers_count];
+        taken_periodic_timers_count += 1;
+    } else if (non_periodic_timers.size() > taken_non_periodic_timers_count) {
+        m_system_timer = non_periodic_timers[taken_non_periodic_timers_count];
+        taken_non_periodic_timers_count += 1;
+    }
 
     m_system_timer->set_callback([this](const RegisterState& regs) {
         // Update the time. We don't really care too much about the
@@ -291,13 +297,18 @@ UNMAP_AFTER_INIT bool TimeManagement::probe_and_set_non_legacy_hardware_timers()
     // can query the timer.
     m_time_keeper_timer = m_system_timer;
 
-    if (periodic_timers.size() > 1)
-        m_profile_timer = periodic_timers[1];
-    else
-        m_profile_timer = non_periodic_timers[1];
+    if (periodic_timers.size() > taken_periodic_timers_count) {
+        m_profile_timer = periodic_timers[taken_periodic_timers_count];
+        taken_periodic_timers_count += 1;
+    } else if (non_periodic_timers.size() > taken_non_periodic_timers_count) {
+        m_profile_timer = non_periodic_timers[taken_non_periodic_timers_count];
+        taken_non_periodic_timers_count += 1;
+    }
 
+    if (m_profile_timer) {
         m_profile_timer->set_callback(PerformanceManager::timer_tick);
         m_profile_timer->try_to_set_frequency(m_profile_timer->calculate_nearest_possible_frequency(1));
+    }
 
     return true;
 }
@@ -389,16 +400,22 @@ void TimeManagement::system_timer_tick(const RegisterState& regs)
     Scheduler::timer_tick(regs);
 }
 
-void TimeManagement::enable_profile_timer()
+bool TimeManagement::enable_profile_timer()
 {
-    if (m_profile_enable_count.fetch_add(1) == 0)
+    if (m_profile_timer && m_profile_enable_count.fetch_add(1) == 0) {
         m_profile_timer->try_to_set_frequency(m_profile_timer->calculate_nearest_possible_frequency(OPTIMAL_PROFILE_TICKS_PER_SECOND_RATE));
+        return true;
+    }
+    return false;
 }
 
-void TimeManagement::disable_profile_timer()
+bool TimeManagement::disable_profile_timer()
 {
-    if (m_profile_enable_count.fetch_sub(1) == 1)
+    if (m_profile_timer && m_profile_enable_count.fetch_sub(1) == 1) {
         m_profile_timer->try_to_set_frequency(m_profile_timer->calculate_nearest_possible_frequency(1));
+        return true;
+    }
+    return false;
 }
 
 }

Kernel/Time/TimeManagement.h

@@ -56,8 +56,8 @@ public:
 
     static bool is_hpet_periodic_mode_allowed();
 
-    void enable_profile_timer();
-    void disable_profile_timer();
+    bool enable_profile_timer();
+    bool disable_profile_timer();
 
     u64 uptime_ms() const;
     static Time now();