On Tue, Oct 01 2024 at 06:58, Jeff Layton wrote: V8 ? I remember that I reviewed v* already :) Also the sentence after the susbsystem prefix starts with an uppercase letter. > Multigrain timestamps allow the kernel to use fine-grained timestamps > when an inode's attributes is being actively observed via ->getattr(). > With this support, it's possible for a file to get a fine-grained > timestamp, and another modified after it to get a coarse-grained stamp > that is earlier than the fine-grained time. If this happens then the > files can appear to have been modified in reverse order, which breaks > VFS ordering guarantees. > > To prevent this, maintain a floor value for multigrain timestamps. > Whenever a fine-grained timestamp is handed out, record it, and when > coarse-grained stamps are handed out, ensure they are not earlier than > that value. If the coarse-grained timestamp is earlier than the > fine-grained floor, return the floor value instead. > > Add a static singleton atomic64_t into timekeeper.c that we can use to s/we can use/is used/ > keep track of the latest fine-grained time ever handed out. This is > tracked as a monotonic ktime_t value to ensure that it isn't affected by > clock jumps. Because it is updated at different times than the rest of > the timekeeper object, the floor value is managed independently of the > timekeeper via a cmpxchg() operation, and sits on its own cacheline. > > This patch also adds two new public interfaces: git grep 'This patch' Docuementation/process/ > - ktime_get_coarse_real_ts64_mg() fills a timespec64 with the later of the > coarse-grained clock and the floor time > > - ktime_get_real_ts64_mg() gets the fine-grained clock value, and tries > to swap it into the floor. A timespec64 is filled with the result. > > Since the floor is global, take care to avoid updating it unless it's > absolutely necessary. If we do the cmpxchg and find that the value has We do nothing. Please describe your changes in passive voice and do not impersonate code. > been updated since we fetched it, then we discard the fine-grained time > that was fetched in favor of the recent update. This still is confusing. Something like this: The floor value is global and updated via a single try_cmpxchg(). If that fails then the operation raced with a concurrent update. It does not matter whether the new value is later than the value, which the failed cmpxchg() tried to write, or not. Add sensible technical explanation. > +/* > + * Multigrain timestamps require that we keep track of the latest fine-grained > + * timestamp that has been issued, and never return a coarse-grained timestamp > + * that is earlier than that value. > + * > + * mg_floor represents the latest fine-grained time that we have handed out as > + * a timestamp on the system. Tracked as a monotonic ktime_t, and converted to > + * the realtime clock on an as-needed basis. > + * > + * This ensures that we never issue a timestamp earlier than one that has > + * already been issued, as long as the realtime clock never experiences a > + * backward clock jump. If the realtime clock is set to an earlier time, then > + * realtime timestamps can appear to go backward. > + */ > +static __cacheline_aligned_in_smp atomic64_t mg_floor; > + > static inline void tk_normalize_xtime(struct timekeeper *tk) > { > while (tk->tkr_mono.xtime_nsec >= ((u64)NSEC_PER_SEC << tk->tkr_mono.shift)) { > @@ -2394,6 +2410,86 @@ void ktime_get_coarse_real_ts64(struct timespec64 *ts) > } > EXPORT_SYMBOL(ktime_get_coarse_real_ts64); > > +/** > + * ktime_get_coarse_real_ts64_mg - return latter of coarse grained time or floor > + * @ts: timespec64 to be filled > + * > + * Fetch the global mg_floor value, convert it to realtime and > + * compare it to the current coarse-grained time. Fill @ts with > + * whichever is latest. Note that this is a filesystem-specific > + * interface and should be avoided outside of that context. > + */ > +void ktime_get_coarse_real_ts64_mg(struct timespec64 *ts) > +{ > + struct timekeeper *tk = &tk_core.timekeeper; > + u64 floor = atomic64_read(&mg_floor); > + ktime_t f_real, offset, coarse; > + unsigned int seq; > + > + do { > + seq = read_seqcount_begin(&tk_core.seq); > + *ts = tk_xtime(tk); > + offset = tk_core.timekeeper.offs_real; > + } while (read_seqcount_retry(&tk_core.seq, seq)); > + > + coarse = timespec64_to_ktime(*ts); > + f_real = ktime_add(floor, offset); > + if (ktime_after(f_real, coarse)) > + *ts = ktime_to_timespec64(f_real); > +} > +EXPORT_SYMBOL_GPL(ktime_get_coarse_real_ts64_mg); > + > +/** > + * ktime_get_real_ts64_mg - attempt to update floor value and return result > + * @ts: pointer to the timespec to be set > + * > + * Get a monotonic fine-grained time value and attempt to swap it into the > + * floor. If it succeeds then accept the new floor value. If it fails > + * then another task raced in during the interim time and updated the floor. > + * That value is just as valid, so accept that value in this case. Why? 'just as valid' does not tell me anything. > + * @ts will be filled with the resulting floor value, regardless of > + * the outcome of the swap. Note that this is a filesystem specific interface > + * and should be avoided outside of that context. > + */ > +void ktime_get_real_ts64_mg(struct timespec64 *ts) > +{ > + struct timekeeper *tk = &tk_core.timekeeper; > + ktime_t old = atomic64_read(&mg_floor); > + ktime_t offset, mono; > + unsigned int seq; > + u64 nsecs; > + > + do { > + seq = read_seqcount_begin(&tk_core.seq); > + > + ts->tv_sec = tk->xtime_sec; > + mono = tk->tkr_mono.base; > + nsecs = timekeeping_get_ns(&tk->tkr_mono); > + offset = tk_core.timekeeper.offs_real; > + } while (read_seqcount_retry(&tk_core.seq, seq)); > + > + mono = ktime_add_ns(mono, nsecs); > + > + /* > + * Attempt to update the floor with the new time value. Accept the > + * resulting floor value regardless of the outcome of the swap. > + */ > + if (atomic64_try_cmpxchg(&mg_floor, &old, mono)) { > + ts->tv_nsec = 0; > + timespec64_add_ns(ts, nsecs); > + } else { > + /* > + * Something has changed mg_floor since "old" was I complained about 'something has changed ...' before. Can we please have proper technical explanations? > + * fetched. "old" has now been updated with the > + * current value of mg_floor, so use that to return > + * the current coarse floor value. This still does not explain why the new floor value is valid even when it is before the value in @mono. Thanks, tglx