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

[Thomas Gleixner <tglx@kernel.org>]

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?

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,