Re: [PATCH v4 1/3] time: add "NOW() good" indicator
Jan Beulich <[email protected]> Wed, 29 Jul 2026 10:57:11 +0200
| Newsgroups | gmane.comp.emulators.xen.devel |
|---|---|
| Message-ID | <[email protected]> |
On 29.07.2026 10:46, Roger Pau Monné wrote: > On Tue, Jul 28, 2026 at 01:37:15PM +0200, Jan Beulich wrote: >> On 28.07.2026 12:32, Roger Pau Monné wrote: >>> On Tue, Jul 28, 2026 at 10:24:38AM +0200, Jan Beulich wrote: >>>> On 28.07.2026 10:17, Roger Pau Monné wrote: >>>>> On Tue, Jun 30, 2026 at 04:06:00PM +0200, Jan Beulich wrote: >>>>>> --- a/xen/arch/x86/time.c >>>>>> +++ b/xen/arch/x86/time.c >>>>>> @@ -2660,6 +2660,7 @@ void __init early_time_init(void) >>>>>> >>>>>> set_time_scale(&t->tsc_scale, tmp); >>>>>> t->stamp.local_tsc = boot_tsc_stamp; >>>>>> + NOW_good = true; >>>>> >>>>> Would you need a barrier here to ensure compiler doesn't re-order the >>>>> writes? Maybe using ACCESS_ONCE(), or a smp_wmb() like it's used in >>>>> other arches? >>>> >>>> Aiui it isn't needed here. The compiler can't re-order the two writes, >>>> due to our use of -fno-strict-aliasing. The barrier is there on other >>>> arch-es to avoid re-ordering in hardware. >>> >>> You are the expert in compilers, but it was my understanding that >>> `no-strict-aliasing` only affects the ordering of pointers accesses, >>> but not plain variables. IOW: NOW_good accesses could be reordered >>> because it's not a pointer. Does the compiler consider t can point to >>> &NOW_good and hence it can't be reordered? >> >> Yes, that's my understanding of how this work. (Yet no, "expert" surely >> is going too far.) Things would be different if NOW_good was a function- >> local variable, I think. > > OK, I wouldn't mind adding a compiler barrier to say on the safe side, > but I also won't object if you think that's enough. Okay, I've added one in, even if it feels a little odd. > Acked-by: Roger Pau Monné <[email protected]> Thanks, Jan