From nobody Mon Nov 25 00:19:30 2024 Received: from galois.linutronix.de (Galois.linutronix.de [193.142.43.55]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id BDBE6187328 for ; Thu, 31 Oct 2024 12:04:09 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=193.142.43.55 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1730376252; cv=none; b=YBnHcuEyS+PhHVY0O/C/+ytp+sbMRPj11PejjZS7s8y+wQE34myIVyDyKpGTm8bny/EIYGm61qKOwr4hb/nYeB5JeQsmEP15Nr9yTo2xIUeRwnBcsZF8qLA/XVD6N7iGBKNhZMwUgZyvLi2Pv24lzmSydbmnvyLoRd+Q3brZ6a4= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1730376252; c=relaxed/simple; bh=YpzKu8nEoV96Wt3cSqQgtIZpcMUeZ9ZtHUGIdaBhKqk=; h=Message-ID:From:To:Cc:Subject:References:MIME-Version: Content-Type:Date; b=VaQHx91ya4EoYBCaCYD0aqxM245uJU1IEDqu2A1h/lgTpu5SF6lVDivudpV+3DZJ51G5JkMozslz89tbOmU4dws4cZfs7bsvbPhf5FbOxP++TB/Z7ntQKnbuaJU/68s26wCIZPIUzwaecHytGPG7Ime/kOfxzqHPzSU/s2gN3Xo= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=linutronix.de; spf=pass smtp.mailfrom=linutronix.de; dkim=pass (2048-bit key) header.d=linutronix.de header.i=@linutronix.de header.b=sqO74uje; dkim=permerror (0-bit key) header.d=linutronix.de header.i=@linutronix.de header.b=XYbMMdYF; arc=none smtp.client-ip=193.142.43.55 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=linutronix.de Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=linutronix.de Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=linutronix.de header.i=@linutronix.de header.b="sqO74uje"; dkim=permerror (0-bit key) header.d=linutronix.de header.i=@linutronix.de header.b="XYbMMdYF" Message-ID: <20241031120328.536010148@linutronix.de> DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linutronix.de; s=2020; t=1730376247; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: references:references; bh=woHrHx/gxc6YVl/Sb4xrXymxodu7bcblbuZnmSYwSvo=; b=sqO74uje4QsJF/7E7KPm81N/DTNzKFCwLnQzy995AAQrKw23h64GvKY9mGAvLXA8yOv7i2 QpVAwJnCJExdsHrTt3oo45xKR4z1Gm4NwVnvrxSD+kbJkhxoZolnKELrCii2cY483aimfE knJZLahuhV58i+euJrEgRtv6uSlEasKaXwgzvylo9rJ98LSCCQCaPB8pAtyCuTAgQ1EfFk /R6ioj8Hjyg08MPiTuxR9jo3GKR92o8z4bq0gvUUjOn7z8bIS9uXCk/gcKb4aKczTBjcoQ TXZDMZPT5D7HbaWoIVioPedyvxbYpFbzcAKMtd6OF5UNXpclnfKrPg8sbJfBnQ== DKIM-Signature: v=1; a=ed25519-sha256; c=relaxed/relaxed; d=linutronix.de; s=2020e; t=1730376247; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: references:references; bh=woHrHx/gxc6YVl/Sb4xrXymxodu7bcblbuZnmSYwSvo=; b=XYbMMdYFLzp/8MVQNq2XAxVe4YnVhEhGVzH3Elwk7xikDKagNv3V4douiXJjd0acv6arsh O56tAcC2R7ENr2CA== From: Thomas Gleixner To: LKML Cc: John Stultz , Anna-Maria Behnsen , Frederic Weisbecker , Stephen Boyd , Peter Zijlstra Subject: [patch 1/2] timekeeping: Remove CONFIG_DEBUG_TIMEKEEPING References: <20241031115448.978498636@linutronix.de> Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Date: Thu, 31 Oct 2024 13:04:07 +0100 (CET) Content-Transfer-Encoding: quoted-printable Content-Type: text/plain; charset="utf-8" Since 135225a363ae timekeeping_cycles_to_ns() handles large offsets which would lead to 64bit multiplication overflows correctly. It's also protected against negative motion of the clocksource unconditionally, which was exclusive to x86 before. timekeeping_advance() handles large offsets already correctly. That means the value of CONFIG_DEBUG_TIMEKEEPING which analyzed these cases is very close to zero. Remove all of it. Signed-off-by: Thomas Gleixner Acked-by: John Stultz --- arch/riscv/configs/defconfig | 1=20 include/linux/timekeeper_internal.h | 16 -- kernel/time/timekeeping.c | 108 ---------------= ----- lib/Kconfig.debug | 13 -- tools/testing/selftests/wireguard/qemu/debug.config | 1=20 5 files changed, 3 insertions(+), 136 deletions(-) --- a/arch/riscv/configs/defconfig +++ b/arch/riscv/configs/defconfig @@ -301,7 +301,6 @@ CONFIG_DEBUG_MEMORY_INIT=3Dy CONFIG_DEBUG_PER_CPU_MAPS=3Dy CONFIG_SOFTLOCKUP_DETECTOR=3Dy CONFIG_WQ_WATCHDOG=3Dy -CONFIG_DEBUG_TIMEKEEPING=3Dy CONFIG_DEBUG_RT_MUTEXES=3Dy CONFIG_DEBUG_SPINLOCK=3Dy CONFIG_DEBUG_MUTEXES=3Dy --- a/include/linux/timekeeper_internal.h +++ b/include/linux/timekeeper_internal.h @@ -76,9 +76,6 @@ struct tk_read_base { * ntp shifted nano seconds. * @ntp_err_mult: Multiplication factor for scaled math conversion * @skip_second_overflow: Flag used to avoid updating NTP twice with same = second - * @last_warning: Warning ratelimiter (DEBUG_TIMEKEEPING) - * @underflow_seen: Underflow warning flag (DEBUG_TIMEKEEPING) - * @overflow_seen: Overflow warning flag (DEBUG_TIMEKEEPING) * * Note: For timespec(64) based interfaces wall_to_monotonic is what * we need to add to xtime (or xtime corrected for sub jiffy times) @@ -147,19 +144,6 @@ struct timekeeper { u32 ntp_error_shift; u32 ntp_err_mult; u32 skip_second_overflow; - -#ifdef CONFIG_DEBUG_TIMEKEEPING - long last_warning; - /* - * These simple flag variables are managed - * without locks, which is racy, but they are - * ok since we don't really care about being - * super precise about how many events were - * seen, just that a problem was observed. - */ - int underflow_seen; - int overflow_seen; -#endif }; =20 #ifdef CONFIG_GENERIC_TIME_VSYSCALL --- a/kernel/time/timekeeping.c +++ b/kernel/time/timekeeping.c @@ -226,97 +226,6 @@ static inline u64 tk_clock_read(const st return clock->read(clock); } =20 -#ifdef CONFIG_DEBUG_TIMEKEEPING -#define WARNING_FREQ (HZ*300) /* 5 minute rate-limiting */ - -static void timekeeping_check_update(struct timekeeper *tk, u64 offset) -{ - - u64 max_cycles =3D tk->tkr_mono.clock->max_cycles; - const char *name =3D tk->tkr_mono.clock->name; - - if (offset > max_cycles) { - printk_deferred("WARNING: timekeeping: Cycle offset (%lld) is larger tha= n allowed by the '%s' clock's max_cycles value (%lld): time overflow danger= \n", - offset, name, max_cycles); - printk_deferred(" timekeeping: Your kernel is sick, but tries to= cope by capping time updates\n"); - } else { - if (offset > (max_cycles >> 1)) { - printk_deferred("INFO: timekeeping: Cycle offset (%lld) is larger than = the '%s' clock's 50%% safety margin (%lld)\n", - offset, name, max_cycles >> 1); - printk_deferred(" timekeeping: Your kernel is still fine, but is f= eeling a bit nervous\n"); - } - } - - if (tk->underflow_seen) { - if (jiffies - tk->last_warning > WARNING_FREQ) { - printk_deferred("WARNING: Underflow in clocksource '%s' observed, time = update ignored.\n", name); - printk_deferred(" Please report this, consider using a differen= t clocksource, if possible.\n"); - printk_deferred(" Your kernel is probably still fine.\n"); - tk->last_warning =3D jiffies; - } - tk->underflow_seen =3D 0; - } - - if (tk->overflow_seen) { - if (jiffies - tk->last_warning > WARNING_FREQ) { - printk_deferred("WARNING: Overflow in clocksource '%s' observed, time u= pdate capped.\n", name); - printk_deferred(" Please report this, consider using a differen= t clocksource, if possible.\n"); - printk_deferred(" Your kernel is probably still fine.\n"); - tk->last_warning =3D jiffies; - } - tk->overflow_seen =3D 0; - } -} - -static inline u64 timekeeping_cycles_to_ns(const struct tk_read_base *tkr,= u64 cycles); - -static inline u64 timekeeping_debug_get_ns(const struct tk_read_base *tkr) -{ - struct timekeeper *tk =3D &tk_core.timekeeper; - u64 now, last, mask, max, delta; - unsigned int seq; - - /* - * Since we're called holding a seqcount, the data may shift - * under us while we're doing the calculation. This can cause - * false positives, since we'd note a problem but throw the - * results away. So nest another seqcount here to atomically - * grab the points we are checking with. - */ - do { - seq =3D read_seqcount_begin(&tk_core.seq); - now =3D tk_clock_read(tkr); - last =3D tkr->cycle_last; - mask =3D tkr->mask; - max =3D tkr->clock->max_cycles; - } while (read_seqcount_retry(&tk_core.seq, seq)); - - delta =3D clocksource_delta(now, last, mask); - - /* - * Try to catch underflows by checking if we are seeing small - * mask-relative negative values. - */ - if (unlikely((~delta & mask) < (mask >> 3))) - tk->underflow_seen =3D 1; - - /* Check for multiplication overflows */ - if (unlikely(delta > max)) - tk->overflow_seen =3D 1; - - /* timekeeping_cycles_to_ns() handles both under and overflow */ - return timekeeping_cycles_to_ns(tkr, now); -} -#else -static inline void timekeeping_check_update(struct timekeeper *tk, u64 off= set) -{ -} -static inline u64 timekeeping_debug_get_ns(const struct tk_read_base *tkr) -{ - BUG(); -} -#endif - /** * tk_setup_internals - Set up internals to use clocksource clock. * @@ -421,19 +330,11 @@ static inline u64 timekeeping_cycles_to_ return ((delta * tkr->mult) + tkr->xtime_nsec) >> tkr->shift; } =20 -static __always_inline u64 __timekeeping_get_ns(const struct tk_read_base = *tkr) +static __always_inline u64 timekeeping_get_ns(const struct tk_read_base *t= kr) { return timekeeping_cycles_to_ns(tkr, tk_clock_read(tkr)); } =20 -static inline u64 timekeeping_get_ns(const struct tk_read_base *tkr) -{ - if (IS_ENABLED(CONFIG_DEBUG_TIMEKEEPING)) - return timekeeping_debug_get_ns(tkr); - - return __timekeeping_get_ns(tkr); -} - /** * update_fast_timekeeper - Update the fast and NMI safe monotonic timekee= per. * @tkr: Timekeeping readout base from which we take the update @@ -477,7 +378,7 @@ static __always_inline u64 __ktime_get_f seq =3D raw_read_seqcount_latch(&tkf->seq); tkr =3D tkf->base + (seq & 0x01); now =3D ktime_to_ns(tkr->base); - now +=3D __timekeeping_get_ns(tkr); + now +=3D timekeeping_get_ns(tkr); } while (raw_read_seqcount_latch_retry(&tkf->seq, seq)); =20 return now; @@ -593,7 +494,7 @@ static __always_inline u64 __ktime_get_r tkr =3D tkf->base + (seq & 0x01); basem =3D ktime_to_ns(tkr->base); baser =3D ktime_to_ns(tkr->base_real); - delta =3D __timekeeping_get_ns(tkr); + delta =3D timekeeping_get_ns(tkr); } while (raw_read_seqcount_latch_retry(&tkf->seq, seq)); =20 if (mono) @@ -2333,9 +2234,6 @@ static bool timekeeping_advance(enum tim if (offset < real_tk->cycle_interval && mode =3D=3D TK_ADV_TICK) return false; =20 - /* Do some additional sanity checking */ - timekeeping_check_update(tk, offset); - /* * With NO_HZ we may have to accumulate many cycle_intervals * (think "ticks") worth of time at once. To do this efficiently, --- a/lib/Kconfig.debug +++ b/lib/Kconfig.debug @@ -1328,19 +1328,6 @@ config SCHEDSTATS =20 endmenu =20 -config DEBUG_TIMEKEEPING - bool "Enable extra timekeeping sanity checking" - help - This option will enable additional timekeeping sanity checks - which may be helpful when diagnosing issues where timekeeping - problems are suspected. - - This may include checks in the timekeeping hotpaths, so this - option may have a (very small) performance impact to some - workloads. - - If unsure, say N. - config DEBUG_PREEMPT bool "Debug preemptible kernel" depends on DEBUG_KERNEL && PREEMPTION && TRACE_IRQFLAGS_SUPPORT --- a/tools/testing/selftests/wireguard/qemu/debug.config +++ b/tools/testing/selftests/wireguard/qemu/debug.config @@ -31,7 +31,6 @@ CONFIG_SCHED_DEBUG=3Dy CONFIG_SCHED_INFO=3Dy CONFIG_SCHEDSTATS=3Dy CONFIG_SCHED_STACK_END_CHECK=3Dy -CONFIG_DEBUG_TIMEKEEPING=3Dy CONFIG_DEBUG_PREEMPT=3Dy CONFIG_DEBUG_RT_MUTEXES=3Dy CONFIG_DEBUG_SPINLOCK=3Dy From nobody Mon Nov 25 00:19:30 2024 Received: from galois.linutronix.de (Galois.linutronix.de [193.142.43.55]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id C008A187864 for ; Thu, 31 Oct 2024 12:04:10 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=193.142.43.55 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1730376252; cv=none; b=YN3R6MH5+7muPFX/K4dYEFWe+DRJ6h8E1dlndPDz7Pg8XzjzdPGm2BF7R4Zrv62UzNykCDIKi8gRjIm4siWpnt1dYebOJRdaDGJQhVhhVS+htm7MsiGFZGQeMvYk66kOUz+IOmBsHCTpF3eR6Bwe47zr970Ah28jeUR3Z61cvyk= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1730376252; c=relaxed/simple; bh=M1eA656eEJ8giqV6WMmEz6tHJn2jOoQrnrwnna6laKc=; h=Message-ID:From:To:Cc:Subject:References:MIME-Version: Content-Type:Date; b=u0SF4CTmEidW9q4k+1+YOQpQEJrz35mjbJ71w+VKYsv2y5HfEIqqpcJeRfmjwHKXjxR932XChWwXAiRdJvDjam/5IpUSH6VglESLVk0u+X14SA9IBk9MCnvH8MVYyP25sXBv+vL8RqoHxx8BYSoYO+UmSvpxueDBR0XmfjrjiYA= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=linutronix.de; spf=pass smtp.mailfrom=linutronix.de; dkim=pass (2048-bit key) header.d=linutronix.de header.i=@linutronix.de header.b=x5dG4y+i; dkim=permerror (0-bit key) header.d=linutronix.de header.i=@linutronix.de header.b=fl1iFoiX; arc=none smtp.client-ip=193.142.43.55 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=linutronix.de Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=linutronix.de Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=linutronix.de header.i=@linutronix.de header.b="x5dG4y+i"; dkim=permerror (0-bit key) header.d=linutronix.de header.i=@linutronix.de header.b="fl1iFoiX" Message-ID: <20241031120328.599430157@linutronix.de> DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linutronix.de; s=2020; t=1730376249; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: references:references; bh=H0Ct+JGAdNC6UcNJLqKMgCSUbsdDOHXxBCiXizWGKOQ=; b=x5dG4y+idX0lxOlu/AlhnI6u/IbCUuXRUpGAs8S7LfqShEs3U4KuLEEymhG2A9EhSVTRtq oSoE//IkB+nHc7kcTdE4Xg2Bg+vyNsUbFwdZ4sRHE91o3CEWrz568snnX0RhnlVI7EbCxu nMS9/lUzknd1Y5UAfJdZEast63roShIBJbDRZWHau2vaO421JUFtTIpN1OHQfHReU/bcYl lrfA6LNkOfoEvdGQFS6HOA6mgr/4bXK+O8s6Egh0Gdr/01lgIykC32HeZk9/ZlVwv61Xoa ZwlBSyLHnGwuaG9KD/Mzo+vWrV4YMhhR3FBtH4n+rMVNcQILx5Bankkhy8YhoQ== DKIM-Signature: v=1; a=ed25519-sha256; c=relaxed/relaxed; d=linutronix.de; s=2020e; t=1730376249; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: references:references; bh=H0Ct+JGAdNC6UcNJLqKMgCSUbsdDOHXxBCiXizWGKOQ=; b=fl1iFoiXWpJigTbu3wyOFanr5dIv5NB8Qc3zFYYXSFiapN0tKysF8147D3GSOI0vhP6Cjk RXGs+SezWtBmV0CA== From: Thomas Gleixner To: LKML Cc: John Stultz , Anna-Maria Behnsen , Frederic Weisbecker , Stephen Boyd , Peter Zijlstra Subject: [patch 2/2] timekeeping: Always check for negative motion References: <20241031115448.978498636@linutronix.de> Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Date: Thu, 31 Oct 2024 13:04:08 +0100 (CET) Content-Transfer-Encoding: quoted-printable Content-Type: text/plain; charset="utf-8" clocksource_delta() has two variants. One with a check for negative motion, which is only selected by x86. This is a historic leftover as this function was previously used in the time getter hot paths. Since 135225a363ae timekeeping_cycles_to_ns() has unconditional protection against this as a by-product of the protection against 64bit math overflow. clocksource_delta() is only used in the clocksource watchdog and in timekeeping_advance(). The extra conditional there is not hurting anyone. Remove the config option and unconditionally prevent negative motion of the readout. Signed-off-by: Thomas Gleixner Acked-by: John Stultz --- arch/x86/Kconfig | 1 - kernel/time/Kconfig | 5 ----- kernel/time/timekeeping_internal.h | 7 ------- 3 files changed, 13 deletions(-) --- a/arch/x86/Kconfig +++ b/arch/x86/Kconfig @@ -145,7 +145,6 @@ config X86 select ARCH_HAS_PARANOID_L1D_FLUSH select BUILDTIME_TABLE_SORT select CLKEVT_I8253 - select CLOCKSOURCE_VALIDATE_LAST_CYCLE select CLOCKSOURCE_WATCHDOG # Word-size accesses may read uninitialized data past the trailing \0 # in strings and cause false KMSAN reports. --- a/kernel/time/Kconfig +++ b/kernel/time/Kconfig @@ -17,11 +17,6 @@ config ARCH_CLOCKSOURCE_DATA config ARCH_CLOCKSOURCE_INIT bool =20 -# Clocksources require validation of the clocksource against the last -# cycle update - x86/TSC misfeature -config CLOCKSOURCE_VALIDATE_LAST_CYCLE - bool - # Timekeeping vsyscall support config GENERIC_TIME_VSYSCALL bool --- a/kernel/time/timekeeping_internal.h +++ b/kernel/time/timekeeping_internal.h @@ -30,7 +30,6 @@ static inline void timekeeping_inc_mg_fl =20 #endif =20 -#ifdef CONFIG_CLOCKSOURCE_VALIDATE_LAST_CYCLE static inline u64 clocksource_delta(u64 now, u64 last, u64 mask) { u64 ret =3D (now - last) & mask; @@ -41,12 +40,6 @@ static inline u64 clocksource_delta(u64 */ return ret & ~(mask >> 1) ? 0 : ret; } -#else -static inline u64 clocksource_delta(u64 now, u64 last, u64 mask) -{ - return (now - last) & mask; -} -#endif =20 /* Semi public for serialization of non timekeeper VDSO updates. */ unsigned long timekeeper_lock_irqsave(void);