vdso: Add support for auxiliary clocks

[PATCH 06/14] vdso/gettimeofday: Return bool from clock_gettime() helpers

Posted by Thomas Weißschuh 3 months, 1 week ago

The internal helpers are effectively using boolean results,
while pretending to use error numbers.

Switch the return type to bool for more clarity.

Signed-off-by: Thomas Weißschuh <thomas.weissschuh@linutronix.de>
---
 lib/vdso/gettimeofday.c | 58 +++++++++++++++++++++++++------------------------
 1 file changed, 30 insertions(+), 28 deletions(-)

diff --git a/lib/vdso/gettimeofday.c b/lib/vdso/gettimeofday.c
index 9b77f23566f6a35887d4c9aaefc61a971131b499..c5266532a097c06f33d12e345c695357d75abf42 100644
--- a/lib/vdso/gettimeofday.c
+++ b/lib/vdso/gettimeofday.c
@@ -82,8 +82,8 @@ const struct vdso_time_data *__arch_get_vdso_u_timens_data(const struct vdso_tim
 #endif /* CONFIG_GENERIC_VDSO_DATA_STORE */
 
 static __always_inline
-int do_hres_timens(const struct vdso_time_data *vdns, const struct vdso_clock *vcns,
-		   clockid_t clk, struct __kernel_timespec *ts)
+bool do_hres_timens(const struct vdso_time_data *vdns, const struct vdso_clock *vcns,
+		    clockid_t clk, struct __kernel_timespec *ts)
 {
 	const struct vdso_time_data *vd = __arch_get_vdso_u_timens_data(vdns);
 	const struct timens_offset *offs = &vcns->offset[clk];
@@ -103,11 +103,11 @@ int do_hres_timens(const struct vdso_time_data *vdns, const struct vdso_clock *v
 		seq = vdso_read_begin(vc);
 
 		if (unlikely(!vdso_clocksource_ok(vc)))
-			return -1;
+			return false;
 
 		cycles = __arch_get_hw_counter(vc->clock_mode, vd);
 		if (unlikely(!vdso_cycles_ok(cycles)))
-			return -1;
+			return false;
 		ns = vdso_calc_ns(vc, cycles, vdso_ts->nsec);
 		sec = vdso_ts->sec;
 	} while (unlikely(vdso_read_retry(vc, seq)));
@@ -123,7 +123,7 @@ int do_hres_timens(const struct vdso_time_data *vdns, const struct vdso_clock *v
 	ts->tv_sec = sec + __iter_div_u64_rem(ns, NSEC_PER_SEC, &ns);
 	ts->tv_nsec = ns;
 
-	return 0;
+	return true;
 }
 #else
 static __always_inline
@@ -133,16 +133,16 @@ const struct vdso_time_data *__arch_get_vdso_u_timens_data(const struct vdso_tim
 }
 
 static __always_inline
-int do_hres_timens(const struct vdso_time_data *vdns, const struct vdso_clock *vcns,
-		   clockid_t clk, struct __kernel_timespec *ts)
+bool do_hres_timens(const struct vdso_time_data *vdns, const struct vdso_clock *vcns,
+		    clockid_t clk, struct __kernel_timespec *ts)
 {
-	return -EINVAL;
+	return false;
 }
 #endif
 
 static __always_inline
-int do_hres(const struct vdso_time_data *vd, const struct vdso_clock *vc,
-	    clockid_t clk, struct __kernel_timespec *ts)
+bool do_hres(const struct vdso_time_data *vd, const struct vdso_clock *vc,
+	     clockid_t clk, struct __kernel_timespec *ts)
 {
 	const struct vdso_timestamp *vdso_ts = &vc->basetime[clk];
 	u64 cycles, sec, ns;
@@ -150,7 +150,7 @@ int do_hres(const struct vdso_time_data *vd, const struct vdso_clock *vc,
 
 	/* Allows to compile the high resolution parts out */
 	if (!__arch_vdso_hres_capable())
-		return -1;
+		return false;
 
 	do {
 		/*
@@ -173,11 +173,11 @@ int do_hres(const struct vdso_time_data *vd, const struct vdso_clock *vc,
 		smp_rmb();
 
 		if (unlikely(!vdso_clocksource_ok(vc)))
-			return -1;
+			return false;
 
 		cycles = __arch_get_hw_counter(vc->clock_mode, vd);
 		if (unlikely(!vdso_cycles_ok(cycles)))
-			return -1;
+			return false;
 		ns = vdso_calc_ns(vc, cycles, vdso_ts->nsec);
 		sec = vdso_ts->sec;
 	} while (unlikely(vdso_read_retry(vc, seq)));
@@ -189,13 +189,13 @@ int do_hres(const struct vdso_time_data *vd, const struct vdso_clock *vc,
 	ts->tv_sec = sec + __iter_div_u64_rem(ns, NSEC_PER_SEC, &ns);
 	ts->tv_nsec = ns;
 
-	return 0;
+	return true;
 }
 
 #ifdef CONFIG_TIME_NS
 static __always_inline
-int do_coarse_timens(const struct vdso_time_data *vdns, const struct vdso_clock *vcns,
-		     clockid_t clk, struct __kernel_timespec *ts)
+bool do_coarse_timens(const struct vdso_time_data *vdns, const struct vdso_clock *vcns,
+		      clockid_t clk, struct __kernel_timespec *ts)
 {
 	const struct vdso_time_data *vd = __arch_get_vdso_u_timens_data(vdns);
 	const struct timens_offset *offs = &vcns->offset[clk];
@@ -223,20 +223,20 @@ int do_coarse_timens(const struct vdso_time_data *vdns, const struct vdso_clock
 	 */
 	ts->tv_sec = sec + __iter_div_u64_rem(nsec, NSEC_PER_SEC, &nsec);
 	ts->tv_nsec = nsec;
-	return 0;
+	return true;
 }
 #else
 static __always_inline
-int do_coarse_timens(const struct vdso_time_data *vdns, const struct vdso_clock *vcns,
-		     clockid_t clk, struct __kernel_timespec *ts)
+bool do_coarse_timens(const struct vdso_time_data *vdns, const struct vdso_clock *vcns,
+		      clockid_t clk, struct __kernel_timespec *ts)
 {
-	return -1;
+	return false;
 }
 #endif
 
 static __always_inline
-int do_coarse(const struct vdso_time_data *vd, const struct vdso_clock *vc,
-	      clockid_t clk, struct __kernel_timespec *ts)
+bool do_coarse(const struct vdso_time_data *vd, const struct vdso_clock *vc,
+	       clockid_t clk, struct __kernel_timespec *ts)
 {
 	const struct vdso_timestamp *vdso_ts = &vc->basetime[clk];
 	u32 seq;
@@ -258,10 +258,10 @@ int do_coarse(const struct vdso_time_data *vd, const struct vdso_clock *vc,
 		ts->tv_nsec = vdso_ts->nsec;
 	} while (unlikely(vdso_read_retry(vc, seq)));
 
-	return 0;
+	return true;
 }
 
-static __always_inline int
+static __always_inline bool
 __cvdso_clock_gettime_common(const struct vdso_time_data *vd, clockid_t clock,
 			     struct __kernel_timespec *ts)
 {
@@ -270,7 +270,7 @@ __cvdso_clock_gettime_common(const struct vdso_time_data *vd, clockid_t clock,
 
 	/* Check for negative values or invalid clocks */
 	if (unlikely((u32) clock >= MAX_CLOCKS))
-		return -1;
+		return false;
 
 	/*
 	 * Convert the clockid to a bitmask and use it to check which
@@ -284,7 +284,7 @@ __cvdso_clock_gettime_common(const struct vdso_time_data *vd, clockid_t clock,
 	else if (msk & VDSO_RAW)
 		vc = &vc[CS_RAW];
 	else
-		return -1;
+		return false;
 
 	return do_hres(vd, vc, clock, ts);
 }
@@ -293,9 +293,11 @@ static __maybe_unused int
 __cvdso_clock_gettime_data(const struct vdso_time_data *vd, clockid_t clock,
 			   struct __kernel_timespec *ts)
 {
-	int ret = __cvdso_clock_gettime_common(vd, clock, ts);
+	bool ok;
 
-	if (unlikely(ret))
+	ok = __cvdso_clock_gettime_common(vd, clock, ts);
+
+	if (unlikely(!ok))
 		return clock_gettime_fallback(clock, ts);
 	return 0;
 }

-- 
2.50.0

Re: [PATCH 06/14] vdso/gettimeofday: Return bool from clock_gettime() helpers

Posted by Mark Brown 3 months ago

On Tue, Jul 01, 2025 at 10:58:00AM +0200, Thomas Weißschuh wrote:
> The internal helpers are effectively using boolean results,
> while pretending to use error numbers.
> 
> Switch the return type to bool for more clarity.

Marek already reported and analysed a hwclock issue with this but I'm
also seeing issues with the vDSO selftest's vdso_test_correcteness on
pine64plus (an arm64 board) which bisect down to this patch:

# [RUN]	Testing gettimeofday...
# 	1752000653.829103 0.000000 1752000653.829106
# [FAIL]	Times are out of sequence

   https://lava.sirena.org.uk/scheduler/job/1550140#L9170

and it causes infrastructure issues due to time on x15 (a 32 bit arm
platform):

| INFO: Generating a skipfile based on /lava-4163764/1/tests/5_kselftest-signal/automated/linux/kselftest/skipfile-lkft.yaml
| fatal error: nanotime returning zero
| goroutine 1 [running, locked to thread]:
| runtime.throw(0x132d83, 0x17)
| 	/usr/lib/golang/src/runtime/panic.go:774 +0x5c fp=0x42c7a4 sp=0x42c790 pc=0x3b740
| runtime.main()
|	/usr/lib/golang/src/runtime/proc.go:152 +0x350 fp=0x42c7e4 sp=0x42c7a4 pc=0x3d308
| runtime.goexit()
|	/usr/lib/golang/src/runtime/asm_arm.s:868 +0x4 fp=0x42c7e4 sp=0x42c7e4 pc=0x645dc
| ERROR: skipgen failed to generate a skipfile: 2

   https://validation.linaro.org/scheduler/job/4163764#L2162

for mystifying reasons which bisect similarly.  I'll try to test Marek's
fix but I'm at a conference this week so no guarantees.

bisect for pine64plus:

# bad: [58ba80c4740212c29a1cf9b48f588e60a7612209] Add linux-next specific files for 20250708
# good: [3a92879a3786d4f28bb3a91a07fd58359b29ca13] Merge branch 'for-linux-next-fixes' of https://gitlab.freedesktop.org/drm/misc/kernel.git
# good: [08dc0f5cc26a203e8008c38d9b436c079e7dbb45] ASoC: soc-dapm: add prefix on soc_dapm_dev_attrs
# good: [571defe0dff3f1e4180bd0db79283d3d5bf74a71] ASoC: codec: rockchip_sai: Remove including of_gpio.h
# good: [2fca750160f29015ab1109bb478537a4e415f7cd] spi: Remove redundant pm_runtime_mark_last_busy() calls
# good: [2bd9648d5a8d329ca734ca2c273a80934867471e] ASoC: SOF: Remove redundant pm_runtime_mark_last_busy() calls
# good: [c61e94e5e4e6bc50064119e6a779564d1d2ac0e7] regulator: stm32-vrefbuf: Remove redundant pm_runtime_mark_last_busy() calls
# good: [bc163baef57002c08b3afe64cdd2f55f55a765eb] ASoC: Use of_reserved_mem_region_to_resource() for "memory-region"
# good: [9f711c9321cffe3e03709176873c277fa911c366] regmap: get rid of redundant debugfs_file_{get,put}()
# good: [baee26a9d6cd3d3c6c3c03c56270aa647a67e4bd] ASoC: fsl_mqs: rename system manager indices for i.MX95
# good: [7105fdd54a14bee49371b39374a61b3c967d74cb] spi: dt-bindings: Convert marvell,orion-spi to DT schema
# good: [913bf8d50cbd144c87e9660b591781179182ff59] spi: spi-qpic-snand: add support for 8 bits ECC strength
# good: [34d340d48e595f8dfd4e72fe4100d2579dbe4a1a] ASoC: qcom: sc8280xp: Add support for QCS8275
# good: [0c0ef1d90967717b91cded41b00dbae05d8e521c] ASoC: amd: acp: Enable acp7.2 platform based DMIC support in machine driver
# good: [3fcd3d2fe44dc9dfca20b6aed117f314a50ba0ff] spi: offload trigger: add ADI Util Sigma-Delta SPI driver
# good: [244bc18e5f1875401a4af87d2eae3f9376d9d720] spi: stm32: delete stray tabs in stm32h7_spi_data_idleness()
# good: [7e1c28fbf235791cb5046fafdac5bc16fe8e788d] spi: spi-pci1xxxx: enable concurrent DMA read/write across SPI transfers
# good: [c4f2c05ab02952c9a56067aeb700ded95b183570] spi: stm32: fix pointer-to-pointer variables usage
# good: [b9ab3b61824190b1c6b2c59e7ba4de591f24eb92] ASoC: SDCA: Add some initial IRQ handlers
# good: [427ceac823e58813b510e585011488f603f0d891] regulator: tps6286x-regulator: Enable REGCACHE_MAPLE
# good: [29ddce17e909779633f856ad1c2f111fbf71c0df] ASoC: codecs: Add calibration function to aw88399 chip
# good: [ac4c064f67d3cdf9118b9b09c1e3b28b6c10a7ea] spi: dt-bindings: add nxp,lpc3220-spi.yaml
# good: [08bf1663c21a3e815eda28fa242d84c945ca3b94] dmaengine: Add devm_dma_request_chan()
# good: [ce57bc9771411d6d27f2ca7b40396cbd7d684ba9] regulator: core: Don't use "proxy" headers
# good: [2555691165a0285a4617230fed859f20dcc51608] spi: atmel-quadspi: Use `devm_dma_request_chan()`
# good: [0f60ecffbfe35e12eb56c99640ba2360244b5bb3] ASoC: sdw_utils: generate combined spk components string
# good: [9a944494c299fabf3cc781798eb7c02a0bece364] spi: dt-bindings: stm32: deprecate `st,spi-midi-ns` property
# good: [3e36c822506d924894ff7de549b9377d3114c2d7] spi: spi-pci1xxxx: Add support for per-instance DMA interrupt vectors
# good: [68fbc70ece40139380380dce74059afa592846b3] ASoC: hisilicon: Standardize ASoC menu
# good: [8f9cf02c8852837923f1cdacfcc92e138513325c] spi: microchip-core-qspi: Add regular transfers
# good: [17cc308b183308bf5ada36e164284fff7eb064ba] ASoC: wm8524: enable constraints when sysclk is configured.
# good: [59566923d955b69bfb1e1163f07dff437dde8c9c] ASoC: SOF: amd: add alternate machines for ACP7.0 and ACP7.1 platforms
# good: [024f39fff6d222cedde361f7fe34d9ba4e6afb92] regulator: mtk-dvfsrc: Add support for MediaTek MT8196 DVFSRC
# good: [19cbc930c209d59a2c9828de4c7b767e9f14667e] regulator: pca9450: Support PWM mode also for pca9451a
# good: [c4ca928a6db1593802cd945f075a7e21dd0430c1] ASoC: hdac_hdmi: Rate limit logging on connection and disconnection
# good: [a48352921f0b15b1f7eff83f5b5613d6ae2350d3] ASoC: codecs: wcd939x: Add defines for major/minor version decoding
# good: [3421d46440ebe0865bec71dbd2330b4e17a425ab] HID: core: Add bus define for SoundWire bus
# good: [a1d203d390e04798ccc1c3c06019cd4411885d6d] ASoC: SOF: ipc4-pcm: Enable delay reporting for ChainDMA streams
# good: [bb48117b79ebc39485f7306d09dc602981fe540f] ASoC: Intel: sof_sdw: Implement add_dai_link to filter HDMI PCMs
# good: [ace9b3daf2b4778358573d3698e34cb1c0fa7e14] ASoC: SOF: ipc4/Intel: Add support for library restore firmware functionality
# good: [2756b7f08ff6ca7c68c8c7dd61c8dc6895c9de34] ASoC: SOF: ipc4-pcm: Harmonize sof_ipc4_set_pipeline_state() dbg print
# good: [5fc2c383125c2b4b6037e02ad8796b776b25e6d0] spi: falcon: mark falcon_sflash_xfer() as static
# good: [cd4da713f99651e99fbce8ed6b6ec8f686c029a8] Documentation: PM: *_autosuspend() functions update last busy time
# good: [7f8924e8785b68c998bc1906e049bf5595865e60] ASoC: dt-bindings: cirrus,cs42xx8: add 'port' property
# good: [3e1c01d06e1f52f78fe00ef26a9cf80dbb0a3115] regulator: rpi-panel-v2: Add shutdown hook
# good: [d9f38d9824bfb1b046d2e720349d2f45959ab184] ASoC: tegra: AHUB: Remove unneeded semicolon
# good: [dce4bc30f42d313b4dc5832316196411b7f07ad0] spi: spi-fsl-dspi: Revert unintended dependency change in config SPI_FSL_DSPI
# good: [47972c1c3315672352f25c68f91dd88543541947] ASoC: Intel: Replace deprecated strcpy() with strscpy()
# good: [5eb8a0d7733d4cd32a776acf1d1aa1c7c01c8a14] ASoC: hdmi-codec: use SND_JACK_AVOUT as jack status
# good: [bb8d8ba4715cb8f997d63d90ba935f6073595df5] ASoC: mediatek: mt8183-afe-pcm: use local `dev` pointer in driver callbacks
# good: [8a5a5cecb79058b608e5562d8998123a3adb313c] ASoC: tas2781: Move the "include linux/debugfs.h" into tas2781.h
# good: [a4eb71ff98c4792f441f108910bd829da7a04092] regulator: rpi-panel-v2: Fix missing OF dependency
# good: [6cafcc53eb5fffd9b9bdfde700bb9bad21e98ed3] spi: spi-mt65xx: Add support for MT6991 Dimensity 9400 SPI IPM
# good: [7e10d7242ea8a5947878880b912ffa5806520705] ASoC: ops: dynamically allocate struct snd_ctl_elem_value
# good: [d6fa0ca959db8efd4462d7beef4bdc5568640fd0] regulator: rpi-panel-v2: Add missing GPIOLIB dependency
# good: [6ba68e5aa9d5d15c8877a655db279fcfc0b38b04] ASoC: renesas: msiof: Convert to <linux/spi/sh_msiof.h>
# good: [1f5cdb6ab45e1c06ae0953609acbb52f8946b3e8] ASoC: codecs: Add support for Richtek RTQ9124
# good: [d49305862fdc4d9ff1b1093b4ed7d8e0cb9971b4] regulator: rpi-panel-v2: Add regulator for 7" Raspberry Pi 720x1280
# good: [03b778d1994827ea5cc971dbdfbb457bbb7bfa5d] ASOC: rockchip: Use helper function devm_clk_get_enabled()
# good: [c459262159f39e6e6336797feb975799344b749b] spi: spi-pci1xxxx: Add support for 25MHz Clock frequency in C0
# good: [267be32b0a7b70cc777f8a46f0904c92c0521d89] ASoC: remove component->id
# good: [548d770c330cd1027549947a6ea899c56b5bc4e4] regulator: pca9450: Add support for mode operations
# good: [111a2c8ab462d77d1519b71b46f13ae1b46920b4] ASoC: imx-card: Use helper function for_each_child_of_node_scoped()
# good: [f6f914893d478b7ba08e5c375de1ced16deb5e92] ASoC: dt-bindings: tas57xx: add tas5753 compatibility
# good: [9a30e332c36c52e92e5316b4a012d45284dedeb5] spi: spi-fsl-dspi: Enable support for S32G platforms
# good: [c95e925daa434ee1a40a86aec6476ce588e4bd77] ASoC: Intel: avs: Add rt5640 machine board
# good: [ac209bde018fd320b79976657a44c23113181af6] ASoC: tas2781: Drop the unnecessary symbol imply
# good: [ece5d881004f041c2e1493436409dbcbea3ad5f8] ASoC: codecs: wcd939x: Drop unused 'struct wcd939x_priv' fields
# good: [b9ecde0bcf6a99a3ff08496d4ba90a385ebbfd68] ASoC: codecs: wcd939x: Add VDD_PX supply
# good: [c8c4694ede7ed42d8d4db0e8927dea9839a3e248] regmap: kunit: Constify regmap_range_cfg array
# good: [e6e8897995a9e6028563ce36c27877e5478c8571] ASoC: qcom: sm8250: Add Fairphone 5 soundcard compatible
git bisect start '58ba80c4740212c29a1cf9b48f588e60a7612209' '3a92879a3786d4f28bb3a91a07fd58359b29ca13' '08dc0f5cc26a203e8008c38d9b436c079e7dbb45' '571defe0dff3f1e4180bd0db79283d3d5bf74a71' '2fca750160f29015ab1109bb478537a4e415f7cd' '2bd9648d5a8d329ca734ca2c273a80934867471e' 'c61e94e5e4e6bc50064119e6a779564d1d2ac0e7' 'bc163baef57002c08b3afe64cdd2f55f55a765eb' '9f711c9321cffe3e03709176873c277fa911c366' 'baee26a9d6cd3d3c6c3c03c56270aa647a67e4bd' '7105fdd54a14bee49371b39374a61b3c967d74cb' '913bf8d50cbd144c87e9660b591781179182ff59' '34d340d48e595f8dfd4e72fe4100d2579dbe4a1a' '0c0ef1d90967717b91cded41b00dbae05d8e521c' '3fcd3d2fe44dc9dfca20b6aed117f314a50ba0ff' '244bc18e5f1875401a4af87d2eae3f9376d9d720' '7e1c28fbf235791cb5046fafdac5bc16fe8e788d' 'c4f2c05ab02952c9a56067aeb700ded95b183570' 'b9ab3b61824190b1c6b2c59e7ba4de591f24eb92' '427ceac823e58813b510e585011488f603f0d891' '29ddce17e909779633f856ad1c2f111fbf71c0df' 'ac4c064f67d3cdf9118b9b09c1e3b28b6c10a7ea' '08bf1663c21a3e815eda28fa242d84c945ca3b94' 'ce57bc9771411d6d27f2ca7b40396cbd7d684ba9' '2555691165a0285a4617230fed859f20dcc51608' '0f60ecffbfe35e12eb56c99640ba2360244b5bb3' '9a944494c299fabf3cc781798eb7c02a0bece364' '3e36c822506d924894ff7de549b9377d3114c2d7' '68fbc70ece40139380380dce74059afa592846b3' '8f9cf02c8852837923f1cdacfcc92e138513325c' '17cc308b183308bf5ada36e164284fff7eb064ba' '59566923d955b69bfb1e1163f07dff437dde8c9c' '024f39fff6d222cedde361f7fe34d9ba4e6afb92' '19cbc930c209d59a2c9828de4c7b767e9f14667e' 'c4ca928a6db1593802cd945f075a7e21dd0430c1' 'a48352921f0b15b1f7eff83f5b5613d6ae2350d3' '3421d46440ebe0865bec71dbd2330b4e17a425ab' 'a1d203d390e04798ccc1c3c06019cd4411885d6d' 'bb48117b79ebc39485f7306d09dc602981fe540f' 'ace9b3daf2b4778358573d3698e34cb1c0fa7e14' '2756b7f08ff6ca7c68c8c7dd61c8dc6895c9de34' '5fc2c383125c2b4b6037e02ad8796b776b25e6d0' 'cd4da713f99651e99fbce8ed6b6ec8f686c029a8' '7f8924e8785b68c998bc1906e049bf5595865e60' '3e1c01d06e1f52f78fe00ef26a9cf80dbb0a3115' 'd9f38d9824bfb1b046d2e720349d2f45959ab184' 'dce4bc30f42d313b4dc5832316196411b7f07ad0' '47972c1c3315672352f25c68f91dd88543541947' '5eb8a0d7733d4cd32a776acf1d1aa1c7c01c8a14' 'bb8d8ba4715cb8f997d63d90ba935f6073595df5' '8a5a5cecb79058b608e5562d8998123a3adb313c' 'a4eb71ff98c4792f441f108910bd829da7a04092' '6cafcc53eb5fffd9b9bdfde700bb9bad21e98ed3' '7e10d7242ea8a5947878880b912ffa5806520705' 'd6fa0ca959db8efd4462d7beef4bdc5568640fd0' '6ba68e5aa9d5d15c8877a655db279fcfc0b38b04' '1f5cdb6ab45e1c06ae0953609acbb52f8946b3e8' 'd49305862fdc4d9ff1b1093b4ed7d8e0cb9971b4' '03b778d1994827ea5cc971dbdfbb457bbb7bfa5d' 'c459262159f39e6e6336797feb975799344b749b' '267be32b0a7b70cc777f8a46f0904c92c0521d89' '548d770c330cd1027549947a6ea899c56b5bc4e4' '111a2c8ab462d77d1519b71b46f13ae1b46920b4' 'f6f914893d478b7ba08e5c375de1ced16deb5e92' '9a30e332c36c52e92e5316b4a012d45284dedeb5' 'c95e925daa434ee1a40a86aec6476ce588e4bd77' 'ac209bde018fd320b79976657a44c23113181af6' 'ece5d881004f041c2e1493436409dbcbea3ad5f8' 'b9ecde0bcf6a99a3ff08496d4ba90a385ebbfd68' 'c8c4694ede7ed42d8d4db0e8927dea9839a3e248' 'e6e8897995a9e6028563ce36c27877e5478c8571'
# test job: [08dc0f5cc26a203e8008c38d9b436c079e7dbb45] https://lava.sirena.org.uk/scheduler/job/1546279
# test job: [571defe0dff3f1e4180bd0db79283d3d5bf74a71] https://lava.sirena.org.uk/scheduler/job/1539758
# test job: [2fca750160f29015ab1109bb478537a4e415f7cd] https://lava.sirena.org.uk/scheduler/job/1540294
# test job: [2bd9648d5a8d329ca734ca2c273a80934867471e] https://lava.sirena.org.uk/scheduler/job/1539587
# test job: [c61e94e5e4e6bc50064119e6a779564d1d2ac0e7] https://lava.sirena.org.uk/scheduler/job/1538605
# test job: [bc163baef57002c08b3afe64cdd2f55f55a765eb] https://lava.sirena.org.uk/scheduler/job/1538760
# test job: [9f711c9321cffe3e03709176873c277fa911c366] https://lava.sirena.org.uk/scheduler/job/1538695
# test job: [baee26a9d6cd3d3c6c3c03c56270aa647a67e4bd] https://lava.sirena.org.uk/scheduler/job/1533838
# test job: [7105fdd54a14bee49371b39374a61b3c967d74cb] https://lava.sirena.org.uk/scheduler/job/1533505
# test job: [913bf8d50cbd144c87e9660b591781179182ff59] https://lava.sirena.org.uk/scheduler/job/1531279
# test job: [34d340d48e595f8dfd4e72fe4100d2579dbe4a1a] https://lava.sirena.org.uk/scheduler/job/1530316
# test job: [0c0ef1d90967717b91cded41b00dbae05d8e521c] https://lava.sirena.org.uk/scheduler/job/1530377
# test job: [3fcd3d2fe44dc9dfca20b6aed117f314a50ba0ff] https://lava.sirena.org.uk/scheduler/job/1528995
# test job: [244bc18e5f1875401a4af87d2eae3f9376d9d720] https://lava.sirena.org.uk/scheduler/job/1528294
# test job: [7e1c28fbf235791cb5046fafdac5bc16fe8e788d] https://lava.sirena.org.uk/scheduler/job/1525609
# test job: [c4f2c05ab02952c9a56067aeb700ded95b183570] https://lava.sirena.org.uk/scheduler/job/1526240
# test job: [b9ab3b61824190b1c6b2c59e7ba4de591f24eb92] https://lava.sirena.org.uk/scheduler/job/1526368
# test job: [427ceac823e58813b510e585011488f603f0d891] https://lava.sirena.org.uk/scheduler/job/1525674
# test job: [29ddce17e909779633f856ad1c2f111fbf71c0df] https://lava.sirena.org.uk/scheduler/job/1523960
# test job: [ac4c064f67d3cdf9118b9b09c1e3b28b6c10a7ea] https://lava.sirena.org.uk/scheduler/job/1515936
# test job: [08bf1663c21a3e815eda28fa242d84c945ca3b94] https://lava.sirena.org.uk/scheduler/job/1517680
# test job: [ce57bc9771411d6d27f2ca7b40396cbd7d684ba9] https://lava.sirena.org.uk/scheduler/job/1515829
# test job: [2555691165a0285a4617230fed859f20dcc51608] https://lava.sirena.org.uk/scheduler/job/1515774
# test job: [0f60ecffbfe35e12eb56c99640ba2360244b5bb3] https://lava.sirena.org.uk/scheduler/job/1511599
# test job: [9a944494c299fabf3cc781798eb7c02a0bece364] https://lava.sirena.org.uk/scheduler/job/1507913
# test job: [3e36c822506d924894ff7de549b9377d3114c2d7] https://lava.sirena.org.uk/scheduler/job/1506345
# test job: [68fbc70ece40139380380dce74059afa592846b3] https://lava.sirena.org.uk/scheduler/job/1504169
# test job: [8f9cf02c8852837923f1cdacfcc92e138513325c] https://lava.sirena.org.uk/scheduler/job/1502871
# test job: [17cc308b183308bf5ada36e164284fff7eb064ba] https://lava.sirena.org.uk/scheduler/job/1501545
# test job: [59566923d955b69bfb1e1163f07dff437dde8c9c] https://lava.sirena.org.uk/scheduler/job/1499651
# test job: [024f39fff6d222cedde361f7fe34d9ba4e6afb92] https://lava.sirena.org.uk/scheduler/job/1499702
# test job: [19cbc930c209d59a2c9828de4c7b767e9f14667e] https://lava.sirena.org.uk/scheduler/job/1497288
# test job: [c4ca928a6db1593802cd945f075a7e21dd0430c1] https://lava.sirena.org.uk/scheduler/job/1496256
# test job: [a48352921f0b15b1f7eff83f5b5613d6ae2350d3] https://lava.sirena.org.uk/scheduler/job/1497361
# test job: [3421d46440ebe0865bec71dbd2330b4e17a425ab] https://lava.sirena.org.uk/scheduler/job/1493081
# test job: [a1d203d390e04798ccc1c3c06019cd4411885d6d] https://lava.sirena.org.uk/scheduler/job/1491513
# test job: [bb48117b79ebc39485f7306d09dc602981fe540f] https://lava.sirena.org.uk/scheduler/job/1489350
# test job: [ace9b3daf2b4778358573d3698e34cb1c0fa7e14] https://lava.sirena.org.uk/scheduler/job/1489286
# test job: [2756b7f08ff6ca7c68c8c7dd61c8dc6895c9de34] https://lava.sirena.org.uk/scheduler/job/1489210
# test job: [5fc2c383125c2b4b6037e02ad8796b776b25e6d0] https://lava.sirena.org.uk/scheduler/job/1486896
# test job: [cd4da713f99651e99fbce8ed6b6ec8f686c029a8] https://lava.sirena.org.uk/scheduler/job/1538863
# test job: [7f8924e8785b68c998bc1906e049bf5595865e60] https://lava.sirena.org.uk/scheduler/job/1486908
# test job: [3e1c01d06e1f52f78fe00ef26a9cf80dbb0a3115] https://lava.sirena.org.uk/scheduler/job/1481717
# test job: [d9f38d9824bfb1b046d2e720349d2f45959ab184] https://lava.sirena.org.uk/scheduler/job/1481619
# test job: [dce4bc30f42d313b4dc5832316196411b7f07ad0] https://lava.sirena.org.uk/scheduler/job/1479449
# test job: [47972c1c3315672352f25c68f91dd88543541947] https://lava.sirena.org.uk/scheduler/job/1479571
# test job: [5eb8a0d7733d4cd32a776acf1d1aa1c7c01c8a14] https://lava.sirena.org.uk/scheduler/job/1474490
# test job: [bb8d8ba4715cb8f997d63d90ba935f6073595df5] https://lava.sirena.org.uk/scheduler/job/1472035
# test job: [8a5a5cecb79058b608e5562d8998123a3adb313c] https://lava.sirena.org.uk/scheduler/job/1471848
# test job: [a4eb71ff98c4792f441f108910bd829da7a04092] https://lava.sirena.org.uk/scheduler/job/1468995
# test job: [6cafcc53eb5fffd9b9bdfde700bb9bad21e98ed3] https://lava.sirena.org.uk/scheduler/job/1468942
# test job: [7e10d7242ea8a5947878880b912ffa5806520705] https://lava.sirena.org.uk/scheduler/job/1466042
# test job: [d6fa0ca959db8efd4462d7beef4bdc5568640fd0] https://lava.sirena.org.uk/scheduler/job/1464679
# test job: [6ba68e5aa9d5d15c8877a655db279fcfc0b38b04] https://lava.sirena.org.uk/scheduler/job/1463330
# test job: [1f5cdb6ab45e1c06ae0953609acbb52f8946b3e8] https://lava.sirena.org.uk/scheduler/job/1462967
# test job: [d49305862fdc4d9ff1b1093b4ed7d8e0cb9971b4] https://lava.sirena.org.uk/scheduler/job/1463039
# test job: [03b778d1994827ea5cc971dbdfbb457bbb7bfa5d] https://lava.sirena.org.uk/scheduler/job/1461893
# test job: [c459262159f39e6e6336797feb975799344b749b] https://lava.sirena.org.uk/scheduler/job/1461009
# test job: [267be32b0a7b70cc777f8a46f0904c92c0521d89] https://lava.sirena.org.uk/scheduler/job/1460419
# test job: [548d770c330cd1027549947a6ea899c56b5bc4e4] https://lava.sirena.org.uk/scheduler/job/1460085
# test job: [111a2c8ab462d77d1519b71b46f13ae1b46920b4] https://lava.sirena.org.uk/scheduler/job/1460877
# test job: [f6f914893d478b7ba08e5c375de1ced16deb5e92] https://lava.sirena.org.uk/scheduler/job/1461476
# test job: [9a30e332c36c52e92e5316b4a012d45284dedeb5] https://lava.sirena.org.uk/scheduler/job/1460560
# test job: [c95e925daa434ee1a40a86aec6476ce588e4bd77] https://lava.sirena.org.uk/scheduler/job/1460134
# test job: [ac209bde018fd320b79976657a44c23113181af6] https://lava.sirena.org.uk/scheduler/job/1461916
# test job: [ece5d881004f041c2e1493436409dbcbea3ad5f8] https://lava.sirena.org.uk/scheduler/job/1461683
# test job: [b9ecde0bcf6a99a3ff08496d4ba90a385ebbfd68] https://lava.sirena.org.uk/scheduler/job/1461078
# test job: [c8c4694ede7ed42d8d4db0e8927dea9839a3e248] https://lava.sirena.org.uk/scheduler/job/1461314
# test job: [e6e8897995a9e6028563ce36c27877e5478c8571] https://lava.sirena.org.uk/scheduler/job/1461772
# test job: [58ba80c4740212c29a1cf9b48f588e60a7612209] https://lava.sirena.org.uk/scheduler/job/1548198
# bad: [58ba80c4740212c29a1cf9b48f588e60a7612209] Add linux-next specific files for 20250708
git bisect bad 58ba80c4740212c29a1cf9b48f588e60a7612209
# test job: [ddadac3880bcacd8ae5ef9c8a975ab0f29365279] https://lava.sirena.org.uk/scheduler/job/1548466
# good: [ddadac3880bcacd8ae5ef9c8a975ab0f29365279] Merge branch 'for-next' of git://git.kernel.org/pub/scm/linux/kernel/git/bpf/bpf-next.git
git bisect good ddadac3880bcacd8ae5ef9c8a975ab0f29365279
# test job: [e3b47c2bac46fc506ecef42040afa3a91776f722] https://lava.sirena.org.uk/scheduler/job/1548653
# good: [e3b47c2bac46fc506ecef42040afa3a91776f722] Merge branch 'for-next' of git://git.kernel.org/pub/scm/linux/kernel/git/sre/linux-power-supply.git
git bisect good e3b47c2bac46fc506ecef42040afa3a91776f722
# test job: [eab3a69ec865243607c349518c09fe39ecc0d936] https://lava.sirena.org.uk/scheduler/job/1548704
# bad: [eab3a69ec865243607c349518c09fe39ecc0d936] Merge branch 'for-next' of git://git.kernel.org/pub/scm/linux/kernel/git/fpga/linux-fpga.git
git bisect bad eab3a69ec865243607c349518c09fe39ecc0d936
# test job: [022880fb3d8f45e408bb117e634edbe9fb1528b2] https://lava.sirena.org.uk/scheduler/job/1548782
# bad: [022880fb3d8f45e408bb117e634edbe9fb1528b2] Merge branch 'next' of git://git.kernel.org/pub/scm/linux/kernel/git/kvmarm/kvmarm.git
git bisect bad 022880fb3d8f45e408bb117e634edbe9fb1528b2
# test job: [a00a9a8b710f09cc6d05a0189e3cd5e41ff8f467] https://lava.sirena.org.uk/scheduler/job/1548826
# bad: [a00a9a8b710f09cc6d05a0189e3cd5e41ff8f467] Merge branch into tip/master: 'x86/kconfig'
git bisect bad a00a9a8b710f09cc6d05a0189e3cd5e41ff8f467
# test job: [05f88a899228a24c1beba9291f63895746351a86] https://lava.sirena.org.uk/scheduler/job/1549063
# good: [05f88a899228a24c1beba9291f63895746351a86] Merge branch into tip/master: 'irq/drivers'
git bisect good 05f88a899228a24c1beba9291f63895746351a86
# test job: [7893ea1006fcbb876ddf53ad4ebba4a054add4b2] https://lava.sirena.org.uk/scheduler/job/1549165
# bad: [7893ea1006fcbb876ddf53ad4ebba4a054add4b2] vdso/gettimeofday: Add support for auxiliary clocks
git bisect bad 7893ea1006fcbb876ddf53ad4ebba4a054add4b2
# test job: [60ecc26ec5af567a55f362ad92c0cac8b894541c] https://lava.sirena.org.uk/scheduler/job/1549250
# good: [60ecc26ec5af567a55f362ad92c0cac8b894541c] timekeeping: Provide time setter for auxiliary clocks
git bisect good 60ecc26ec5af567a55f362ad92c0cac8b894541c
# test job: [d878e2960cb638faf3cc9f1409c6a2a3f9283ec1] https://lava.sirena.org.uk/scheduler/job/1549360
# good: [d878e2960cb638faf3cc9f1409c6a2a3f9283ec1] vdso/vsyscall: Introduce a helper to fill clock configurations
git bisect good d878e2960cb638faf3cc9f1409c6a2a3f9283ec1
# test job: [b5df72706b044b30b17f5d623fe040b83e98be36] https://lava.sirena.org.uk/scheduler/job/1549506
# bad: [b5df72706b044b30b17f5d623fe040b83e98be36] vdso/gettimeofday: Introduce vdso_clockid_valid()
git bisect bad b5df72706b044b30b17f5d623fe040b83e98be36
# test job: [9916785ef2ce464edd83fce80eaa11fab7792547] https://lava.sirena.org.uk/scheduler/job/1549963
# good: [9916785ef2ce464edd83fce80eaa11fab7792547] vdso/helpers: Add helpers for seqlocks of single vdso_clock
git bisect good 9916785ef2ce464edd83fce80eaa11fab7792547
# test job: [fcc8e46f768ff508dab0e3cfc2001e96dcb388e2] https://lava.sirena.org.uk/scheduler/job/1550140
# bad: [fcc8e46f768ff508dab0e3cfc2001e96dcb388e2] vdso/gettimeofday: Return bool from clock_gettime() helpers
git bisect bad fcc8e46f768ff508dab0e3cfc2001e96dcb388e2
# test job: [7413d7c640aa1d9620aa467922cfe3b8df51272e] https://lava.sirena.org.uk/scheduler/job/1550370
# good: [7413d7c640aa1d9620aa467922cfe3b8df51272e] vdso/gettimeofday: Return bool from clock_getres() helpers
git bisect good 7413d7c640aa1d9620aa467922cfe3b8df51272e
# first bad commit: [fcc8e46f768ff508dab0e3cfc2001e96dcb388e2] vdso/gettimeofday: Return bool from clock_gettime() helpers

Re: [PATCH 06/14] vdso/gettimeofday: Return bool from clock_gettime() helpers

Posted by Marek Szyprowski 3 months ago

On 01.07.2025 10:58, Thomas Weißschuh wrote:
> The internal helpers are effectively using boolean results,
> while pretending to use error numbers.
>
> Switch the return type to bool for more clarity.
>
> Signed-off-by: Thomas Weißschuh <thomas.weissschuh@linutronix.de>
> ---
>   lib/vdso/gettimeofday.c | 58 +++++++++++++++++++++++++------------------------
>   1 file changed, 30 insertions(+), 28 deletions(-)

This patch landed in today's linux-next as commit fcc8e46f768f 
("vdso/gettimeofday: Return bool from clock_gettime() helpers"). In my 
tests I found that it causes serious problem with hwclock operation on 
some of my ARM 32bit test boards. I observe that calling "hwclock -w -f 
/dev/rtc0" never ends on those boards. Disabling vdso support (by 
removing ARM architected timer) fixes this issue.


> diff --git a/lib/vdso/gettimeofday.c b/lib/vdso/gettimeofday.c
> index 9b77f23566f6a35887d4c9aaefc61a971131b499..c5266532a097c06f33d12e345c695357d75abf42 100644
> --- a/lib/vdso/gettimeofday.c
> +++ b/lib/vdso/gettimeofday.c
> @@ -82,8 +82,8 @@ const struct vdso_time_data *__arch_get_vdso_u_timens_data(const struct vdso_tim
>   #endif /* CONFIG_GENERIC_VDSO_DATA_STORE */
>   
>   static __always_inline
> -int do_hres_timens(const struct vdso_time_data *vdns, const struct vdso_clock *vcns,
> -		   clockid_t clk, struct __kernel_timespec *ts)
> +bool do_hres_timens(const struct vdso_time_data *vdns, const struct vdso_clock *vcns,
> +		    clockid_t clk, struct __kernel_timespec *ts)
>   {
>   	const struct vdso_time_data *vd = __arch_get_vdso_u_timens_data(vdns);
>   	const struct timens_offset *offs = &vcns->offset[clk];
> @@ -103,11 +103,11 @@ int do_hres_timens(const struct vdso_time_data *vdns, const struct vdso_clock *v
>   		seq = vdso_read_begin(vc);
>   
>   		if (unlikely(!vdso_clocksource_ok(vc)))
> -			return -1;
> +			return false;
>   
>   		cycles = __arch_get_hw_counter(vc->clock_mode, vd);
>   		if (unlikely(!vdso_cycles_ok(cycles)))
> -			return -1;
> +			return false;
>   		ns = vdso_calc_ns(vc, cycles, vdso_ts->nsec);
>   		sec = vdso_ts->sec;
>   	} while (unlikely(vdso_read_retry(vc, seq)));
> @@ -123,7 +123,7 @@ int do_hres_timens(const struct vdso_time_data *vdns, const struct vdso_clock *v
>   	ts->tv_sec = sec + __iter_div_u64_rem(ns, NSEC_PER_SEC, &ns);
>   	ts->tv_nsec = ns;
>   
> -	return 0;
> +	return true;
>   }
>   #else
>   static __always_inline
> @@ -133,16 +133,16 @@ const struct vdso_time_data *__arch_get_vdso_u_timens_data(const struct vdso_tim
>   }
>   
>   static __always_inline
> -int do_hres_timens(const struct vdso_time_data *vdns, const struct vdso_clock *vcns,
> -		   clockid_t clk, struct __kernel_timespec *ts)
> +bool do_hres_timens(const struct vdso_time_data *vdns, const struct vdso_clock *vcns,
> +		    clockid_t clk, struct __kernel_timespec *ts)
>   {
> -	return -EINVAL;
> +	return false;
>   }
>   #endif
>   
>   static __always_inline
> -int do_hres(const struct vdso_time_data *vd, const struct vdso_clock *vc,
> -	    clockid_t clk, struct __kernel_timespec *ts)
> +bool do_hres(const struct vdso_time_data *vd, const struct vdso_clock *vc,
> +	     clockid_t clk, struct __kernel_timespec *ts)
>   {
>   	const struct vdso_timestamp *vdso_ts = &vc->basetime[clk];
>   	u64 cycles, sec, ns;
> @@ -150,7 +150,7 @@ int do_hres(const struct vdso_time_data *vd, const struct vdso_clock *vc,
>   
>   	/* Allows to compile the high resolution parts out */
>   	if (!__arch_vdso_hres_capable())
> -		return -1;
> +		return false;
>   
>   	do {
>   		/*
> @@ -173,11 +173,11 @@ int do_hres(const struct vdso_time_data *vd, const struct vdso_clock *vc,
>   		smp_rmb();
>   
>   		if (unlikely(!vdso_clocksource_ok(vc)))
> -			return -1;
> +			return false;
>   
>   		cycles = __arch_get_hw_counter(vc->clock_mode, vd);
>   		if (unlikely(!vdso_cycles_ok(cycles)))
> -			return -1;
> +			return false;
>   		ns = vdso_calc_ns(vc, cycles, vdso_ts->nsec);
>   		sec = vdso_ts->sec;
>   	} while (unlikely(vdso_read_retry(vc, seq)));
> @@ -189,13 +189,13 @@ int do_hres(const struct vdso_time_data *vd, const struct vdso_clock *vc,
>   	ts->tv_sec = sec + __iter_div_u64_rem(ns, NSEC_PER_SEC, &ns);
>   	ts->tv_nsec = ns;
>   
> -	return 0;
> +	return true;
>   }
>   
>   #ifdef CONFIG_TIME_NS
>   static __always_inline
> -int do_coarse_timens(const struct vdso_time_data *vdns, const struct vdso_clock *vcns,
> -		     clockid_t clk, struct __kernel_timespec *ts)
> +bool do_coarse_timens(const struct vdso_time_data *vdns, const struct vdso_clock *vcns,
> +		      clockid_t clk, struct __kernel_timespec *ts)
>   {
>   	const struct vdso_time_data *vd = __arch_get_vdso_u_timens_data(vdns);
>   	const struct timens_offset *offs = &vcns->offset[clk];
> @@ -223,20 +223,20 @@ int do_coarse_timens(const struct vdso_time_data *vdns, const struct vdso_clock
>   	 */
>   	ts->tv_sec = sec + __iter_div_u64_rem(nsec, NSEC_PER_SEC, &nsec);
>   	ts->tv_nsec = nsec;
> -	return 0;
> +	return true;
>   }
>   #else
>   static __always_inline
> -int do_coarse_timens(const struct vdso_time_data *vdns, const struct vdso_clock *vcns,
> -		     clockid_t clk, struct __kernel_timespec *ts)
> +bool do_coarse_timens(const struct vdso_time_data *vdns, const struct vdso_clock *vcns,
> +		      clockid_t clk, struct __kernel_timespec *ts)
>   {
> -	return -1;
> +	return false;
>   }
>   #endif
>   
>   static __always_inline
> -int do_coarse(const struct vdso_time_data *vd, const struct vdso_clock *vc,
> -	      clockid_t clk, struct __kernel_timespec *ts)
> +bool do_coarse(const struct vdso_time_data *vd, const struct vdso_clock *vc,
> +	       clockid_t clk, struct __kernel_timespec *ts)
>   {
>   	const struct vdso_timestamp *vdso_ts = &vc->basetime[clk];
>   	u32 seq;
> @@ -258,10 +258,10 @@ int do_coarse(const struct vdso_time_data *vd, const struct vdso_clock *vc,
>   		ts->tv_nsec = vdso_ts->nsec;
>   	} while (unlikely(vdso_read_retry(vc, seq)));
>   
> -	return 0;
> +	return true;
>   }
>   
> -static __always_inline int
> +static __always_inline bool
>   __cvdso_clock_gettime_common(const struct vdso_time_data *vd, clockid_t clock,
>   			     struct __kernel_timespec *ts)
>   {
> @@ -270,7 +270,7 @@ __cvdso_clock_gettime_common(const struct vdso_time_data *vd, clockid_t clock,
>   
>   	/* Check for negative values or invalid clocks */
>   	if (unlikely((u32) clock >= MAX_CLOCKS))
> -		return -1;
> +		return false;
>   
>   	/*
>   	 * Convert the clockid to a bitmask and use it to check which
> @@ -284,7 +284,7 @@ __cvdso_clock_gettime_common(const struct vdso_time_data *vd, clockid_t clock,
>   	else if (msk & VDSO_RAW)
>   		vc = &vc[CS_RAW];
>   	else
> -		return -1;
> +		return false;
>   
>   	return do_hres(vd, vc, clock, ts);
>   }
> @@ -293,9 +293,11 @@ static __maybe_unused int
>   __cvdso_clock_gettime_data(const struct vdso_time_data *vd, clockid_t clock,
>   			   struct __kernel_timespec *ts)
>   {
> -	int ret = __cvdso_clock_gettime_common(vd, clock, ts);
> +	bool ok;
>   
> -	if (unlikely(ret))
> +	ok = __cvdso_clock_gettime_common(vd, clock, ts);
> +
> +	if (unlikely(!ok))
>   		return clock_gettime_fallback(clock, ts);
>   	return 0;
>   }
>
Best regards
-- 
Marek Szyprowski, PhD
Samsung R&D Institute Poland

Re: [PATCH 06/14] vdso/gettimeofday: Return bool from clock_gettime() helpers

Posted by Marek Szyprowski 3 months ago

On 08.07.2025 17:17, Marek Szyprowski wrote:
> On 01.07.2025 10:58, Thomas Weißschuh wrote:
>> The internal helpers are effectively using boolean results,
>> while pretending to use error numbers.
>>
>> Switch the return type to bool for more clarity.
>>
>> Signed-off-by: Thomas Weißschuh <thomas.weissschuh@linutronix.de>
>> ---
>>   lib/vdso/gettimeofday.c | 58 
>> +++++++++++++++++++++++++------------------------
>>   1 file changed, 30 insertions(+), 28 deletions(-)
>
> This patch landed in today's linux-next as commit fcc8e46f768f 
> ("vdso/gettimeofday: Return bool from clock_gettime() helpers"). In my 
> tests I found that it causes serious problem with hwclock operation on 
> some of my ARM 32bit test boards. I observe that calling "hwclock -w 
> -f /dev/rtc0" never ends on those boards. Disabling vdso support (by 
> removing ARM architected timer) fixes this issue.

I spent some time analyzing the code refactored in this patch and it 
looks that the following change is missing:

diff --git a/lib/vdso/gettimeofday.c b/lib/vdso/gettimeofday.c
index c5266532a097..7e79b02839b0 100644
--- a/lib/vdso/gettimeofday.c
+++ b/lib/vdso/gettimeofday.c
@@ -344,7 +344,7 @@ __cvdso_gettimeofday_data(const struct 
vdso_time_data *vd,
         if (likely(tv != NULL)) {
                 struct __kernel_timespec ts;

-               if (do_hres(vd, &vc[CS_HRES_COARSE], CLOCK_REALTIME, &ts))
+               if (!do_hres(vd, &vc[CS_HRES_COARSE], CLOCK_REALTIME, &ts))
                         return gettimeofday_fallback(tv, tz);

                 tv->tv_sec = ts.tv_sec;


In my tests this fixed the hwclock issue on the mentioned boards.

>
>> diff --git a/lib/vdso/gettimeofday.c b/lib/vdso/gettimeofday.c
>> index 
>> 9b77f23566f6a35887d4c9aaefc61a971131b499..c5266532a097c06f33d12e345c695357d75abf42 
>> 100644
>> --- a/lib/vdso/gettimeofday.c
>> +++ b/lib/vdso/gettimeofday.c
>> @@ -82,8 +82,8 @@ const struct vdso_time_data 
>> *__arch_get_vdso_u_timens_data(const struct vdso_tim
>>   #endif /* CONFIG_GENERIC_VDSO_DATA_STORE */
>>     static __always_inline
>> -int do_hres_timens(const struct vdso_time_data *vdns, const struct 
>> vdso_clock *vcns,
>> -           clockid_t clk, struct __kernel_timespec *ts)
>> +bool do_hres_timens(const struct vdso_time_data *vdns, const struct 
>> vdso_clock *vcns,
>> +            clockid_t clk, struct __kernel_timespec *ts)
>>   {
>>       const struct vdso_time_data *vd = 
>> __arch_get_vdso_u_timens_data(vdns);
>>       const struct timens_offset *offs = &vcns->offset[clk];
>> @@ -103,11 +103,11 @@ int do_hres_timens(const struct vdso_time_data 
>> *vdns, const struct vdso_clock *v
>>           seq = vdso_read_begin(vc);
>>             if (unlikely(!vdso_clocksource_ok(vc)))
>> -            return -1;
>> +            return false;
>>             cycles = __arch_get_hw_counter(vc->clock_mode, vd);
>>           if (unlikely(!vdso_cycles_ok(cycles)))
>> -            return -1;
>> +            return false;
>>           ns = vdso_calc_ns(vc, cycles, vdso_ts->nsec);
>>           sec = vdso_ts->sec;
>>       } while (unlikely(vdso_read_retry(vc, seq)));
>> @@ -123,7 +123,7 @@ int do_hres_timens(const struct vdso_time_data 
>> *vdns, const struct vdso_clock *v
>>       ts->tv_sec = sec + __iter_div_u64_rem(ns, NSEC_PER_SEC, &ns);
>>       ts->tv_nsec = ns;
>>   -    return 0;
>> +    return true;
>>   }
>>   #else
>>   static __always_inline
>> @@ -133,16 +133,16 @@ const struct vdso_time_data 
>> *__arch_get_vdso_u_timens_data(const struct vdso_tim
>>   }
>>     static __always_inline
>> -int do_hres_timens(const struct vdso_time_data *vdns, const struct 
>> vdso_clock *vcns,
>> -           clockid_t clk, struct __kernel_timespec *ts)
>> +bool do_hres_timens(const struct vdso_time_data *vdns, const struct 
>> vdso_clock *vcns,
>> +            clockid_t clk, struct __kernel_timespec *ts)
>>   {
>> -    return -EINVAL;
>> +    return false;
>>   }
>>   #endif
>>     static __always_inline
>> -int do_hres(const struct vdso_time_data *vd, const struct vdso_clock 
>> *vc,
>> -        clockid_t clk, struct __kernel_timespec *ts)
>> +bool do_hres(const struct vdso_time_data *vd, const struct 
>> vdso_clock *vc,
>> +         clockid_t clk, struct __kernel_timespec *ts)
>>   {
>>       const struct vdso_timestamp *vdso_ts = &vc->basetime[clk];
>>       u64 cycles, sec, ns;
>> @@ -150,7 +150,7 @@ int do_hres(const struct vdso_time_data *vd, 
>> const struct vdso_clock *vc,
>>         /* Allows to compile the high resolution parts out */
>>       if (!__arch_vdso_hres_capable())
>> -        return -1;
>> +        return false;
>>         do {
>>           /*
>> @@ -173,11 +173,11 @@ int do_hres(const struct vdso_time_data *vd, 
>> const struct vdso_clock *vc,
>>           smp_rmb();
>>             if (unlikely(!vdso_clocksource_ok(vc)))
>> -            return -1;
>> +            return false;
>>             cycles = __arch_get_hw_counter(vc->clock_mode, vd);
>>           if (unlikely(!vdso_cycles_ok(cycles)))
>> -            return -1;
>> +            return false;
>>           ns = vdso_calc_ns(vc, cycles, vdso_ts->nsec);
>>           sec = vdso_ts->sec;
>>       } while (unlikely(vdso_read_retry(vc, seq)));
>> @@ -189,13 +189,13 @@ int do_hres(const struct vdso_time_data *vd, 
>> const struct vdso_clock *vc,
>>       ts->tv_sec = sec + __iter_div_u64_rem(ns, NSEC_PER_SEC, &ns);
>>       ts->tv_nsec = ns;
>>   -    return 0;
>> +    return true;
>>   }
>>     #ifdef CONFIG_TIME_NS
>>   static __always_inline
>> -int do_coarse_timens(const struct vdso_time_data *vdns, const struct 
>> vdso_clock *vcns,
>> -             clockid_t clk, struct __kernel_timespec *ts)
>> +bool do_coarse_timens(const struct vdso_time_data *vdns, const 
>> struct vdso_clock *vcns,
>> +              clockid_t clk, struct __kernel_timespec *ts)
>>   {
>>       const struct vdso_time_data *vd = 
>> __arch_get_vdso_u_timens_data(vdns);
>>       const struct timens_offset *offs = &vcns->offset[clk];
>> @@ -223,20 +223,20 @@ int do_coarse_timens(const struct 
>> vdso_time_data *vdns, const struct vdso_clock
>>        */
>>       ts->tv_sec = sec + __iter_div_u64_rem(nsec, NSEC_PER_SEC, &nsec);
>>       ts->tv_nsec = nsec;
>> -    return 0;
>> +    return true;
>>   }
>>   #else
>>   static __always_inline
>> -int do_coarse_timens(const struct vdso_time_data *vdns, const struct 
>> vdso_clock *vcns,
>> -             clockid_t clk, struct __kernel_timespec *ts)
>> +bool do_coarse_timens(const struct vdso_time_data *vdns, const 
>> struct vdso_clock *vcns,
>> +              clockid_t clk, struct __kernel_timespec *ts)
>>   {
>> -    return -1;
>> +    return false;
>>   }
>>   #endif
>>     static __always_inline
>> -int do_coarse(const struct vdso_time_data *vd, const struct 
>> vdso_clock *vc,
>> -          clockid_t clk, struct __kernel_timespec *ts)
>> +bool do_coarse(const struct vdso_time_data *vd, const struct 
>> vdso_clock *vc,
>> +           clockid_t clk, struct __kernel_timespec *ts)
>>   {
>>       const struct vdso_timestamp *vdso_ts = &vc->basetime[clk];
>>       u32 seq;
>> @@ -258,10 +258,10 @@ int do_coarse(const struct vdso_time_data *vd, 
>> const struct vdso_clock *vc,
>>           ts->tv_nsec = vdso_ts->nsec;
>>       } while (unlikely(vdso_read_retry(vc, seq)));
>>   -    return 0;
>> +    return true;
>>   }
>>   -static __always_inline int
>> +static __always_inline bool
>>   __cvdso_clock_gettime_common(const struct vdso_time_data *vd, 
>> clockid_t clock,
>>                    struct __kernel_timespec *ts)
>>   {
>> @@ -270,7 +270,7 @@ __cvdso_clock_gettime_common(const struct 
>> vdso_time_data *vd, clockid_t clock,
>>         /* Check for negative values or invalid clocks */
>>       if (unlikely((u32) clock >= MAX_CLOCKS))
>> -        return -1;
>> +        return false;
>>         /*
>>        * Convert the clockid to a bitmask and use it to check which
>> @@ -284,7 +284,7 @@ __cvdso_clock_gettime_common(const struct 
>> vdso_time_data *vd, clockid_t clock,
>>       else if (msk & VDSO_RAW)
>>           vc = &vc[CS_RAW];
>>       else
>> -        return -1;
>> +        return false;
>>         return do_hres(vd, vc, clock, ts);
>>   }
>> @@ -293,9 +293,11 @@ static __maybe_unused int
>>   __cvdso_clock_gettime_data(const struct vdso_time_data *vd, 
>> clockid_t clock,
>>                  struct __kernel_timespec *ts)
>>   {
>> -    int ret = __cvdso_clock_gettime_common(vd, clock, ts);
>> +    bool ok;
>>   -    if (unlikely(ret))
>> +    ok = __cvdso_clock_gettime_common(vd, clock, ts);
>> +
>> +    if (unlikely(!ok))
>>           return clock_gettime_fallback(clock, ts);
>>       return 0;
>>   }
>>
> Best regards

Best regards
-- 
Marek Szyprowski, PhD
Samsung R&D Institute Poland

Re: [PATCH 06/14] vdso/gettimeofday: Return bool from clock_gettime() helpers

Posted by Thomas Weißschuh 3 months ago

Hi Marek,

On Tue, Jul 08, 2025 at 05:49:18PM +0200, Marek Szyprowski wrote:
> On 08.07.2025 17:17, Marek Szyprowski wrote:
> > On 01.07.2025 10:58, Thomas Weißschuh wrote:
> >> The internal helpers are effectively using boolean results,
> >> while pretending to use error numbers.
> >>
> >> Switch the return type to bool for more clarity.
> >>
> >> Signed-off-by: Thomas Weißschuh <thomas.weissschuh@linutronix.de>
> >> ---
> >>   lib/vdso/gettimeofday.c | 58 
> >> +++++++++++++++++++++++++------------------------
> >>   1 file changed, 30 insertions(+), 28 deletions(-)
> >
> > This patch landed in today's linux-next as commit fcc8e46f768f 
> > ("vdso/gettimeofday: Return bool from clock_gettime() helpers"). In my 
> > tests I found that it causes serious problem with hwclock operation on 
> > some of my ARM 32bit test boards. I observe that calling "hwclock -w 
> > -f /dev/rtc0" never ends on those boards. Disabling vdso support (by 
> > removing ARM architected timer) fixes this issue.
> 
> I spent some time analyzing the code refactored in this patch and it 
> looks that the following change is missing:

Thanks for the report and investigation!

> diff --git a/lib/vdso/gettimeofday.c b/lib/vdso/gettimeofday.c
> index c5266532a097..7e79b02839b0 100644
> --- a/lib/vdso/gettimeofday.c
> +++ b/lib/vdso/gettimeofday.c
> @@ -344,7 +344,7 @@ __cvdso_gettimeofday_data(const struct 
> vdso_time_data *vd,
>          if (likely(tv != NULL)) {
>                  struct __kernel_timespec ts;
> 
> -               if (do_hres(vd, &vc[CS_HRES_COARSE], CLOCK_REALTIME, &ts))
> +               if (!do_hres(vd, &vc[CS_HRES_COARSE], CLOCK_REALTIME, &ts))
>                          return gettimeofday_fallback(tv, tz);
> 
>                  tv->tv_sec = ts.tv_sec;
> 
> 
> In my tests this fixed the hwclock issue on the mentioned boards.

This fix looks correct to me.


tglx:

Are you going to fold the fix into the commit or do you want a proper patch?


Marek:

If a new patch is required, do you want to send it? You found and fixed the
issue after all. If not, I'll take care of it.


Thomas

Re: [PATCH 06/14] vdso/gettimeofday: Return bool from clock_gettime() helpers

Posted by Marek Szyprowski 3 months ago

On 09.07.2025 09:34, Thomas Weißschuh wrote:
> On Tue, Jul 08, 2025 at 05:49:18PM +0200, Marek Szyprowski wrote:
>> On 08.07.2025 17:17, Marek Szyprowski wrote:
>>> On 01.07.2025 10:58, Thomas Weißschuh wrote:
>>>> The internal helpers are effectively using boolean results,
>>>> while pretending to use error numbers.
>>>>
>>>> Switch the return type to bool for more clarity.
>>>>
>>>> Signed-off-by: Thomas Weißschuh <thomas.weissschuh@linutronix.de>
>>>> ---
>>>>    lib/vdso/gettimeofday.c | 58
>>>> +++++++++++++++++++++++++------------------------
>>>>    1 file changed, 30 insertions(+), 28 deletions(-)
>>> This patch landed in today's linux-next as commit fcc8e46f768f
>>> ("vdso/gettimeofday: Return bool from clock_gettime() helpers"). In my
>>> tests I found that it causes serious problem with hwclock operation on
>>> some of my ARM 32bit test boards. I observe that calling "hwclock -w
>>> -f /dev/rtc0" never ends on those boards. Disabling vdso support (by
>>> removing ARM architected timer) fixes this issue.
>> I spent some time analyzing the code refactored in this patch and it
>> looks that the following change is missing:
> Thanks for the report and investigation!
>
>> diff --git a/lib/vdso/gettimeofday.c b/lib/vdso/gettimeofday.c
>> index c5266532a097..7e79b02839b0 100644
>> --- a/lib/vdso/gettimeofday.c
>> +++ b/lib/vdso/gettimeofday.c
>> @@ -344,7 +344,7 @@ __cvdso_gettimeofday_data(const struct
>> vdso_time_data *vd,
>>           if (likely(tv != NULL)) {
>>                   struct __kernel_timespec ts;
>>
>> -               if (do_hres(vd, &vc[CS_HRES_COARSE], CLOCK_REALTIME, &ts))
>> +               if (!do_hres(vd, &vc[CS_HRES_COARSE], CLOCK_REALTIME, &ts))
>>                           return gettimeofday_fallback(tv, tz);
>>
>>                   tv->tv_sec = ts.tv_sec;
>>
>>
>> In my tests this fixed the hwclock issue on the mentioned boards.
> This fix looks correct to me.
>
>
> tglx:
>
> Are you going to fold the fix into the commit or do you want a proper patch?
>
>
> Marek:
>
> If a new patch is required, do you want to send it? You found and fixed the
> issue after all. If not, I'll take care of it.

If it is possible to fold it into original patch then go ahead, it would 
make less noise imho. If you need a formal patch, I can send it in a few 
minutes.

Best regards
-- 
Marek Szyprowski, PhD
Samsung R&D Institute Poland

Re: [PATCH 06/14] vdso/gettimeofday: Return bool from clock_gettime() helpers

Posted by Mark Brown 2 months, 3 weeks ago

On Wed, Jul 09, 2025 at 10:04:21AM +0200, Marek Szyprowski wrote:
> On 09.07.2025 09:34, Thomas Weißschuh wrote:
> > On Tue, Jul 08, 2025 at 05:49:18PM +0200, Marek Szyprowski wrote:

> > This fix looks correct to me.

> > tglx:

> > Are you going to fold the fix into the commit or do you want a proper patch?

> > Marek:

> > If a new patch is required, do you want to send it? You found and fixed the
> > issue after all. If not, I'll take care of it.

> If it is possible to fold it into original patch then go ahead, it would 
> make less noise imho. If you need a formal patch, I can send it in a few 
> minutes.

This issue has been present in -next for a week and is causing a bunch
of disruption to tests that end up relying on the vDSO - do we have any
news on getting a fix merged?  Perhaps it makes sense for Marek to just
send his patch so that it's there if needed?

Re: [PATCH 06/14] vdso/gettimeofday: Return bool from clock_gettime() helpers

Posted by Thomas Weißschuh 2 months, 3 weeks ago

On Wed, Jul 16, 2025 at 01:25:06PM +0100, Mark Brown wrote:
> On Wed, Jul 09, 2025 at 10:04:21AM +0200, Marek Szyprowski wrote:
> > On 09.07.2025 09:34, Thomas Weißschuh wrote:
> > > On Tue, Jul 08, 2025 at 05:49:18PM +0200, Marek Szyprowski wrote:
> 
> > > This fix looks correct to me.
> 
> > > tglx:
> 
> > > Are you going to fold the fix into the commit or do you want a proper patch?
> 
> > > Marek:
> 
> > > If a new patch is required, do you want to send it? You found and fixed the
> > > issue after all. If not, I'll take care of it.
> 
> > If it is possible to fold it into original patch then go ahead, it would 
> > make less noise imho. If you need a formal patch, I can send it in a few 
> > minutes.
> 
> This issue has been present in -next for a week and is causing a bunch
> of disruption to tests that end up relying on the vDSO - do we have any
> news on getting a fix merged?  Perhaps it makes sense for Marek to just
> send his patch so that it's there if needed?

That fix has been in -next since next-20250710.
If you still have issues, I'll take a look.


Thomas

Re: [PATCH 06/14] vdso/gettimeofday: Return bool from clock_gettime() helpers

Posted by Mark Brown 2 months, 3 weeks ago

On Wed, Jul 16, 2025 at 02:34:52PM +0200, Thomas Weißschuh wrote:
> On Wed, Jul 16, 2025 at 01:25:06PM +0100, Mark Brown wrote:

> > This issue has been present in -next for a week and is causing a bunch
> > of disruption to tests that end up relying on the vDSO - do we have any
> > news on getting a fix merged?  Perhaps it makes sense for Marek to just
> > send his patch so that it's there if needed?

> That fix has been in -next since next-20250710.
> If you still have issues, I'll take a look.

Ah, sorry - I'd not seen followup mails in the thread and was still
seeing issues that appeared at the same time that had previously
bisected here.  One is:

| INFO: Generating a skipfile based on /lava-4170058/1/tests/6_kselftest-dev-errlogs/automated/linux/kselftest/skipfile-lkft.yaml
| fatal error: nanotime returning zero
| goroutine 1 [running, locked to thread]:
| runtime.throw(0x132d83, 0x17)
| 	/usr/lib/golang/src/runtime/panic.go:774 +0x5c fp=0x42c7a4 sp=0x42c790 pc=0x3b740
| runtime.main()
| 	/usr/lib/golang/src/runtime/proc.go:152 +0x350 fp=0x42c7e4 sp=0x42c7a4 pc=0x3d308
|A runtime.goexit()
| 	/usr/lib/golang/src/runtime/asm_arm.s:868 +0x4 fp=0x42c7e4 sp=0x42c7e4 pc=0x645dc
| ERROR: skipgen failed to generate a skipfile: 2

I'll just kick of a clean bisect for that and see what it comes up with.

Full log:

  https://validation.linaro.org/scheduler/job/4170058#L2215

Re: [PATCH 06/14] vdso/gettimeofday: Return bool from clock_gettime() helpers

Posted by Thomas Weißschuh 2 months, 3 weeks ago

On Wed, Jul 16, 2025 at 01:50:22PM +0100, Mark Brown wrote:
> On Wed, Jul 16, 2025 at 02:34:52PM +0200, Thomas Weißschuh wrote:
> > On Wed, Jul 16, 2025 at 01:25:06PM +0100, Mark Brown wrote:
> 
> > > This issue has been present in -next for a week and is causing a bunch
> > > of disruption to tests that end up relying on the vDSO - do we have any
> > > news on getting a fix merged?  Perhaps it makes sense for Marek to just
> > > send his patch so that it's there if needed?
> 
> > That fix has been in -next since next-20250710.
> > If you still have issues, I'll take a look.
> 
> Ah, sorry - I'd not seen followup mails in the thread and was still
> seeing issues that appeared at the same time that had previously
> bisected here.  One is:
> 
> | INFO: Generating a skipfile based on /lava-4170058/1/tests/6_kselftest-dev-errlogs/automated/linux/kselftest/skipfile-lkft.yaml
> | fatal error: nanotime returning zero
> | goroutine 1 [running, locked to thread]:
> | runtime.throw(0x132d83, 0x17)
> | 	/usr/lib/golang/src/runtime/panic.go:774 +0x5c fp=0x42c7a4 sp=0x42c790 pc=0x3b740
> | runtime.main()
> | 	/usr/lib/golang/src/runtime/proc.go:152 +0x350 fp=0x42c7e4 sp=0x42c7a4 pc=0x3d308
> |A runtime.goexit()
> | 	/usr/lib/golang/src/runtime/asm_arm.s:868 +0x4 fp=0x42c7e4 sp=0x42c7e4 pc=0x645dc
> | ERROR: skipgen failed to generate a skipfile: 2
> 
> I'll just kick of a clean bisect for that and see what it comes up with.

Can you try the following?
I missed this despite the double-checking after the last reported issue.

diff --git a/lib/vdso/gettimeofday.c b/lib/vdso/gettimeofday.c
index 97aa9059a5c9..5e0106130e07 100644
--- a/lib/vdso/gettimeofday.c
+++ b/lib/vdso/gettimeofday.c
@@ -365,14 +365,14 @@ __cvdso_clock_gettime32_data(const struct vdso_time_data *vd, clockid_t clock,
                             struct old_timespec32 *res)
 {
        struct __kernel_timespec ts;
-       int ret;
+       bool ok;
 
-       ret = __cvdso_clock_gettime_common(vd, clock, &ts);
+       ok = __cvdso_clock_gettime_common(vd, clock, &ts);
 
-       if (unlikely(ret))
+       if (unlikely(!ok))
                return clock_gettime32_fallback(clock, res);
 
-       /* For ret == 0 */
+       /* For ok == true */
        res->tv_sec = ts.tv_sec;
        res->tv_nsec = ts.tv_nsec;


Sorry for all the breakage.


Thomas

Re: [PATCH 06/14] vdso/gettimeofday: Return bool from clock_gettime() helpers

Posted by Mark Brown 2 months, 3 weeks ago

On Wed, Jul 16, 2025 at 03:23:24PM +0200, Thomas Weißschuh wrote:

> Can you try the following?
> I missed this despite the double-checking after the last reported issue.

I needed to fix that up a bit, it was missing an update of the final ret
in the function and didn't apply directly to -next for some reason so I
had to manually apply but it seems to do the trick, thanks!

Tested-by: Mark Brown <broonie@kernel.org>

with this against -next:

diff --git a/lib/vdso/gettimeofday.c b/lib/vdso/gettimeofday.c
index 97aa9059a5c97..487e3458e536e 100644
--- a/lib/vdso/gettimeofday.c
+++ b/lib/vdso/gettimeofday.c
@@ -365,18 +365,18 @@ __cvdso_clock_gettime32_data(const struct vdso_time_data *vd, clockid_t clock,
 			     struct old_timespec32 *res)
 {
 	struct __kernel_timespec ts;
-	int ret;
+	bool ok;
 
-	ret = __cvdso_clock_gettime_common(vd, clock, &ts);
+	ok = __cvdso_clock_gettime_common(vd, clock, &ts);
 
-	if (unlikely(ret))
+	if (unlikely(!ok))
 		return clock_gettime32_fallback(clock, res);
 
-	/* For ret == 0 */
+	/* For ok == true */
 	res->tv_sec = ts.tv_sec;
 	res->tv_nsec = ts.tv_nsec;
 
-	return ret;
+	return 0;
 }
 
 static __maybe_unused int

Re: [PATCH 06/14] vdso/gettimeofday: Return bool from clock_gettime() helpers

Posted by David Laight 2 months, 2 weeks ago

On Wed, 16 Jul 2025 15:35:09 +0100
Mark Brown <broonie@kernel.org> wrote:

> On Wed, Jul 16, 2025 at 03:23:24PM +0200, Thomas Weißschuh wrote:
> 
> > Can you try the following?
> > I missed this despite the double-checking after the last reported issue.  
> 
> I needed to fix that up a bit, it was missing an update of the final ret
> in the function and didn't apply directly to -next for some reason so I
> had to manually apply but it seems to do the trick, thanks!
> 
> Tested-by: Mark Brown <broonie@kernel.org>
> 
> with this against -next:
> 
> diff --git a/lib/vdso/gettimeofday.c b/lib/vdso/gettimeofday.c
> index 97aa9059a5c97..487e3458e536e 100644
> --- a/lib/vdso/gettimeofday.c
> +++ b/lib/vdso/gettimeofday.c
> @@ -365,18 +365,18 @@ __cvdso_clock_gettime32_data(const struct vdso_time_data *vd, clockid_t clock,
>  			     struct old_timespec32 *res)
>  {
>  	struct __kernel_timespec ts;
> -	int ret;
> +	bool ok;
>  
> -	ret = __cvdso_clock_gettime_common(vd, clock, &ts);
> +	ok = __cvdso_clock_gettime_common(vd, clock, &ts);
>  
> -	if (unlikely(ret))
> +	if (unlikely(!ok))

Do you even need 'ok' at all, just:
	if (unlikely(!__cvdso_clock_gettime_common(vd, clock, &ts)))

>  		return clock_gettime32_fallback(clock, res);
>  
> -	/* For ret == 0 */
> +	/* For ok == true */
>  	res->tv_sec = ts.tv_sec;
>  	res->tv_nsec = ts.tv_nsec;
>  
> -	return ret;
> +	return 0;
>  }
>  
>  static __maybe_unused int

	David

Re: [PATCH 06/14] vdso/gettimeofday: Return bool from clock_gettime() helpers

Posted by Thomas Weißschuh 2 months, 3 weeks ago

Hi Mark,

On Wed, Jul 16, 2025 at 03:35:09PM +0100, Mark Brown wrote:
> On Wed, Jul 16, 2025 at 03:23:24PM +0200, Thomas Weißschuh wrote:
> 
> > Can you try the following?
> > I missed this despite the double-checking after the last reported issue.
> 
> I needed to fix that up a bit, it was missing an update of the final ret
> in the function and didn't apply directly to -next for some reason so I
> had to manually apply but it seems to do the trick, thanks!
> 
> Tested-by: Mark Brown <broonie@kernel.org>

The fix for this is now in tip/timers/ptp and should be in -next tomorrow.
Thanks again for the report and sorry for the breakage.

(...)

Thomas

CONFIG_DEVMEM=y breaks gettimeofday in next-20250708

Posted by Bert Karwatzki 3 months ago

Recently I found that my RAM has an error (memtest86+ reproducibly reports
a failing address) (this error may lead to random crashes every few days). 
To further investigate the issue I tried using memtester which needs access 
to /dev/mem and so I recompiled linux next-20250708 with CONFIG_DEMEM=y 
and found a strange and unusual side effect:

a) the time displayed by xfce is stuck at 1.1.1970 01:00 (UTC + 1)
b) most certificates in firefox-esr fail to work due to the date being 1.1.1970 
(this includes www.google.de, www.duckduckgo.com, wikipedia and youtube and many more)
c) some certificates in firefox-esr still work (kernel.org, xkcd.com, www.spiegel.de)
d) the shell built-in time (and also /usr/bin/time) fail to work, e.g.
$ time sleep 5
real	0m0,000s
user	0m0,000s
sys	0m0,002s
(even though it actually take 5 seconds for this)
e) date still works correctly, e.g.
$ date
Mi 9. Jul 11:51:20 CEST 2025
f) This example program 

#include <stdlib.h>
#include <stdio.h>
#include <sys/time.h>

int main()
{
	int ret;
	struct timeval tv;
	struct timezone tz;

	ret = gettimeofday(&tv, &tz);
	printf("gettimeofday returns ret = %d, tv.tv_sec = %lu tv.tv_usec = %lu\n", ret, tv.tv_sec, tv.tv_usec);

	return 0;
}

gives the following output on affected versions:

$
gettimeofday returns ret = 0, tv.tv_sec = 0 tv.tv_usec = 0


These errors do not occur when using v6.16-rc5 with CONFIG_DEVMEM=y, and are 100%
reproducible so are not related to the RAM error. 

I bisected the issue in between
v6.16-rc5 and next-20250708 and found commit fcc8e46f768f ("vdso/gettimeofday:
Return bool from clock_gettime() helpers") as the first bad commit.

Bert Karwatzki

Re: CONFIG_DEVMEM=y breaks gettimeofday in next-20250708

Posted by Thomas Weißschuh 3 months ago

Hi Bert,

On Wed, Jul 09, 2025 at 02:42:15PM +0200, Bert Karwatzki wrote:
> Recently I found that my RAM has an error (memtest86+ reproducibly reports
> a failing address) (this error may lead to random crashes every few days). 
> To further investigate the issue I tried using memtester which needs access 
> to /dev/mem and so I recompiled linux next-20250708 with CONFIG_DEMEM=y 
> and found a strange and unusual side effect:
> 
> a) the time displayed by xfce is stuck at 1.1.1970 01:00 (UTC + 1)
> b) most certificates in firefox-esr fail to work due to the date being 1.1.1970 
> (this includes www.google.de, www.duckduckgo.com, wikipedia and youtube and many more)
> c) some certificates in firefox-esr still work (kernel.org, xkcd.com, www.spiegel.de)
> d) the shell built-in time (and also /usr/bin/time) fail to work, e.g.
> $ time sleep 5
> real	0m0,000s
> user	0m0,000s
> sys	0m0,002s
> (even though it actually take 5 seconds for this)
> e) date still works correctly, e.g.
> $ date
> Mi 9. Jul 11:51:20 CEST 2025
> f) This example program 
> 
> #include <stdlib.h>
> #include <stdio.h>
> #include <sys/time.h>
> 
> int main()
> {
> 	int ret;
> 	struct timeval tv;
> 	struct timezone tz;
> 
> 	ret = gettimeofday(&tv, &tz);
> 	printf("gettimeofday returns ret = %d, tv.tv_sec = %lu tv.tv_usec = %lu\n", ret, tv.tv_sec, tv.tv_usec);
> 
> 	return 0;
> }
> 
> gives the following output on affected versions:
> 
> $
> gettimeofday returns ret = 0, tv.tv_sec = 0 tv.tv_usec = 0

Thanks for the report. Can you try the fix posted by Marek [0]?
It looks like the same issue where the vDSO fastpath is unavailable on your
hardware but I broke the check for it.
If it is the same issue it still leaves the question why the fastpath is
broken by CONFIG_DEVMEM. Can you describe your setup a bit?
Are there any entries in dmesg about the tsc or the clocksource subsystem?

> These errors do not occur when using v6.16-rc5 with CONFIG_DEVMEM=y, and are 100%
> reproducible so are not related to the RAM error. 
> 
> I bisected the issue in between
> v6.16-rc5 and next-20250708 and found commit fcc8e46f768f ("vdso/gettimeofday:
> Return bool from clock_gettime() helpers") as the first bad commit.

[0] https://lore.kernel.org/lkml/e8c6b9a7-eaa6-4947-98e1-9d6fecc958d4@samsung.com/

Re: CONFIG_DEVMEM=y breaks gettimeofday in next-20250708

Posted by Bert Karwatzki 3 months ago

Am Mittwoch, dem 09.07.2025 um 15:17 +0200 schrieb Thomas Weißschuh:
> Hi Bert,
> 
> On Wed, Jul 09, 2025 at 02:42:15PM +0200, Bert Karwatzki wrote:
> > Recently I found that my RAM has an error (memtest86+ reproducibly reports
> > a failing address) (this error may lead to random crashes every few days). 
> > To further investigate the issue I tried using memtester which needs access 
> > to /dev/mem and so I recompiled linux next-20250708 with CONFIG_DEMEM=y 
> > and found a strange and unusual side effect:
> > 
> > a) the time displayed by xfce is stuck at 1.1.1970 01:00 (UTC + 1)
> > b) most certificates in firefox-esr fail to work due to the date being 1.1.1970 
> > (this includes www.google.de, www.duckduckgo.com, wikipedia and youtube and many more)
> > c) some certificates in firefox-esr still work (kernel.org, xkcd.com, www.spiegel.de)
> > d) the shell built-in time (and also /usr/bin/time) fail to work, e.g.
> > $ time sleep 5
> > real	0m0,000s
> > user	0m0,000s
> > sys	0m0,002s
> > (even though it actually take 5 seconds for this)
> > e) date still works correctly, e.g.
> > $ date
> > Mi 9. Jul 11:51:20 CEST 2025
> > f) This example program 
> > 
> > #include <stdlib.h>
> > #include <stdio.h>
> > #include <sys/time.h>
> > 
> > int main()
> > {
> > 	int ret;
> > 	struct timeval tv;
> > 	struct timezone tz;
> > 
> > 	ret = gettimeofday(&tv, &tz);
> > 	printf("gettimeofday returns ret = %d, tv.tv_sec = %lu tv.tv_usec = %lu\n", ret, tv.tv_sec, tv.tv_usec);
> > 
> > 	return 0;
> > }
> > 
> > gives the following output on affected versions:
> > 
> > $
> > gettimeofday returns ret = 0, tv.tv_sec = 0 tv.tv_usec = 0
> 
> Thanks for the report. Can you try the fix posted by Marek [0]?
> It looks like the same issue where the vDSO fastpath is unavailable on your
> hardware but I broke the check for it.

Yes, this fixes the issue for me.

> If it is the same issue it still leaves the question why the fastpath is
> broken by CONFIG_DEVMEM. Can you describe your setup a bit?
> Are there any entries in dmesg about the tsc or the clocksource subsystem?

I use a MSI Alpha 15 B5EEK/MS-158L amd64 laptop (AMD Ryzen 7 5800H) running debian sid. When booting 
without "tsc=unstable" I get this message in dmesg (this is from next-20250708 with CONFIG_DEVMEM=y):

[    C7] clocksource: timekeeping watchdog on CPU7: Marking clocksource 'tsc' as unstable because the skew is too large:
[    C7] clocksource:                       'hpet' wd_nsec: 495997815 wd_now: 36f24f3 wd_last: 302c799 mask: ffffffff
[    C7] clocksource:                       'tsc' cs_nsec: 497721239 cs_now: 1ea2752100 cs_last: 1e43b3e700 mask: ffffffffffffffff
[    C7] clocksource:                       Clocksource 'tsc' skewed 1723424 ns (1 ms) over watchdog 'hpet' interval of 495997815 ns (495 ms)
[    C7] clocksource:                       'hpet' (not 'tsc') is current clocksource.
[    C7] tsc: Marking TSC unstable due to clocksource watchdog
[  T233] TSC found unstable after boot, most likely due to broken BIOS. Use 'tsc=unstable'.
[  T233] sched_clock: Marking unstable (4040049255, 1223348)<-(4058551237, -15591933)

I usually boot with "tsc=unstable" but the issue occurs there, too. I can't say yet why
this only occurs with CONFIG_DEVMEM=y.

Bert Karwatzki

[tip: timers/ptp] vdso/gettimeofday: Return bool from clock_gettime() helpers

Posted by tip-bot2 for Thomas Weißschuh 2 months, 3 weeks ago

The following commit has been merged into the timers/ptp branch of tip:

Commit-ID:     fb61bdb27fd730c393a8bddbda2401c37a919667
Gitweb:        https://git.kernel.org/tip/fb61bdb27fd730c393a8bddbda2401c37a919667
Author:        Thomas Weißschuh <thomas.weissschuh@linutronix.de>
AuthorDate:    Tue, 01 Jul 2025 10:58:00 +02:00
Committer:     Thomas Gleixner <tglx@linutronix.de>
CommitterDate: Fri, 18 Jul 2025 13:45:32 +02:00

vdso/gettimeofday: Return bool from clock_gettime() helpers

The internal helpers are effectively using boolean results,
while pretending to use error numbers.

Switch the return type to bool for more clarity.

Signed-off-by: Thomas Weißschuh <thomas.weissschuh@linutronix.de>
Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
Link: https://lore.kernel.org/all/20250701-vdso-auxclock-v1-6-df7d9f87b9b8@linutronix.de

---
 lib/vdso/gettimeofday.c | 70 ++++++++++++++++++++--------------------
 1 file changed, 36 insertions(+), 34 deletions(-)

diff --git a/lib/vdso/gettimeofday.c b/lib/vdso/gettimeofday.c
index 9b77f23..32e568d 100644
--- a/lib/vdso/gettimeofday.c
+++ b/lib/vdso/gettimeofday.c
@@ -82,8 +82,8 @@ const struct vdso_time_data *__arch_get_vdso_u_timens_data(const struct vdso_tim
 #endif /* CONFIG_GENERIC_VDSO_DATA_STORE */
 
 static __always_inline
-int do_hres_timens(const struct vdso_time_data *vdns, const struct vdso_clock *vcns,
-		   clockid_t clk, struct __kernel_timespec *ts)
+bool do_hres_timens(const struct vdso_time_data *vdns, const struct vdso_clock *vcns,
+		    clockid_t clk, struct __kernel_timespec *ts)
 {
 	const struct vdso_time_data *vd = __arch_get_vdso_u_timens_data(vdns);
 	const struct timens_offset *offs = &vcns->offset[clk];
@@ -103,11 +103,11 @@ int do_hres_timens(const struct vdso_time_data *vdns, const struct vdso_clock *v
 		seq = vdso_read_begin(vc);
 
 		if (unlikely(!vdso_clocksource_ok(vc)))
-			return -1;
+			return false;
 
 		cycles = __arch_get_hw_counter(vc->clock_mode, vd);
 		if (unlikely(!vdso_cycles_ok(cycles)))
-			return -1;
+			return false;
 		ns = vdso_calc_ns(vc, cycles, vdso_ts->nsec);
 		sec = vdso_ts->sec;
 	} while (unlikely(vdso_read_retry(vc, seq)));
@@ -123,7 +123,7 @@ int do_hres_timens(const struct vdso_time_data *vdns, const struct vdso_clock *v
 	ts->tv_sec = sec + __iter_div_u64_rem(ns, NSEC_PER_SEC, &ns);
 	ts->tv_nsec = ns;
 
-	return 0;
+	return true;
 }
 #else
 static __always_inline
@@ -133,16 +133,16 @@ const struct vdso_time_data *__arch_get_vdso_u_timens_data(const struct vdso_tim
 }
 
 static __always_inline
-int do_hres_timens(const struct vdso_time_data *vdns, const struct vdso_clock *vcns,
-		   clockid_t clk, struct __kernel_timespec *ts)
+bool do_hres_timens(const struct vdso_time_data *vdns, const struct vdso_clock *vcns,
+		    clockid_t clk, struct __kernel_timespec *ts)
 {
-	return -EINVAL;
+	return false;
 }
 #endif
 
 static __always_inline
-int do_hres(const struct vdso_time_data *vd, const struct vdso_clock *vc,
-	    clockid_t clk, struct __kernel_timespec *ts)
+bool do_hres(const struct vdso_time_data *vd, const struct vdso_clock *vc,
+	     clockid_t clk, struct __kernel_timespec *ts)
 {
 	const struct vdso_timestamp *vdso_ts = &vc->basetime[clk];
 	u64 cycles, sec, ns;
@@ -150,7 +150,7 @@ int do_hres(const struct vdso_time_data *vd, const struct vdso_clock *vc,
 
 	/* Allows to compile the high resolution parts out */
 	if (!__arch_vdso_hres_capable())
-		return -1;
+		return false;
 
 	do {
 		/*
@@ -173,11 +173,11 @@ int do_hres(const struct vdso_time_data *vd, const struct vdso_clock *vc,
 		smp_rmb();
 
 		if (unlikely(!vdso_clocksource_ok(vc)))
-			return -1;
+			return false;
 
 		cycles = __arch_get_hw_counter(vc->clock_mode, vd);
 		if (unlikely(!vdso_cycles_ok(cycles)))
-			return -1;
+			return false;
 		ns = vdso_calc_ns(vc, cycles, vdso_ts->nsec);
 		sec = vdso_ts->sec;
 	} while (unlikely(vdso_read_retry(vc, seq)));
@@ -189,13 +189,13 @@ int do_hres(const struct vdso_time_data *vd, const struct vdso_clock *vc,
 	ts->tv_sec = sec + __iter_div_u64_rem(ns, NSEC_PER_SEC, &ns);
 	ts->tv_nsec = ns;
 
-	return 0;
+	return true;
 }
 
 #ifdef CONFIG_TIME_NS
 static __always_inline
-int do_coarse_timens(const struct vdso_time_data *vdns, const struct vdso_clock *vcns,
-		     clockid_t clk, struct __kernel_timespec *ts)
+bool do_coarse_timens(const struct vdso_time_data *vdns, const struct vdso_clock *vcns,
+		      clockid_t clk, struct __kernel_timespec *ts)
 {
 	const struct vdso_time_data *vd = __arch_get_vdso_u_timens_data(vdns);
 	const struct timens_offset *offs = &vcns->offset[clk];
@@ -223,20 +223,20 @@ int do_coarse_timens(const struct vdso_time_data *vdns, const struct vdso_clock 
 	 */
 	ts->tv_sec = sec + __iter_div_u64_rem(nsec, NSEC_PER_SEC, &nsec);
 	ts->tv_nsec = nsec;
-	return 0;
+	return true;
 }
 #else
 static __always_inline
-int do_coarse_timens(const struct vdso_time_data *vdns, const struct vdso_clock *vcns,
-		     clockid_t clk, struct __kernel_timespec *ts)
+bool do_coarse_timens(const struct vdso_time_data *vdns, const struct vdso_clock *vcns,
+		      clockid_t clk, struct __kernel_timespec *ts)
 {
-	return -1;
+	return false;
 }
 #endif
 
 static __always_inline
-int do_coarse(const struct vdso_time_data *vd, const struct vdso_clock *vc,
-	      clockid_t clk, struct __kernel_timespec *ts)
+bool do_coarse(const struct vdso_time_data *vd, const struct vdso_clock *vc,
+	       clockid_t clk, struct __kernel_timespec *ts)
 {
 	const struct vdso_timestamp *vdso_ts = &vc->basetime[clk];
 	u32 seq;
@@ -258,10 +258,10 @@ int do_coarse(const struct vdso_time_data *vd, const struct vdso_clock *vc,
 		ts->tv_nsec = vdso_ts->nsec;
 	} while (unlikely(vdso_read_retry(vc, seq)));
 
-	return 0;
+	return true;
 }
 
-static __always_inline int
+static __always_inline bool
 __cvdso_clock_gettime_common(const struct vdso_time_data *vd, clockid_t clock,
 			     struct __kernel_timespec *ts)
 {
@@ -270,7 +270,7 @@ __cvdso_clock_gettime_common(const struct vdso_time_data *vd, clockid_t clock,
 
 	/* Check for negative values or invalid clocks */
 	if (unlikely((u32) clock >= MAX_CLOCKS))
-		return -1;
+		return false;
 
 	/*
 	 * Convert the clockid to a bitmask and use it to check which
@@ -284,7 +284,7 @@ __cvdso_clock_gettime_common(const struct vdso_time_data *vd, clockid_t clock,
 	else if (msk & VDSO_RAW)
 		vc = &vc[CS_RAW];
 	else
-		return -1;
+		return false;
 
 	return do_hres(vd, vc, clock, ts);
 }
@@ -293,9 +293,11 @@ static __maybe_unused int
 __cvdso_clock_gettime_data(const struct vdso_time_data *vd, clockid_t clock,
 			   struct __kernel_timespec *ts)
 {
-	int ret = __cvdso_clock_gettime_common(vd, clock, ts);
+	bool ok;
+
+	ok = __cvdso_clock_gettime_common(vd, clock, ts);
 
-	if (unlikely(ret))
+	if (unlikely(!ok))
 		return clock_gettime_fallback(clock, ts);
 	return 0;
 }
@@ -312,18 +314,18 @@ __cvdso_clock_gettime32_data(const struct vdso_time_data *vd, clockid_t clock,
 			     struct old_timespec32 *res)
 {
 	struct __kernel_timespec ts;
-	int ret;
+	bool ok;
 
-	ret = __cvdso_clock_gettime_common(vd, clock, &ts);
+	ok = __cvdso_clock_gettime_common(vd, clock, &ts);
 
-	if (unlikely(ret))
+	if (unlikely(!ok))
 		return clock_gettime32_fallback(clock, res);
 
-	/* For ret == 0 */
+	/* For ok == true */
 	res->tv_sec = ts.tv_sec;
 	res->tv_nsec = ts.tv_nsec;
 
-	return ret;
+	return 0;
 }
 
 static __maybe_unused int
@@ -342,7 +344,7 @@ __cvdso_gettimeofday_data(const struct vdso_time_data *vd,
 	if (likely(tv != NULL)) {
 		struct __kernel_timespec ts;
 
-		if (do_hres(vd, &vc[CS_HRES_COARSE], CLOCK_REALTIME, &ts))
+		if (!do_hres(vd, &vc[CS_HRES_COARSE], CLOCK_REALTIME, &ts))
 			return gettimeofday_fallback(tv, tz);
 
 		tv->tv_sec = ts.tv_sec;

[tip: timers/ptp] vdso/gettimeofday: Return bool from clock_gettime() helpers

Posted by tip-bot2 for Thomas Weißschuh 3 months ago

The following commit has been merged into the timers/ptp branch of tip:

Commit-ID:     75540b791bf1d9454dfd2c670943c42901382604
Gitweb:        https://git.kernel.org/tip/75540b791bf1d9454dfd2c670943c42901382604
Author:        Thomas Weißschuh <thomas.weissschuh@linutronix.de>
AuthorDate:    Tue, 01 Jul 2025 10:58:00 +02:00
Committer:     Thomas Gleixner <tglx@linutronix.de>
CommitterDate: Wed, 09 Jul 2025 11:52:34 +02:00

vdso/gettimeofday: Return bool from clock_gettime() helpers

The internal helpers are effectively using boolean results,
while pretending to use error numbers.

Switch the return type to bool for more clarity.

Signed-off-by: Thomas Weißschuh <thomas.weissschuh@linutronix.de>
Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
Link: https://lore.kernel.org/all/20250701-vdso-auxclock-v1-6-df7d9f87b9b8@linutronix.de

---
 lib/vdso/gettimeofday.c | 60 ++++++++++++++++++++--------------------
 1 file changed, 31 insertions(+), 29 deletions(-)

diff --git a/lib/vdso/gettimeofday.c b/lib/vdso/gettimeofday.c
index 9b77f23..7e79b02 100644
--- a/lib/vdso/gettimeofday.c
+++ b/lib/vdso/gettimeofday.c
@@ -82,8 +82,8 @@ const struct vdso_time_data *__arch_get_vdso_u_timens_data(const struct vdso_tim
 #endif /* CONFIG_GENERIC_VDSO_DATA_STORE */
 
 static __always_inline
-int do_hres_timens(const struct vdso_time_data *vdns, const struct vdso_clock *vcns,
-		   clockid_t clk, struct __kernel_timespec *ts)
+bool do_hres_timens(const struct vdso_time_data *vdns, const struct vdso_clock *vcns,
+		    clockid_t clk, struct __kernel_timespec *ts)
 {
 	const struct vdso_time_data *vd = __arch_get_vdso_u_timens_data(vdns);
 	const struct timens_offset *offs = &vcns->offset[clk];
@@ -103,11 +103,11 @@ int do_hres_timens(const struct vdso_time_data *vdns, const struct vdso_clock *v
 		seq = vdso_read_begin(vc);
 
 		if (unlikely(!vdso_clocksource_ok(vc)))
-			return -1;
+			return false;
 
 		cycles = __arch_get_hw_counter(vc->clock_mode, vd);
 		if (unlikely(!vdso_cycles_ok(cycles)))
-			return -1;
+			return false;
 		ns = vdso_calc_ns(vc, cycles, vdso_ts->nsec);
 		sec = vdso_ts->sec;
 	} while (unlikely(vdso_read_retry(vc, seq)));
@@ -123,7 +123,7 @@ int do_hres_timens(const struct vdso_time_data *vdns, const struct vdso_clock *v
 	ts->tv_sec = sec + __iter_div_u64_rem(ns, NSEC_PER_SEC, &ns);
 	ts->tv_nsec = ns;
 
-	return 0;
+	return true;
 }
 #else
 static __always_inline
@@ -133,16 +133,16 @@ const struct vdso_time_data *__arch_get_vdso_u_timens_data(const struct vdso_tim
 }
 
 static __always_inline
-int do_hres_timens(const struct vdso_time_data *vdns, const struct vdso_clock *vcns,
-		   clockid_t clk, struct __kernel_timespec *ts)
+bool do_hres_timens(const struct vdso_time_data *vdns, const struct vdso_clock *vcns,
+		    clockid_t clk, struct __kernel_timespec *ts)
 {
-	return -EINVAL;
+	return false;
 }
 #endif
 
 static __always_inline
-int do_hres(const struct vdso_time_data *vd, const struct vdso_clock *vc,
-	    clockid_t clk, struct __kernel_timespec *ts)
+bool do_hres(const struct vdso_time_data *vd, const struct vdso_clock *vc,
+	     clockid_t clk, struct __kernel_timespec *ts)
 {
 	const struct vdso_timestamp *vdso_ts = &vc->basetime[clk];
 	u64 cycles, sec, ns;
@@ -150,7 +150,7 @@ int do_hres(const struct vdso_time_data *vd, const struct vdso_clock *vc,
 
 	/* Allows to compile the high resolution parts out */
 	if (!__arch_vdso_hres_capable())
-		return -1;
+		return false;
 
 	do {
 		/*
@@ -173,11 +173,11 @@ int do_hres(const struct vdso_time_data *vd, const struct vdso_clock *vc,
 		smp_rmb();
 
 		if (unlikely(!vdso_clocksource_ok(vc)))
-			return -1;
+			return false;
 
 		cycles = __arch_get_hw_counter(vc->clock_mode, vd);
 		if (unlikely(!vdso_cycles_ok(cycles)))
-			return -1;
+			return false;
 		ns = vdso_calc_ns(vc, cycles, vdso_ts->nsec);
 		sec = vdso_ts->sec;
 	} while (unlikely(vdso_read_retry(vc, seq)));
@@ -189,13 +189,13 @@ int do_hres(const struct vdso_time_data *vd, const struct vdso_clock *vc,
 	ts->tv_sec = sec + __iter_div_u64_rem(ns, NSEC_PER_SEC, &ns);
 	ts->tv_nsec = ns;
 
-	return 0;
+	return true;
 }
 
 #ifdef CONFIG_TIME_NS
 static __always_inline
-int do_coarse_timens(const struct vdso_time_data *vdns, const struct vdso_clock *vcns,
-		     clockid_t clk, struct __kernel_timespec *ts)
+bool do_coarse_timens(const struct vdso_time_data *vdns, const struct vdso_clock *vcns,
+		      clockid_t clk, struct __kernel_timespec *ts)
 {
 	const struct vdso_time_data *vd = __arch_get_vdso_u_timens_data(vdns);
 	const struct timens_offset *offs = &vcns->offset[clk];
@@ -223,20 +223,20 @@ int do_coarse_timens(const struct vdso_time_data *vdns, const struct vdso_clock 
 	 */
 	ts->tv_sec = sec + __iter_div_u64_rem(nsec, NSEC_PER_SEC, &nsec);
 	ts->tv_nsec = nsec;
-	return 0;
+	return true;
 }
 #else
 static __always_inline
-int do_coarse_timens(const struct vdso_time_data *vdns, const struct vdso_clock *vcns,
-		     clockid_t clk, struct __kernel_timespec *ts)
+bool do_coarse_timens(const struct vdso_time_data *vdns, const struct vdso_clock *vcns,
+		      clockid_t clk, struct __kernel_timespec *ts)
 {
-	return -1;
+	return false;
 }
 #endif
 
 static __always_inline
-int do_coarse(const struct vdso_time_data *vd, const struct vdso_clock *vc,
-	      clockid_t clk, struct __kernel_timespec *ts)
+bool do_coarse(const struct vdso_time_data *vd, const struct vdso_clock *vc,
+	       clockid_t clk, struct __kernel_timespec *ts)
 {
 	const struct vdso_timestamp *vdso_ts = &vc->basetime[clk];
 	u32 seq;
@@ -258,10 +258,10 @@ int do_coarse(const struct vdso_time_data *vd, const struct vdso_clock *vc,
 		ts->tv_nsec = vdso_ts->nsec;
 	} while (unlikely(vdso_read_retry(vc, seq)));
 
-	return 0;
+	return true;
 }
 
-static __always_inline int
+static __always_inline bool
 __cvdso_clock_gettime_common(const struct vdso_time_data *vd, clockid_t clock,
 			     struct __kernel_timespec *ts)
 {
@@ -270,7 +270,7 @@ __cvdso_clock_gettime_common(const struct vdso_time_data *vd, clockid_t clock,
 
 	/* Check for negative values or invalid clocks */
 	if (unlikely((u32) clock >= MAX_CLOCKS))
-		return -1;
+		return false;
 
 	/*
 	 * Convert the clockid to a bitmask and use it to check which
@@ -284,7 +284,7 @@ __cvdso_clock_gettime_common(const struct vdso_time_data *vd, clockid_t clock,
 	else if (msk & VDSO_RAW)
 		vc = &vc[CS_RAW];
 	else
-		return -1;
+		return false;
 
 	return do_hres(vd, vc, clock, ts);
 }
@@ -293,9 +293,11 @@ static __maybe_unused int
 __cvdso_clock_gettime_data(const struct vdso_time_data *vd, clockid_t clock,
 			   struct __kernel_timespec *ts)
 {
-	int ret = __cvdso_clock_gettime_common(vd, clock, ts);
+	bool ok;
 
-	if (unlikely(ret))
+	ok = __cvdso_clock_gettime_common(vd, clock, ts);
+
+	if (unlikely(!ok))
 		return clock_gettime_fallback(clock, ts);
 	return 0;
 }
@@ -342,7 +344,7 @@ __cvdso_gettimeofday_data(const struct vdso_time_data *vd,
 	if (likely(tv != NULL)) {
 		struct __kernel_timespec ts;
 
-		if (do_hres(vd, &vc[CS_HRES_COARSE], CLOCK_REALTIME, &ts))
+		if (!do_hres(vd, &vc[CS_HRES_COARSE], CLOCK_REALTIME, &ts))
 			return gettimeofday_fallback(tv, tz);
 
 		tv->tv_sec = ts.tv_sec;

[tip: timers/ptp] vdso/gettimeofday: Return bool from clock_gettime() helpers

Posted by tip-bot2 for Thomas Weißschuh 3 months ago

The following commit has been merged into the timers/ptp branch of tip:

Commit-ID:     fcc8e46f768ff508dab0e3cfc2001e96dcb388e2
Gitweb:        https://git.kernel.org/tip/fcc8e46f768ff508dab0e3cfc2001e96dcb388e2
Author:        Thomas Weißschuh <thomas.weissschuh@linutronix.de>
AuthorDate:    Tue, 01 Jul 2025 10:58:00 +02:00
Committer:     Thomas Gleixner <tglx@linutronix.de>
CommitterDate: Mon, 07 Jul 2025 08:58:51 +02:00

vdso/gettimeofday: Return bool from clock_gettime() helpers

The internal helpers are effectively using boolean results,
while pretending to use error numbers.

Switch the return type to bool for more clarity.

Signed-off-by: Thomas Weißschuh <thomas.weissschuh@linutronix.de>
Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
Link: https://lore.kernel.org/all/20250701-vdso-auxclock-v1-6-df7d9f87b9b8@linutronix.de

---
 lib/vdso/gettimeofday.c | 58 ++++++++++++++++++++--------------------
 1 file changed, 30 insertions(+), 28 deletions(-)

diff --git a/lib/vdso/gettimeofday.c b/lib/vdso/gettimeofday.c
index 9b77f23..c526653 100644
--- a/lib/vdso/gettimeofday.c
+++ b/lib/vdso/gettimeofday.c
@@ -82,8 +82,8 @@ const struct vdso_time_data *__arch_get_vdso_u_timens_data(const struct vdso_tim
 #endif /* CONFIG_GENERIC_VDSO_DATA_STORE */
 
 static __always_inline
-int do_hres_timens(const struct vdso_time_data *vdns, const struct vdso_clock *vcns,
-		   clockid_t clk, struct __kernel_timespec *ts)
+bool do_hres_timens(const struct vdso_time_data *vdns, const struct vdso_clock *vcns,
+		    clockid_t clk, struct __kernel_timespec *ts)
 {
 	const struct vdso_time_data *vd = __arch_get_vdso_u_timens_data(vdns);
 	const struct timens_offset *offs = &vcns->offset[clk];
@@ -103,11 +103,11 @@ int do_hres_timens(const struct vdso_time_data *vdns, const struct vdso_clock *v
 		seq = vdso_read_begin(vc);
 
 		if (unlikely(!vdso_clocksource_ok(vc)))
-			return -1;
+			return false;
 
 		cycles = __arch_get_hw_counter(vc->clock_mode, vd);
 		if (unlikely(!vdso_cycles_ok(cycles)))
-			return -1;
+			return false;
 		ns = vdso_calc_ns(vc, cycles, vdso_ts->nsec);
 		sec = vdso_ts->sec;
 	} while (unlikely(vdso_read_retry(vc, seq)));
@@ -123,7 +123,7 @@ int do_hres_timens(const struct vdso_time_data *vdns, const struct vdso_clock *v
 	ts->tv_sec = sec + __iter_div_u64_rem(ns, NSEC_PER_SEC, &ns);
 	ts->tv_nsec = ns;
 
-	return 0;
+	return true;
 }
 #else
 static __always_inline
@@ -133,16 +133,16 @@ const struct vdso_time_data *__arch_get_vdso_u_timens_data(const struct vdso_tim
 }
 
 static __always_inline
-int do_hres_timens(const struct vdso_time_data *vdns, const struct vdso_clock *vcns,
-		   clockid_t clk, struct __kernel_timespec *ts)
+bool do_hres_timens(const struct vdso_time_data *vdns, const struct vdso_clock *vcns,
+		    clockid_t clk, struct __kernel_timespec *ts)
 {
-	return -EINVAL;
+	return false;
 }
 #endif
 
 static __always_inline
-int do_hres(const struct vdso_time_data *vd, const struct vdso_clock *vc,
-	    clockid_t clk, struct __kernel_timespec *ts)
+bool do_hres(const struct vdso_time_data *vd, const struct vdso_clock *vc,
+	     clockid_t clk, struct __kernel_timespec *ts)
 {
 	const struct vdso_timestamp *vdso_ts = &vc->basetime[clk];
 	u64 cycles, sec, ns;
@@ -150,7 +150,7 @@ int do_hres(const struct vdso_time_data *vd, const struct vdso_clock *vc,
 
 	/* Allows to compile the high resolution parts out */
 	if (!__arch_vdso_hres_capable())
-		return -1;
+		return false;
 
 	do {
 		/*
@@ -173,11 +173,11 @@ int do_hres(const struct vdso_time_data *vd, const struct vdso_clock *vc,
 		smp_rmb();
 
 		if (unlikely(!vdso_clocksource_ok(vc)))
-			return -1;
+			return false;
 
 		cycles = __arch_get_hw_counter(vc->clock_mode, vd);
 		if (unlikely(!vdso_cycles_ok(cycles)))
-			return -1;
+			return false;
 		ns = vdso_calc_ns(vc, cycles, vdso_ts->nsec);
 		sec = vdso_ts->sec;
 	} while (unlikely(vdso_read_retry(vc, seq)));
@@ -189,13 +189,13 @@ int do_hres(const struct vdso_time_data *vd, const struct vdso_clock *vc,
 	ts->tv_sec = sec + __iter_div_u64_rem(ns, NSEC_PER_SEC, &ns);
 	ts->tv_nsec = ns;
 
-	return 0;
+	return true;
 }
 
 #ifdef CONFIG_TIME_NS
 static __always_inline
-int do_coarse_timens(const struct vdso_time_data *vdns, const struct vdso_clock *vcns,
-		     clockid_t clk, struct __kernel_timespec *ts)
+bool do_coarse_timens(const struct vdso_time_data *vdns, const struct vdso_clock *vcns,
+		      clockid_t clk, struct __kernel_timespec *ts)
 {
 	const struct vdso_time_data *vd = __arch_get_vdso_u_timens_data(vdns);
 	const struct timens_offset *offs = &vcns->offset[clk];
@@ -223,20 +223,20 @@ int do_coarse_timens(const struct vdso_time_data *vdns, const struct vdso_clock 
 	 */
 	ts->tv_sec = sec + __iter_div_u64_rem(nsec, NSEC_PER_SEC, &nsec);
 	ts->tv_nsec = nsec;
-	return 0;
+	return true;
 }
 #else
 static __always_inline
-int do_coarse_timens(const struct vdso_time_data *vdns, const struct vdso_clock *vcns,
-		     clockid_t clk, struct __kernel_timespec *ts)
+bool do_coarse_timens(const struct vdso_time_data *vdns, const struct vdso_clock *vcns,
+		      clockid_t clk, struct __kernel_timespec *ts)
 {
-	return -1;
+	return false;
 }
 #endif
 
 static __always_inline
-int do_coarse(const struct vdso_time_data *vd, const struct vdso_clock *vc,
-	      clockid_t clk, struct __kernel_timespec *ts)
+bool do_coarse(const struct vdso_time_data *vd, const struct vdso_clock *vc,
+	       clockid_t clk, struct __kernel_timespec *ts)
 {
 	const struct vdso_timestamp *vdso_ts = &vc->basetime[clk];
 	u32 seq;
@@ -258,10 +258,10 @@ int do_coarse(const struct vdso_time_data *vd, const struct vdso_clock *vc,
 		ts->tv_nsec = vdso_ts->nsec;
 	} while (unlikely(vdso_read_retry(vc, seq)));
 
-	return 0;
+	return true;
 }
 
-static __always_inline int
+static __always_inline bool
 __cvdso_clock_gettime_common(const struct vdso_time_data *vd, clockid_t clock,
 			     struct __kernel_timespec *ts)
 {
@@ -270,7 +270,7 @@ __cvdso_clock_gettime_common(const struct vdso_time_data *vd, clockid_t clock,
 
 	/* Check for negative values or invalid clocks */
 	if (unlikely((u32) clock >= MAX_CLOCKS))
-		return -1;
+		return false;
 
 	/*
 	 * Convert the clockid to a bitmask and use it to check which
@@ -284,7 +284,7 @@ __cvdso_clock_gettime_common(const struct vdso_time_data *vd, clockid_t clock,
 	else if (msk & VDSO_RAW)
 		vc = &vc[CS_RAW];
 	else
-		return -1;
+		return false;
 
 	return do_hres(vd, vc, clock, ts);
 }
@@ -293,9 +293,11 @@ static __maybe_unused int
 __cvdso_clock_gettime_data(const struct vdso_time_data *vd, clockid_t clock,
 			   struct __kernel_timespec *ts)
 {
-	int ret = __cvdso_clock_gettime_common(vd, clock, ts);
+	bool ok;
 
-	if (unlikely(ret))
+	ok = __cvdso_clock_gettime_common(vd, clock, ts);
+
+	if (unlikely(!ok))
 		return clock_gettime_fallback(clock, ts);
 	return 0;
 }