Re: [patch 09/24] timekeeping: Add CLOCK_AUX support for ktime_get_snapshot_id()

[Thomas Weißschuh]

On Fri, Jun 26, 2026 at 05:17:52PM +0200, Thomas Gleixner wrote:
> On Fri, Jun 26 2026 at 13:03, Thomas Weißschuh wrote:
> > On Fri, Jun 26, 2026 at 12:49:41PM +0200, Thomas Gleixner wrote:
> >> On Fri, Jun 26 2026 at 10:48, Thomas Weißschuh wrote:
> >> > On Tue, May 26, 2026 at 07:14:13PM +0200, Thomas Gleixner wrote:
> >> > (...)
> >> >
> >> >>  static inline void tk_update_aux_offs(struct timekeeper *tk, ktime_t offs)
> >> >> @@ -1218,6 +1223,12 @@ bool ktime_get_snapshot_id(struct system
> >> >>  		tkd = &tk_core;
> >> >>  		offs = &tk_core.timekeeper.offs_boot;
> >> >>  		break;
> >> >> +	case CLOCK_AUX ... CLOCK_AUX_LAST:
> >> >> +		tkd = aux_get_tk_data(clock_id);
> >> >> +		if (!tkd)
> >> >> +			return false;
> >> >> +		offs = &tkd->timekeeper.offs_aux;
> >> >> +		break;
> >> >
> >> > 'tkd' is also used to compute 'monoraw'. However 'tkr_raw' and 'tkr_mono'
> >> > are the same for auxilary clocks, so this will compute a wrong 'monoraw'.
> >> 
> >> AUX clocks are independent in the first place and the MONORAW part is
> >> the "MONORAW" related to the AUX clock itself. 
> >> 
> >> > Instead 'monoraw' should be computed based on 'tk_core'.
> >> > Which then also requires the sequence locking of 'tk_core'.
> >> 
> >> No. From a PTP and steering point of view you want the "raw" value which
> >> is related to the AUX clock itself and not the global one.
> >
> > Ack.
> >
> > However the kdocs call it 'CLOCK_MONOTONIC_RAW'. Can we clean this up?
> 
> Yes. Something like the below?

Looks good, thanks.
The corresponding structure definitions are a also affected, though.

> Thanks,
> 
>         tglx
> ---
> --- a/kernel/time/timekeeping.c
> +++ b/kernel/time/timekeeping.c
> @@ -1202,10 +1202,21 @@ static inline u64 tk_clock_read_snapshot
>  
>  /**
>   * ktime_get_snapshot_id -  Simultaneously snapshot a given clock ID with
> - *			    CLOCK_MONOTONIC_RAW and the underlying
> + *			    the corresponding monotonic raw the underlying
>   *			    clocksource counter value.
>   * @clock_id:		The clock ID to snapshot
>   * @systime_snapshot:	Pointer to struct receiving the system time snapshot
> + *
> + * For the system time keeping clocks (REALTIME, MONOTONIC and BOOTTIME) the
> + * monotonic raw clock is CLOCK_MONOTONIC_RAW. For AUX clocks this is the
> + * monotonic raw clock related to the AUX clock. These AUX clock related
> + * monotonic raw clocks have a strict linear offset to the system time
> + * CLOCK_MONOTONIC_RAW:
> + *
> + *	MONOTONIC_RAW(AUX$N) = CLOCK_MONOTONIC_RAW(system) + offset(AUX$N)
> + *
> + * The offset is established when a AUX clock is initialized, but it is
> + * currently not accessible.
>   */
>  void ktime_get_snapshot_id(clockid_t clock_id, struct system_time_snapshot *systime_snapshot)
>  {
> @@ -1512,6 +1523,9 @@ EXPORT_SYMBOL_GPL(ktime_real_to_base_clo
>   * @xtstamp:		Receives simultaneously captured system and device time
>   *
>   * Reads a timestamp from a device and correlates it to system time
> + *
> + * See documentation for ktime_get_snapshot_id() for information about the raw
> + * monotonic time stamp which is used here.
>   */
>  int get_device_system_crosststamp(int (*get_time_fn)
>  				  (ktime_t *device_time,
>