Messages in this thread | | | Date | Tue, 8 Dec 2020 09:01:08 +0100 | From | Peter Zijlstra <> | Subject | Re: [patch 3/3] tick: Annotate tick_do_timer_cpu data races |
| |
On Mon, Dec 07, 2020 at 07:19:51PM +0100, Marco Elver wrote: > On Mon, 7 Dec 2020 at 18:46, Thomas Gleixner <tglx@linutronix.de> wrote: > > On Mon, Dec 07 2020 at 13:09, Peter Zijlstra wrote: > > > On Sun, Dec 06, 2020 at 10:12:56PM +0100, Thomas Gleixner wrote: > > >> + if (data_race(tick_do_timer_cpu) == TICK_DO_TIMER_BOOT) { > > > > > > I prefer the form: > > > > > > if (data_race(tick_do_timer_cpu == TICK_DO_TIMER_BOOT)) { > > > > > > But there doesn't yet seem to be sufficient data_race() usage in the > > > kernel to see which of the forms is preferred. Do we want to bike-shed > > > this now and document the outcome somewhere? > > > > Yes please before we get a gazillion of patches changing half of them > > half a year from now. > > That rule should be as simple as possible. The simplest would be: > "Only enclose the smallest required expression in data_race(); keep > the number of required data_race() expressions to a minimum." (=> want > least amount of code inside data_race() with the least number of > data_race()s). > > In the case here, that'd be the "if (data_race(tick_do_timer_cpu) == > ..." variant.
So I was worried that data_race(var) == const, would not allow the compiler to emit
cmpq $CONST, ();
but would instead force a separate load. But I checked and it does generate the right code.
| |