From 52964a5b9f85a6fde83684a1e5abe33feb132a0e Mon Sep 17 00:00:00 2001 From: "Sean T. Allen" Date: Thu, 11 Jun 2020 11:09:52 +0000 Subject: [PATCH] Fix incorrect cpu.sleeper accounting Under contention for the CPU lock, you could end up in a scenario where waiters on the cpu.sem would be awoken earlier than they should have been. The problem crosses two functions. lkl_cpu_get and lkl_cpu_put: https://github.com/lsds/lkl/blob/master/arch/lkl/kernel/cpu.c#L95 https://github.com/lsds/lkl/blob/master/arch/lkl/kernel/cpu.c#L113 The handling of "cpu.sleepers" is incorrect. "cpu.sleepers" should be incremented when a thread is waiting to get the cpu.sem so that in lkl_cpu_put, it can call sem_up to wake a sleeper if any exists. However what it is currently doing is in lkl_cpu_get is incrementing the sleepers thread count every time it can't get the cpu lock. https://github.com/lsds/lkl/blob/master/arch/lkl/kernel/cpu.c#L102 So, if a thread fails to get the lock more than once it will increment the count of sleeping threads more than once. This means as it stands, in lkl_cpu_put, that one thread trying to get the lock several times can account for multiple calls to sem_up. Given that it is awoken each time in lkl_cpu_get, that is wrong. The correct logic is to decrement the sleepers count after the sem_down returns in lkl_cpu_get and not in lkl_cpu_put. That will correctly match up cpu.sleepers with the number of threads waiting on cpu.sem. --- arch/lkl/kernel/cpu.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/arch/lkl/kernel/cpu.c b/arch/lkl/kernel/cpu.c index 636b8f1b63a7d7..0704067acfb8df 100644 --- a/arch/lkl/kernel/cpu.c +++ b/arch/lkl/kernel/cpu.c @@ -103,6 +103,8 @@ int lkl_cpu_get(void) __cpu_try_get_unlock(ret, 0); lkl_ops->sem_down(cpu.sem); ret = __cpu_try_get_lock(0); + if (ret > -2) + cpu.sleepers--; } __cpu_try_get_unlock(ret, 1); @@ -140,7 +142,6 @@ void lkl_cpu_put(void) } if (cpu.sleepers) { - cpu.sleepers--; lkl_ops->sem_up(cpu.sem); }