Re: [RFC PATCH v2 01/11] time: Add do_settimeofday64() safe version(using timespec64)

From: Thomas Gleixner
Date: Thu Oct 30 2014 - 09:24:50 EST


On Thu, 30 Oct 2014, pang.xunlei wrote:

Your subject line

> time: Add do_settimeofday64() safe version(using timespec64)

is confusing at best.

time: Provide y2038 safe do_settimeofday() replacement

That explains what the patch is doing in a very precice way.

> /**
> - * do_settimeofday - Sets the time of day
> - * @tv: pointer to the timespec variable containing the new time
> + * do_settimeofday64 - Sets the time of day
> + * @ts: pointer to the timespec64 variable containing the new time
> *
> * Sets the time of day to the new time and update NTP and notify hrtimers
> + * Safe version for 2038 safety.

I rather put this into the function description above:

+ * do_settimeofday64 - Sets the time of day. 2038 safe.

Way simpler to grep for.

> */
> -int do_settimeofday(const struct timespec *tv)
> +int do_settimeofday64(const struct timespec64 *ts)
> {
> struct timekeeper *tk = &tk_core.timekeeper;
> - struct timespec64 ts_delta, xt, tmp;
> + struct timespec64 ts_delta, xt;
> unsigned long flags;
>
> - if (!timespec_valid_strict(tv))
> + if (!timespec64_valid_strict(ts))
> return -EINVAL;
>
> raw_spin_lock_irqsave(&timekeeper_lock, flags);
> @@ -723,13 +724,12 @@ int do_settimeofday(const struct timespec *tv)
> timekeeping_forward_now(tk);
>
> xt = tk_xtime(tk);
> - ts_delta.tv_sec = tv->tv_sec - xt.tv_sec;
> - ts_delta.tv_nsec = tv->tv_nsec - xt.tv_nsec;
> + ts_delta.tv_sec = ts->tv_sec - xt.tv_sec;
> + ts_delta.tv_nsec = ts->tv_nsec - xt.tv_nsec;
>
> tk_set_wall_to_mono(tk, timespec64_sub(tk->wall_to_monotonic, ts_delta));
>
> - tmp = timespec_to_timespec64(*tv);
> - tk_set_xtime(tk, &tmp);
> + tk_set_xtime(tk, ts);
>
> timekeeping_update(tk, TK_CLEAR_NTP | TK_MIRROR | TK_CLOCK_WAS_SET);
>
> @@ -741,6 +741,22 @@ int do_settimeofday(const struct timespec *tv)
>
> return 0;
> }
> +EXPORT_SYMBOL(do_settimeofday64);

EXPORT_SYMBOL_GPL please.

> +
> +/**
> + * do_settimeofday - Sets the time of day
> + * @tv: pointer to the timespec variable containing the new time
> + *
> + * Sets the time of day to the new time and update NTP and notify hrtimers
> + * TODO: [2038 safety] should be replaced by do_settimeofday64().

The TODO is not to replace do_settimeofday() by do_settimeofday64(). The TODO
is to fix the call sites.

So instead of adding this you can simply say:

+ * do_settimeofday - Sets the time of day. Deprecated. Use do_settimeofday64()

We want things like this easy to find and not be burried in some
cryptic comment.

Thanks,

tglx
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/