From 1a2b85f1e2a93a3f84243e654d225e4088735336 Mon Sep 17 00:00:00 2001 From: Davidlohr Bueso Date: Wed, 21 Oct 2020 12:07:49 -0700 Subject: [PATCH 01/46] timekeeping: Convert jiffies_seq to seqcount_raw_spinlock_t Use the new api and associate the seqcounter to the jiffies_lock enabling lockdep support - although for this particular case the write-side locking and non-preemptibility are quite obvious. Signed-off-by: Davidlohr Bueso Signed-off-by: Thomas Gleixner Link: https://lore.kernel.org/r/20201021190749.19363-1-dave@stgolabs.net --- kernel/time/jiffies.c | 3 ++- kernel/time/timekeeping.h | 2 +- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/kernel/time/jiffies.c b/kernel/time/jiffies.c index eddcf4970444..a5cffe2a1770 100644 --- a/kernel/time/jiffies.c +++ b/kernel/time/jiffies.c @@ -59,7 +59,8 @@ static struct clocksource clocksource_jiffies = { }; __cacheline_aligned_in_smp DEFINE_RAW_SPINLOCK(jiffies_lock); -__cacheline_aligned_in_smp seqcount_t jiffies_seq; +__cacheline_aligned_in_smp seqcount_raw_spinlock_t jiffies_seq = + SEQCNT_RAW_SPINLOCK_ZERO(jiffies_seq, &jiffies_lock); #if (BITS_PER_LONG < 64) u64 get_jiffies_64(void) diff --git a/kernel/time/timekeeping.h b/kernel/time/timekeeping.h index 099737f6f10c..6c2cbd9ef999 100644 --- a/kernel/time/timekeeping.h +++ b/kernel/time/timekeeping.h @@ -26,7 +26,7 @@ extern void do_timer(unsigned long ticks); extern void update_wall_time(void); extern raw_spinlock_t jiffies_lock; -extern seqcount_t jiffies_seq; +extern seqcount_raw_spinlock_t jiffies_seq; #define CS_NAME_LEN 32 From da88f9b3113620dcd30fc203236aa53d5430ee98 Mon Sep 17 00:00:00 2001 From: Helge Deller Date: Wed, 4 Nov 2020 17:34:01 +0100 Subject: [PATCH 02/46] timer_list: Use printk format instead of open-coded symbol lookup Use the "%ps" printk format string to resolve symbol names. This works on all platforms, including ia64, ppc64 and parisc64 on which one needs to dereference pointers to function descriptors instead of function pointers. Signed-off-by: Helge Deller Signed-off-by: Thomas Gleixner Link: https://lore.kernel.org/r/20201104163401.GA3984@ls3530.fritz.box --- kernel/time/timer_list.c | 66 ++++++++++++---------------------------- 1 file changed, 19 insertions(+), 47 deletions(-) diff --git a/kernel/time/timer_list.c b/kernel/time/timer_list.c index acb326f5f50a..6939140ab7c5 100644 --- a/kernel/time/timer_list.c +++ b/kernel/time/timer_list.c @@ -42,24 +42,11 @@ static void SEQ_printf(struct seq_file *m, const char *fmt, ...) va_end(args); } -static void print_name_offset(struct seq_file *m, void *sym) -{ - char symname[KSYM_NAME_LEN]; - - if (lookup_symbol_name((unsigned long)sym, symname) < 0) - SEQ_printf(m, "<%pK>", sym); - else - SEQ_printf(m, "%s", symname); -} - static void print_timer(struct seq_file *m, struct hrtimer *taddr, struct hrtimer *timer, int idx, u64 now) { - SEQ_printf(m, " #%d: ", idx); - print_name_offset(m, taddr); - SEQ_printf(m, ", "); - print_name_offset(m, timer->function); + SEQ_printf(m, " #%d: <%pK>, %ps", idx, taddr, timer->function); SEQ_printf(m, ", S:%02x", timer->state); SEQ_printf(m, "\n"); SEQ_printf(m, " # expires at %Lu-%Lu nsecs [in %Ld to %Ld nsecs]\n", @@ -116,9 +103,7 @@ print_base(struct seq_file *m, struct hrtimer_clock_base *base, u64 now) SEQ_printf(m, " .resolution: %u nsecs\n", hrtimer_resolution); - SEQ_printf(m, " .get_time: "); - print_name_offset(m, base->get_time); - SEQ_printf(m, "\n"); + SEQ_printf(m, " .get_time: %ps\n", base->get_time); #ifdef CONFIG_HIGH_RES_TIMERS SEQ_printf(m, " .offset: %Lu nsecs\n", (unsigned long long) ktime_to_ns(base->offset)); @@ -218,42 +203,29 @@ print_tickdevice(struct seq_file *m, struct tick_device *td, int cpu) SEQ_printf(m, " next_event: %Ld nsecs\n", (unsigned long long) ktime_to_ns(dev->next_event)); - SEQ_printf(m, " set_next_event: "); - print_name_offset(m, dev->set_next_event); - SEQ_printf(m, "\n"); + SEQ_printf(m, " set_next_event: %ps\n", dev->set_next_event); - if (dev->set_state_shutdown) { - SEQ_printf(m, " shutdown: "); - print_name_offset(m, dev->set_state_shutdown); - SEQ_printf(m, "\n"); - } + if (dev->set_state_shutdown) + SEQ_printf(m, " shutdown: %ps\n", + dev->set_state_shutdown); - if (dev->set_state_periodic) { - SEQ_printf(m, " periodic: "); - print_name_offset(m, dev->set_state_periodic); - SEQ_printf(m, "\n"); - } + if (dev->set_state_periodic) + SEQ_printf(m, " periodic: %ps\n", + dev->set_state_periodic); - if (dev->set_state_oneshot) { - SEQ_printf(m, " oneshot: "); - print_name_offset(m, dev->set_state_oneshot); - SEQ_printf(m, "\n"); - } + if (dev->set_state_oneshot) + SEQ_printf(m, " oneshot: %ps\n", + dev->set_state_oneshot); - if (dev->set_state_oneshot_stopped) { - SEQ_printf(m, " oneshot stopped: "); - print_name_offset(m, dev->set_state_oneshot_stopped); - SEQ_printf(m, "\n"); - } + if (dev->set_state_oneshot_stopped) + SEQ_printf(m, " oneshot stopped: %ps\n", + dev->set_state_oneshot_stopped); - if (dev->tick_resume) { - SEQ_printf(m, " resume: "); - print_name_offset(m, dev->tick_resume); - SEQ_printf(m, "\n"); - } + if (dev->tick_resume) + SEQ_printf(m, " resume: %ps\n", + dev->tick_resume); - SEQ_printf(m, " event_handler: "); - print_name_offset(m, dev->event_handler); + SEQ_printf(m, " event_handler: %ps\n", dev->event_handler); SEQ_printf(m, "\n"); SEQ_printf(m, " retries: %lu\n", dev->retries); SEQ_printf(m, "\n"); From c725dafc95f1b37027840aaeaa8b7e4e9cd20516 Mon Sep 17 00:00:00 2001 From: Sebastian Andrzej Siewior Date: Tue, 3 Nov 2020 20:09:37 +0100 Subject: [PATCH 03/46] timers: Don't block on ->expiry_lock for TIMER_IRQSAFE timers PREEMPT_RT does not spin and wait until a running timer completes its callback but instead it blocks on a sleeping lock to prevent a livelock in the case that the task waiting for the callback completion preempted the callback. This cannot be done for timers flagged with TIMER_IRQSAFE. These timers can be canceled from an interrupt disabled context even on RT kernels. The expiry callback of such timers is invoked with interrupts disabled so there is no need to use the expiry lock mechanism because obviously the callback cannot be preempted even on RT kernels. Do not use the timer_base::expiry_lock mechanism when waiting for a running callback to complete if the timer is flagged with TIMER_IRQSAFE. Also add a lockdep assertion for RT kernels to validate that the expiry lock mechanism is always invoked in preemptible context. Reported-by: Mike Galbraith Signed-off-by: Sebastian Andrzej Siewior Signed-off-by: Thomas Gleixner Link: https://lore.kernel.org/r/20201103190937.hga67rqhvknki3tp@linutronix.de --- kernel/time/timer.c | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/kernel/time/timer.c b/kernel/time/timer.c index de37e33a868d..af9ddfbd447d 100644 --- a/kernel/time/timer.c +++ b/kernel/time/timer.c @@ -1288,7 +1288,7 @@ static void del_timer_wait_running(struct timer_list *timer) u32 tf; tf = READ_ONCE(timer->flags); - if (!(tf & TIMER_MIGRATING)) { + if (!(tf & (TIMER_MIGRATING | TIMER_IRQSAFE))) { struct timer_base *base = get_timer_base(tf); /* @@ -1372,6 +1372,13 @@ int del_timer_sync(struct timer_list *timer) */ WARN_ON(in_irq() && !(timer->flags & TIMER_IRQSAFE)); + /* + * Must be able to sleep on PREEMPT_RT because of the slowpath in + * del_timer_wait_running(). + */ + if (IS_ENABLED(CONFIG_PREEMPT_RT) && !(timer->flags & TIMER_IRQSAFE)) + lockdep_assert_preemption_enabled(); + do { ret = try_to_del_timer_sync(timer); From a0f5a65fa5faeef708d022698d5fcba290a35856 Mon Sep 17 00:00:00 2001 From: Alex Shi Date: Fri, 13 Nov 2020 15:24:30 +0800 Subject: [PATCH 04/46] time: Add missing colons for parameter documentation of time64_to_tm() Address these kernel-doc warnings: kernel/time/timeconv.c:79: warning: Function parameter or member 'totalsecs' not described in 'time64_to_tm' kernel/time/timeconv.c:79: warning: Function parameter or member 'offset' not described in 'time64_to_tm' kernel/time/timeconv.c:79: warning: Function parameter or member 'result' not described in 'time64_to_tm' The parameters are described but lack colons after the parameter name. [ tglx: Massaged changelog ] Signed-off-by: Alex Shi Signed-off-by: Thomas Gleixner Link: https://lore.kernel.org/r/1605252275-63652-1-git-send-email-alex.shi@linux.alibaba.com --- kernel/time/timeconv.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/kernel/time/timeconv.c b/kernel/time/timeconv.c index 589e0a552129..62e3b46717a6 100644 --- a/kernel/time/timeconv.c +++ b/kernel/time/timeconv.c @@ -70,10 +70,10 @@ static const unsigned short __mon_yday[2][13] = { /** * time64_to_tm - converts the calendar time to local broken-down time * - * @totalsecs the number of seconds elapsed since 00:00:00 on January 1, 1970, + * @totalsecs: the number of seconds elapsed since 00:00:00 on January 1, 1970, * Coordinated Universal Time (UTC). - * @offset offset seconds adding to totalsecs. - * @result pointer to struct tm variable to receive broken-down time + * @offset: offset seconds adding to totalsecs. + * @result: pointer to struct tm variable to receive broken-down time */ void time64_to_tm(time64_t totalsecs, int offset, struct tm *result) { From 199d280c884de44c3b0daeb77438db43f6db01a2 Mon Sep 17 00:00:00 2001 From: Alex Shi Date: Fri, 13 Nov 2020 15:24:33 +0800 Subject: [PATCH 05/46] timekeeping: Remove static functions from kernel-doc markup Various static functions in the timekeeping code have function comments which pretend to be kernel-doc, but are incomplete and trigger parser warnings. As these functions are local to the timekeeping core code there is no need to expose them via kernel-doc. Remove the double star kernel-doc marker and remove excess newlines. [ tglx: Massaged changelog and removed excess newlines ] Signed-off-by: Alex Shi Signed-off-by: Thomas Gleixner Link: https://lore.kernel.org/r/1605252275-63652-4-git-send-email-alex.shi@linux.alibaba.com --- kernel/time/timekeeping.c | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-) diff --git a/kernel/time/timekeeping.c b/kernel/time/timekeeping.c index 6858a31364b6..570fc500d263 100644 --- a/kernel/time/timekeeping.c +++ b/kernel/time/timekeeping.c @@ -1415,9 +1415,8 @@ void timekeeping_warp_clock(void) } } -/** +/* * __timekeeping_set_tai_offset - Sets the TAI offset from UTC and monotonic - * */ static void __timekeeping_set_tai_offset(struct timekeeper *tk, s32 tai_offset) { @@ -1425,7 +1424,7 @@ static void __timekeeping_set_tai_offset(struct timekeeper *tk, s32 tai_offset) tk->offs_tai = ktime_add(tk->offs_real, ktime_set(tai_offset, 0)); } -/** +/* * change_clocksource - Swaps clocksources if a new one is available * * Accumulates current time interval and initializes new clocksource @@ -2023,13 +2022,12 @@ static void timekeeping_adjust(struct timekeeper *tk, s64 offset) } } -/** +/* * accumulate_nsecs_to_secs - Accumulates nsecs into secs * * Helper function that accumulates the nsecs greater than a second * from the xtime_nsec field to the xtime_secs field. * It also calls into the NTP code to handle leapsecond processing. - * */ static inline unsigned int accumulate_nsecs_to_secs(struct timekeeper *tk) { @@ -2071,7 +2069,7 @@ static inline unsigned int accumulate_nsecs_to_secs(struct timekeeper *tk) return clock_set; } -/** +/* * logarithmic_accumulation - shifted accumulation of cycles * * This functions accumulates a shifted interval of cycles into @@ -2314,7 +2312,7 @@ ktime_t ktime_get_update_offsets_now(unsigned int *cwsseq, ktime_t *offs_real, return base; } -/** +/* * timekeeping_validate_timex - Ensures the timex is ok for use in do_adjtimex */ static int timekeeping_validate_timex(const struct __kernel_timex *txc) From e025b03113d27139ce2b28b82599018e4d8fa5f6 Mon Sep 17 00:00:00 2001 From: Alex Shi Date: Fri, 13 Nov 2020 15:24:31 +0800 Subject: [PATCH 06/46] timekeeping: Add missing parameter documentation for update_fast_timekeeper() Address the following warning: kernel/time/timekeeping.c:415: warning: Function parameter or member 'tkf' not described in 'update_fast_timekeeper' [ tglx: Remove the bogus ktime_get_mono_fast_ns() part ] Signed-off-by: Alex Shi Signed-off-by: Thomas Gleixner Link: https://lore.kernel.org/r/1605252275-63652-2-git-send-email-alex.shi@linux.alibaba.com --- kernel/time/timekeeping.c | 1 + 1 file changed, 1 insertion(+) diff --git a/kernel/time/timekeeping.c b/kernel/time/timekeeping.c index 570fc500d263..a823703c905e 100644 --- a/kernel/time/timekeeping.c +++ b/kernel/time/timekeeping.c @@ -407,6 +407,7 @@ static inline u64 timekeeping_cycles_to_ns(const struct tk_read_base *tkr, u64 c /** * update_fast_timekeeper - Update the fast and NMI safe monotonic timekeeper. * @tkr: Timekeeping readout base from which we take the update + * @tkf: Pointer to NMI safe timekeeper * * We want to use this from any context including NMI and tracing / * instrumenting the timekeeping code itself. From c1ce406e80fb15fa52b2b48dfd48fad6f3d2a32f Mon Sep 17 00:00:00 2001 From: Thomas Gleixner Date: Sun, 15 Nov 2020 21:09:31 +0100 Subject: [PATCH 07/46] timekeeping: Fix up function documentation for the NMI safe accessors Alex reported the following warning: kernel/time/timekeeping.c:464: warning: Function parameter or member 'tkf' not described in '__ktime_get_fast_ns' which is not entirely correct because the documented function is ktime_get_mono_fast_ns() which does not have a parameter, but the kernel-doc parser looks at the function declaration which follows the comment and complains about the missing parameter documentation. Aside of that the documentation for the rest of the NMI safe accessors is either incomplete or missing. - Move the function documentation to the right place - Fixup the references and inconsistencies - Add the missing documentation for ktime_get_raw_fast_ns() Reported-by: Alex Shi Signed-off-by: Thomas Gleixner --- kernel/time/timekeeping.c | 58 ++++++++++++++++++++++----------------- 1 file changed, 33 insertions(+), 25 deletions(-) diff --git a/kernel/time/timekeeping.c b/kernel/time/timekeeping.c index a823703c905e..ab4b83186331 100644 --- a/kernel/time/timekeeping.c +++ b/kernel/time/timekeeping.c @@ -437,6 +437,27 @@ static void update_fast_timekeeper(const struct tk_read_base *tkr, memcpy(base + 1, base, sizeof(*base)); } +static __always_inline u64 __ktime_get_fast_ns(struct tk_fast *tkf) +{ + struct tk_read_base *tkr; + unsigned int seq; + u64 now; + + do { + seq = raw_read_seqcount_latch(&tkf->seq); + tkr = tkf->base + (seq & 0x01); + now = ktime_to_ns(tkr->base); + + now += timekeeping_delta_to_ns(tkr, + clocksource_delta( + tk_clock_read(tkr), + tkr->cycle_last, + tkr->mask)); + } while (read_seqcount_latch_retry(&tkf->seq, seq)); + + return now; +} + /** * ktime_get_mono_fast_ns - Fast NMI safe access to clock monotonic * @@ -463,39 +484,24 @@ static void update_fast_timekeeper(const struct tk_read_base *tkr, * * So reader 6 will observe time going backwards versus reader 5. * - * While other CPUs are likely to be able observe that, the only way + * While other CPUs are likely to be able to observe that, the only way * for a CPU local observation is when an NMI hits in the middle of * the update. Timestamps taken from that NMI context might be ahead * of the following timestamps. Callers need to be aware of that and * deal with it. */ -static __always_inline u64 __ktime_get_fast_ns(struct tk_fast *tkf) -{ - struct tk_read_base *tkr; - unsigned int seq; - u64 now; - - do { - seq = raw_read_seqcount_latch(&tkf->seq); - tkr = tkf->base + (seq & 0x01); - now = ktime_to_ns(tkr->base); - - now += timekeeping_delta_to_ns(tkr, - clocksource_delta( - tk_clock_read(tkr), - tkr->cycle_last, - tkr->mask)); - } while (read_seqcount_latch_retry(&tkf->seq, seq)); - - return now; -} - u64 ktime_get_mono_fast_ns(void) { return __ktime_get_fast_ns(&tk_fast_mono); } EXPORT_SYMBOL_GPL(ktime_get_mono_fast_ns); +/** + * ktime_get_raw_fast_ns - Fast NMI safe access to clock monotonic raw + * + * Contrary to ktime_get_mono_fast_ns() this is always correct because the + * conversion factor is not affected by NTP/PTP correction. + */ u64 ktime_get_raw_fast_ns(void) { return __ktime_get_fast_ns(&tk_fast_raw); @@ -522,6 +528,9 @@ EXPORT_SYMBOL_GPL(ktime_get_raw_fast_ns); * (2) On 32-bit systems, the 64-bit boot offset (tk->offs_boot) may be * partially updated. Since the tk->offs_boot update is a rare event, this * should be a rare occurrence which postprocessing should be able to handle. + * + * The caveats vs. timestamp ordering as documented for ktime_get_fast_ns() + * apply as well. */ u64 notrace ktime_get_boot_fast_ns(void) { @@ -531,9 +540,6 @@ u64 notrace ktime_get_boot_fast_ns(void) } EXPORT_SYMBOL_GPL(ktime_get_boot_fast_ns); -/* - * See comment for __ktime_get_fast_ns() vs. timestamp ordering - */ static __always_inline u64 __ktime_get_real_fast(struct tk_fast *tkf, u64 *mono) { struct tk_read_base *tkr; @@ -558,6 +564,8 @@ static __always_inline u64 __ktime_get_real_fast(struct tk_fast *tkf, u64 *mono) /** * ktime_get_real_fast_ns: - NMI safe and fast access to clock realtime. + * + * See ktime_get_fast_ns() for documentation of the time stamp ordering. */ u64 ktime_get_real_fast_ns(void) { From f27f7c3f100e74a7f451a63a15788f50c52f7cce Mon Sep 17 00:00:00 2001 From: Alex Shi Date: Fri, 13 Nov 2020 15:24:32 +0800 Subject: [PATCH 08/46] timekeeping: Add missing parameter docs for pvclock_gtod_[un]register_notifier() The kernel-doc parser complains about: kernel/time/timekeeping.c:651: warning: Function parameter or member 'nb' not described in 'pvclock_gtod_register_notifier' kernel/time/timekeeping.c:670: warning: Function parameter or member 'nb' not described in 'pvclock_gtod_unregister_notifier' Add the missing parameter explanations. [ tglx: Massaged changelog ] Signed-off-by: Alex Shi Signed-off-by: Thomas Gleixner Link: https://lore.kernel.org/r/1605252275-63652-3-git-send-email-alex.shi@linux.alibaba.com --- kernel/time/timekeeping.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/kernel/time/timekeeping.c b/kernel/time/timekeeping.c index ab4b83186331..9c9392360ade 100644 --- a/kernel/time/timekeeping.c +++ b/kernel/time/timekeeping.c @@ -663,6 +663,7 @@ static void update_pvclock_gtod(struct timekeeper *tk, bool was_set) /** * pvclock_gtod_register_notifier - register a pvclock timedata update listener + * @nb: Pointer to the notifier block to register */ int pvclock_gtod_register_notifier(struct notifier_block *nb) { @@ -682,6 +683,7 @@ EXPORT_SYMBOL_GPL(pvclock_gtod_register_notifier); /** * pvclock_gtod_unregister_notifier - unregister a pvclock * timedata update listener + * @nb: Pointer to the notifier block to unregister */ int pvclock_gtod_unregister_notifier(struct notifier_block *nb) { From 29efc4612ac1b888e65da408b41dafa4dd00842f Mon Sep 17 00:00:00 2001 From: Alex Shi Date: Fri, 13 Nov 2020 15:24:35 +0800 Subject: [PATCH 09/46] timekeeping: Fix parameter docs of read_persistent_wall_and_boot_offset() Address the following kernel-doc markup warnings: kernel/time/timekeeping.c:1563: warning: Function parameter or member 'wall_time' not described in 'read_persistent_wall_and_boot_offset' kernel/time/timekeeping.c:1563: warning: Function parameter or member 'boot_offset' not described in 'read_persistent_wall_and_boot_offset' The parameters are described but miss the leading '@' and the colon after the parameter names. [ tglx: Massaged changelog ] Signed-off-by: Alex Shi Signed-off-by: Thomas Gleixner Link: https://lore.kernel.org/r/1605252275-63652-6-git-send-email-alex.shi@linux.alibaba.com --- kernel/time/timekeeping.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/kernel/time/timekeeping.c b/kernel/time/timekeeping.c index 9c9392360ade..75cba958ae29 100644 --- a/kernel/time/timekeeping.c +++ b/kernel/time/timekeeping.c @@ -1576,8 +1576,9 @@ void __weak read_persistent_clock64(struct timespec64 *ts) * from the boot. * * Weak dummy function for arches that do not yet support it. - * wall_time - current time as returned by persistent clock - * boot_offset - offset that is defined as wall_time - boot_time + * @wall_time: - current time as returned by persistent clock + * @boot_offset: - offset that is defined as wall_time - boot_time + * * The default function calculates offset based on the current value of * local_clock(). This way architectures that support sched_clock() but don't * support dedicated boot time clock will provide the best estimate of the From 6e5a91901c2dff3a0f2eb9f10e427dce2b0488fc Mon Sep 17 00:00:00 2001 From: Alex Shi Date: Fri, 13 Nov 2020 15:24:34 +0800 Subject: [PATCH 10/46] timekeeping: Address parameter documentation issues for various functions The kernel-doc parser complains: kernel/time/timekeeping.c:1543: warning: Function parameter or member 'ts' not described in 'read_persistent_clock64' kernel/time/timekeeping.c:764: warning: Function parameter or member 'tk' not described in 'timekeeping_forward_now' kernel/time/timekeeping.c:1331: warning: Function parameter or member 'ts' not described in 'timekeeping_inject_offset' kernel/time/timekeeping.c:1331: warning: Excess function parameter 'tv' description in 'timekeeping_inject_offset' Add the missing parameter documentations and rename the 'tv' parameter of timekeeping_inject_offset() to 'ts' so it matches the implemention. [ tglx: Reworded a few docs and massaged changelog ] Signed-off-by: Alex Shi Signed-off-by: Thomas Gleixner Link: https://lore.kernel.org/r/1605252275-63652-5-git-send-email-alex.shi@linux.alibaba.com --- kernel/time/timekeeping.c | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/kernel/time/timekeeping.c b/kernel/time/timekeeping.c index 75cba958ae29..74503c0151e5 100644 --- a/kernel/time/timekeeping.c +++ b/kernel/time/timekeeping.c @@ -774,6 +774,7 @@ static void timekeeping_update(struct timekeeper *tk, unsigned int action) /** * timekeeping_forward_now - update clock to the current time + * @tk: Pointer to the timekeeper to update * * Forward the current clock to update its state since the last call to * update_wall_time(). This is useful before significant clock changes, @@ -1350,7 +1351,7 @@ EXPORT_SYMBOL(do_settimeofday64); /** * timekeeping_inject_offset - Adds or subtracts from the current time. - * @tv: pointer to the timespec variable containing the offset + * @ts: Pointer to the timespec variable containing the offset * * Adds or subtracts an offset value from the current time. */ @@ -1558,6 +1559,7 @@ u64 timekeeping_max_deferment(void) /** * read_persistent_clock64 - Return time from the persistent clock. + * @ts: Pointer to the storage for the readout value * * Weak dummy function for arches that do not yet support it. * Reads the time from the battery backed persistent clock. @@ -1663,7 +1665,8 @@ static struct timespec64 timekeeping_suspend_time; /** * __timekeeping_inject_sleeptime - Internal function to add sleep interval - * @delta: pointer to a timespec delta value + * @tk: Pointer to the timekeeper to be updated + * @delta: Pointer to the delta value in timespec64 format * * Takes a timespec offset measuring a suspend interval and properly * adds the sleep offset to the timekeeping variables. From cc947f2b9c04113d84eeef67cc7c6326e1982019 Mon Sep 17 00:00:00 2001 From: Thomas Gleixner Date: Mon, 16 Nov 2020 10:53:38 +0100 Subject: [PATCH 11/46] timers: Make run_local_timers() static No users outside of the timer code. Move the caller below this function to avoid a pointless forward declaration. Signed-off-by: Thomas Gleixner --- include/linux/timer.h | 1 - kernel/time/timer.c | 48 +++++++++++++++++++++---------------------- 2 files changed, 24 insertions(+), 25 deletions(-) diff --git a/include/linux/timer.h b/include/linux/timer.h index d10bc7e73b41..fda13c9d1256 100644 --- a/include/linux/timer.h +++ b/include/linux/timer.h @@ -193,7 +193,6 @@ extern int try_to_del_timer_sync(struct timer_list *timer); #define del_singleshot_timer_sync(t) del_timer_sync(t) extern void init_timers(void); -extern void run_local_timers(void); struct hrtimer; extern enum hrtimer_restart it_real_fn(struct hrtimer *); diff --git a/kernel/time/timer.c b/kernel/time/timer.c index af9ddfbd447d..ebf3b26d2501 100644 --- a/kernel/time/timer.c +++ b/kernel/time/timer.c @@ -1705,29 +1705,6 @@ void timer_clear_idle(void) } #endif -/* - * Called from the timer interrupt handler to charge one tick to the current - * process. user_tick is 1 if the tick is user time, 0 for system. - */ -void update_process_times(int user_tick) -{ - struct task_struct *p = current; - - PRANDOM_ADD_NOISE(jiffies, user_tick, p, 0); - - /* Note: this timer irq context must be accounted for as well. */ - account_process_tick(p, user_tick); - run_local_timers(); - rcu_sched_clock_irq(user_tick); -#ifdef CONFIG_IRQ_WORK - if (in_irq()) - irq_work_tick(); -#endif - scheduler_tick(); - if (IS_ENABLED(CONFIG_POSIX_TIMERS)) - run_posix_cpu_timers(); -} - /** * __run_timers - run all expired timers (if any) on this CPU. * @base: the timer vector to be processed. @@ -1777,7 +1754,7 @@ static __latent_entropy void run_timer_softirq(struct softirq_action *h) /* * Called by the local, per-CPU timer interrupt on SMP. */ -void run_local_timers(void) +static void run_local_timers(void) { struct timer_base *base = this_cpu_ptr(&timer_bases[BASE_STD]); @@ -1794,6 +1771,29 @@ void run_local_timers(void) raise_softirq(TIMER_SOFTIRQ); } +/* + * Called from the timer interrupt handler to charge one tick to the current + * process. user_tick is 1 if the tick is user time, 0 for system. + */ +void update_process_times(int user_tick) +{ + struct task_struct *p = current; + + PRANDOM_ADD_NOISE(jiffies, user_tick, p, 0); + + /* Note: this timer irq context must be accounted for as well. */ + account_process_tick(p, user_tick); + run_local_timers(); + rcu_sched_clock_irq(user_tick); +#ifdef CONFIG_IRQ_WORK + if (in_irq()) + irq_work_tick(); +#endif + scheduler_tick(); + if (IS_ENABLED(CONFIG_POSIX_TIMERS)) + run_posix_cpu_timers(); +} + /* * Since schedule_timeout()'s timer is defined on the stack, it must store * the target task on the stack as well. From 66981c37b3199d293c58f84cf2366e86a06e1a3d Mon Sep 17 00:00:00 2001 From: Mauro Carvalho Chehab Date: Mon, 16 Nov 2020 11:18:14 +0100 Subject: [PATCH 12/46] hrtimer: Fix kernel-doc markups The hrtimer_get_remaining() markup is documenting, instead, __hrtimer_get_remaining(), as it is placed at the C file. In order to properly document it, a kernel-doc markup is needed together with the function prototype. So, add a new one, while preserving the existing one, just fixing the function name. The hrtimer_is_queued prototype has a typo: it is using '=' instead of '-' to split: identifier - description as required by kernel-doc markup. Signed-off-by: Mauro Carvalho Chehab Signed-off-by: Thomas Gleixner Link: https://lore.kernel.org/r/9dc87808c2fd07b7e050bafcd033c5ef05808fea.1605521731.git.mchehab+huawei@kernel.org --- include/linux/hrtimer.h | 6 +++++- kernel/time/hrtimer.c | 2 +- 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/include/linux/hrtimer.h b/include/linux/hrtimer.h index 107cedd7019a..bb5e7b0a4274 100644 --- a/include/linux/hrtimer.h +++ b/include/linux/hrtimer.h @@ -447,6 +447,10 @@ static inline void hrtimer_restart(struct hrtimer *timer) /* Query timers: */ extern ktime_t __hrtimer_get_remaining(const struct hrtimer *timer, bool adjust); +/** + * hrtimer_get_remaining - get remaining time for the timer + * @timer: the timer to read + */ static inline ktime_t hrtimer_get_remaining(const struct hrtimer *timer) { return __hrtimer_get_remaining(timer, false); @@ -458,7 +462,7 @@ extern u64 hrtimer_next_event_without(const struct hrtimer *exclude); extern bool hrtimer_active(const struct hrtimer *timer); /** - * hrtimer_is_queued = check, whether the timer is on one of the queues + * hrtimer_is_queued - check, whether the timer is on one of the queues * @timer: Timer to check * * Returns: True if the timer is queued, false otherwise diff --git a/kernel/time/hrtimer.c b/kernel/time/hrtimer.c index 3624b9b5835d..61c39ff68439 100644 --- a/kernel/time/hrtimer.c +++ b/kernel/time/hrtimer.c @@ -1289,7 +1289,7 @@ int hrtimer_cancel(struct hrtimer *timer) EXPORT_SYMBOL_GPL(hrtimer_cancel); /** - * hrtimer_get_remaining - get remaining time for the timer + * __hrtimer_get_remaining - get remaining time for the timer * @timer: the timer to read * @adjust: adjust relative timers when CONFIG_TIME_LOW_RES=y */ From f73f64d5687192bc8eb7f3d9521ca6256b79f224 Mon Sep 17 00:00:00 2001 From: Thomas Gleixner Date: Tue, 17 Nov 2020 14:19:43 +0100 Subject: [PATCH 13/46] tick/broadcast: Serialize access to tick_next_period tick_broadcast_setup_oneshot() accesses tick_next_period twice without any serialization. This is wrong in two aspects: - Reading it twice might make the broadcast data inconsistent if the variable is updated concurrently. - On 32bit systems the access might see an partial update Protect it with jiffies_lock. That's safe as none of the callchains leading up to this function can create a lock ordering violation: timer interrupt run_local_timers() hrtimer_run_queues() hrtimer_switch_to_hres() tick_init_highres() tick_switch_to_oneshot() tick_broadcast_switch_to_oneshot() or tick_check_oneshot_change() tick_nohz_switch_to_nohz() tick_switch_to_oneshot() tick_broadcast_switch_to_oneshot() Signed-off-by: Thomas Gleixner Link: https://lore.kernel.org/r/20201117132006.061341507@linutronix.de --- kernel/time/tick-broadcast.c | 23 ++++++++++++++++++++--- 1 file changed, 20 insertions(+), 3 deletions(-) diff --git a/kernel/time/tick-broadcast.c b/kernel/time/tick-broadcast.c index 36d7464c8962..2a47c8f80e53 100644 --- a/kernel/time/tick-broadcast.c +++ b/kernel/time/tick-broadcast.c @@ -877,6 +877,22 @@ static void tick_broadcast_init_next_event(struct cpumask *mask, } } +static inline ktime_t tick_get_next_period(void) +{ + ktime_t next; + + /* + * Protect against concurrent updates (store /load tearing on + * 32bit). It does not matter if the time is already in the + * past. The broadcast device which is about to be programmed will + * fire in any case. + */ + raw_spin_lock(&jiffies_lock); + next = tick_next_period; + raw_spin_unlock(&jiffies_lock); + return next; +} + /** * tick_broadcast_setup_oneshot - setup the broadcast device */ @@ -905,10 +921,11 @@ static void tick_broadcast_setup_oneshot(struct clock_event_device *bc) tick_broadcast_oneshot_mask, tmpmask); if (was_periodic && !cpumask_empty(tmpmask)) { + ktime_t nextevt = tick_get_next_period(); + clockevents_switch_state(bc, CLOCK_EVT_STATE_ONESHOT); - tick_broadcast_init_next_event(tmpmask, - tick_next_period); - tick_broadcast_set_event(bc, cpu, tick_next_period); + tick_broadcast_init_next_event(tmpmask, nextevt); + tick_broadcast_set_event(bc, cpu, nextevt); } else bc->next_event = KTIME_MAX; } else { From c398960cd82b233886fbff163986f998b5a5c008 Mon Sep 17 00:00:00 2001 From: Thomas Gleixner Date: Tue, 17 Nov 2020 14:19:44 +0100 Subject: [PATCH 14/46] tick: Document protections for tick related data The protection rules for tick_next_period and last_jiffies_update are blury at best. Clarify this. Signed-off-by: Thomas Gleixner Link: https://lore.kernel.org/r/20201117132006.197713794@linutronix.de --- kernel/time/tick-common.c | 4 +++- kernel/time/tick-sched.c | 4 +++- 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/kernel/time/tick-common.c b/kernel/time/tick-common.c index 6c9c342dd0e5..68504eb0d38b 100644 --- a/kernel/time/tick-common.c +++ b/kernel/time/tick-common.c @@ -27,7 +27,9 @@ */ DEFINE_PER_CPU(struct tick_device, tick_cpu_device); /* - * Tick next event: keeps track of the tick time + * Tick next event: keeps track of the tick time. It's updated by the + * CPU which handles the tick and protected by jiffies_lock. There is + * no requirement to write hold the jiffies seqcount for it. */ ktime_t tick_next_period; ktime_t tick_period; diff --git a/kernel/time/tick-sched.c b/kernel/time/tick-sched.c index 81632cd5e3b7..15360e652c85 100644 --- a/kernel/time/tick-sched.c +++ b/kernel/time/tick-sched.c @@ -44,7 +44,9 @@ struct tick_sched *tick_get_tick_sched(int cpu) #if defined(CONFIG_NO_HZ_COMMON) || defined(CONFIG_HIGH_RES_TIMERS) /* - * The time, when the last jiffy update happened. Protected by jiffies_lock. + * The time, when the last jiffy update happened. Write access must hold + * jiffies_lock and jiffies_seq. tick_nohz_next_event() needs to get a + * consistent view of jiffies and last_jiffies_update. */ static ktime_t last_jiffies_update; From 372acbbaa80940189593f9d69c7c069955f24f7a Mon Sep 17 00:00:00 2001 From: Thomas Gleixner Date: Tue, 17 Nov 2020 14:19:45 +0100 Subject: [PATCH 15/46] tick/sched: Use tick_next_period for lockless quick check No point in doing calculations. tick_next_period = last_jiffies_update + tick_period Just check whether now is before tick_next_period to figure out whether jiffies need an update. Add a comment why the intentional data race in the quick check is safe or not so safe in a 32bit corner case and why we don't worry about it. Signed-off-by: Thomas Gleixner Link: https://lore.kernel.org/r/20201117132006.337366695@linutronix.de --- kernel/time/tick-sched.c | 46 ++++++++++++++++++++++++++++------------ 1 file changed, 33 insertions(+), 13 deletions(-) diff --git a/kernel/time/tick-sched.c b/kernel/time/tick-sched.c index 15360e652c85..b4b6abc81e4a 100644 --- a/kernel/time/tick-sched.c +++ b/kernel/time/tick-sched.c @@ -59,11 +59,29 @@ static void tick_do_update_jiffies64(ktime_t now) ktime_t delta; /* - * Do a quick check without holding jiffies_lock: - * The READ_ONCE() pairs with two updates done later in this function. + * Do a quick check without holding jiffies_lock. The READ_ONCE() + * pairs with the update done later in this function. + * + * This is also an intentional data race which is even safe on + * 32bit in theory. If there is a concurrent update then the check + * might give a random answer. It does not matter because if it + * returns then the concurrent update is already taking care, if it + * falls through then it will pointlessly contend on jiffies_lock. + * + * Though there is one nasty case on 32bit due to store tearing of + * the 64bit value. If the first 32bit store makes the quick check + * return on all other CPUs and the writing CPU context gets + * delayed to complete the second store (scheduled out on virt) + * then jiffies can become stale for up to ~2^32 nanoseconds + * without noticing. After that point all CPUs will wait for + * jiffies lock. + * + * OTOH, this is not any different than the situation with NOHZ=off + * where one CPU is responsible for updating jiffies and + * timekeeping. If that CPU goes out for lunch then all other CPUs + * will operate on stale jiffies until it decides to come back. */ - delta = ktime_sub(now, READ_ONCE(last_jiffies_update)); - if (delta < tick_period) + if (ktime_before(now, READ_ONCE(tick_next_period))) return; /* Reevaluate with jiffies_lock held */ @@ -74,9 +92,8 @@ static void tick_do_update_jiffies64(ktime_t now) if (delta >= tick_period) { delta = ktime_sub(delta, tick_period); - /* Pairs with the lockless read in this function. */ - WRITE_ONCE(last_jiffies_update, - ktime_add(last_jiffies_update, tick_period)); + last_jiffies_update = ktime_add(last_jiffies_update, + tick_period); /* Slow path for long timeouts */ if (unlikely(delta >= tick_period)) { @@ -84,15 +101,18 @@ static void tick_do_update_jiffies64(ktime_t now) ticks = ktime_divns(delta, incr); - /* Pairs with the lockless read in this function. */ - WRITE_ONCE(last_jiffies_update, - ktime_add_ns(last_jiffies_update, - incr * ticks)); + last_jiffies_update = ktime_add_ns(last_jiffies_update, + incr * ticks); } do_timer(++ticks); - /* Keep the tick_next_period variable up to date */ - tick_next_period = ktime_add(last_jiffies_update, tick_period); + /* + * Keep the tick_next_period variable up to date. + * WRITE_ONCE() pairs with the READ_ONCE() in the lockless + * quick check above. + */ + WRITE_ONCE(tick_next_period, + ktime_add(last_jiffies_update, tick_period)); } else { write_seqcount_end(&jiffies_seq); raw_spin_unlock(&jiffies_lock); From 94ad2e3cedb82af034f6d97c58022f162b669f9b Mon Sep 17 00:00:00 2001 From: Yunfeng Ye Date: Tue, 17 Nov 2020 14:19:46 +0100 Subject: [PATCH 16/46] tick/sched: Reduce seqcount held scope in tick_do_update_jiffies64() If jiffies are up to date already (caller lost the race against another CPU) there is no point to change the sequence count. Doing that just forces other CPUs into the seqcount retry loop in tick_nohz_next_event() for nothing. Just bail out early. [ tglx: Rewrote most of it ] Signed-off-by: Yunfeng Ye Signed-off-by: Thomas Gleixner Link: https://lore.kernel.org/r/20201117132006.462195901@linutronix.de --- kernel/time/tick-sched.c | 55 +++++++++++++++++++--------------------- 1 file changed, 26 insertions(+), 29 deletions(-) diff --git a/kernel/time/tick-sched.c b/kernel/time/tick-sched.c index b4b6abc81e4a..ca9191ced4b5 100644 --- a/kernel/time/tick-sched.c +++ b/kernel/time/tick-sched.c @@ -86,38 +86,35 @@ static void tick_do_update_jiffies64(ktime_t now) /* Reevaluate with jiffies_lock held */ raw_spin_lock(&jiffies_lock); - write_seqcount_begin(&jiffies_seq); - - delta = ktime_sub(now, last_jiffies_update); - if (delta >= tick_period) { - - delta = ktime_sub(delta, tick_period); - last_jiffies_update = ktime_add(last_jiffies_update, - tick_period); - - /* Slow path for long timeouts */ - if (unlikely(delta >= tick_period)) { - s64 incr = ktime_to_ns(tick_period); - - ticks = ktime_divns(delta, incr); - - last_jiffies_update = ktime_add_ns(last_jiffies_update, - incr * ticks); - } - do_timer(++ticks); - - /* - * Keep the tick_next_period variable up to date. - * WRITE_ONCE() pairs with the READ_ONCE() in the lockless - * quick check above. - */ - WRITE_ONCE(tick_next_period, - ktime_add(last_jiffies_update, tick_period)); - } else { - write_seqcount_end(&jiffies_seq); + if (ktime_before(now, tick_next_period)) { raw_spin_unlock(&jiffies_lock); return; } + + write_seqcount_begin(&jiffies_seq); + + last_jiffies_update = ktime_add(last_jiffies_update, tick_period); + + delta = ktime_sub(now, tick_next_period); + if (unlikely(delta >= tick_period)) { + /* Slow path for long idle sleep times */ + s64 incr = ktime_to_ns(tick_period); + + ticks = ktime_divns(delta, incr); + + last_jiffies_update = ktime_add_ns(last_jiffies_update, + incr * ticks); + } + + do_timer(++ticks); + + /* + * Keep the tick_next_period variable up to date. WRITE_ONCE() + * pairs with the READ_ONCE() in the lockless quick check above. + */ + WRITE_ONCE(tick_next_period, + ktime_add(last_jiffies_update, tick_period)); + write_seqcount_end(&jiffies_seq); raw_spin_unlock(&jiffies_lock); update_wall_time(); From 7a35bf2a6a871cd0252cd371d741e7d070b53af9 Mon Sep 17 00:00:00 2001 From: Thomas Gleixner Date: Tue, 17 Nov 2020 14:19:47 +0100 Subject: [PATCH 17/46] tick/sched: Optimize tick_do_update_jiffies64() further Now that it's clear that there is always one tick to account, simplify the calculations some more. Signed-off-by: Thomas Gleixner Link: https://lore.kernel.org/r/20201117132006.565663056@linutronix.de --- kernel/time/tick-sched.c | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/kernel/time/tick-sched.c b/kernel/time/tick-sched.c index ca9191ced4b5..306adeb6ce4c 100644 --- a/kernel/time/tick-sched.c +++ b/kernel/time/tick-sched.c @@ -55,7 +55,7 @@ static ktime_t last_jiffies_update; */ static void tick_do_update_jiffies64(ktime_t now) { - unsigned long ticks = 0; + unsigned long ticks = 1; ktime_t delta; /* @@ -93,20 +93,21 @@ static void tick_do_update_jiffies64(ktime_t now) write_seqcount_begin(&jiffies_seq); - last_jiffies_update = ktime_add(last_jiffies_update, tick_period); - delta = ktime_sub(now, tick_next_period); if (unlikely(delta >= tick_period)) { /* Slow path for long idle sleep times */ s64 incr = ktime_to_ns(tick_period); - ticks = ktime_divns(delta, incr); + ticks += ktime_divns(delta, incr); last_jiffies_update = ktime_add_ns(last_jiffies_update, incr * ticks); + } else { + last_jiffies_update = ktime_add(last_jiffies_update, + tick_period); } - do_timer(++ticks); + do_timer(ticks); /* * Keep the tick_next_period variable up to date. WRITE_ONCE() From 896b969e6732b68ee3c12ae4e1aeddf5db99bc46 Mon Sep 17 00:00:00 2001 From: Yunfeng Ye Date: Tue, 17 Nov 2020 14:19:48 +0100 Subject: [PATCH 18/46] tick/sched: Release seqcount before invoking calc_load_global() calc_load_global() does not need the sequence count protection. [ tglx: Split it up properly and added comments ] Signed-off-by: Yunfeng Ye Signed-off-by: Thomas Gleixner Link: https://lore.kernel.org/r/20201117132006.660902274@linutronix.de --- kernel/time/tick-sched.c | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/kernel/time/tick-sched.c b/kernel/time/tick-sched.c index 306adeb6ce4c..33c897bb88c6 100644 --- a/kernel/time/tick-sched.c +++ b/kernel/time/tick-sched.c @@ -20,6 +20,7 @@ #include #include #include +#include #include #include #include @@ -107,7 +108,8 @@ static void tick_do_update_jiffies64(ktime_t now) tick_period); } - do_timer(ticks); + /* Advance jiffies to complete the jiffies_seq protected job */ + jiffies_64 += ticks; /* * Keep the tick_next_period variable up to date. WRITE_ONCE() @@ -116,7 +118,15 @@ static void tick_do_update_jiffies64(ktime_t now) WRITE_ONCE(tick_next_period, ktime_add(last_jiffies_update, tick_period)); + /* + * Release the sequence count. calc_global_load() below is not + * protected by it, but jiffies_lock needs to be held to prevent + * concurrent invocations. + */ write_seqcount_end(&jiffies_seq); + + calc_global_load(); + raw_spin_unlock(&jiffies_lock); update_wall_time(); } From b996544916429946bf4934c1c01a306d1690972c Mon Sep 17 00:00:00 2001 From: Thomas Gleixner Date: Tue, 17 Nov 2020 14:19:49 +0100 Subject: [PATCH 19/46] tick: Get rid of tick_period The variable tick_period is initialized to NSEC_PER_TICK / HZ during boot and never updated again. If NSEC_PER_TICK is not an integer multiple of HZ this computation is less accurate than TICK_NSEC which has proper rounding in place. Aside of the inaccuracy there is no reason for having this variable at all. It's just a pointless indirection and all usage sites can just use the TICK_NSEC constant. Signed-off-by: Thomas Gleixner Link: https://lore.kernel.org/r/20201117132006.766643526@linutronix.de --- kernel/time/tick-broadcast.c | 2 +- kernel/time/tick-common.c | 8 +++----- kernel/time/tick-internal.h | 1 - kernel/time/tick-sched.c | 22 +++++++++++----------- 4 files changed, 15 insertions(+), 18 deletions(-) diff --git a/kernel/time/tick-broadcast.c b/kernel/time/tick-broadcast.c index 2a47c8f80e53..5a23829372c7 100644 --- a/kernel/time/tick-broadcast.c +++ b/kernel/time/tick-broadcast.c @@ -331,7 +331,7 @@ static void tick_handle_periodic_broadcast(struct clock_event_device *dev) bc_local = tick_do_periodic_broadcast(); if (clockevent_state_oneshot(dev)) { - ktime_t next = ktime_add(dev->next_event, tick_period); + ktime_t next = ktime_add_ns(dev->next_event, TICK_NSEC); clockevents_program_event(dev, next, true); } diff --git a/kernel/time/tick-common.c b/kernel/time/tick-common.c index 68504eb0d38b..a03764df5366 100644 --- a/kernel/time/tick-common.c +++ b/kernel/time/tick-common.c @@ -32,7 +32,6 @@ DEFINE_PER_CPU(struct tick_device, tick_cpu_device); * no requirement to write hold the jiffies seqcount for it. */ ktime_t tick_next_period; -ktime_t tick_period; /* * tick_do_timer_cpu is a timer core internal variable which holds the CPU NR @@ -90,7 +89,7 @@ static void tick_periodic(int cpu) write_seqcount_begin(&jiffies_seq); /* Keep track of the next tick event */ - tick_next_period = ktime_add(tick_next_period, tick_period); + tick_next_period = ktime_add_ns(tick_next_period, TICK_NSEC); do_timer(1); write_seqcount_end(&jiffies_seq); @@ -129,7 +128,7 @@ void tick_handle_periodic(struct clock_event_device *dev) * Setup the next period for devices, which do not have * periodic mode: */ - next = ktime_add(next, tick_period); + next = ktime_add_ns(next, TICK_NSEC); if (!clockevents_program_event(dev, next, false)) return; @@ -175,7 +174,7 @@ void tick_setup_periodic(struct clock_event_device *dev, int broadcast) for (;;) { if (!clockevents_program_event(dev, next, false)) return; - next = ktime_add(next, tick_period); + next = ktime_add_ns(next, TICK_NSEC); } } } @@ -222,7 +221,6 @@ static void tick_setup_device(struct tick_device *td, tick_do_timer_cpu = cpu; tick_next_period = ktime_get(); - tick_period = NSEC_PER_SEC / HZ; #ifdef CONFIG_NO_HZ_FULL /* * The boot CPU may be nohz_full, in which case set diff --git a/kernel/time/tick-internal.h b/kernel/time/tick-internal.h index 7b2496136729..7a981c9e87a4 100644 --- a/kernel/time/tick-internal.h +++ b/kernel/time/tick-internal.h @@ -15,7 +15,6 @@ DECLARE_PER_CPU(struct tick_device, tick_cpu_device); extern ktime_t tick_next_period; -extern ktime_t tick_period; extern int tick_do_timer_cpu __read_mostly; extern void tick_setup_periodic(struct clock_event_device *dev, int broadcast); diff --git a/kernel/time/tick-sched.c b/kernel/time/tick-sched.c index 33c897bb88c6..cc7cba20382e 100644 --- a/kernel/time/tick-sched.c +++ b/kernel/time/tick-sched.c @@ -95,17 +95,17 @@ static void tick_do_update_jiffies64(ktime_t now) write_seqcount_begin(&jiffies_seq); delta = ktime_sub(now, tick_next_period); - if (unlikely(delta >= tick_period)) { + if (unlikely(delta >= TICK_NSEC)) { /* Slow path for long idle sleep times */ - s64 incr = ktime_to_ns(tick_period); + s64 incr = TICK_NSEC; ticks += ktime_divns(delta, incr); last_jiffies_update = ktime_add_ns(last_jiffies_update, incr * ticks); } else { - last_jiffies_update = ktime_add(last_jiffies_update, - tick_period); + last_jiffies_update = ktime_add_ns(last_jiffies_update, + TICK_NSEC); } /* Advance jiffies to complete the jiffies_seq protected job */ @@ -116,7 +116,7 @@ static void tick_do_update_jiffies64(ktime_t now) * pairs with the READ_ONCE() in the lockless quick check above. */ WRITE_ONCE(tick_next_period, - ktime_add(last_jiffies_update, tick_period)); + ktime_add_ns(last_jiffies_update, TICK_NSEC)); /* * Release the sequence count. calc_global_load() below is not @@ -691,7 +691,7 @@ static void tick_nohz_restart(struct tick_sched *ts, ktime_t now) hrtimer_set_expires(&ts->sched_timer, ts->last_tick); /* Forward the time to expire in the future */ - hrtimer_forward(&ts->sched_timer, now, tick_period); + hrtimer_forward(&ts->sched_timer, now, TICK_NSEC); if (ts->nohz_mode == NOHZ_MODE_HIGHRES) { hrtimer_start_expires(&ts->sched_timer, @@ -1260,7 +1260,7 @@ static void tick_nohz_handler(struct clock_event_device *dev) if (unlikely(ts->tick_stopped)) return; - hrtimer_forward(&ts->sched_timer, now, tick_period); + hrtimer_forward(&ts->sched_timer, now, TICK_NSEC); tick_program_event(hrtimer_get_expires(&ts->sched_timer), 1); } @@ -1297,7 +1297,7 @@ static void tick_nohz_switch_to_nohz(void) next = tick_init_jiffy_update(); hrtimer_set_expires(&ts->sched_timer, next); - hrtimer_forward_now(&ts->sched_timer, tick_period); + hrtimer_forward_now(&ts->sched_timer, TICK_NSEC); tick_program_event(hrtimer_get_expires(&ts->sched_timer), 1); tick_nohz_activate(ts, NOHZ_MODE_LOWRES); } @@ -1363,7 +1363,7 @@ static enum hrtimer_restart tick_sched_timer(struct hrtimer *timer) if (unlikely(ts->tick_stopped)) return HRTIMER_NORESTART; - hrtimer_forward(timer, now, tick_period); + hrtimer_forward(timer, now, TICK_NSEC); return HRTIMER_RESTART; } @@ -1397,13 +1397,13 @@ void tick_setup_sched_timer(void) /* Offset the tick to avert jiffies_lock contention. */ if (sched_skew_tick) { - u64 offset = ktime_to_ns(tick_period) >> 1; + u64 offset = TICK_NSEC >> 1; do_div(offset, num_possible_cpus()); offset *= smp_processor_id(); hrtimer_add_expires_ns(&ts->sched_timer, offset); } - hrtimer_forward(&ts->sched_timer, now, tick_period); + hrtimer_forward(&ts->sched_timer, now, TICK_NSEC); hrtimer_start_expires(&ts->sched_timer, HRTIMER_MODE_ABS_PINNED_HARD); tick_nohz_activate(ts, NOHZ_MODE_HIGHRES); } From 3c0a4b185f6c82c06025720b00a490c719a6f0ff Mon Sep 17 00:00:00 2001 From: Zhen Lei Date: Wed, 21 Oct 2020 09:22:59 +0800 Subject: [PATCH 20/46] clocksource/drivers/sp804: Add static for functions such as sp804_clockevents_init() Add static for sp804_clocksource_and_sched_clock_init() and sp804_clockevents_init(), they are only used in timer-sp804.c now. Otherwise, the following warning will be reported: drivers/clocksource/timer-sp804.c:68:12: warning: no previous prototype \ for 'sp804_clocksource_and_sched_clock_init' [-Wmissing-prototypes] drivers/clocksource/timer-sp804.c:162:12: warning: no previous prototype \ for 'sp804_clockevents_init' [-Wmissing-prototypes] Fixes: 975434f8b24a ("clocksource/drivers/sp804: Delete the leading "__" of some functions") Fixes: 65f4d7ddc7b6 ("clocksource/drivers/sp804: Remove unused sp804_timer_disable() and timer-sp804.h") Reported-by: kernel test robot Signed-off-by: Zhen Lei Signed-off-by: Daniel Lezcano Link: https://lore.kernel.org/r/20201021012259.2067-2-thunder.leizhen@huawei.com --- drivers/clocksource/timer-sp804.c | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/drivers/clocksource/timer-sp804.c b/drivers/clocksource/timer-sp804.c index 6e8ad4a4ea3c..db5330cc49bc 100644 --- a/drivers/clocksource/timer-sp804.c +++ b/drivers/clocksource/timer-sp804.c @@ -117,10 +117,10 @@ static u64 notrace sp804_read(void) return ~readl_relaxed(sched_clkevt->value); } -int __init sp804_clocksource_and_sched_clock_init(void __iomem *base, - const char *name, - struct clk *clk, - int use_sched_clock) +static int __init sp804_clocksource_and_sched_clock_init(void __iomem *base, + const char *name, + struct clk *clk, + int use_sched_clock) { long rate; struct sp804_clkevt *clkevt; @@ -216,8 +216,8 @@ static struct clock_event_device sp804_clockevent = { .rating = 300, }; -int __init sp804_clockevents_init(void __iomem *base, unsigned int irq, - struct clk *clk, const char *name) +static int __init sp804_clockevents_init(void __iomem *base, unsigned int irq, + struct clk *clk, const char *name) { struct clock_event_device *evt = &sp804_clockevent; long rate; From 3c07bf0fc3558f680374f8ac6d148b0082aa08c6 Mon Sep 17 00:00:00 2001 From: Kefeng Wang Date: Thu, 29 Oct 2020 20:33:14 +0800 Subject: [PATCH 21/46] clocksource/drivers/sp804: Make some symbol static drivers/clocksource/timer-sp804.c:38:31: warning: symbol 'arm_sp804_timer' was not declared. Should it be static? drivers/clocksource/timer-sp804.c:47:31: warning: symbol 'hisi_sp804_timer' was not declared. Should it be static? drivers/clocksource/timer-sp804.c:120:12: warning: symbol 'sp804_clocksource_and_sched_clock_init' was not declared. Should it be static? drivers/clocksource/timer-sp804.c:219:12: warning: symbol 'sp804_clockevents_init' was not declared. Should it be static? And move __initdata after the variables. Signed-off-by: Kefeng Wang Signed-off-by: Daniel Lezcano Link: https://lore.kernel.org/r/20201029123317.90286-2-wangkefeng.wang@huawei.com --- drivers/clocksource/timer-sp804.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/drivers/clocksource/timer-sp804.c b/drivers/clocksource/timer-sp804.c index db5330cc49bc..22a68cb83cf3 100644 --- a/drivers/clocksource/timer-sp804.c +++ b/drivers/clocksource/timer-sp804.c @@ -34,8 +34,7 @@ #define HISI_TIMER_BGLOAD 0x20 #define HISI_TIMER_BGLOAD_H 0x24 - -struct sp804_timer __initdata arm_sp804_timer = { +static struct sp804_timer arm_sp804_timer __initdata = { .load = TIMER_LOAD, .value = TIMER_VALUE, .ctrl = TIMER_CTRL, @@ -44,7 +43,7 @@ struct sp804_timer __initdata arm_sp804_timer = { .width = 32, }; -struct sp804_timer __initdata hisi_sp804_timer = { +static struct sp804_timer hisi_sp804_timer __initdata = { .load = HISI_TIMER_LOAD, .load_h = HISI_TIMER_LOAD_H, .value = HISI_TIMER_VALUE, From 9d4965eb438f0c9f93e91ce6bfec72bbb8def988 Mon Sep 17 00:00:00 2001 From: Kefeng Wang Date: Thu, 29 Oct 2020 20:33:15 +0800 Subject: [PATCH 22/46] clocksource/drivers/sp804: Use clk_prepare_enable and clk_disable_unprepare Directly use clk_prepare_enable and clk_disable_unprepare. Signed-off-by: Kefeng Wang Signed-off-by: Daniel Lezcano Link: https://lore.kernel.org/r/20201029123317.90286-3-wangkefeng.wang@huawei.com --- drivers/clocksource/timer-sp804.c | 13 ++----------- 1 file changed, 2 insertions(+), 11 deletions(-) diff --git a/drivers/clocksource/timer-sp804.c b/drivers/clocksource/timer-sp804.c index 22a68cb83cf3..d74788b47802 100644 --- a/drivers/clocksource/timer-sp804.c +++ b/drivers/clocksource/timer-sp804.c @@ -68,17 +68,9 @@ static long __init sp804_get_clock_rate(struct clk *clk, const char *name) return PTR_ERR(clk); } - err = clk_prepare(clk); - if (err) { - pr_err("sp804: clock failed to prepare: %d\n", err); - clk_put(clk); - return err; - } - - err = clk_enable(clk); + err = clk_prepare_enable(clk); if (err) { pr_err("sp804: clock failed to enable: %d\n", err); - clk_unprepare(clk); clk_put(clk); return err; } @@ -86,8 +78,7 @@ static long __init sp804_get_clock_rate(struct clk *clk, const char *name) rate = clk_get_rate(clk); if (rate < 0) { pr_err("sp804: clock failed to get rate: %ld\n", rate); - clk_disable(clk); - clk_unprepare(clk); + clk_disable_unprepare(clk); clk_put(clk); } From dca54f8ce1c3c979caf06cfdcdf8eab05a00f5ff Mon Sep 17 00:00:00 2001 From: Kefeng Wang Date: Thu, 29 Oct 2020 20:33:16 +0800 Subject: [PATCH 23/46] clocksource/drivers/sp804: Correct clk_get_rate handle clk_get_rate won't return negative value, correct clk_get_rate handle. Signed-off-by: Kefeng Wang Signed-off-by: Daniel Lezcano Link: https://lore.kernel.org/r/20201029123317.90286-4-wangkefeng.wang@huawei.com --- drivers/clocksource/timer-sp804.c | 10 +--------- 1 file changed, 1 insertion(+), 9 deletions(-) diff --git a/drivers/clocksource/timer-sp804.c b/drivers/clocksource/timer-sp804.c index d74788b47802..fcce839670cb 100644 --- a/drivers/clocksource/timer-sp804.c +++ b/drivers/clocksource/timer-sp804.c @@ -58,7 +58,6 @@ static struct sp804_clkevt sp804_clkevt[NR_TIMERS]; static long __init sp804_get_clock_rate(struct clk *clk, const char *name) { - long rate; int err; if (!clk) @@ -75,14 +74,7 @@ static long __init sp804_get_clock_rate(struct clk *clk, const char *name) return err; } - rate = clk_get_rate(clk); - if (rate < 0) { - pr_err("sp804: clock failed to get rate: %ld\n", rate); - clk_disable_unprepare(clk); - clk_put(clk); - } - - return rate; + return clk_get_rate(clk); } static struct sp804_clkevt * __init sp804_clkevt_get(void __iomem *base) From 19f7ce8e36c09f4a2491b065dabd9162018309b6 Mon Sep 17 00:00:00 2001 From: Kefeng Wang Date: Thu, 29 Oct 2020 20:33:17 +0800 Subject: [PATCH 24/46] clocksource/drivers/sp804: Use pr_fmt Add pr_fmt to prefix pr_ output. Signed-off-by: Kefeng Wang Signed-off-by: Daniel Lezcano Link: https://lore.kernel.org/r/20201029123317.90286-5-wangkefeng.wang@huawei.com --- drivers/clocksource/timer-sp804.c | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/drivers/clocksource/timer-sp804.c b/drivers/clocksource/timer-sp804.c index fcce839670cb..401d592e85f5 100644 --- a/drivers/clocksource/timer-sp804.c +++ b/drivers/clocksource/timer-sp804.c @@ -5,6 +5,9 @@ * Copyright (C) 1999 - 2003 ARM Limited * Copyright (C) 2000 Deep Blue Solutions Ltd */ + +#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt + #include #include #include @@ -63,13 +66,13 @@ static long __init sp804_get_clock_rate(struct clk *clk, const char *name) if (!clk) clk = clk_get_sys("sp804", name); if (IS_ERR(clk)) { - pr_err("sp804: %s clock not found: %ld\n", name, PTR_ERR(clk)); + pr_err("%s clock not found: %ld\n", name, PTR_ERR(clk)); return PTR_ERR(clk); } err = clk_prepare_enable(clk); if (err) { - pr_err("sp804: clock failed to enable: %d\n", err); + pr_err("clock failed to enable: %d\n", err); clk_put(clk); return err; } @@ -218,7 +221,7 @@ static int __init sp804_clockevents_init(void __iomem *base, unsigned int irq, if (request_irq(irq, sp804_timer_interrupt, IRQF_TIMER | IRQF_IRQPOLL, "timer", &sp804_clockevent)) - pr_err("%s: request_irq() failed\n", "timer"); + pr_err("request_irq() failed\n"); clockevents_config_and_register(evt, rate, 0xf, 0xffffffff); return 0; @@ -280,7 +283,7 @@ static int __init sp804_of_init(struct device_node *np, struct sp804_timer *time if (of_clk_get_parent_count(np) == 3) { clk2 = of_clk_get(np, 1); if (IS_ERR(clk2)) { - pr_err("sp804: %pOFn clock not found: %d\n", np, + pr_err("%pOFn clock not found: %d\n", np, (int)PTR_ERR(clk2)); clk2 = NULL; } From 0fce2e02a29ca5420472f03d3f2858eedded3fe7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E5=91=A8=E7=90=B0=E6=9D=B0=20=28Zhou=20Yanjie=29?= Date: Mon, 26 Oct 2020 23:58:42 +0800 Subject: [PATCH 25/46] dt-bindings: timer: Add new OST support for the upcoming new driver. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The new OST has one global timer and two or four percpu timers, so there will be three combinations in the upcoming new OST driver: the original GLOBAL_TIMER + PERCPU_TIMER, the new GLOBAL_TIMER + PERCPU_TIMER0/1 and GLOBAL_TIMER + PERCPU_TIMER0/1/2/3, For this, add the macro definition about OST_CLK_PERCPU_TIMER0/1/2/3. And in order to ensure that all the combinations work normally, the original ABI values of OST_CLK_PERCPU_TIMER and OST_CLK_GLOBAL_TIMER need to be exchanged to ensure that in any combinations, the clock can be registered (by calling clk_hw_register()) from index 0. Before this patch, OST_CLK_PERCPU_TIMER and OST_CLK_GLOBAL_TIMER are only used in two places, one is when using "assigned-clocks" to configure the clocks in the DTS file; the other is when registering the clocks in the sysost driver. When the values of these two ABIs are exchanged, the ABI value used by sysost driver when registering the clock, and the ABI value used by DTS when configuring the clock using "assigned-clocks" will also change accordingly. Therefore, there is no situation that causes the wrong clock to the configured. Therefore, exchanging ABI values will not cause errors in the existing codes when registering and configuring the clocks. Currently, in the mainline, only X1000 and X1830 are using sysost driver, and the upcoming X2000 will also use sysost driver. This patch has been tested on all three SoCs and all works fine. Tested-by: 周正 (Zhou Zheng) Signed-off-by: 周琰杰 (Zhou Yanjie) Reviewed-by: Rob Herring Signed-off-by: Daniel Lezcano Link: https://lore.kernel.org/r/20201026155842.10196-2-zhouyanjie@wanyeetech.com --- include/dt-bindings/clock/ingenic,sysost.h | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/include/dt-bindings/clock/ingenic,sysost.h b/include/dt-bindings/clock/ingenic,sysost.h index 9ac88e90babf..063791b01ab3 100644 --- a/include/dt-bindings/clock/ingenic,sysost.h +++ b/include/dt-bindings/clock/ingenic,sysost.h @@ -1,12 +1,16 @@ /* SPDX-License-Identifier: GPL-2.0 */ /* - * This header provides clock numbers for the ingenic,tcu DT binding. + * This header provides clock numbers for the Ingenic OST DT binding. */ #ifndef __DT_BINDINGS_CLOCK_INGENIC_OST_H__ #define __DT_BINDINGS_CLOCK_INGENIC_OST_H__ -#define OST_CLK_PERCPU_TIMER 0 -#define OST_CLK_GLOBAL_TIMER 1 +#define OST_CLK_PERCPU_TIMER 1 +#define OST_CLK_GLOBAL_TIMER 0 +#define OST_CLK_PERCPU_TIMER0 1 +#define OST_CLK_PERCPU_TIMER1 2 +#define OST_CLK_PERCPU_TIMER2 3 +#define OST_CLK_PERCPU_TIMER3 4 #endif /* __DT_BINDINGS_CLOCK_INGENIC_OST_H__ */ From b6ea209ef124dad4045772a759e2aecd191534c0 Mon Sep 17 00:00:00 2001 From: Vineet Gupta Date: Thu, 5 Nov 2020 13:22:08 -0800 Subject: [PATCH 26/46] clocksource/drivers/nps: Remove EZChip NPS clocksource driver NPS platform has been removed from ARC port and there are no in-tree users of it now. So RIP ! Cc: Daniel Lezcano Cc: Thomas Gleixner Cc: linux-kernel@vger.kernel.org Signed-off-by: Vineet Gupta Signed-off-by: Daniel Lezcano Link: https://lore.kernel.org/r/20201105212210.1891598-2-vgupta@synopsys.com --- drivers/clocksource/Kconfig | 10 -- drivers/clocksource/Makefile | 1 - drivers/clocksource/timer-nps.c | 284 -------------------------------- 3 files changed, 295 deletions(-) delete mode 100644 drivers/clocksource/timer-nps.c diff --git a/drivers/clocksource/Kconfig b/drivers/clocksource/Kconfig index 68b087bff59c..390c27cd926d 100644 --- a/drivers/clocksource/Kconfig +++ b/drivers/clocksource/Kconfig @@ -275,16 +275,6 @@ config CLKSRC_TI_32K This option enables support for Texas Instruments 32.768 Hz clocksource available on many OMAP-like platforms. -config CLKSRC_NPS - bool "NPS400 clocksource driver" if COMPILE_TEST - depends on !PHYS_ADDR_T_64BIT - select CLKSRC_MMIO - select TIMER_OF if OF - help - NPS400 clocksource support. - It has a 64-bit counter with update rate up to 1000MHz. - This counter is accessed via couple of 32-bit memory-mapped registers. - config CLKSRC_STM32 bool "Clocksource for STM32 SoCs" if !ARCH_STM32 depends on OF && ARM && (ARCH_STM32 || COMPILE_TEST) diff --git a/drivers/clocksource/Makefile b/drivers/clocksource/Makefile index 1c444cc3bb44..3c75cbbf8533 100644 --- a/drivers/clocksource/Makefile +++ b/drivers/clocksource/Makefile @@ -56,7 +56,6 @@ obj-$(CONFIG_CLKSRC_QCOM) += timer-qcom.o obj-$(CONFIG_MTK_TIMER) += timer-mediatek.o obj-$(CONFIG_CLKSRC_PISTACHIO) += timer-pistachio.o obj-$(CONFIG_CLKSRC_TI_32K) += timer-ti-32k.o -obj-$(CONFIG_CLKSRC_NPS) += timer-nps.o obj-$(CONFIG_OXNAS_RPS_TIMER) += timer-oxnas-rps.o obj-$(CONFIG_OWL_TIMER) += timer-owl.o obj-$(CONFIG_MILBEAUT_TIMER) += timer-milbeaut.o diff --git a/drivers/clocksource/timer-nps.c b/drivers/clocksource/timer-nps.c deleted file mode 100644 index 7b6bb0df96ae..000000000000 --- a/drivers/clocksource/timer-nps.c +++ /dev/null @@ -1,284 +0,0 @@ -/* - * Copyright (c) 2016, Mellanox Technologies. All rights reserved. - * - * This software is available to you under a choice of one of two - * licenses. You may choose to be licensed under the terms of the GNU - * General Public License (GPL) Version 2, available from the file - * COPYING in the main directory of this source tree, or the - * OpenIB.org BSD license below: - * - * Redistribution and use in source and binary forms, with or - * without modification, are permitted provided that the following - * conditions are met: - * - * - Redistributions of source code must retain the above - * copyright notice, this list of conditions and the following - * disclaimer. - * - * - Redistributions in binary form must reproduce the above - * copyright notice, this list of conditions and the following - * disclaimer in the documentation and/or other materials - * provided with the distribution. - * - * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, - * EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF - * MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND - * NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS - * BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN - * ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN - * CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE - * SOFTWARE. - */ - -#include -#include -#include -#include -#include -#include -#include -#include - -#define NPS_MSU_TICK_LOW 0xC8 -#define NPS_CLUSTER_OFFSET 8 -#define NPS_CLUSTER_NUM 16 - -/* This array is per cluster of CPUs (Each NPS400 cluster got 256 CPUs) */ -static void *nps_msu_reg_low_addr[NPS_CLUSTER_NUM] __read_mostly; - -static int __init nps_get_timer_clk(struct device_node *node, - unsigned long *timer_freq, - struct clk **clk) -{ - int ret; - - *clk = of_clk_get(node, 0); - ret = PTR_ERR_OR_ZERO(*clk); - if (ret) { - pr_err("timer missing clk\n"); - return ret; - } - - ret = clk_prepare_enable(*clk); - if (ret) { - pr_err("Couldn't enable parent clk\n"); - clk_put(*clk); - return ret; - } - - *timer_freq = clk_get_rate(*clk); - if (!(*timer_freq)) { - pr_err("Couldn't get clk rate\n"); - clk_disable_unprepare(*clk); - clk_put(*clk); - return -EINVAL; - } - - return 0; -} - -static u64 nps_clksrc_read(struct clocksource *clksrc) -{ - int cluster = raw_smp_processor_id() >> NPS_CLUSTER_OFFSET; - - return (u64)ioread32be(nps_msu_reg_low_addr[cluster]); -} - -static int __init nps_setup_clocksource(struct device_node *node) -{ - int ret, cluster; - struct clk *clk; - unsigned long nps_timer1_freq; - - - for (cluster = 0; cluster < NPS_CLUSTER_NUM; cluster++) - nps_msu_reg_low_addr[cluster] = - nps_host_reg((cluster << NPS_CLUSTER_OFFSET), - NPS_MSU_BLKID, NPS_MSU_TICK_LOW); - - ret = nps_get_timer_clk(node, &nps_timer1_freq, &clk); - if (ret) - return ret; - - ret = clocksource_mmio_init(nps_msu_reg_low_addr, "nps-tick", - nps_timer1_freq, 300, 32, nps_clksrc_read); - if (ret) { - pr_err("Couldn't register clock source.\n"); - clk_disable_unprepare(clk); - } - - return ret; -} - -TIMER_OF_DECLARE(ezchip_nps400_clksrc, "ezchip,nps400-timer", - nps_setup_clocksource); -TIMER_OF_DECLARE(ezchip_nps400_clk_src, "ezchip,nps400-timer1", - nps_setup_clocksource); - -#ifdef CONFIG_EZNPS_MTM_EXT -#include - -/* Timer related Aux registers */ -#define NPS_REG_TIMER0_TSI 0xFFFFF850 -#define NPS_REG_TIMER0_LIMIT 0x23 -#define NPS_REG_TIMER0_CTRL 0x22 -#define NPS_REG_TIMER0_CNT 0x21 - -/* - * Interrupt Enabled (IE) - re-arm the timer - * Not Halted (NH) - is cleared when working with JTAG (for debug) - */ -#define TIMER0_CTRL_IE BIT(0) -#define TIMER0_CTRL_NH BIT(1) - -static unsigned long nps_timer0_freq; -static unsigned long nps_timer0_irq; - -static void nps_clkevent_rm_thread(void) -{ - int thread; - unsigned int cflags, enabled_threads; - - hw_schd_save(&cflags); - - enabled_threads = read_aux_reg(NPS_REG_TIMER0_TSI); - - /* remove thread from TSI1 */ - thread = read_aux_reg(CTOP_AUX_THREAD_ID); - enabled_threads &= ~(1 << thread); - write_aux_reg(NPS_REG_TIMER0_TSI, enabled_threads); - - /* Acknowledge and if needed re-arm the timer */ - if (!enabled_threads) - write_aux_reg(NPS_REG_TIMER0_CTRL, TIMER0_CTRL_NH); - else - write_aux_reg(NPS_REG_TIMER0_CTRL, - TIMER0_CTRL_IE | TIMER0_CTRL_NH); - - hw_schd_restore(cflags); -} - -static void nps_clkevent_add_thread(unsigned long delta) -{ - int thread; - unsigned int cflags, enabled_threads; - - hw_schd_save(&cflags); - - /* add thread to TSI1 */ - thread = read_aux_reg(CTOP_AUX_THREAD_ID); - enabled_threads = read_aux_reg(NPS_REG_TIMER0_TSI); - enabled_threads |= (1 << thread); - write_aux_reg(NPS_REG_TIMER0_TSI, enabled_threads); - - /* set next timer event */ - write_aux_reg(NPS_REG_TIMER0_LIMIT, delta); - write_aux_reg(NPS_REG_TIMER0_CNT, 0); - write_aux_reg(NPS_REG_TIMER0_CTRL, - TIMER0_CTRL_IE | TIMER0_CTRL_NH); - - hw_schd_restore(cflags); -} - -/* - * Whenever anyone tries to change modes, we just mask interrupts - * and wait for the next event to get set. - */ -static int nps_clkevent_set_state(struct clock_event_device *dev) -{ - nps_clkevent_rm_thread(); - disable_percpu_irq(nps_timer0_irq); - - return 0; -} - -static int nps_clkevent_set_next_event(unsigned long delta, - struct clock_event_device *dev) -{ - nps_clkevent_add_thread(delta); - enable_percpu_irq(nps_timer0_irq, IRQ_TYPE_NONE); - - return 0; -} - -static DEFINE_PER_CPU(struct clock_event_device, nps_clockevent_device) = { - .name = "NPS Timer0", - .features = CLOCK_EVT_FEAT_ONESHOT, - .rating = 300, - .set_next_event = nps_clkevent_set_next_event, - .set_state_oneshot = nps_clkevent_set_state, - .set_state_oneshot_stopped = nps_clkevent_set_state, - .set_state_shutdown = nps_clkevent_set_state, - .tick_resume = nps_clkevent_set_state, -}; - -static irqreturn_t timer_irq_handler(int irq, void *dev_id) -{ - struct clock_event_device *evt = dev_id; - - nps_clkevent_rm_thread(); - evt->event_handler(evt); - - return IRQ_HANDLED; -} - -static int nps_timer_starting_cpu(unsigned int cpu) -{ - struct clock_event_device *evt = this_cpu_ptr(&nps_clockevent_device); - - evt->cpumask = cpumask_of(smp_processor_id()); - - clockevents_config_and_register(evt, nps_timer0_freq, 0, ULONG_MAX); - enable_percpu_irq(nps_timer0_irq, IRQ_TYPE_NONE); - - return 0; -} - -static int nps_timer_dying_cpu(unsigned int cpu) -{ - disable_percpu_irq(nps_timer0_irq); - return 0; -} - -static int __init nps_setup_clockevent(struct device_node *node) -{ - struct clk *clk; - int ret; - - nps_timer0_irq = irq_of_parse_and_map(node, 0); - if (nps_timer0_irq <= 0) { - pr_err("clockevent: missing irq\n"); - return -EINVAL; - } - - ret = nps_get_timer_clk(node, &nps_timer0_freq, &clk); - if (ret) - return ret; - - /* Needs apriori irq_set_percpu_devid() done in intc map function */ - ret = request_percpu_irq(nps_timer0_irq, timer_irq_handler, - "Timer0 (per-cpu-tick)", - &nps_clockevent_device); - if (ret) { - pr_err("Couldn't request irq\n"); - clk_disable_unprepare(clk); - return ret; - } - - ret = cpuhp_setup_state(CPUHP_AP_ARC_TIMER_STARTING, - "clockevents/nps:starting", - nps_timer_starting_cpu, - nps_timer_dying_cpu); - if (ret) { - pr_err("Failed to setup hotplug state\n"); - clk_disable_unprepare(clk); - free_percpu_irq(nps_timer0_irq, &nps_clockevent_device); - return ret; - } - - return 0; -} - -TIMER_OF_DECLARE(ezchip_nps400_clk_evt, "ezchip,nps400-timer0", - nps_setup_clockevent); -#endif /* CONFIG_EZNPS_MTM_EXT */ From c1e6cad00aa2f17845e7270e38ff3cc82c7b022a Mon Sep 17 00:00:00 2001 From: Yang Yingliang Date: Wed, 11 Nov 2020 14:47:06 +0800 Subject: [PATCH 27/46] clocksource/drivers/orion: Add missing clk_disable_unprepare() on error path After calling clk_prepare_enable(), clk_disable_unprepare() need be called on error path. Fixes: fbe4b3566ddc ("clocksource/drivers/orion: Convert init function...") Reported-by: Hulk Robot Signed-off-by: Yang Yingliang Signed-off-by: Daniel Lezcano Link: https://lore.kernel.org/r/20201111064706.3397156-1-yangyingliang@huawei.com --- drivers/clocksource/timer-orion.c | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/drivers/clocksource/timer-orion.c b/drivers/clocksource/timer-orion.c index d01ff4181867..5101e834d78f 100644 --- a/drivers/clocksource/timer-orion.c +++ b/drivers/clocksource/timer-orion.c @@ -143,7 +143,8 @@ static int __init orion_timer_init(struct device_node *np) irq = irq_of_parse_and_map(np, 1); if (irq <= 0) { pr_err("%pOFn: unable to parse timer1 irq\n", np); - return -EINVAL; + ret = -EINVAL; + goto out_unprep_clk; } rate = clk_get_rate(clk); @@ -160,7 +161,7 @@ static int __init orion_timer_init(struct device_node *np) clocksource_mmio_readl_down); if (ret) { pr_err("Failed to initialize mmio timer\n"); - return ret; + goto out_unprep_clk; } sched_clock_register(orion_read_sched_clock, 32, rate); @@ -170,7 +171,7 @@ static int __init orion_timer_init(struct device_node *np) "orion_event", NULL); if (ret) { pr_err("%pOFn: unable to setup irq\n", np); - return ret; + goto out_unprep_clk; } ticks_per_jiffy = (clk_get_rate(clk) + HZ/2) / HZ; @@ -183,5 +184,9 @@ static int __init orion_timer_init(struct device_node *np) orion_delay_timer_init(rate); return 0; + +out_unprep_clk: + clk_disable_unprepare(clk); + return ret; } TIMER_OF_DECLARE(orion_timer, "marvell,orion-timer", orion_timer_init); From db08e6c0e2513d1341369ec6a4f1774ee20b290b Mon Sep 17 00:00:00 2001 From: Marian-Cristian Rotariu Date: Tue, 10 Nov 2020 17:20:13 +0100 Subject: [PATCH 28/46] dt-bindings: timer: renesas: tmu: Document r8a774e1 bindings MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Document RZ/G2H (R8A774E1) SoC in the Renesas TMU bindings. Signed-off-by: Marian-Cristian Rotariu Signed-off-by: Lad Prabhakar Signed-off-by: Geert Uytterhoeven Reviewed-by: Niklas Söderlund Signed-off-by: Daniel Lezcano Link: https://lore.kernel.org/r/20201110162014.3290109-2-geert+renesas@glider.be --- Documentation/devicetree/bindings/timer/renesas,tmu.txt | 1 + 1 file changed, 1 insertion(+) diff --git a/Documentation/devicetree/bindings/timer/renesas,tmu.txt b/Documentation/devicetree/bindings/timer/renesas,tmu.txt index 29159f4e65ab..a36cd61e74fb 100644 --- a/Documentation/devicetree/bindings/timer/renesas,tmu.txt +++ b/Documentation/devicetree/bindings/timer/renesas,tmu.txt @@ -13,6 +13,7 @@ Required Properties: - "renesas,tmu-r8a774a1" for the r8a774A1 TMU - "renesas,tmu-r8a774b1" for the r8a774B1 TMU - "renesas,tmu-r8a774c0" for the r8a774C0 TMU + - "renesas,tmu-r8a774e1" for the r8a774E1 TMU - "renesas,tmu-r8a7778" for the r8a7778 TMU - "renesas,tmu-r8a7779" for the r8a7779 TMU - "renesas,tmu-r8a77970" for the r8a77970 TMU From b7c0fed5ccf2c5cb4bb43ddc6b1625f042a83d0a Mon Sep 17 00:00:00 2001 From: Geert Uytterhoeven Date: Tue, 10 Nov 2020 17:20:14 +0100 Subject: [PATCH 29/46] dt-bindings: timer: renesas: tmu: Convert to json-schema MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Convert the Renesas R-Mobile/R-Car Timer Unit (TMU) Device Tree binding documentation to json-schema. Document missing properties. Update the example to match reality. Signed-off-by: Geert Uytterhoeven Reviewed-by: Rob Herring Signed-off-by: Daniel Lezcano Reviewed-by: Niklas Söderlund Link: https://lore.kernel.org/r/20201110162014.3290109-3-geert+renesas@glider.be --- .../devicetree/bindings/timer/renesas,tmu.txt | 50 ---------- .../bindings/timer/renesas,tmu.yaml | 99 +++++++++++++++++++ 2 files changed, 99 insertions(+), 50 deletions(-) delete mode 100644 Documentation/devicetree/bindings/timer/renesas,tmu.txt create mode 100644 Documentation/devicetree/bindings/timer/renesas,tmu.yaml diff --git a/Documentation/devicetree/bindings/timer/renesas,tmu.txt b/Documentation/devicetree/bindings/timer/renesas,tmu.txt deleted file mode 100644 index a36cd61e74fb..000000000000 --- a/Documentation/devicetree/bindings/timer/renesas,tmu.txt +++ /dev/null @@ -1,50 +0,0 @@ -* Renesas R-Mobile/R-Car Timer Unit (TMU) - -The TMU is a 32-bit timer/counter with configurable clock inputs and -programmable compare match. - -Channels share hardware resources but their counter and compare match value -are independent. The TMU hardware supports up to three channels. - -Required Properties: - - - compatible: must contain one or more of the following: - - "renesas,tmu-r8a7740" for the r8a7740 TMU - - "renesas,tmu-r8a774a1" for the r8a774A1 TMU - - "renesas,tmu-r8a774b1" for the r8a774B1 TMU - - "renesas,tmu-r8a774c0" for the r8a774C0 TMU - - "renesas,tmu-r8a774e1" for the r8a774E1 TMU - - "renesas,tmu-r8a7778" for the r8a7778 TMU - - "renesas,tmu-r8a7779" for the r8a7779 TMU - - "renesas,tmu-r8a77970" for the r8a77970 TMU - - "renesas,tmu-r8a77980" for the r8a77980 TMU - - "renesas,tmu" for any TMU. - This is a fallback for the above renesas,tmu-* entries - - - reg: base address and length of the registers block for the timer module. - - - interrupts: interrupt-specifier for the timer, one per channel. - - - clocks: a list of phandle + clock-specifier pairs, one for each entry - in clock-names. - - clock-names: must contain "fck" for the functional clock. - -Optional Properties: - - - #renesas,channels: number of channels implemented by the timer, must be 2 - or 3 (if not specified the value defaults to 3). - - -Example: R8A7779 (R-Car H1) TMU0 node - - tmu0: timer@ffd80000 { - compatible = "renesas,tmu-r8a7779", "renesas,tmu"; - reg = <0xffd80000 0x30>; - interrupts = <0 32 IRQ_TYPE_LEVEL_HIGH>, - <0 33 IRQ_TYPE_LEVEL_HIGH>, - <0 34 IRQ_TYPE_LEVEL_HIGH>; - clocks = <&mstp0_clks R8A7779_CLK_TMU0>; - clock-names = "fck"; - - #renesas,channels = <3>; - }; diff --git a/Documentation/devicetree/bindings/timer/renesas,tmu.yaml b/Documentation/devicetree/bindings/timer/renesas,tmu.yaml new file mode 100644 index 000000000000..c54188731a1b --- /dev/null +++ b/Documentation/devicetree/bindings/timer/renesas,tmu.yaml @@ -0,0 +1,99 @@ +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) +%YAML 1.2 +--- +$id: http://devicetree.org/schemas/timer/renesas,tmu.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# + +title: Renesas R-Mobile/R-Car Timer Unit (TMU) + +maintainers: + - Geert Uytterhoeven + - Laurent Pinchart + +description: + The TMU is a 32-bit timer/counter with configurable clock inputs and + programmable compare match. + + Channels share hardware resources but their counter and compare match value + are independent. The TMU hardware supports up to three channels. + +properties: + compatible: + items: + - enum: + - renesas,tmu-r8a7740 # R-Mobile A1 + - renesas,tmu-r8a774a1 # RZ/G2M + - renesas,tmu-r8a774b1 # RZ/G2N + - renesas,tmu-r8a774c0 # RZ/G2E + - renesas,tmu-r8a774e1 # RZ/G2H + - renesas,tmu-r8a7778 # R-Car M1A + - renesas,tmu-r8a7779 # R-Car H1 + - renesas,tmu-r8a77970 # R-Car V3M + - renesas,tmu-r8a77980 # R-Car V3H + - const: renesas,tmu + + reg: + maxItems: 1 + + interrupts: + minItems: 2 + maxItems: 3 + + clocks: + maxItems: 1 + + clock-names: + const: fck + + power-domains: + maxItems: 1 + + resets: + maxItems: 1 + + '#renesas,channels': + description: + Number of channels implemented by the timer. + $ref: /schemas/types.yaml#/definitions/uint32 + enum: [ 2, 3 ] + default: 3 + +required: + - compatible + - reg + - interrupts + - clocks + - clock-names + - power-domains + +if: + not: + properties: + compatible: + contains: + enum: + - renesas,tmu-r8a7740 + - renesas,tmu-r8a7778 + - renesas,tmu-r8a7779 +then: + required: + - resets + +additionalProperties: false + +examples: + - | + #include + #include + #include + tmu0: timer@ffd80000 { + compatible = "renesas,tmu-r8a7779", "renesas,tmu"; + reg = <0xffd80000 0x30>; + interrupts = , + , + ; + clocks = <&mstp0_clks R8A7779_CLK_TMU0>; + clock-names = "fck"; + power-domains = <&sysc R8A7779_PD_ALWAYS_ON>; + #renesas,channels = <3>; + }; From eee422c46e6840a81c9db18a497b74387a557b29 Mon Sep 17 00:00:00 2001 From: Yu Kuai Date: Mon, 16 Nov 2020 21:51:23 +0800 Subject: [PATCH 30/46] clocksource/drivers/cadence_ttc: Fix memory leak in ttc_setup_clockevent() If clk_notifier_register() failed, ttc_setup_clockevent() will return without freeing 'ttcce', which will leak memory. Fixes: 70504f311d4b ("clocksource/drivers/cadence_ttc: Convert init function to return error") Reported-by: Hulk Robot Signed-off-by: Yu Kuai Signed-off-by: Daniel Lezcano Link: https://lore.kernel.org/r/20201116135123.2164033-1-yukuai3@huawei.com --- drivers/clocksource/timer-cadence-ttc.c | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/drivers/clocksource/timer-cadence-ttc.c b/drivers/clocksource/timer-cadence-ttc.c index 80e960602030..4efd0cf3b602 100644 --- a/drivers/clocksource/timer-cadence-ttc.c +++ b/drivers/clocksource/timer-cadence-ttc.c @@ -413,10 +413,8 @@ static int __init ttc_setup_clockevent(struct clk *clk, ttcce->ttc.clk = clk; err = clk_prepare_enable(ttcce->ttc.clk); - if (err) { - kfree(ttcce); - return err; - } + if (err) + goto out_kfree; ttcce->ttc.clk_rate_change_nb.notifier_call = ttc_rate_change_clockevent_cb; @@ -426,7 +424,7 @@ static int __init ttc_setup_clockevent(struct clk *clk, &ttcce->ttc.clk_rate_change_nb); if (err) { pr_warn("Unable to register clock notifier.\n"); - return err; + goto out_kfree; } ttcce->ttc.freq = clk_get_rate(ttcce->ttc.clk); @@ -455,15 +453,17 @@ static int __init ttc_setup_clockevent(struct clk *clk, err = request_irq(irq, ttc_clock_event_interrupt, IRQF_TIMER, ttcce->ce.name, ttcce); - if (err) { - kfree(ttcce); - return err; - } + if (err) + goto out_kfree; clockevents_config_and_register(&ttcce->ce, ttcce->ttc.freq / PRESCALE, 1, 0xfffe); return 0; + +out_kfree: + kfree(ttcce); + return err; } static int __init ttc_timer_probe(struct platform_device *pdev) From 5bd7cb29eceb52e4b108917786fdbf2a2c2048ef Mon Sep 17 00:00:00 2001 From: Daniel Lezcano Date: Wed, 25 Nov 2020 11:23:45 +0100 Subject: [PATCH 31/46] clocksource/drivers/ingenic: Fix section mismatch MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The function ingenic_tcu_get_clock() is annotated for the __init section but it is actually called from the online cpu callback. That will lead to a crash if a CPU is hotplugged after boot time. Remove the __init annotation for the ingenic_tcu_get_clock() function. Fixes: f19d838d08fc (clocksource/drivers/ingenic: Add high resolution timer support for SMP/SMT) Reported-by: kernel test robot Signed-off-by: Daniel Lezcano Reviewed-by: Paul Cercueil Tested-by: 周琰杰 (Zhou Yanjie) Link: https://lore.kernel.org/r/20201125102346.1816310-1-daniel.lezcano@linaro.org --- drivers/clocksource/ingenic-timer.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/clocksource/ingenic-timer.c b/drivers/clocksource/ingenic-timer.c index 58fd9189fab7..905fd6b163a8 100644 --- a/drivers/clocksource/ingenic-timer.c +++ b/drivers/clocksource/ingenic-timer.c @@ -127,7 +127,7 @@ static irqreturn_t ingenic_tcu_cevt_cb(int irq, void *dev_id) return IRQ_HANDLED; } -static struct clk * __init ingenic_tcu_get_clock(struct device_node *np, int id) +static struct clk *ingenic_tcu_get_clock(struct device_node *np, int id) { struct of_phandle_args args; From ab3105446f1ec4e98fadfc998ee24feec271c16c Mon Sep 17 00:00:00 2001 From: Kefeng Wang Date: Wed, 28 Oct 2020 21:12:30 +0800 Subject: [PATCH 32/46] clocksource/drivers/riscv: Make RISCV_TIMER depends on RISCV_SBI The riscv timer is set via SBI timer call, let's make RISCV_TIMER depends on RISCV_SBI, and it also fixes some build issue. Fixes: d5be89a8d118 ("RISC-V: Resurrect the MMIO timer implementation for M-mode systems") Signed-off-by: Kefeng Wang Reviewed-by: Palmer Dabbelt Acked-by: Palmer Dabbelt Signed-off-by: Daniel Lezcano Link: https://lore.kernel.org/r/20201028131230.72907-1-wangkefeng.wang@huawei.com --- drivers/clocksource/Kconfig | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/clocksource/Kconfig b/drivers/clocksource/Kconfig index 390c27cd926d..9f00b8385fd4 100644 --- a/drivers/clocksource/Kconfig +++ b/drivers/clocksource/Kconfig @@ -644,7 +644,7 @@ config ATCPIT100_TIMER config RISCV_TIMER bool "Timer for the RISC-V platform" if COMPILE_TEST - depends on GENERIC_SCHED_CLOCK && RISCV + depends on GENERIC_SCHED_CLOCK && RISCV && RISCV_SBI select TIMER_PROBE select TIMER_OF help From 5d9814df0aec56a638bbf20795abb4cfaf3cd331 Mon Sep 17 00:00:00 2001 From: Dinh Nguyen Date: Sat, 5 Dec 2020 04:52:23 -0600 Subject: [PATCH 33/46] clocksource/drivers/dw_apb_timer_of: Add error handling if no clock available commit ("b0fc70ce1f02 arm64: berlin: Select DW_APB_TIMER_OF") added the support for the dw_apb_timer into the arm64 defconfig. However, for some platforms like the Intel Stratix10 and Agilex, the clock manager doesn't get loaded until after the timer driver get loaded. Thus, the driver hits the panic "No clock nor clock-frequency property for" because it cannot properly get the clock. This patch adds the error handling needed for the timer driver so that the kernel can continue booting instead of just hitting the panic. Signed-off-by: Dinh Nguyen Signed-off-by: Daniel Lezcano Link: https://lore.kernel.org/r/20201205105223.208604-1-dinguyen@kernel.org --- drivers/clocksource/dw_apb_timer_of.c | 57 ++++++++++++++++++--------- 1 file changed, 39 insertions(+), 18 deletions(-) diff --git a/drivers/clocksource/dw_apb_timer_of.c b/drivers/clocksource/dw_apb_timer_of.c index ab3ddebe8344..42e7e43b8fcd 100644 --- a/drivers/clocksource/dw_apb_timer_of.c +++ b/drivers/clocksource/dw_apb_timer_of.c @@ -14,12 +14,13 @@ #include #include -static void __init timer_get_base_and_rate(struct device_node *np, +static int __init timer_get_base_and_rate(struct device_node *np, void __iomem **base, u32 *rate) { struct clk *timer_clk; struct clk *pclk; struct reset_control *rstc; + int ret; *base = of_iomap(np, 0); @@ -46,55 +47,67 @@ static void __init timer_get_base_and_rate(struct device_node *np, pr_warn("pclk for %pOFn is present, but could not be activated\n", np); + if (!of_property_read_u32(np, "clock-freq", rate) && + !of_property_read_u32(np, "clock-frequency", rate)) + return 0; + timer_clk = of_clk_get_by_name(np, "timer"); if (IS_ERR(timer_clk)) - goto try_clock_freq; + return PTR_ERR(timer_clk); - if (!clk_prepare_enable(timer_clk)) { - *rate = clk_get_rate(timer_clk); - return; - } + ret = clk_prepare_enable(timer_clk); + if (ret) + return ret; -try_clock_freq: - if (of_property_read_u32(np, "clock-freq", rate) && - of_property_read_u32(np, "clock-frequency", rate)) - panic("No clock nor clock-frequency property for %pOFn", np); + *rate = clk_get_rate(timer_clk); + if (!(*rate)) + return -EINVAL; + + return 0; } -static void __init add_clockevent(struct device_node *event_timer) +static int __init add_clockevent(struct device_node *event_timer) { void __iomem *iobase; struct dw_apb_clock_event_device *ced; u32 irq, rate; + int ret = 0; irq = irq_of_parse_and_map(event_timer, 0); if (irq == 0) panic("No IRQ for clock event timer"); - timer_get_base_and_rate(event_timer, &iobase, &rate); + ret = timer_get_base_and_rate(event_timer, &iobase, &rate); + if (ret) + return ret; ced = dw_apb_clockevent_init(-1, event_timer->name, 300, iobase, irq, rate); if (!ced) - panic("Unable to initialise clockevent device"); + return -EINVAL; dw_apb_clockevent_register(ced); + + return 0; } static void __iomem *sched_io_base; static u32 sched_rate; -static void __init add_clocksource(struct device_node *source_timer) +static int __init add_clocksource(struct device_node *source_timer) { void __iomem *iobase; struct dw_apb_clocksource *cs; u32 rate; + int ret; - timer_get_base_and_rate(source_timer, &iobase, &rate); + ret = timer_get_base_and_rate(source_timer, &iobase, &rate); + if (ret) + return ret; cs = dw_apb_clocksource_init(300, source_timer->name, iobase, rate); if (!cs) - panic("Unable to initialise clocksource device"); + return -EINVAL; dw_apb_clocksource_start(cs); dw_apb_clocksource_register(cs); @@ -106,6 +119,8 @@ static void __init add_clocksource(struct device_node *source_timer) */ sched_io_base = iobase + 0x04; sched_rate = rate; + + return 0; } static u64 notrace read_sched_clock(void) @@ -146,10 +161,14 @@ static struct delay_timer dw_apb_delay_timer = { static int num_called; static int __init dw_apb_timer_init(struct device_node *timer) { + int ret = 0; + switch (num_called) { case 1: pr_debug("%s: found clocksource timer\n", __func__); - add_clocksource(timer); + ret = add_clocksource(timer); + if (ret) + return ret; init_sched_clock(); #ifdef CONFIG_ARM dw_apb_delay_timer.freq = sched_rate; @@ -158,7 +177,9 @@ static int __init dw_apb_timer_init(struct device_node *timer) break; default: pr_debug("%s: found clockevent timer\n", __func__); - add_clockevent(timer); + ret = add_clockevent(timer); + if (ret) + return ret; break; } From d8cc3905b8073c7cfbff94af889fa8dc71f21dd5 Mon Sep 17 00:00:00 2001 From: Keqian Zhu Date: Fri, 4 Dec 2020 15:31:25 +0800 Subject: [PATCH 34/46] clocksource/drivers/arm_arch_timer: Use stable count reader in erratum sne In commit 0ea415390cd3 ("clocksource/arm_arch_timer: Use arch_timer_read_counter to access stable counters"), we separate stable and normal count reader to omit unnecessary overhead on systems that have no timer erratum. However, in erratum_set_next_event_tval_generic(), count reader becomes normal reader. This converts it to stable reader. Fixes: 0ea415390cd3 ("clocksource/arm_arch_timer: Use arch_timer_read_counter to access stable counters") Acked-by: Marc Zyngier Signed-off-by: Keqian Zhu Signed-off-by: Daniel Lezcano Link: https://lore.kernel.org/r/20201204073126.6920-2-zhukeqian1@huawei.com --- drivers/clocksource/arm_arch_timer.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/clocksource/arm_arch_timer.c b/drivers/clocksource/arm_arch_timer.c index 6c3e84180146..777d38cb39b0 100644 --- a/drivers/clocksource/arm_arch_timer.c +++ b/drivers/clocksource/arm_arch_timer.c @@ -396,10 +396,10 @@ static void erratum_set_next_event_tval_generic(const int access, unsigned long ctrl &= ~ARCH_TIMER_CTRL_IT_MASK; if (access == ARCH_TIMER_PHYS_ACCESS) { - cval = evt + arch_counter_get_cntpct(); + cval = evt + arch_counter_get_cntpct_stable(); write_sysreg(cval, cntp_cval_el0); } else { - cval = evt + arch_counter_get_cntvct(); + cval = evt + arch_counter_get_cntvct_stable(); write_sysreg(cval, cntv_cval_el0); } From 8b7770b877d187bfdae1eaf587bd2b792479a31c Mon Sep 17 00:00:00 2001 From: Keqian Zhu Date: Fri, 4 Dec 2020 15:31:26 +0800 Subject: [PATCH 35/46] clocksource/drivers/arm_arch_timer: Correct fault programming of CNTKCTL_EL1.EVNTI ARM virtual counter supports event stream, it can only trigger an event when the trigger bit (the value of CNTKCTL_EL1.EVNTI) of CNTVCT_EL0 changes, so the actual period of event stream is 2^(cntkctl_evnti + 1). For example, when the trigger bit is 0, then virtual counter trigger an event for every two cycles. While we're at it, rework the way we compute the trigger bit position by making it more obvious that when bits [n:n-1] are both set (with n being the most significant bit), we pick bit (n + 1). Fixes: 037f637767a8 ("drivers: clocksource: add support for ARM architected timer event stream") Suggested-by: Marc Zyngier Signed-off-by: Keqian Zhu Acked-by: Marc Zyngier Signed-off-by: Daniel Lezcano Link: https://lore.kernel.org/r/20201204073126.6920-3-zhukeqian1@huawei.com --- drivers/clocksource/arm_arch_timer.c | 23 ++++++++++++++++------- 1 file changed, 16 insertions(+), 7 deletions(-) diff --git a/drivers/clocksource/arm_arch_timer.c b/drivers/clocksource/arm_arch_timer.c index 777d38cb39b0..d0177824c518 100644 --- a/drivers/clocksource/arm_arch_timer.c +++ b/drivers/clocksource/arm_arch_timer.c @@ -822,15 +822,24 @@ static void arch_timer_evtstrm_enable(int divider) static void arch_timer_configure_evtstream(void) { - int evt_stream_div, pos; + int evt_stream_div, lsb; + + /* + * As the event stream can at most be generated at half the frequency + * of the counter, use half the frequency when computing the divider. + */ + evt_stream_div = arch_timer_rate / ARCH_TIMER_EVT_STREAM_FREQ / 2; + + /* + * Find the closest power of two to the divisor. If the adjacent bit + * of lsb (last set bit, starts from 0) is set, then we use (lsb + 1). + */ + lsb = fls(evt_stream_div) - 1; + if (lsb > 0 && (evt_stream_div & BIT(lsb - 1))) + lsb++; - /* Find the closest power of two to the divisor */ - evt_stream_div = arch_timer_rate / ARCH_TIMER_EVT_STREAM_FREQ; - pos = fls(evt_stream_div); - if (pos > 1 && !(evt_stream_div & (1 << (pos - 2)))) - pos--; /* enable event stream */ - arch_timer_evtstrm_enable(min(pos, 15)); + arch_timer_evtstrm_enable(max(0, min(lsb, 15))); } static void arch_counter_set_user_access(void) From 8ae954caf49ac403c177d117fb8e05cbc866aa3c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Niklas=20S=C3=B6derlund?= Date: Sat, 5 Dec 2020 03:19:20 +0100 Subject: [PATCH 36/46] clocksource/drivers/sh_cmt: Fix potential deadlock when calling runtime PM MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The ch->lock is used to protect the whole enable() and read() of sh_cmt's implementation of struct clocksource. The enable() implementation calls pm_runtime_get_sync() which may result in the clock source to be read() triggering a cyclic lockdep warning for the ch->lock. The sh_cmt driver implement its own balancing of calls to sh_cmt_{enable,disable}() with flags in sh_cmt_{start,stop}(). It does this to deal with that start and stop are shared between the clock source and clock event providers. While this could be improved on verifying corner cases based on any substantial rework on all devices this driver supports might prove hard. As a first step separate the PM handling for clock event and clock source. Always put/get the device when enabling/disabling the clock source but keep the clock event logic unchanged. This allows the sh_cmt implementation of struct clocksource to call PM without holding the ch->lock and avoiding the deadlock. Triggering and log of the deadlock warning, # echo e60f0000.timer > /sys/devices/system/clocksource/clocksource0/current_clocksource [ 46.948370] ====================================================== [ 46.954730] WARNING: possible circular locking dependency detected [ 46.961094] 5.10.0-rc6-arm64-renesas-00001-g0e5fd7414e8b #36 Not tainted [ 46.967985] ------------------------------------------------------ [ 46.974342] migration/0/11 is trying to acquire lock: [ 46.979543] ffff0000403ed220 (&dev->power.lock){-...}-{2:2}, at: __pm_runtime_resume+0x40/0x74 [ 46.988445] [ 46.988445] but task is already holding lock: [ 46.994441] ffff000040ad0298 (&ch->lock){....}-{2:2}, at: sh_cmt_start+0x28/0x210 [ 47.002173] [ 47.002173] which lock already depends on the new lock. [ 47.002173] [ 47.010573] [ 47.010573] the existing dependency chain (in reverse order) is: [ 47.018262] [ 47.018262] -> #3 (&ch->lock){....}-{2:2}: [ 47.024033] lock_acquire.part.0+0x120/0x330 [ 47.028970] lock_acquire+0x64/0x80 [ 47.033105] _raw_spin_lock_irqsave+0x7c/0xc4 [ 47.038130] sh_cmt_start+0x28/0x210 [ 47.042352] sh_cmt_clocksource_enable+0x28/0x50 [ 47.047644] change_clocksource+0x9c/0x160 [ 47.052402] multi_cpu_stop+0xa4/0x190 [ 47.056799] cpu_stopper_thread+0x90/0x154 [ 47.061557] smpboot_thread_fn+0x244/0x270 [ 47.066310] kthread+0x154/0x160 [ 47.070175] ret_from_fork+0x10/0x20 [ 47.074390] [ 47.074390] -> #2 (tk_core.seq.seqcount){----}-{0:0}: [ 47.081136] lock_acquire.part.0+0x120/0x330 [ 47.086070] lock_acquire+0x64/0x80 [ 47.090203] seqcount_lockdep_reader_access.constprop.0+0x74/0x100 [ 47.097096] ktime_get+0x28/0xa0 [ 47.100960] hrtimer_start_range_ns+0x210/0x2dc [ 47.106164] generic_sched_clock_init+0x70/0x88 [ 47.111364] sched_clock_init+0x40/0x64 [ 47.115853] start_kernel+0x494/0x524 [ 47.120156] [ 47.120156] -> #1 (hrtimer_bases.lock){-.-.}-{2:2}: [ 47.126721] lock_acquire.part.0+0x120/0x330 [ 47.136042] lock_acquire+0x64/0x80 [ 47.144461] _raw_spin_lock_irqsave+0x7c/0xc4 [ 47.153721] hrtimer_start_range_ns+0x68/0x2dc [ 47.163054] rpm_suspend+0x308/0x5dc [ 47.171473] rpm_idle+0xc4/0x2a4 [ 47.179550] pm_runtime_work+0x98/0xc0 [ 47.188209] process_one_work+0x294/0x6f0 [ 47.197142] worker_thread+0x70/0x45c [ 47.205661] kthread+0x154/0x160 [ 47.213673] ret_from_fork+0x10/0x20 [ 47.221957] [ 47.221957] -> #0 (&dev->power.lock){-...}-{2:2}: [ 47.236292] check_noncircular+0x128/0x140 [ 47.244907] __lock_acquire+0x13b0/0x204c [ 47.253332] lock_acquire.part.0+0x120/0x330 [ 47.262033] lock_acquire+0x64/0x80 [ 47.269826] _raw_spin_lock_irqsave+0x7c/0xc4 [ 47.278430] __pm_runtime_resume+0x40/0x74 [ 47.286758] sh_cmt_start+0x84/0x210 [ 47.294537] sh_cmt_clocksource_enable+0x28/0x50 [ 47.303449] change_clocksource+0x9c/0x160 [ 47.311783] multi_cpu_stop+0xa4/0x190 [ 47.319720] cpu_stopper_thread+0x90/0x154 [ 47.328022] smpboot_thread_fn+0x244/0x270 [ 47.336298] kthread+0x154/0x160 [ 47.343708] ret_from_fork+0x10/0x20 [ 47.351445] [ 47.351445] other info that might help us debug this: [ 47.351445] [ 47.370225] Chain exists of: [ 47.370225] &dev->power.lock --> tk_core.seq.seqcount --> &ch->lock [ 47.370225] [ 47.392003] Possible unsafe locking scenario: [ 47.392003] [ 47.405314] CPU0 CPU1 [ 47.413569] ---- ---- [ 47.421768] lock(&ch->lock); [ 47.428425] lock(tk_core.seq.seqcount); [ 47.438701] lock(&ch->lock); [ 47.447930] lock(&dev->power.lock); [ 47.455172] [ 47.455172] *** DEADLOCK *** [ 47.455172] [ 47.471433] 3 locks held by migration/0/11: [ 47.479099] #0: ffff8000113c9278 (timekeeper_lock){-.-.}-{2:2}, at: change_clocksource+0x2c/0x160 [ 47.491834] #1: ffff8000113c8f88 (tk_core.seq.seqcount){----}-{0:0}, at: multi_cpu_stop+0xa4/0x190 [ 47.504727] #2: ffff000040ad0298 (&ch->lock){....}-{2:2}, at: sh_cmt_start+0x28/0x210 [ 47.516541] [ 47.516541] stack backtrace: [ 47.528480] CPU: 0 PID: 11 Comm: migration/0 Not tainted 5.10.0-rc6-arm64-renesas-00001-g0e5fd7414e8b #36 [ 47.542147] Hardware name: Renesas Salvator-X 2nd version board based on r8a77965 (DT) [ 47.554241] Call trace: [ 47.560832] dump_backtrace+0x0/0x190 [ 47.568670] show_stack+0x14/0x30 [ 47.576144] dump_stack+0xe8/0x130 [ 47.583670] print_circular_bug+0x1f0/0x200 [ 47.592015] check_noncircular+0x128/0x140 [ 47.600289] __lock_acquire+0x13b0/0x204c [ 47.608486] lock_acquire.part.0+0x120/0x330 [ 47.616953] lock_acquire+0x64/0x80 [ 47.624582] _raw_spin_lock_irqsave+0x7c/0xc4 [ 47.633114] __pm_runtime_resume+0x40/0x74 [ 47.641371] sh_cmt_start+0x84/0x210 [ 47.649115] sh_cmt_clocksource_enable+0x28/0x50 [ 47.657916] change_clocksource+0x9c/0x160 [ 47.666165] multi_cpu_stop+0xa4/0x190 [ 47.674056] cpu_stopper_thread+0x90/0x154 [ 47.682308] smpboot_thread_fn+0x244/0x270 [ 47.690560] kthread+0x154/0x160 [ 47.697927] ret_from_fork+0x10/0x20 [ 47.708447] clocksource: Switched to clocksource e60f0000.timer Signed-off-by: Niklas Söderlund Reviewed-by: Geert Uytterhoeven Signed-off-by: Daniel Lezcano Link: https://lore.kernel.org/r/20201205021921.1456190-2-niklas.soderlund+renesas@ragnatech.se --- drivers/clocksource/sh_cmt.c | 18 ++++++++++++++---- 1 file changed, 14 insertions(+), 4 deletions(-) diff --git a/drivers/clocksource/sh_cmt.c b/drivers/clocksource/sh_cmt.c index 760777458a90..19fa3ef75e3b 100644 --- a/drivers/clocksource/sh_cmt.c +++ b/drivers/clocksource/sh_cmt.c @@ -319,7 +319,6 @@ static int sh_cmt_enable(struct sh_cmt_channel *ch) { int k, ret; - pm_runtime_get_sync(&ch->cmt->pdev->dev); dev_pm_syscore_device(&ch->cmt->pdev->dev, true); /* enable clock */ @@ -394,7 +393,6 @@ static void sh_cmt_disable(struct sh_cmt_channel *ch) clk_disable(ch->cmt->clk); dev_pm_syscore_device(&ch->cmt->pdev->dev, false); - pm_runtime_put(&ch->cmt->pdev->dev); } /* private flags */ @@ -562,10 +560,16 @@ static int sh_cmt_start(struct sh_cmt_channel *ch, unsigned long flag) int ret = 0; unsigned long flags; + if (flag & FLAG_CLOCKSOURCE) + pm_runtime_get_sync(&ch->cmt->pdev->dev); + raw_spin_lock_irqsave(&ch->lock, flags); - if (!(ch->flags & (FLAG_CLOCKEVENT | FLAG_CLOCKSOURCE))) + if (!(ch->flags & (FLAG_CLOCKEVENT | FLAG_CLOCKSOURCE))) { + if (flag & FLAG_CLOCKEVENT) + pm_runtime_get_sync(&ch->cmt->pdev->dev); ret = sh_cmt_enable(ch); + } if (ret) goto out; @@ -590,14 +594,20 @@ static void sh_cmt_stop(struct sh_cmt_channel *ch, unsigned long flag) f = ch->flags & (FLAG_CLOCKEVENT | FLAG_CLOCKSOURCE); ch->flags &= ~flag; - if (f && !(ch->flags & (FLAG_CLOCKEVENT | FLAG_CLOCKSOURCE))) + if (f && !(ch->flags & (FLAG_CLOCKEVENT | FLAG_CLOCKSOURCE))) { sh_cmt_disable(ch); + if (flag & FLAG_CLOCKEVENT) + pm_runtime_put(&ch->cmt->pdev->dev); + } /* adjust the timeout to maximum if only clocksource left */ if ((flag == FLAG_CLOCKEVENT) && (ch->flags & FLAG_CLOCKSOURCE)) __sh_cmt_set_next(ch, ch->max_match_value); raw_spin_unlock_irqrestore(&ch->lock, flags); + + if (flag & FLAG_CLOCKSOURCE) + pm_runtime_put(&ch->cmt->pdev->dev); } static struct sh_cmt_channel *cs_to_sh_cmt(struct clocksource *cs) From 05a0302c35481e9b47fb90ba40922b0a4cae40d8 Mon Sep 17 00:00:00 2001 From: Thomas Gleixner Date: Sun, 6 Dec 2020 22:46:14 +0100 Subject: [PATCH 37/46] rtc: mc146818: Prevent reading garbage The MC146818 driver is prone to read garbage from the RTC. There are several issues all related to the update cycle of the MC146818. The chip increments seconds obviously once per second and indicates that by a bit in a register. The bit goes high 244us before the actual update starts. During the update the readout of the time values is undefined. The code just checks whether the update in progress bit (UIP) is set before reading the clock. If it's set it waits arbitrary 20ms before retrying, which is ample because the maximum update time is ~2ms. But this check does not guarantee that the UIP bit goes high and the actual update happens during the readout. So the following can happen 0.997 UIP = False -> Interrupt/NMI/preemption 0.998 UIP -> True 0.999 Readout <- Undefined To prevent this rework the code so it checks UIP before and after the readout and if set after the readout try again. But that's not enough to cover the following: 0.997 UIP = False Readout seconds -> NMI (or vCPU scheduled out) 0.998 UIP -> True update completes UIP -> False 1.000 Readout minutes,.... UIP check succeeds That can make the readout wrong up to 59 seconds. To prevent this, read the seconds value before the first UIP check, validate it after checking UIP and after reading out the rest. It's amazing that the original i386 code had this actually correct and the generic implementation of the MC146818 driver got it wrong in 2002 and it stayed that way until today. Signed-off-by: Thomas Gleixner Acked-by: Alexandre Belloni Link: https://lore.kernel.org/r/20201206220541.594826678@linutronix.de --- drivers/rtc/rtc-mc146818-lib.c | 64 +++++++++++++++++++++------------- 1 file changed, 39 insertions(+), 25 deletions(-) diff --git a/drivers/rtc/rtc-mc146818-lib.c b/drivers/rtc/rtc-mc146818-lib.c index 2ecd8752b088..98048bba1c14 100644 --- a/drivers/rtc/rtc-mc146818-lib.c +++ b/drivers/rtc/rtc-mc146818-lib.c @@ -8,41 +8,41 @@ #include #endif -/* - * Returns true if a clock update is in progress - */ -static inline unsigned char mc146818_is_updating(void) -{ - unsigned char uip; - unsigned long flags; - - spin_lock_irqsave(&rtc_lock, flags); - uip = (CMOS_READ(RTC_FREQ_SELECT) & RTC_UIP); - spin_unlock_irqrestore(&rtc_lock, flags); - return uip; -} - unsigned int mc146818_get_time(struct rtc_time *time) { unsigned char ctrl; unsigned long flags; unsigned char century = 0; + bool retry; #ifdef CONFIG_MACH_DECSTATION unsigned int real_year; #endif +again: + spin_lock_irqsave(&rtc_lock, flags); /* - * read RTC once any update in progress is done. The update - * can take just over 2ms. We wait 20ms. There is no need to - * to poll-wait (up to 1s - eeccch) for the falling edge of RTC_UIP. - * If you need to know *exactly* when a second has started, enable - * periodic update complete interrupts, (via ioctl) and then - * immediately read /dev/rtc which will block until you get the IRQ. - * Once the read clears, read the RTC time (again via ioctl). Easy. + * Check whether there is an update in progress during which the + * readout is unspecified. The maximum update time is ~2ms. Poll + * every msec for completion. + * + * Store the second value before checking UIP so a long lasting NMI + * which happens to hit after the UIP check cannot make an update + * cycle invisible. */ - if (mc146818_is_updating()) - mdelay(20); + time->tm_sec = CMOS_READ(RTC_SECONDS); + + if (CMOS_READ(RTC_FREQ_SELECT) & RTC_UIP) { + spin_unlock_irqrestore(&rtc_lock, flags); + mdelay(1); + goto again; + } + + /* Revalidate the above readout */ + if (time->tm_sec != CMOS_READ(RTC_SECONDS)) { + spin_unlock_irqrestore(&rtc_lock, flags); + goto again; + } /* * Only the values that we read from the RTC are set. We leave @@ -50,8 +50,6 @@ unsigned int mc146818_get_time(struct rtc_time *time) * RTC has RTC_DAY_OF_WEEK, we ignore it, as it is only updated * by the RTC when initially set to a non-zero value. */ - spin_lock_irqsave(&rtc_lock, flags); - time->tm_sec = CMOS_READ(RTC_SECONDS); time->tm_min = CMOS_READ(RTC_MINUTES); time->tm_hour = CMOS_READ(RTC_HOURS); time->tm_mday = CMOS_READ(RTC_DAY_OF_MONTH); @@ -66,8 +64,24 @@ unsigned int mc146818_get_time(struct rtc_time *time) century = CMOS_READ(acpi_gbl_FADT.century); #endif ctrl = CMOS_READ(RTC_CONTROL); + /* + * Check for the UIP bit again. If it is set now then + * the above values may contain garbage. + */ + retry = CMOS_READ(RTC_FREQ_SELECT) & RTC_UIP; + /* + * A NMI might have interrupted the above sequence so check whether + * the seconds value has changed which indicates that the NMI took + * longer than the UIP bit was set. Unlikely, but possible and + * there is also virt... + */ + retry |= time->tm_sec != CMOS_READ(RTC_SECONDS); + spin_unlock_irqrestore(&rtc_lock, flags); + if (retry) + goto again; + if (!(ctrl & RTC_DM_BINARY) || RTC_ALWAYS_BCD) { time->tm_sec = bcd2bin(time->tm_sec); From dcf257e92622ba0e25fdc4b6699683e7ae67e2a1 Mon Sep 17 00:00:00 2001 From: Thomas Gleixner Date: Sun, 6 Dec 2020 22:46:15 +0100 Subject: [PATCH 38/46] rtc: mc146818: Reduce spinlock section in mc146818_set_time() No need to hold the lock and disable interrupts for doing math. Signed-off-by: Thomas Gleixner Acked-by: Alexandre Belloni Link: https://lore.kernel.org/r/20201206220541.709243630@linutronix.de --- drivers/rtc/rtc-mc146818-lib.c | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/drivers/rtc/rtc-mc146818-lib.c b/drivers/rtc/rtc-mc146818-lib.c index 98048bba1c14..972a5b9a629d 100644 --- a/drivers/rtc/rtc-mc146818-lib.c +++ b/drivers/rtc/rtc-mc146818-lib.c @@ -135,7 +135,6 @@ int mc146818_set_time(struct rtc_time *time) if (yrs > 255) /* They are unsigned */ return -EINVAL; - spin_lock_irqsave(&rtc_lock, flags); #ifdef CONFIG_MACH_DECSTATION real_yrs = yrs; leap_yr = ((!((yrs + 1900) % 4) && ((yrs + 1900) % 100)) || @@ -164,10 +163,8 @@ int mc146818_set_time(struct rtc_time *time) /* These limits and adjustments are independent of * whether the chip is in binary mode or not. */ - if (yrs > 169) { - spin_unlock_irqrestore(&rtc_lock, flags); + if (yrs > 169) return -EINVAL; - } if (yrs >= 100) yrs -= 100; @@ -183,6 +180,7 @@ int mc146818_set_time(struct rtc_time *time) century = bin2bcd(century); } + spin_lock_irqsave(&rtc_lock, flags); save_control = CMOS_READ(RTC_CONTROL); CMOS_WRITE((save_control|RTC_SET), RTC_CONTROL); save_freq_select = CMOS_READ(RTC_FREQ_SELECT); From b0ecd8e8c5ef376777277c4c2db7de92ac59f23f Mon Sep 17 00:00:00 2001 From: Thomas Gleixner Date: Sun, 6 Dec 2020 22:46:16 +0100 Subject: [PATCH 39/46] rtc: cmos: Make rtc_cmos sync offset correct The offset for rtc_cmos must be -500ms to work correctly with the current implementation of rtc_set_ntp_time() due to the following: tsched twrite(t2.tv_sec - 1) t2 (seconds increment) twrite - tsched is the transport time for the write to hit the device, which is negligible for this chip because it's accessed directly. t2 - twrite = 500ms according to the datasheet. But rtc_set_ntp_time() calculation of tsched is: tsched = t2 - 1sec - (t2 - twrite) The default for the sync offset is 500ms which means that the write happens at t2 - 1.5 seconds which is obviously off by a second for this device. Make the offset -500ms so it works correct. Signed-off-by: Thomas Gleixner Reviewed-by: Jason Gunthorpe Acked-by: Alexandre Belloni Link: https://lore.kernel.org/r/20201206220541.830517160@linutronix.de --- drivers/rtc/rtc-cmos.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/drivers/rtc/rtc-cmos.c b/drivers/rtc/rtc-cmos.c index c633319cdb91..7728faca01b4 100644 --- a/drivers/rtc/rtc-cmos.c +++ b/drivers/rtc/rtc-cmos.c @@ -868,6 +868,9 @@ cmos_do_probe(struct device *dev, struct resource *ports, int rtc_irq) if (retval) goto cleanup2; + /* Set the sync offset for the periodic 11min update correct */ + cmos_rtc.rtc->set_offset_nsec = -(NSEC_PER_SEC / 2); + /* export at least the first block of NVRAM */ nvmem_cfg.size = address_space - NVRAM_OFFSET; if (rtc_nvmem_register(cmos_rtc.rtc, &nvmem_cfg)) From 354c796b9270eb4780e59e3bdb83a3ae4930a832 Mon Sep 17 00:00:00 2001 From: Thomas Gleixner Date: Sun, 6 Dec 2020 22:46:17 +0100 Subject: [PATCH 40/46] rtc: core: Make the sync offset default more realistic The offset which is used to steer the start of an RTC synchronization update via rtc_set_ntp_time() is huge. The math behind this is: tsched twrite(t2.tv_sec - 1) t2 (seconds increment) twrite - tsched is the transport time for the write to hit the device. t2 - twrite depends on the chip and is for most chips one second. The rtc_set_ntp_time() calculation of tsched is: tsched = t2 - 1sec - (t2 - twrite) The default for the sync offset is 500ms which means that twrite - tsched is 500ms assumed that t2 - twrite is one second. This is 0.5 seconds off for RTCs which are directly accessible by IO writes and probably for the majority of i2C/SPI based RTC off by an order of magnitude. Set it to 5ms which should bring it closer to reality. The default can be adjusted by drivers (rtc_cmos does so) and could be adjusted further by a calibration method which is an orthogonal problem. Signed-off-by: Thomas Gleixner Reviewed-by: Jason Gunthorpe Acked-by: Alexandre Belloni Link: https://lore.kernel.org/r/20201206220541.960333166@linutronix.de --- drivers/rtc/class.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/rtc/class.c b/drivers/rtc/class.c index 7c88d190c51f..d7957376eb96 100644 --- a/drivers/rtc/class.c +++ b/drivers/rtc/class.c @@ -201,7 +201,7 @@ static struct rtc_device *rtc_allocate_device(void) device_initialize(&rtc->dev); /* Drivers can revise this default after allocating the device. */ - rtc->set_offset_nsec = NSEC_PER_SEC / 2; + rtc->set_offset_nsec = 5 * NSEC_PER_MSEC; rtc->irq_freq = 1; rtc->max_user_freq = 64; From c9e6189fb03123a7dfb93589280347b46f30b161 Mon Sep 17 00:00:00 2001 From: Thomas Gleixner Date: Sun, 6 Dec 2020 22:46:18 +0100 Subject: [PATCH 41/46] ntp: Make the RTC synchronization more reliable Miroslav reported that the periodic RTC synchronization in the NTP code fails more often than not to hit the specified update window. The reason is that the code uses delayed_work to schedule the update which needs to be in thread context as the underlying RTC might be connected via a slow bus, e.g. I2C. In the update function it verifies whether the current time is correct vs. the requirements of the underlying RTC. But delayed_work is using the timer wheel for scheduling which is inaccurate by design. Depending on the distance to the expiry the wheel gets less granular to allow batching and to avoid the cascading of the original timer wheel. See 500462a9de65 ("timers: Switch to a non-cascading wheel") and the code for further details. The code already deals with this by splitting the 660 seconds period into a long 659 seconds timer and then retrying with a smaller delta. But looking at the actual granularities of the timer wheel (which depend on the HZ configuration) the 659 seconds timer ends up in an outer wheel level and is affected by a worst case granularity of: HZ Granularity 1000 32s 250 16s 100 40s So the initial timer can be already off by max 12.5% which is not a big issue as the period of the sync is defined as ~11 minutes. The fine grained second attempt schedules to the desired update point with a timer expiring less than a second from now. Depending on the actual delta and the HZ setting even the second attempt can end up in outer wheel levels which have a large enough granularity to make the correctness check fail. As this is a fundamental property of the timer wheel there is no way to make this more accurate short of iterating in one jiffies steps towards the update point. Switch it to an hrtimer instead which schedules the actual update work. The hrtimer will expire precisely (max 1 jiffie delay when high resolution timers are not available). The actual scheduling delay of the work is the same as before. The update is triggered from do_adjtimex() which is a bit racy but not much more racy than it was before: if (ntp_synced()) queue_delayed_work(system_power_efficient_wq, &sync_work, 0); which is racy when the work is currently executed and has not managed to reschedule itself. This becomes now: if (ntp_synced() && !hrtimer_is_queued(&sync_hrtimer)) queue_work(system_power_efficient_wq, &sync_work, 0); which is racy when the hrtimer has expired and the work is currently executed and has not yet managed to rearm the hrtimer. Not a big problem as it just schedules work for nothing. The new implementation has a safe guard in place to catch the case where the hrtimer is queued on entry to the work function and avoids an extra update attempt of the RTC that way. Reported-by: Miroslav Lichvar Signed-off-by: Thomas Gleixner Tested-by: Miroslav Lichvar Reviewed-by: Jason Gunthorpe Acked-by: Alexandre Belloni Link: https://lore.kernel.org/r/20201206220542.062910520@linutronix.de --- include/linux/timex.h | 1 - kernel/time/ntp.c | 92 ++++++++++++++++++++------------------ kernel/time/ntp_internal.h | 7 +++ 3 files changed, 56 insertions(+), 44 deletions(-) diff --git a/include/linux/timex.h b/include/linux/timex.h index ce0859763670..9c2e54faf9b7 100644 --- a/include/linux/timex.h +++ b/include/linux/timex.h @@ -157,7 +157,6 @@ extern int do_clock_adjtime(const clockid_t which_clock, struct __kernel_timex * extern void hardpps(const struct timespec64 *, const struct timespec64 *); int read_current_timer(unsigned long *timer_val); -void ntp_notify_cmos_timer(void); /* The clock frequency of the i8253/i8254 PIT */ #define PIT_TICK_RATE 1193182ul diff --git a/kernel/time/ntp.c b/kernel/time/ntp.c index 069ca78fb0bf..ff1a7b8ec4ef 100644 --- a/kernel/time/ntp.c +++ b/kernel/time/ntp.c @@ -494,65 +494,55 @@ out: return leap; } +#if defined(CONFIG_GENERIC_CMOS_UPDATE) || defined(CONFIG_RTC_SYSTOHC) static void sync_hw_clock(struct work_struct *work); -static DECLARE_DELAYED_WORK(sync_work, sync_hw_clock); - -static void sched_sync_hw_clock(struct timespec64 now, - unsigned long target_nsec, bool fail) +static DECLARE_WORK(sync_work, sync_hw_clock); +static struct hrtimer sync_hrtimer; +#define SYNC_PERIOD_NS (11UL * 60 * NSEC_PER_SEC) +static enum hrtimer_restart sync_timer_callback(struct hrtimer *timer) { - struct timespec64 next; + queue_work(system_power_efficient_wq, &sync_work); - ktime_get_real_ts64(&next); - if (!fail) - next.tv_sec = 659; - else { - /* - * Try again as soon as possible. Delaying long periods - * decreases the accuracy of the work queue timer. Due to this - * the algorithm is very likely to require a short-sleep retry - * after the above long sleep to synchronize ts_nsec. - */ - next.tv_sec = 0; - } + return HRTIMER_NORESTART; +} - /* Compute the needed delay that will get to tv_nsec == target_nsec */ - next.tv_nsec = target_nsec - next.tv_nsec; - if (next.tv_nsec <= 0) - next.tv_nsec += NSEC_PER_SEC; - if (next.tv_nsec >= NSEC_PER_SEC) { - next.tv_sec++; - next.tv_nsec -= NSEC_PER_SEC; - } +static void sched_sync_hw_clock(unsigned long offset_nsec, bool retry) +{ + ktime_t exp = ktime_set(ktime_get_real_seconds(), 0); - queue_delayed_work(system_power_efficient_wq, &sync_work, - timespec64_to_jiffies(&next)); + if (retry) + exp = ktime_add_ns(exp, 2 * NSEC_PER_SEC - offset_nsec); + else + exp = ktime_add_ns(exp, SYNC_PERIOD_NS - offset_nsec); + + hrtimer_start(&sync_hrtimer, exp, HRTIMER_MODE_ABS); } static void sync_rtc_clock(void) { - unsigned long target_nsec; - struct timespec64 adjust, now; + unsigned long offset_nsec; + struct timespec64 adjust; int rc; if (!IS_ENABLED(CONFIG_RTC_SYSTOHC)) return; - ktime_get_real_ts64(&now); + ktime_get_real_ts64(&adjust); - adjust = now; if (persistent_clock_is_local) adjust.tv_sec -= (sys_tz.tz_minuteswest * 60); /* - * The current RTC in use will provide the target_nsec it wants to be - * called at, and does rtc_tv_nsec_ok internally. + * The current RTC in use will provide the nanoseconds offset prior + * to a full second it wants to be called at, and invokes + * rtc_tv_nsec_ok() internally. */ - rc = rtc_set_ntp_time(adjust, &target_nsec); + rc = rtc_set_ntp_time(adjust, &offset_nsec); if (rc == -ENODEV) return; - sched_sync_hw_clock(now, target_nsec, rc); + sched_sync_hw_clock(offset_nsec, rc != 0); } #ifdef CONFIG_GENERIC_CMOS_UPDATE @@ -599,7 +589,7 @@ static bool sync_cmos_clock(void) } } - sched_sync_hw_clock(now, target_nsec, rc); + sched_sync_hw_clock(target_nsec, rc != 0); return true; } @@ -613,7 +603,12 @@ static bool sync_cmos_clock(void) */ static void sync_hw_clock(struct work_struct *work) { - if (!ntp_synced()) + /* + * Don't update if STA_UNSYNC is set and if ntp_notify_cmos_timer() + * managed to schedule the work between the timer firing and the + * work being able to rearm the timer. Wait for the timer to expire. + */ + if (!ntp_synced() || hrtimer_is_queued(&sync_hrtimer)) return; if (sync_cmos_clock()) @@ -624,14 +619,24 @@ static void sync_hw_clock(struct work_struct *work) void ntp_notify_cmos_timer(void) { - if (!ntp_synced()) - return; - - if (IS_ENABLED(CONFIG_GENERIC_CMOS_UPDATE) || - IS_ENABLED(CONFIG_RTC_SYSTOHC)) - queue_delayed_work(system_power_efficient_wq, &sync_work, 0); + /* + * When the work is currently executed but has not yet the timer + * rearmed this queues the work immediately again. No big issue, + * just a pointless work scheduled. + */ + if (ntp_synced() && !hrtimer_is_queued(&sync_hrtimer)) + queue_work(system_power_efficient_wq, &sync_work); } +static void __init ntp_init_cmos_sync(void) +{ + hrtimer_init(&sync_hrtimer, CLOCK_REALTIME, HRTIMER_MODE_ABS); + sync_hrtimer.function = sync_timer_callback; +} +#else /* CONFIG_GENERIC_CMOS_UPDATE) || defined(CONFIG_RTC_SYSTOHC) */ +static inline void __init ntp_init_cmos_sync(void) { } +#endif /* !CONFIG_GENERIC_CMOS_UPDATE) || defined(CONFIG_RTC_SYSTOHC) */ + /* * Propagate a new txc->status value into the NTP state: */ @@ -1044,4 +1049,5 @@ __setup("ntp_tick_adj=", ntp_tick_adj_setup); void __init ntp_init(void) { ntp_clear(); + ntp_init_cmos_sync(); } diff --git a/kernel/time/ntp_internal.h b/kernel/time/ntp_internal.h index 908ecaa65fc3..23d1b74c3065 100644 --- a/kernel/time/ntp_internal.h +++ b/kernel/time/ntp_internal.h @@ -12,4 +12,11 @@ extern int __do_adjtimex(struct __kernel_timex *txc, const struct timespec64 *ts, s32 *time_tai, struct audit_ntp_data *ad); extern void __hardpps(const struct timespec64 *phase_ts, const struct timespec64 *raw_ts); + +#if defined(CONFIG_GENERIC_CMOS_UPDATE) || defined(CONFIG_RTC_SYSTOHC) +extern void ntp_notify_cmos_timer(void); +#else +static inline void ntp_notify_cmos_timer(void) { } +#endif + #endif /* _LINUX_NTP_INTERNAL_H */ From 33e62e832384c8cb523044e0e9d99d7133f98e93 Mon Sep 17 00:00:00 2001 From: Thomas Gleixner Date: Sun, 6 Dec 2020 22:46:19 +0100 Subject: [PATCH 42/46] ntp, rtc: Move rtc_set_ntp_time() to ntp code rtc_set_ntp_time() is not really RTC functionality as the code is just a user of RTC. Move it into the NTP code which allows further cleanups. Requested-by: Alexandre Belloni Signed-off-by: Thomas Gleixner Reviewed-by: Jason Gunthorpe Acked-by: Alexandre Belloni Link: https://lore.kernel.org/r/20201206220542.166871172@linutronix.de --- drivers/rtc/Makefile | 1 - drivers/rtc/systohc.c | 61 ------------------------------ include/linux/rtc.h | 34 ----------------- kernel/time/ntp.c | 88 +++++++++++++++++++++++++++++++++++++++++-- 4 files changed, 85 insertions(+), 99 deletions(-) delete mode 100644 drivers/rtc/systohc.c diff --git a/drivers/rtc/Makefile b/drivers/rtc/Makefile index bfb57464118d..bb8f319b09fb 100644 --- a/drivers/rtc/Makefile +++ b/drivers/rtc/Makefile @@ -6,7 +6,6 @@ ccflags-$(CONFIG_RTC_DEBUG) := -DDEBUG obj-$(CONFIG_RTC_LIB) += lib.o -obj-$(CONFIG_RTC_SYSTOHC) += systohc.o obj-$(CONFIG_RTC_CLASS) += rtc-core.o obj-$(CONFIG_RTC_MC146818_LIB) += rtc-mc146818-lib.o rtc-core-y := class.o interface.o diff --git a/drivers/rtc/systohc.c b/drivers/rtc/systohc.c deleted file mode 100644 index 8b70f0520e13..000000000000 --- a/drivers/rtc/systohc.c +++ /dev/null @@ -1,61 +0,0 @@ -// SPDX-License-Identifier: GPL-2.0 -#include -#include - -/** - * rtc_set_ntp_time - Save NTP synchronized time to the RTC - * @now: Current time of day - * @target_nsec: pointer for desired now->tv_nsec value - * - * Replacement for the NTP platform function update_persistent_clock64 - * that stores time for later retrieval by rtc_hctosys. - * - * Returns 0 on successful RTC update, -ENODEV if a RTC update is not - * possible at all, and various other -errno for specific temporary failure - * cases. - * - * -EPROTO is returned if now.tv_nsec is not close enough to *target_nsec. - * - * If temporary failure is indicated the caller should try again 'soon' - */ -int rtc_set_ntp_time(struct timespec64 now, unsigned long *target_nsec) -{ - struct rtc_device *rtc; - struct rtc_time tm; - struct timespec64 to_set; - int err = -ENODEV; - bool ok; - - rtc = rtc_class_open(CONFIG_RTC_SYSTOHC_DEVICE); - if (!rtc) - goto out_err; - - if (!rtc->ops || !rtc->ops->set_time) - goto out_close; - - /* Compute the value of tv_nsec we require the caller to supply in - * now.tv_nsec. This is the value such that (now + - * set_offset_nsec).tv_nsec == 0. - */ - set_normalized_timespec64(&to_set, 0, -rtc->set_offset_nsec); - *target_nsec = to_set.tv_nsec; - - /* The ntp code must call this with the correct value in tv_nsec, if - * it does not we update target_nsec and return EPROTO to make the ntp - * code try again later. - */ - ok = rtc_tv_nsec_ok(rtc->set_offset_nsec, &to_set, &now); - if (!ok) { - err = -EPROTO; - goto out_close; - } - - rtc_time64_to_tm(to_set.tv_sec, &tm); - - err = rtc_set_time(rtc, &tm); - -out_close: - rtc_class_close(rtc); -out_err: - return err; -} diff --git a/include/linux/rtc.h b/include/linux/rtc.h index 22d1575e4991..ff62680b48ca 100644 --- a/include/linux/rtc.h +++ b/include/linux/rtc.h @@ -165,7 +165,6 @@ int __rtc_register_device(struct module *owner, struct rtc_device *rtc); extern int rtc_read_time(struct rtc_device *rtc, struct rtc_time *tm); extern int rtc_set_time(struct rtc_device *rtc, struct rtc_time *tm); -extern int rtc_set_ntp_time(struct timespec64 now, unsigned long *target_nsec); int __rtc_read_alarm(struct rtc_device *rtc, struct rtc_wkalrm *alarm); extern int rtc_read_alarm(struct rtc_device *rtc, struct rtc_wkalrm *alrm); @@ -205,39 +204,6 @@ static inline bool is_leap_year(unsigned int year) return (!(year % 4) && (year % 100)) || !(year % 400); } -/* Determine if we can call to driver to set the time. Drivers can only be - * called to set a second aligned time value, and the field set_offset_nsec - * specifies how far away from the second aligned time to call the driver. - * - * This also computes 'to_set' which is the time we are trying to set, and has - * a zero in tv_nsecs, such that: - * to_set - set_delay_nsec == now +/- FUZZ - * - */ -static inline bool rtc_tv_nsec_ok(s64 set_offset_nsec, - struct timespec64 *to_set, - const struct timespec64 *now) -{ - /* Allowed error in tv_nsec, arbitarily set to 5 jiffies in ns. */ - const unsigned long TIME_SET_NSEC_FUZZ = TICK_NSEC * 5; - struct timespec64 delay = {.tv_sec = 0, - .tv_nsec = set_offset_nsec}; - - *to_set = timespec64_add(*now, delay); - - if (to_set->tv_nsec < TIME_SET_NSEC_FUZZ) { - to_set->tv_nsec = 0; - return true; - } - - if (to_set->tv_nsec > NSEC_PER_SEC - TIME_SET_NSEC_FUZZ) { - to_set->tv_sec++; - to_set->tv_nsec = 0; - return true; - } - return false; -} - #define rtc_register_device(device) \ __rtc_register_device(THIS_MODULE, device) diff --git a/kernel/time/ntp.c b/kernel/time/ntp.c index ff1a7b8ec4ef..84a554622cee 100644 --- a/kernel/time/ntp.c +++ b/kernel/time/ntp.c @@ -519,15 +519,94 @@ static void sched_sync_hw_clock(unsigned long offset_nsec, bool retry) hrtimer_start(&sync_hrtimer, exp, HRTIMER_MODE_ABS); } +/* + * Determine if we can call to driver to set the time. Drivers can only be + * called to set a second aligned time value, and the field set_offset_nsec + * specifies how far away from the second aligned time to call the driver. + * + * This also computes 'to_set' which is the time we are trying to set, and has + * a zero in tv_nsecs, such that: + * to_set - set_delay_nsec == now +/- FUZZ + * + */ +static inline bool rtc_tv_nsec_ok(long set_offset_nsec, + struct timespec64 *to_set, + const struct timespec64 *now) +{ + /* Allowed error in tv_nsec, arbitarily set to 5 jiffies in ns. */ + const unsigned long TIME_SET_NSEC_FUZZ = TICK_NSEC * 5; + struct timespec64 delay = {.tv_sec = 0, + .tv_nsec = set_offset_nsec}; + + *to_set = timespec64_add(*now, delay); + + if (to_set->tv_nsec < TIME_SET_NSEC_FUZZ) { + to_set->tv_nsec = 0; + return true; + } + + if (to_set->tv_nsec > NSEC_PER_SEC - TIME_SET_NSEC_FUZZ) { + to_set->tv_sec++; + to_set->tv_nsec = 0; + return true; + } + return false; +} + +#ifdef CONFIG_RTC_SYSTOHC +/* + * rtc_set_ntp_time - Save NTP synchronized time to the RTC + */ +static int rtc_set_ntp_time(struct timespec64 now, unsigned long *target_nsec) +{ + struct rtc_device *rtc; + struct rtc_time tm; + struct timespec64 to_set; + int err = -ENODEV; + bool ok; + + rtc = rtc_class_open(CONFIG_RTC_SYSTOHC_DEVICE); + if (!rtc) + goto out_err; + + if (!rtc->ops || !rtc->ops->set_time) + goto out_close; + + /* + * Compute the value of tv_nsec we require the caller to supply in + * now.tv_nsec. This is the value such that (now + + * set_offset_nsec).tv_nsec == 0. + */ + set_normalized_timespec64(&to_set, 0, -rtc->set_offset_nsec); + *target_nsec = to_set.tv_nsec; + + /* + * The ntp code must call this with the correct value in tv_nsec, if + * it does not we update target_nsec and return EPROTO to make the ntp + * code try again later. + */ + ok = rtc_tv_nsec_ok(rtc->set_offset_nsec, &to_set, &now); + if (!ok) { + err = -EPROTO; + goto out_close; + } + + rtc_time64_to_tm(to_set.tv_sec, &tm); + + err = rtc_set_time(rtc, &tm); + +out_close: + rtc_class_close(rtc); +out_err: + return err; +} + static void sync_rtc_clock(void) { unsigned long offset_nsec; struct timespec64 adjust; int rc; - if (!IS_ENABLED(CONFIG_RTC_SYSTOHC)) - return; - ktime_get_real_ts64(&adjust); if (persistent_clock_is_local) @@ -544,6 +623,9 @@ static void sync_rtc_clock(void) sched_sync_hw_clock(offset_nsec, rc != 0); } +#else +static inline void sync_rtc_clock(void) { } +#endif #ifdef CONFIG_GENERIC_CMOS_UPDATE int __weak update_persistent_clock64(struct timespec64 now64) From 69eca258c85000564577642ba28335eb4e1df8f0 Mon Sep 17 00:00:00 2001 From: Thomas Gleixner Date: Sun, 6 Dec 2020 22:46:20 +0100 Subject: [PATCH 43/46] ntp: Make the RTC sync offset less obscure The current RTC set_offset_nsec value is not really intuitive to understand. tsched twrite(t2.tv_sec - 1) t2 (seconds increment) The offset is calculated from twrite based on the assumption that t2 - twrite == 1s. That means for the MC146818 RTC the offset needs to be negative so that the write happens 500ms before t2. It's easier to understand when the whole calculation is based on t2. That avoids negative offsets and the meaning is obvious: t2 - twrite: The time defined by the chip when seconds increment after the write. twrite - tsched: The time for the transport to the point where the chip is updated. ==> set_offset_nsec = t2 - tsched ttransport = twrite - tsched tRTCinc = t2 - twrite ==> set_offset_nsec = ttransport + tRTCinc tRTCinc is a chip property and can be obtained from the data sheet. ttransport depends on how the RTC is connected. It is close to 0 for directly accessible RTCs. For RTCs behind a slow bus, e.g. i2c, it's the time required to send the update over the bus. This can be estimated or even calibrated, but that's a different problem. Adjust the implementation and update comments accordingly. Signed-off-by: Thomas Gleixner Acked-by: Alexandre Belloni Link: https://lore.kernel.org/r/20201206220542.263204937@linutronix.de --- drivers/rtc/class.c | 9 ++++++-- drivers/rtc/rtc-cmos.c | 2 +- include/linux/rtc.h | 35 +++++++++++++++++++++++++------ kernel/time/ntp.c | 47 +++++++++++++++++++++--------------------- 4 files changed, 61 insertions(+), 32 deletions(-) diff --git a/drivers/rtc/class.c b/drivers/rtc/class.c index d7957376eb96..5855aa2eef62 100644 --- a/drivers/rtc/class.c +++ b/drivers/rtc/class.c @@ -200,8 +200,13 @@ static struct rtc_device *rtc_allocate_device(void) device_initialize(&rtc->dev); - /* Drivers can revise this default after allocating the device. */ - rtc->set_offset_nsec = 5 * NSEC_PER_MSEC; + /* + * Drivers can revise this default after allocating the device. + * The default is what most RTCs do: Increment seconds exactly one + * second after the write happened. This adds a default transport + * time of 5ms which is at least halfways close to reality. + */ + rtc->set_offset_nsec = NSEC_PER_SEC + 5 * NSEC_PER_MSEC; rtc->irq_freq = 1; rtc->max_user_freq = 64; diff --git a/drivers/rtc/rtc-cmos.c b/drivers/rtc/rtc-cmos.c index 7728faca01b4..c5bcd2adc9fe 100644 --- a/drivers/rtc/rtc-cmos.c +++ b/drivers/rtc/rtc-cmos.c @@ -869,7 +869,7 @@ cmos_do_probe(struct device *dev, struct resource *ports, int rtc_irq) goto cleanup2; /* Set the sync offset for the periodic 11min update correct */ - cmos_rtc.rtc->set_offset_nsec = -(NSEC_PER_SEC / 2); + cmos_rtc.rtc->set_offset_nsec = NSEC_PER_SEC / 2; /* export at least the first block of NVRAM */ nvmem_cfg.size = address_space - NVRAM_OFFSET; diff --git a/include/linux/rtc.h b/include/linux/rtc.h index ff62680b48ca..b829382de6c3 100644 --- a/include/linux/rtc.h +++ b/include/linux/rtc.h @@ -110,13 +110,36 @@ struct rtc_device { /* Some hardware can't support UIE mode */ int uie_unsupported; - /* Number of nsec it takes to set the RTC clock. This influences when - * the set ops are called. An offset: - * - of 0.5 s will call RTC set for wall clock time 10.0 s at 9.5 s - * - of 1.5 s will call RTC set for wall clock time 10.0 s at 8.5 s - * - of -0.5 s will call RTC set for wall clock time 10.0 s at 10.5 s + /* + * This offset specifies the update timing of the RTC. + * + * tsched t1 write(t2.tv_sec - 1sec)) t2 RTC increments seconds + * + * The offset defines how tsched is computed so that the write to + * the RTC (t2.tv_sec - 1sec) is correct versus the time required + * for the transport of the write and the time which the RTC needs + * to increment seconds the first time after the write (t2). + * + * For direct accessible RTCs tsched ~= t1 because the write time + * is negligible. For RTCs behind slow busses the transport time is + * significant and has to be taken into account. + * + * The time between the write (t1) and the first increment after + * the write (t2) is RTC specific. For a MC146818 RTC it's 500ms, + * for many others it's exactly 1 second. Consult the datasheet. + * + * The value of this offset is also used to calculate the to be + * written value (t2.tv_sec - 1sec) at tsched. + * + * The default value for this is NSEC_PER_SEC + 10 msec default + * transport time. The offset can be adjusted by drivers so the + * calculation for the to be written value at tsched becomes + * correct: + * + * newval = tsched + set_offset_nsec - NSEC_PER_SEC + * and (tsched + set_offset_nsec) % NSEC_PER_SEC == 0 */ - long set_offset_nsec; + unsigned long set_offset_nsec; bool registered; diff --git a/kernel/time/ntp.c b/kernel/time/ntp.c index 84a554622cee..a34ac069335f 100644 --- a/kernel/time/ntp.c +++ b/kernel/time/ntp.c @@ -520,22 +520,33 @@ static void sched_sync_hw_clock(unsigned long offset_nsec, bool retry) } /* - * Determine if we can call to driver to set the time. Drivers can only be - * called to set a second aligned time value, and the field set_offset_nsec - * specifies how far away from the second aligned time to call the driver. + * Check whether @now is correct versus the required time to update the RTC + * and calculate the value which needs to be written to the RTC so that the + * next seconds increment of the RTC after the write is aligned with the next + * seconds increment of clock REALTIME. * - * This also computes 'to_set' which is the time we are trying to set, and has - * a zero in tv_nsecs, such that: - * to_set - set_delay_nsec == now +/- FUZZ + * tsched t1 write(t2.tv_sec - 1sec)) t2 RTC increments seconds * + * t2.tv_nsec == 0 + * tsched = t2 - set_offset_nsec + * newval = t2 - NSEC_PER_SEC + * + * ==> neval = tsched + set_offset_nsec - NSEC_PER_SEC + * + * As the execution of this code is not guaranteed to happen exactly at + * tsched this allows it to happen within a fuzzy region: + * + * abs(now - tsched) < FUZZ + * + * If @now is not inside the allowed window the function returns false. */ -static inline bool rtc_tv_nsec_ok(long set_offset_nsec, +static inline bool rtc_tv_nsec_ok(unsigned long set_offset_nsec, struct timespec64 *to_set, const struct timespec64 *now) { /* Allowed error in tv_nsec, arbitarily set to 5 jiffies in ns. */ const unsigned long TIME_SET_NSEC_FUZZ = TICK_NSEC * 5; - struct timespec64 delay = {.tv_sec = 0, + struct timespec64 delay = {.tv_sec = -1, .tv_nsec = set_offset_nsec}; *to_set = timespec64_add(*now, delay); @@ -557,11 +568,11 @@ static inline bool rtc_tv_nsec_ok(long set_offset_nsec, /* * rtc_set_ntp_time - Save NTP synchronized time to the RTC */ -static int rtc_set_ntp_time(struct timespec64 now, unsigned long *target_nsec) +static int rtc_set_ntp_time(struct timespec64 now, unsigned long *offset_nsec) { + struct timespec64 to_set; struct rtc_device *rtc; struct rtc_time tm; - struct timespec64 to_set; int err = -ENODEV; bool ok; @@ -572,19 +583,9 @@ static int rtc_set_ntp_time(struct timespec64 now, unsigned long *target_nsec) if (!rtc->ops || !rtc->ops->set_time) goto out_close; - /* - * Compute the value of tv_nsec we require the caller to supply in - * now.tv_nsec. This is the value such that (now + - * set_offset_nsec).tv_nsec == 0. - */ - set_normalized_timespec64(&to_set, 0, -rtc->set_offset_nsec); - *target_nsec = to_set.tv_nsec; + /* Store the update offset for this RTC */ + *offset_nsec = rtc->set_offset_nsec; - /* - * The ntp code must call this with the correct value in tv_nsec, if - * it does not we update target_nsec and return EPROTO to make the ntp - * code try again later. - */ ok = rtc_tv_nsec_ok(rtc->set_offset_nsec, &to_set, &now); if (!ok) { err = -EPROTO; @@ -657,7 +658,7 @@ static bool sync_cmos_clock(void) * implement this legacy API. */ ktime_get_real_ts64(&now); - if (rtc_tv_nsec_ok(-1 * target_nsec, &adjust, &now)) { + if (rtc_tv_nsec_ok(target_nsec, &adjust, &now)) { if (persistent_clock_is_local) adjust.tv_sec -= (sys_tz.tz_minuteswest * 60); rc = update_persistent_clock64(adjust); From 76e87d96b30b5fee91b381fbc444a3eabcd9469a Mon Sep 17 00:00:00 2001 From: Thomas Gleixner Date: Sun, 6 Dec 2020 22:46:21 +0100 Subject: [PATCH 44/46] ntp: Consolidate the RTC update implementation The code for the legacy RTC and the RTC class based update are pretty much the same. Consolidate the common parts into one function and just invoke the actual setter functions. For RTC class based devices the update code checks whether the offset is valid for the device, which is usually not the case for the first invocation. If it's not the same it stores the correct offset and lets the caller try again. That's not much different from the previous approach where the first invocation had a pretty low probability to actually hit the allowed window. Signed-off-by: Thomas Gleixner Reviewed-by: Jason Gunthorpe Acked-by: Alexandre Belloni Link: https://lore.kernel.org/r/20201206220542.355743355@linutronix.de --- kernel/time/ntp.c | 162 +++++++++++++++++----------------------------- 1 file changed, 61 insertions(+), 101 deletions(-) diff --git a/kernel/time/ntp.c b/kernel/time/ntp.c index a34ac069335f..7404d3831527 100644 --- a/kernel/time/ntp.c +++ b/kernel/time/ntp.c @@ -564,117 +564,52 @@ static inline bool rtc_tv_nsec_ok(unsigned long set_offset_nsec, return false; } -#ifdef CONFIG_RTC_SYSTOHC -/* - * rtc_set_ntp_time - Save NTP synchronized time to the RTC - */ -static int rtc_set_ntp_time(struct timespec64 now, unsigned long *offset_nsec) -{ - struct timespec64 to_set; - struct rtc_device *rtc; - struct rtc_time tm; - int err = -ENODEV; - bool ok; - - rtc = rtc_class_open(CONFIG_RTC_SYSTOHC_DEVICE); - if (!rtc) - goto out_err; - - if (!rtc->ops || !rtc->ops->set_time) - goto out_close; - - /* Store the update offset for this RTC */ - *offset_nsec = rtc->set_offset_nsec; - - ok = rtc_tv_nsec_ok(rtc->set_offset_nsec, &to_set, &now); - if (!ok) { - err = -EPROTO; - goto out_close; - } - - rtc_time64_to_tm(to_set.tv_sec, &tm); - - err = rtc_set_time(rtc, &tm); - -out_close: - rtc_class_close(rtc); -out_err: - return err; -} - -static void sync_rtc_clock(void) -{ - unsigned long offset_nsec; - struct timespec64 adjust; - int rc; - - ktime_get_real_ts64(&adjust); - - if (persistent_clock_is_local) - adjust.tv_sec -= (sys_tz.tz_minuteswest * 60); - - /* - * The current RTC in use will provide the nanoseconds offset prior - * to a full second it wants to be called at, and invokes - * rtc_tv_nsec_ok() internally. - */ - rc = rtc_set_ntp_time(adjust, &offset_nsec); - if (rc == -ENODEV) - return; - - sched_sync_hw_clock(offset_nsec, rc != 0); -} -#else -static inline void sync_rtc_clock(void) { } -#endif - #ifdef CONFIG_GENERIC_CMOS_UPDATE int __weak update_persistent_clock64(struct timespec64 now64) { return -ENODEV; } +#else +static inline int update_persistent_clock64(struct timespec64 now64) +{ + return -ENODEV; +} #endif -static bool sync_cmos_clock(void) +#ifdef CONFIG_RTC_SYSTOHC +/* Save NTP synchronized time to the RTC */ +static int update_rtc(struct timespec64 *to_set, unsigned long *offset_nsec) { - static bool no_cmos; - struct timespec64 now; - struct timespec64 adjust; - int rc = -EPROTO; - long target_nsec = NSEC_PER_SEC / 2; + struct rtc_device *rtc; + struct rtc_time tm; + int err = -ENODEV; - if (!IS_ENABLED(CONFIG_GENERIC_CMOS_UPDATE)) - return false; + rtc = rtc_class_open(CONFIG_RTC_SYSTOHC_DEVICE); + if (!rtc) + return -ENODEV; - if (no_cmos) - return false; + if (!rtc->ops || !rtc->ops->set_time) + goto out_close; - /* - * Historically update_persistent_clock64() has followed x86 - * semantics, which match the MC146818A/etc RTC. This RTC will store - * 'adjust' and then in .5s it will advance once second. - * - * Architectures are strongly encouraged to use rtclib and not - * implement this legacy API. - */ - ktime_get_real_ts64(&now); - if (rtc_tv_nsec_ok(target_nsec, &adjust, &now)) { - if (persistent_clock_is_local) - adjust.tv_sec -= (sys_tz.tz_minuteswest * 60); - rc = update_persistent_clock64(adjust); - /* - * The machine does not support update_persistent_clock64 even - * though it defines CONFIG_GENERIC_CMOS_UPDATE. - */ - if (rc == -ENODEV) { - no_cmos = true; - return false; - } + /* First call might not have the correct offset */ + if (*offset_nsec == rtc->set_offset_nsec) { + rtc_time64_to_tm(to_set->tv_sec, &tm); + err = rtc_set_time(rtc, &tm); + } else { + /* Store the update offset and let the caller try again */ + *offset_nsec = rtc->set_offset_nsec; + err = -EAGAIN; } - - sched_sync_hw_clock(target_nsec, rc != 0); - return true; +out_close: + rtc_class_close(rtc); + return err; } +#else +static inline int update_rtc(struct timespec64 *to_set, unsigned long *offset_nsec) +{ + return -ENODEV; +} +#endif /* * If we have an externally synchronized Linux clock, then update RTC clock @@ -686,6 +621,15 @@ static bool sync_cmos_clock(void) */ static void sync_hw_clock(struct work_struct *work) { + /* + * The default synchronization offset is 500ms for the deprecated + * update_persistent_clock64() under the assumption that it uses + * the infamous CMOS clock (MC146818). + */ + static unsigned long offset_nsec = NSEC_PER_SEC / 2; + struct timespec64 now, to_set; + int res = -EAGAIN; + /* * Don't update if STA_UNSYNC is set and if ntp_notify_cmos_timer() * managed to schedule the work between the timer firing and the @@ -694,10 +638,26 @@ static void sync_hw_clock(struct work_struct *work) if (!ntp_synced() || hrtimer_is_queued(&sync_hrtimer)) return; - if (sync_cmos_clock()) - return; + ktime_get_real_ts64(&now); + /* If @now is not in the allowed window, try again */ + if (!rtc_tv_nsec_ok(offset_nsec, &to_set, &now)) + goto rearm; - sync_rtc_clock(); + /* Take timezone adjusted RTCs into account */ + if (persistent_clock_is_local) + to_set.tv_sec -= (sys_tz.tz_minuteswest * 60); + + /* Try the legacy RTC first. */ + res = update_persistent_clock64(to_set); + if (res != -ENODEV) + goto rearm; + + /* Try the RTC class */ + res = update_rtc(&to_set, &offset_nsec); + if (res == -ENODEV) + return; +rearm: + sched_sync_hw_clock(offset_nsec, res != 0); } void ntp_notify_cmos_timer(void) From aa3b66f401b372598b29421bab4d17b631b92407 Mon Sep 17 00:00:00 2001 From: Thomas Gleixner Date: Fri, 4 Dec 2020 11:55:19 +0100 Subject: [PATCH 45/46] tick/sched: Make jiffies update quick check more robust The quick check in tick_do_update_jiffies64() whether jiffies need to be updated is not really correct under all circumstances and on all architectures, especially not on 32bit systems. The quick check does: if (now < READ_ONCE(tick_next_period)) return; and the counterpart in the update is: WRITE_ONCE(tick_next_period, next_update_time); This has two problems: 1) On weakly ordered architectures there is no guarantee that the stores before the WRITE_ONCE() are visible which means that other CPUs can operate on a stale jiffies value. 2) On 32bit the store of tick_next_period which is an u64 is split into two 32bit stores. If the first 32bit store advances tick_next_period far out and the second 32bit store is delayed (virt, NMI ...) then jiffies will become stale until the second 32bit store happens. Address this by seperating the handling for 32bit and 64bit. On 64bit problem #1 is addressed by replacing READ_ONCE() / WRITE_ONCE() with smp_load_acquire() / smp_store_release(). On 32bit problem #2 is addressed by protecting the quick check with the jiffies sequence counter. The load and stores can be plain because the sequence count mechanics provides the required barriers already. Signed-off-by: Thomas Gleixner Reviewed-by: Frederic Weisbecker Link: https://lore.kernel.org/r/87czzpc02w.fsf@nanos.tec.linutronix.de --- kernel/time/tick-sched.c | 74 +++++++++++++++++++++++++--------------- 1 file changed, 47 insertions(+), 27 deletions(-) diff --git a/kernel/time/tick-sched.c b/kernel/time/tick-sched.c index cc7cba20382e..a9e68936822d 100644 --- a/kernel/time/tick-sched.c +++ b/kernel/time/tick-sched.c @@ -57,36 +57,42 @@ static ktime_t last_jiffies_update; static void tick_do_update_jiffies64(ktime_t now) { unsigned long ticks = 1; - ktime_t delta; + ktime_t delta, nextp; /* - * Do a quick check without holding jiffies_lock. The READ_ONCE() + * 64bit can do a quick check without holding jiffies lock and + * without looking at the sequence count. The smp_load_acquire() * pairs with the update done later in this function. * - * This is also an intentional data race which is even safe on - * 32bit in theory. If there is a concurrent update then the check - * might give a random answer. It does not matter because if it - * returns then the concurrent update is already taking care, if it - * falls through then it will pointlessly contend on jiffies_lock. - * - * Though there is one nasty case on 32bit due to store tearing of - * the 64bit value. If the first 32bit store makes the quick check - * return on all other CPUs and the writing CPU context gets - * delayed to complete the second store (scheduled out on virt) - * then jiffies can become stale for up to ~2^32 nanoseconds - * without noticing. After that point all CPUs will wait for - * jiffies lock. - * - * OTOH, this is not any different than the situation with NOHZ=off - * where one CPU is responsible for updating jiffies and - * timekeeping. If that CPU goes out for lunch then all other CPUs - * will operate on stale jiffies until it decides to come back. + * 32bit cannot do that because the store of tick_next_period + * consists of two 32bit stores and the first store could move it + * to a random point in the future. */ - if (ktime_before(now, READ_ONCE(tick_next_period))) - return; + if (IS_ENABLED(CONFIG_64BIT)) { + if (ktime_before(now, smp_load_acquire(&tick_next_period))) + return; + } else { + unsigned int seq; - /* Reevaluate with jiffies_lock held */ + /* + * Avoid contention on jiffies_lock and protect the quick + * check with the sequence count. + */ + do { + seq = read_seqcount_begin(&jiffies_seq); + nextp = tick_next_period; + } while (read_seqcount_retry(&jiffies_seq, seq)); + + if (ktime_before(now, nextp)) + return; + } + + /* Quick check failed, i.e. update is required. */ raw_spin_lock(&jiffies_lock); + /* + * Reevaluate with the lock held. Another CPU might have done the + * update already. + */ if (ktime_before(now, tick_next_period)) { raw_spin_unlock(&jiffies_lock); return; @@ -112,11 +118,25 @@ static void tick_do_update_jiffies64(ktime_t now) jiffies_64 += ticks; /* - * Keep the tick_next_period variable up to date. WRITE_ONCE() - * pairs with the READ_ONCE() in the lockless quick check above. + * Keep the tick_next_period variable up to date. */ - WRITE_ONCE(tick_next_period, - ktime_add_ns(last_jiffies_update, TICK_NSEC)); + nextp = ktime_add_ns(last_jiffies_update, TICK_NSEC); + + if (IS_ENABLED(CONFIG_64BIT)) { + /* + * Pairs with smp_load_acquire() in the lockless quick + * check above and ensures that the update to jiffies_64 is + * not reordered vs. the store to tick_next_period, neither + * by the compiler nor by the CPU. + */ + smp_store_release(&tick_next_period, nextp); + } else { + /* + * A plain store is good enough on 32bit as the quick check + * above is protected by the sequence count. + */ + tick_next_period = nextp; + } /* * Release the sequence count. calc_global_load() below is not From 3cabca87b329cbcbdf295be0094adbd72c7b1f67 Mon Sep 17 00:00:00 2001 From: Ingo Molnar Date: Sat, 12 Dec 2020 18:29:20 +0100 Subject: [PATCH 46/46] ntp: Fix prototype in the !CONFIG_GENERIC_CMOS_UPDATE case In the !CONFIG_GENERIC_CMOS_UPDATE case the update_persistent_clock64() function gets defined as a stub in ntp.c - make the prototype in conditional on CONFIG_GENERIC_CMOS_UPDATE as well. Fixes: 76e87d96b30b5 ("ntp: Consolidate the RTC update implementation") Signed-off-by: Ingo Molnar Acked-by: Thomas Gleixner --- include/linux/timekeeping.h | 2 ++ 1 file changed, 2 insertions(+) diff --git a/include/linux/timekeeping.h b/include/linux/timekeeping.h index 7f7e4a3f4394..929d3f3937c0 100644 --- a/include/linux/timekeeping.h +++ b/include/linux/timekeeping.h @@ -303,6 +303,8 @@ extern int persistent_clock_is_local; extern void read_persistent_clock64(struct timespec64 *ts); void read_persistent_wall_and_boot_offset(struct timespec64 *wall_clock, struct timespec64 *boot_offset); +#ifdef CONFIG_GENERIC_CMOS_UPDATE extern int update_persistent_clock64(struct timespec64 now); +#endif #endif