On 08.07.2025 17:17, Marek Szyprowski wrote:

On 01.07.2025 10:58, Thomas Weißschuh wrote:

quoted

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 <redacted>
---
  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.

quoted

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