Re: [PATCH v4 1/3] time: add "NOW() good" indicator

[Roger Pau Monné <roger@xxxxxxxxxxxxxx>]

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.

Acked-by: Roger Pau Monné <roger@xxxxxxxxxxxxxx>

Thanks, Roger.