Skip to content

Commit

Permalink
Fix incorrect cpu.sleeper accounting
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
SeanTAllen committed Jun 11, 2020
1 parent 583b1ce commit 52964a5
Showing 1 changed file with 2 additions and 1 deletion.
3 changes: 2 additions & 1 deletion arch/lkl/kernel/cpu.c
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -140,7 +142,6 @@ void lkl_cpu_put(void)
}

if (cpu.sleepers) {
cpu.sleepers--;
lkl_ops->sem_up(cpu.sem);
}

Expand Down

0 comments on commit 52964a5

Please sign in to comment.