Messages in this thread | | | From | Eric Dumazet <> | Date | Fri, 31 Jan 2020 10:48:21 -0800 | Subject | Re: Confused about hlist_unhashed_lockless() |
| |
On Fri, Jan 31, 2020 at 10:43 AM Peter Zijlstra <peterz@infradead.org> wrote: > > On Fri, Jan 31, 2020 at 08:48:05AM -0800, Eric Dumazet wrote: > > BUG: KCSAN: data-race in del_timer / detach_if_pending > > > diff --git a/include/linux/timer.h b/include/linux/timer.h > > index 1e6650ed066d5d28251b0bd385fc37ef94c96532..0dc19a8c39c9e49a7cde3d34bfa4be8871cbc1c2 > > 100644 > > --- a/include/linux/timer.h > > +++ b/include/linux/timer.h > > @@ -164,7 +164,7 @@ static inline void destroy_timer_on_stack(struct > > timer_list *timer) { } > > */ > > static inline int timer_pending(const struct timer_list * timer) > > { > > - return timer->entry.pprev != NULL; > > + return !hlist_unhashed_lockless(&timer->entry); > > } > > That's just completely wrong. > > Aside from any memory barrier issues that might or might not be there > (I'm waaaay to tired atm to tell), the above code is perfectly fine. > > In fact, this is a KCSAN compiler infrastructure 'bug'. > > Any load that is only compared to zero is immune to load-tearing issues. > > The correct thing to do here is something like: > > static inline int timer_pending(const struct timer_list *timer) > { > /* > * KCSAN compiler infrastructure is insuffiently clever to > * realize that a 'load compared to zero' is immune to > * load-tearing. > */ > return data_race(timer->entry.pprev != NULL); > }
This is nice, now with have data_race()
Remember these patches were sent 2 months ago, at a time we were trying to sort out things.
data_race() was merged a few days ago.
| |