On Tue, May 26, 2026 at 07:13:33PM +0200, Thomas Gleixner wrote: > ktime_get_snapshot() provides a snapshot of the underlying clocksource > counter value and the corresponding CLOCK_MONOTONIC_RAW, CLOCK_REALTIME and > CLOCK_BOOTTIME timestamps. > > There is no usage of CLOCK_REALTIME and CLOCK_BOOTTIME at the same time and > CLOCK_BOOTTIME support was just added for the ARM64 KVM tracing mechanism, > which needs CLOCK_BOOTTIME and the underlying clocksource counter value. > > ktime_get_snapshot() is also not suitable for usage with CLOCK_AUX, but > that's a prerequisite to support PTP hardware timestamping for CLOCK_AUX > steering. > > As a first step, rename ktime_get_snapshot() to ktime_get_snapshot_id(), > which now takes a clockid argument to select the clock which needs to be > captured. The result is stored in system_time_snapshot::sys, which will > replace the system_time_snapshot::real/boot members once all usage sites > have been converted. > > ktime_get_snapshot() is a simple wrapper which hands in CLOCK_REALTIME as > clockid argument for the conversion period. That means CLOCK_REALTIME is > now captured twice, but that redunancy is only temporary. > > No functional change vs. current users of ktime_get_snapshot() > > Signed-off-by: Thomas Gleixner <[email protected]>
Reviewed-by: Thomas Weißschuh <[email protected]> Plus some subjective nitpicks below. > --- > include/linux/timekeeping.h | 29 ++++++++++----- > kernel/time/timekeeping.c | 84 > +++++++++++++++++++++++++++++++++----------- > 2 files changed, 84 insertions(+), 29 deletions(-) > > --- a/include/linux/timekeeping.h > +++ b/include/linux/timekeeping.h > @@ -276,24 +276,28 @@ static inline bool ktime_get_aux_ts64(cl > #endif > > /** > - * struct system_time_snapshot - simultaneous raw/real time capture with > - * counter value > - * @cycles: Clocksource counter value to produce the system times > - * @real: Realtime system time > - * @boot: Boot time > - * @raw: Monotonic raw system time > - * @cs_id: Clocksource ID > + * struct system_time_snapshot - Simultaneous time capture of > CLOCK_MONOTONIC_RAW, > + * a selected CLOCK_* and the clocksource counter > value > + * @cycles: Clocksource counter value to produce the system times > + * @sys: The system time of the selected CLOCK ID > + * @real: Realtime system time > + * @boot: Boot time > + * @raw: Monotonic raw system time > + * @cs_id: Clocksource ID > * @clock_was_set_seq: The sequence number of clock-was-set events > * @cs_was_changed_seq: The sequence number of clocksource change events > + * @valid: True if the snapshot is valid > */ > struct system_time_snapshot { > u64 cycles; > + ktime_t sys; > ktime_t real; > ktime_t boot; > ktime_t raw; > enum clocksource_ids cs_id; > unsigned int clock_was_set_seq; > u8 cs_was_changed_seq; > + u8 valid; > }; > > /** > @@ -341,9 +345,16 @@ extern int get_device_system_crosststamp > struct system_device_crosststamp *xtstamp); > > /* > - * Simultaneously snapshot realtime and monotonic raw clocks > + * Simultaneously snapshot a given clock with MONOTONIC_RAW and the > underlying > + * clocksource counter value. > */ > -extern void ktime_get_snapshot(struct system_time_snapshot > *systime_snapshot); > +extern bool ktime_get_snapshot_id(struct system_time_snapshot > *systime_snapshot, > + clockid_t clock_id); None of the callers (except the wrapper below, which gets removed later) is checking the return value. Having both the return value and the valid member looks a bit weird, too. Clockid parameter first for consistency? > + > +static inline void ktime_get_snapshot(struct system_time_snapshot > *systime_snapshot) > +{ > + WARN_ON_ONCE(!ktime_get_snapshot_id(systime_snapshot, CLOCK_REALTIME)); > +} > > /* > * Persistent clock related interfaces > --- a/kernel/time/timekeeping.c > +++ b/kernel/time/timekeeping.c > @@ -1183,43 +1183,87 @@ noinstr time64_t __ktime_get_real_second > } > > /** > - * ktime_get_snapshot - snapshots the realtime/monotonic raw clocks with > counter > - * @systime_snapshot: pointer to struct receiving the system time > snapshot > + * ktime_get_snapshot_id - Simultaneously snapshot a given clock ID with > + * CLOCK_MONOTONIC_RAW and the underlying > + * clocksource counter value. > + * @systime_snapshot: Pointer to struct receiving the system time > snapshot > + * @clock_id: The clock ID to snapshot > */ > -void ktime_get_snapshot(struct system_time_snapshot *systime_snapshot) > +bool ktime_get_snapshot_id(struct system_time_snapshot *systime_snapshot, > clockid_t clock_id) > { > - struct timekeeper *tk = &tk_core.timekeeper; > + ktime_t base_raw, base_sys, offs_sys, *offs, offs_zero = 0; > + u64 nsec_raw, nsec_sys, now; > + struct timekeeper *tk; > + struct tk_data *tkd; This indirection only makes sense with the support for AUX clocks added later. > unsigned int seq; > - ktime_t base_raw; > ktime_t base_real; > ktime_t base_boot; The ktime_t variable declarations are weird now. > - u64 nsec_raw; > - u64 nsec_real; > - u64 now; > > - WARN_ON_ONCE(timekeeping_suspended); > + /* Invalidate the snapshot for all failure cases */ > + systime_snapshot->valid = false; > + > + if (WARN_ON_ONCE(timekeeping_suspended)) > + return false; > + > + switch (clock_id) { > + case CLOCK_REALTIME: > + tkd = &tk_core; > + offs = &tk_core.timekeeper.offs_real; > + break; > + /* Map RAW to MONOTONIC so the loop below is trivial */ > + case CLOCK_MONOTONIC_RAW: > + case CLOCK_MONOTONIC: > + tkd = &tk_core; > + offs = &offs_zero; > + break; > + case CLOCK_BOOTTIME: > + tkd = &tk_core; > + offs = &tk_core.timekeeper.offs_boot; > + break; > + default: > + WARN_ON_ONCE(1); > + return false; > + } > + > + tk = &tkd->timekeeper; > > do { > - seq = read_seqcount_begin(&tk_core.seq); > + seq = read_seqcount_begin(&tkd->seq); > + > now = tk_clock_read(&tk->tkr_mono); > systime_snapshot->cs_id = tk->tkr_mono.clock->id; > systime_snapshot->cs_was_changed_seq = tk->cs_was_changed_seq; > systime_snapshot->clock_was_set_seq = tk->clock_was_set_seq; > - base_real = ktime_add(tk->tkr_mono.base, > - tk_core.timekeeper.offs_real); > - base_boot = ktime_add(tk->tkr_mono.base, > - tk_core.timekeeper.offs_boot); > + > + base_sys = tk->tkr_mono.base; > + offs_sys = *offs; > base_raw = tk->tkr_raw.base; > - nsec_real = timekeeping_cycles_to_ns(&tk->tkr_mono, now); > - nsec_raw = timekeeping_cycles_to_ns(&tk->tkr_raw, now); > - } while (read_seqcount_retry(&tk_core.seq, seq)); > + > + /* Kept around until the callers are fixed up */ > + base_real = ktime_add(base_sys, tk_core.timekeeper.offs_real); > + base_boot = ktime_add(base_sys, tk_core.timekeeper.offs_boot); > + > + nsec_sys = timekeeping_cycles_to_ns(&tk->tkr_mono, now); > + nsec_raw = timekeeping_cycles_to_ns(&tk->tkr_raw, now); > + } while (read_seqcount_retry(&tkd->seq, seq)); > > systime_snapshot->cycles = now; > - systime_snapshot->real = ktime_add_ns(base_real, nsec_real); > - systime_snapshot->boot = ktime_add_ns(base_boot, nsec_real); > + systime_snapshot->sys = ktime_add_ns(base_sys, offs_sys + nsec_sys); Technically offs_sys is ktime_t and not u64, so would need ktime_add(). (Not that it makes a difference) > + systime_snapshot->real = ktime_add_ns(base_real, nsec_sys); > + systime_snapshot->boot = ktime_add_ns(base_boot, nsec_sys); > systime_snapshot->raw = ktime_add_ns(base_raw, nsec_raw); > + > + /* > + * Special case for PTP. Just transfer the raw time into sys, > + * so the call sites can consistently use snap::sys. > + */ > + if (clock_id == CLOCK_MONOTONIC_RAW) > + systime_snapshot->sys = systime_snapshot->raw; > + /* Tell the consumer that this snapshot is valid */ > + systime_snapshot->valid = true; > + return true; > } > -EXPORT_SYMBOL_GPL(ktime_get_snapshot); > +EXPORT_SYMBOL_GPL(ktime_get_snapshot_id); > > /* Scale base by mult/div checking for overflow */ > static int scale64_check_overflow(u64 mult, u64 div, u64 *base) >
