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

[Jan Beulich <jbeulich@xxxxxxxx>]

On 22.05.2026 09:46, Oleksii Kurochko wrote:
> 
> 
> On 5/20/26 4:45 PM, Jan Beulich wrote:
>> printk_start_of_line() checks for a value of 0 right now. In order to be
>> able to have NOW() return at least monotonically increasing values, that
>> needs replacing by an explicit indicator.
>>
>> Signed-off-by: Jan Beulich <jbeulich@xxxxxxxx>
>> ---
>> Arm and RISC-V may want to consider whether their initial get_cycles()
>> can't be moved yet earlier, such that the indicator also can be set
>> yet earlier.
>> ---
> 
> At least, for RISC-V ...
> 
>> v3: New.
>>
>> --- a/xen/arch/arm/time.c
>> +++ b/xen/arch/arm/time.c
>> @@ -145,6 +145,7 @@ void __init preinit_xen_time(void)
>>           panic("Timer: Cannot initialize platform timer\n");
>>   
>>       boot_count = get_cycles();
>> +    NOW_good = true;
>>   }
>>   
>>   static void __init init_dt_xen_time(void)
>> --- a/xen/arch/riscv/time.c
>> +++ b/xen/arch/riscv/time.c
>> @@ -87,6 +87,7 @@ void __init preinit_xen_time(void)
>>           panic("%s: ACPI isn't supported\n", __func__);
>>   
>>       boot_clock_cycles = get_cycles();
>> +    NOW_good = true;
> 
> ...  preinit_xen_time() call could be moved a little bit above just 
> after riscv_fill_hwcap() as it is using riscv_isa_extension_available() 
> inside.
> 
> preinit_xen_time() could be splited so it can be moved just above 
> tasklet_subsys_init() after device tree is initialized but I don't think
> there is to much sense for that.
> 
>>   
>>       /* set_xen_timer must have been set by sbi_init() already */
>>       ASSERT(set_xen_timer);
> 
> The ASSERT is harmless, but NOW_good could technically go after it.

"could" or do you perhaps even mean "should"? I'd like to keep the write
next to that of boot_clock_cycles, but of course only if there's no
dependency on set_xen_timer.

> boot_clock_cycles and cpu_khz must be visible to any reader of NOW_good 
> == true. There's no explicit ordering between those stores and the 
> NOW_good = true store. In practice this is fine because SMP isn't active 
> at this point, but a WRITE_ONCE(NOW_good, true) or a compiler barrier 
> would make the intent explicit and protect against future reordering by 
> an optimizing compiler.

Hmm, yes. Logically it would want to be smp_wmb(), and then I'll also
have to add smp_rmb() in printk_start_of_line().

Jan