| Message ID | 1512581776.2224.129.camel@kernel.crashing.org |
|---|---|
| State | Accepted |
| Headers | show |
| Series | timer: Stop calling list_top() racily | expand |
Benjamin Herrenschmidt <benh@kernel.crashing.org> writes: > This will trip the debug checks in debug builds under some circumstances > and is actually a rather bad idea as we might look at a timer that is > concurrently being removed and modified, and thus incorrectly assume > there is no work to do. > > Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org> > --- > core/timer.c | 9 +++++---- > 1 file changed, 5 insertions(+), 4 deletions(-) THanks, merged to master as of b504f2737e9ba3b6935b4ea1c015b3643aefaf51 and 5.9.x as of 6e975e1d0fdafdf6cf534da7742b57a538c4fa28
diff --git a/core/timer.c b/core/timer.c index 4b5925fe..b10a5646 100644 --- a/core/timer.c +++ b/core/timer.c @@ -223,7 +223,6 @@ static void __check_timers(uint64_t now) void check_timers(bool from_interrupt) { - struct timer *t; uint64_t now = mftb(); /* This is the polling variant, the SLW interrupt path, when it @@ -231,9 +230,11 @@ void check_timers(bool from_interrupt) * the pollers */ - /* Lockless "peek", a bit racy but shouldn't be a problem */ - t = list_top(&timer_list, struct timer, link); - if (list_empty_nocheck(&timer_poll_list) && (!t || t->target > now)) + /* Lockless "peek", a bit racy but shouldn't be a problem as + * we are only looking at whether the list is empty + */ + if (list_empty_nocheck(&timer_poll_list) && + list_empty_nocheck(&timer_list)) return; /* Take lock and try again */
This will trip the debug checks in debug builds under some circumstances and is actually a rather bad idea as we might look at a timer that is concurrently being removed and modified, and thus incorrectly assume there is no work to do. Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org> --- core/timer.c | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-)