* [PATCH V2] y2038: replace timespec with timespec64 in clock_settime
@ 2020-09-29 7:19 chensong
2020-10-27 16:33 ` Jan Kiszka
0 siblings, 1 reply; 5+ messages in thread
From: chensong @ 2020-09-29 7:19 UTC (permalink / raw)
To: xenomai
Upstream has used timespec64 to replace timespec inside kernel and
used __kernel_timespec to replace timespect in syscalls, therefore
we must keep aligned with upstream.
clock_settime is a point to get started at, it involves 2 parts:
1, syscall in ./include/trace/events/cobalt-posix.h for 64bits
2, compat syscall in kernel/cobalt/posix/syscall32.c for 32bits
3, define __kernel_timespec in wrapper.h for compatibility
some new functions are implemented to keep the compatibility with
other syscalls which haven't been handled yet and will be removed
in the end of the work.
Signed-off-by: chensong <chensong@tj.kylinos.cn>
CC: Jan Kiszka <jan.kiszka@siemens.com>
CC: Henning Schild <henning.schild@siemens.com>
---
include/cobalt/kernel/clock.h | 6 ++---
include/cobalt/kernel/compat.h | 3 +++
.../cobalt/include/asm-generic/xenomai/wrappers.h | 9 +++++++
kernel/cobalt/posix/clock.c | 28 +++++++++++++++++-----
kernel/cobalt/posix/clock.h | 15 ++++++++++--
kernel/cobalt/posix/compat.c | 11 +++++++++
kernel/cobalt/posix/syscall32.c | 6 ++---
kernel/cobalt/trace/cobalt-posix.h | 24 +++++++++++++++++--
8 files changed, 86 insertions(+), 16 deletions(-)
diff --git a/include/cobalt/kernel/clock.h b/include/cobalt/kernel/clock.h
index c26776a..cf9d5ec 100644
--- a/include/cobalt/kernel/clock.h
+++ b/include/cobalt/kernel/clock.h
@@ -52,7 +52,7 @@ struct xnclock {
xnticks_t (*read_raw)(struct xnclock *clock);
xnticks_t (*read_monotonic)(struct xnclock *clock);
int (*set_time)(struct xnclock *clock,
- const struct timespec *ts);
+ const struct timespec64 *ts);
xnsticks_t (*ns_to_ticks)(struct xnclock *clock,
xnsticks_t ns);
xnsticks_t (*ticks_to_ns)(struct xnclock *clock,
@@ -213,7 +213,7 @@ static inline xnticks_t xnclock_read_monotonic(struct xnclock *clock)
}
static inline int xnclock_set_time(struct xnclock *clock,
- const struct timespec *ts)
+ const struct timespec64 *ts)
{
if (likely(clock == &nkclock))
return -EINVAL;
@@ -266,7 +266,7 @@ static inline xnticks_t xnclock_read_monotonic(struct xnclock *clock)
}
static inline int xnclock_set_time(struct xnclock *clock,
- const struct timespec *ts)
+ const struct timespec64 *ts)
{
/*
* There is no way to change the core clock's idea of time.
diff --git a/include/cobalt/kernel/compat.h b/include/cobalt/kernel/compat.h
index 05754cb..70e2c03 100644
--- a/include/cobalt/kernel/compat.h
+++ b/include/cobalt/kernel/compat.h
@@ -89,6 +89,9 @@ struct compat_rtdm_mmap_request {
int sys32_get_timespec(struct timespec *ts,
const struct compat_timespec __user *cts);
+int sys64_get_timespec(struct timespec64 *ts,
+ const struct compat_timespec __user *cts);
+
int sys32_put_timespec(struct compat_timespec __user *cts,
const struct timespec *ts);
diff --git a/kernel/cobalt/include/asm-generic/xenomai/wrappers.h b/kernel/cobalt/include/asm-generic/xenomai/wrappers.h
index 23993dc..e5375de 100644
--- a/kernel/cobalt/include/asm-generic/xenomai/wrappers.h
+++ b/kernel/cobalt/include/asm-generic/xenomai/wrappers.h
@@ -156,4 +156,13 @@ devm_hwmon_device_register_with_groups(struct device *dev, const char *name,
#error "Xenomai/cobalt requires Linux kernel 3.10 or above"
#endif /* < 3.10 */
+#if LINUX_VERSION_CODE < KERNEL_VERSION(4, 18, 0)
+typedef long long __kernel_time64_t;
+struct __kernel_timespec {
+ __kernel_time64_t tv_sec; /* seconds */
+ long long tv_nsec; /* nanoseconds */
+};
+#endif /* < 4.18 */
+
+
#endif /* _COBALT_ASM_GENERIC_WRAPPERS_H */
diff --git a/kernel/cobalt/posix/clock.c b/kernel/cobalt/posix/clock.c
index 561358e..c960d7b 100644
--- a/kernel/cobalt/posix/clock.c
+++ b/kernel/cobalt/posix/clock.c
@@ -194,7 +194,7 @@ COBALT_SYSCALL(clock_gettime, current,
return 0;
}
-int __cobalt_clock_settime(clockid_t clock_id, const struct timespec *ts)
+int __cobalt_clock_settime(clockid_t clock_id, const struct timespec64 *ts)
{
int _ret, ret = 0;
xnticks_t now;
@@ -207,7 +207,7 @@ int __cobalt_clock_settime(clockid_t clock_id, const struct timespec *ts)
case CLOCK_REALTIME:
xnlock_get_irqsave(&nklock, s);
now = xnclock_read_realtime(&nkclock);
- xnclock_adjust(&nkclock, (xnsticks_t) (ts2ns(ts) - now));
+ xnclock_adjust(&nkclock, (xnsticks_t) (ts2ns64(ts) - now));
xnlock_put_irqrestore(&nklock, s);
break;
default:
@@ -242,15 +242,31 @@ int __cobalt_clock_adjtime(clockid_t clock_id, struct timex *tx)
return 0;
}
+static int __cobalt_get_timespec64(struct timespec64 *ts,
+ const struct __kernel_timespec __user *uts)
+{
+ struct __kernel_timespec kts;
+ int ret;
+
+ ret = cobalt_copy_from_user(&kts, uts, sizeof(kts));
+ if (ret)
+ return -EFAULT;
+
+ ts->tv_sec = kts.tv_sec;
+ ts->tv_nsec = kts.tv_nsec;
+
+ return 0;
+
+}
COBALT_SYSCALL(clock_settime, current,
- (clockid_t clock_id, const struct timespec __user *u_ts))
+ (clockid_t clock_id, const struct __kernel_timespec __user *u_ts))
{
- struct timespec ts;
+ struct timespec64 new_ts;
- if (cobalt_copy_from_user(&ts, u_ts, sizeof(ts)))
+ if (__cobalt_get_timespec64(&new_ts, u_ts))
return -EFAULT;
- return __cobalt_clock_settime(clock_id, &ts);
+ return __cobalt_clock_settime(clock_id, &new_ts);
}
COBALT_SYSCALL(clock_adjtime, current,
diff --git a/kernel/cobalt/posix/clock.h b/kernel/cobalt/posix/clock.h
index 0b06b93..d7328b0 100644
--- a/kernel/cobalt/posix/clock.h
+++ b/kernel/cobalt/posix/clock.h
@@ -43,6 +43,16 @@ static inline xnticks_t ts2ns(const struct timespec *ts)
return nsecs;
}
+static inline xnticks_t ts2ns64(const struct timespec64 *ts)
+{
+ xnticks_t nsecs = ts->tv_nsec;
+
+ if (ts->tv_sec)
+ nsecs += (xnticks_t)ts->tv_sec * ONE_BILLION;
+
+ return nsecs;
+}
+
static inline xnticks_t tv2ns(const struct timeval *tv)
{
xnticks_t nsecs = tv->tv_usec * 1000;
@@ -86,7 +96,7 @@ int __cobalt_clock_gettime(clockid_t clock_id,
struct timespec *ts);
int __cobalt_clock_settime(clockid_t clock_id,
- const struct timespec *ts);
+ const struct timespec64 *ts);
int __cobalt_clock_adjtime(clockid_t clock_id,
struct timex *tx);
@@ -102,7 +112,8 @@ COBALT_SYSCALL_DECL(clock_gettime,
(clockid_t clock_id, struct timespec __user *u_ts));
COBALT_SYSCALL_DECL(clock_settime,
- (clockid_t clock_id, const struct timespec __user *u_ts));
+ (clockid_t clock_id,
+ const struct __kernel_timespec __user *u_ts));
COBALT_SYSCALL_DECL(clock_adjtime,
(clockid_t clock_id, struct timex __user *u_tx));
diff --git a/kernel/cobalt/posix/compat.c b/kernel/cobalt/posix/compat.c
index 17968bf..08aedcf 100644
--- a/kernel/cobalt/posix/compat.c
+++ b/kernel/cobalt/posix/compat.c
@@ -32,6 +32,17 @@ int sys32_get_timespec(struct timespec *ts,
}
EXPORT_SYMBOL_GPL(sys32_get_timespec);
+int sys64_get_timespec(struct timespec64 *ts,
+ const struct compat_timespec __user *cts)
+{
+ return (cts == NULL ||
+ !access_rok(cts, sizeof(*cts)) ||
+ __xn_get_user(ts->tv_sec, &cts->tv_sec) ||
+ __xn_get_user(ts->tv_nsec, &cts->tv_nsec)) ? -EFAULT : 0;
+}
+EXPORT_SYMBOL_GPL(sys64_get_timespec);
+
+
int sys32_put_timespec(struct compat_timespec __user *cts,
const struct timespec *ts)
{
diff --git a/kernel/cobalt/posix/syscall32.c b/kernel/cobalt/posix/syscall32.c
index faa7ef5..4b1dfcc 100644
--- a/kernel/cobalt/posix/syscall32.c
+++ b/kernel/cobalt/posix/syscall32.c
@@ -161,14 +161,14 @@ COBALT_SYSCALL32emu(clock_settime, current,
(clockid_t clock_id,
const struct compat_timespec __user *u_ts))
{
- struct timespec ts;
+ struct timespec64 new_ts;
int ret;
- ret = sys32_get_timespec(&ts, u_ts);
+ ret = sys64_get_timespec(&new_ts, u_ts);
if (ret)
return ret;
- return __cobalt_clock_settime(clock_id, &ts);
+ return __cobalt_clock_settime(clock_id, &new_ts);
}
COBALT_SYSCALL32emu(clock_adjtime, current,
diff --git a/kernel/cobalt/trace/cobalt-posix.h b/kernel/cobalt/trace/cobalt-posix.h
index aa78efb..f764872 100644
--- a/kernel/cobalt/trace/cobalt-posix.h
+++ b/kernel/cobalt/trace/cobalt-posix.h
@@ -748,6 +748,26 @@ DECLARE_EVENT_CLASS(cobalt_clock_timespec,
)
);
+DECLARE_EVENT_CLASS(cobalt_clock_timespec64,
+ TP_PROTO(clockid_t clk_id, const struct timespec64 *val),
+ TP_ARGS(clk_id, val),
+
+ TP_STRUCT__entry(
+ __field(clockid_t, clk_id)
+ __timespec_fields(val)
+ ),
+
+ TP_fast_assign(
+ __entry->clk_id = clk_id;
+ __assign_timespec(val, val);
+ ),
+
+ TP_printk("clock_id=%d timeval=(%ld.%09ld)",
+ __entry->clk_id,
+ __timespec_args(val)
+ )
+);
+
DEFINE_EVENT(cobalt_clock_timespec, cobalt_clock_getres,
TP_PROTO(clockid_t clk_id, const struct timespec *res),
TP_ARGS(clk_id, res)
@@ -758,8 +778,8 @@ DEFINE_EVENT(cobalt_clock_timespec, cobalt_clock_gettime,
TP_ARGS(clk_id, time)
);
-DEFINE_EVENT(cobalt_clock_timespec, cobalt_clock_settime,
- TP_PROTO(clockid_t clk_id, const struct timespec *time),
+DEFINE_EVENT(cobalt_clock_timespec64, cobalt_clock_settime,
+ TP_PROTO(clockid_t clk_id, const struct timespec64 *time),
TP_ARGS(clk_id, time)
);
--
2.7.4
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH V2] y2038: replace timespec with timespec64 in clock_settime
2020-09-29 7:19 [PATCH V2] y2038: replace timespec with timespec64 in clock_settime chensong
@ 2020-10-27 16:33 ` Jan Kiszka
2020-10-30 8:50 ` chensong
0 siblings, 1 reply; 5+ messages in thread
From: Jan Kiszka @ 2020-10-27 16:33 UTC (permalink / raw)
To: chensong, xenomai
On 29.09.20 09:19, chensong wrote:
> Upstream has used timespec64 to replace timespec inside kernel and
> used __kernel_timespec to replace timespect in syscalls, therefore
> we must keep aligned with upstream.
>
> clock_settime is a point to get started at, it involves 2 parts:
> 1, syscall in ./include/trace/events/cobalt-posix.h for 64bits
> 2, compat syscall in kernel/cobalt/posix/syscall32.c for 32bits
> 3, define __kernel_timespec in wrapper.h for compatibility
>
> some new functions are implemented to keep the compatibility with
> other syscalls which haven't been handled yet and will be removed
> in the end of the work.
>
> Signed-off-by: chensong <chensong@tj.kylinos.cn>
>
> CC: Jan Kiszka <jan.kiszka@siemens.com>
> CC: Henning Schild <henning.schild@siemens.com>
> ---
> include/cobalt/kernel/clock.h | 6 ++---
> include/cobalt/kernel/compat.h | 3 +++
> .../cobalt/include/asm-generic/xenomai/wrappers.h | 9 +++++++
> kernel/cobalt/posix/clock.c | 28 +++++++++++++++++-----
> kernel/cobalt/posix/clock.h | 15 ++++++++++--
> kernel/cobalt/posix/compat.c | 11 +++++++++
> kernel/cobalt/posix/syscall32.c | 6 ++---
> kernel/cobalt/trace/cobalt-posix.h | 24 +++++++++++++++++--
> 8 files changed, 86 insertions(+), 16 deletions(-)
>
> diff --git a/include/cobalt/kernel/clock.h b/include/cobalt/kernel/clock.h
> index c26776a..cf9d5ec 100644
> --- a/include/cobalt/kernel/clock.h
> +++ b/include/cobalt/kernel/clock.h
> @@ -52,7 +52,7 @@ struct xnclock {
> xnticks_t (*read_raw)(struct xnclock *clock);
> xnticks_t (*read_monotonic)(struct xnclock *clock);
> int (*set_time)(struct xnclock *clock,
> - const struct timespec *ts);
> + const struct timespec64 *ts);
> xnsticks_t (*ns_to_ticks)(struct xnclock *clock,
> xnsticks_t ns);
> xnsticks_t (*ticks_to_ns)(struct xnclock *clock,
> @@ -213,7 +213,7 @@ static inline xnticks_t xnclock_read_monotonic(struct xnclock *clock)
> }
>
> static inline int xnclock_set_time(struct xnclock *clock,
> - const struct timespec *ts)
> + const struct timespec64 *ts)
> {
> if (likely(clock == &nkclock))
> return -EINVAL;
> @@ -266,7 +266,7 @@ static inline xnticks_t xnclock_read_monotonic(struct xnclock *clock)
> }
>
> static inline int xnclock_set_time(struct xnclock *clock,
> - const struct timespec *ts)
> + const struct timespec64 *ts)
> {
> /*
> * There is no way to change the core clock's idea of time.
> diff --git a/include/cobalt/kernel/compat.h b/include/cobalt/kernel/compat.h
> index 05754cb..70e2c03 100644
> --- a/include/cobalt/kernel/compat.h
> +++ b/include/cobalt/kernel/compat.h
> @@ -89,6 +89,9 @@ struct compat_rtdm_mmap_request {
> int sys32_get_timespec(struct timespec *ts,
> const struct compat_timespec __user *cts);
>
> +int sys64_get_timespec(struct timespec64 *ts,
> + const struct compat_timespec __user *cts);
> +
> int sys32_put_timespec(struct compat_timespec __user *cts,
> const struct timespec *ts);
>
> diff --git a/kernel/cobalt/include/asm-generic/xenomai/wrappers.h b/kernel/cobalt/include/asm-generic/xenomai/wrappers.h
> index 23993dc..e5375de 100644
> --- a/kernel/cobalt/include/asm-generic/xenomai/wrappers.h
> +++ b/kernel/cobalt/include/asm-generic/xenomai/wrappers.h
> @@ -156,4 +156,13 @@ devm_hwmon_device_register_with_groups(struct device *dev, const char *name,
> #error "Xenomai/cobalt requires Linux kernel 3.10 or above"
> #endif /* < 3.10 */
>
> +#if LINUX_VERSION_CODE < KERNEL_VERSION(4, 18, 0)
> +typedef long long __kernel_time64_t;
> +struct __kernel_timespec {
> + __kernel_time64_t tv_sec; /* seconds */
> + long long tv_nsec; /* nanoseconds */
> +};
> +#endif /* < 4.18 */
> +
> +
> #endif /* _COBALT_ASM_GENERIC_WRAPPERS_H */
> diff --git a/kernel/cobalt/posix/clock.c b/kernel/cobalt/posix/clock.c
> index 561358e..c960d7b 100644
> --- a/kernel/cobalt/posix/clock.c
> +++ b/kernel/cobalt/posix/clock.c
> @@ -194,7 +194,7 @@ COBALT_SYSCALL(clock_gettime, current,
> return 0;
> }
>
> -int __cobalt_clock_settime(clockid_t clock_id, const struct timespec *ts)
> +int __cobalt_clock_settime(clockid_t clock_id, const struct timespec64 *ts)
> {
> int _ret, ret = 0;
> xnticks_t now;
> @@ -207,7 +207,7 @@ int __cobalt_clock_settime(clockid_t clock_id, const struct timespec *ts)
> case CLOCK_REALTIME:
> xnlock_get_irqsave(&nklock, s);
> now = xnclock_read_realtime(&nkclock);
> - xnclock_adjust(&nkclock, (xnsticks_t) (ts2ns(ts) - now));
> + xnclock_adjust(&nkclock, (xnsticks_t) (ts2ns64(ts) - now));
> xnlock_put_irqrestore(&nklock, s);
> break;
> default:
> @@ -242,15 +242,31 @@ int __cobalt_clock_adjtime(clockid_t clock_id, struct timex *tx)
> return 0;
> }
>
> +static int __cobalt_get_timespec64(struct timespec64 *ts,
> + const struct __kernel_timespec __user *uts)
> +{
> + struct __kernel_timespec kts;
> + int ret;
> +
> + ret = cobalt_copy_from_user(&kts, uts, sizeof(kts));
> + if (ret)
> + return -EFAULT;
> +
> + ts->tv_sec = kts.tv_sec;
> + ts->tv_nsec = kts.tv_nsec;
> +
> + return 0;
> +
> +}
> COBALT_SYSCALL(clock_settime, current,
> - (clockid_t clock_id, const struct timespec __user *u_ts))
> + (clockid_t clock_id, const struct __kernel_timespec __user *u_ts))
This is still a point where I see ABI breakage: On a 32-bit system,
struct timespec was using a 32-bit tv_sec. Now with struct
_kernel_timespec, this will become a 64-bit value. That is what we want
for new applications being built after this patch. However, existing
application - and that includes existing Xenomai libs built before this
change (stable kernel ABI...) will continue to deliver 32-bit tv_sec
here and break over this syscall change.
To model such a change in an ABI-preserving way, we need some
"clock_settime64" syscall that new userspace will pick by default, and
an unmodified (those 2038-wise broken) clock_settime. Same for all other
syscalls and drivers taking time_t directly or indirectly.
Jan
> {
> - struct timespec ts;
> + struct timespec64 new_ts;
>
> - if (cobalt_copy_from_user(&ts, u_ts, sizeof(ts)))
> + if (__cobalt_get_timespec64(&new_ts, u_ts))
> return -EFAULT;
>
> - return __cobalt_clock_settime(clock_id, &ts);
> + return __cobalt_clock_settime(clock_id, &new_ts);
> }
>
> COBALT_SYSCALL(clock_adjtime, current,
> diff --git a/kernel/cobalt/posix/clock.h b/kernel/cobalt/posix/clock.h
> index 0b06b93..d7328b0 100644
> --- a/kernel/cobalt/posix/clock.h
> +++ b/kernel/cobalt/posix/clock.h
> @@ -43,6 +43,16 @@ static inline xnticks_t ts2ns(const struct timespec *ts)
> return nsecs;
> }
>
> +static inline xnticks_t ts2ns64(const struct timespec64 *ts)
> +{
> + xnticks_t nsecs = ts->tv_nsec;
> +
> + if (ts->tv_sec)
> + nsecs += (xnticks_t)ts->tv_sec * ONE_BILLION;
> +
> + return nsecs;
> +}
> +
> static inline xnticks_t tv2ns(const struct timeval *tv)
> {
> xnticks_t nsecs = tv->tv_usec * 1000;
> @@ -86,7 +96,7 @@ int __cobalt_clock_gettime(clockid_t clock_id,
> struct timespec *ts);
>
> int __cobalt_clock_settime(clockid_t clock_id,
> - const struct timespec *ts);
> + const struct timespec64 *ts);
>
> int __cobalt_clock_adjtime(clockid_t clock_id,
> struct timex *tx);
> @@ -102,7 +112,8 @@ COBALT_SYSCALL_DECL(clock_gettime,
> (clockid_t clock_id, struct timespec __user *u_ts));
>
> COBALT_SYSCALL_DECL(clock_settime,
> - (clockid_t clock_id, const struct timespec __user *u_ts));
> + (clockid_t clock_id,
> + const struct __kernel_timespec __user *u_ts));
>
> COBALT_SYSCALL_DECL(clock_adjtime,
> (clockid_t clock_id, struct timex __user *u_tx));
> diff --git a/kernel/cobalt/posix/compat.c b/kernel/cobalt/posix/compat.c
> index 17968bf..08aedcf 100644
> --- a/kernel/cobalt/posix/compat.c
> +++ b/kernel/cobalt/posix/compat.c
> @@ -32,6 +32,17 @@ int sys32_get_timespec(struct timespec *ts,
> }
> EXPORT_SYMBOL_GPL(sys32_get_timespec);
>
> +int sys64_get_timespec(struct timespec64 *ts,
> + const struct compat_timespec __user *cts)
> +{
> + return (cts == NULL ||
> + !access_rok(cts, sizeof(*cts)) ||
> + __xn_get_user(ts->tv_sec, &cts->tv_sec) ||
> + __xn_get_user(ts->tv_nsec, &cts->tv_nsec)) ? -EFAULT : 0;
> +}
> +EXPORT_SYMBOL_GPL(sys64_get_timespec);
> +
> +
> int sys32_put_timespec(struct compat_timespec __user *cts,
> const struct timespec *ts)
> {
> diff --git a/kernel/cobalt/posix/syscall32.c b/kernel/cobalt/posix/syscall32.c
> index faa7ef5..4b1dfcc 100644
> --- a/kernel/cobalt/posix/syscall32.c
> +++ b/kernel/cobalt/posix/syscall32.c
> @@ -161,14 +161,14 @@ COBALT_SYSCALL32emu(clock_settime, current,
> (clockid_t clock_id,
> const struct compat_timespec __user *u_ts))
> {
> - struct timespec ts;
> + struct timespec64 new_ts;
> int ret;
>
> - ret = sys32_get_timespec(&ts, u_ts);
> + ret = sys64_get_timespec(&new_ts, u_ts);
> if (ret)
> return ret;
>
> - return __cobalt_clock_settime(clock_id, &ts);
> + return __cobalt_clock_settime(clock_id, &new_ts);
> }
>
> COBALT_SYSCALL32emu(clock_adjtime, current,
> diff --git a/kernel/cobalt/trace/cobalt-posix.h b/kernel/cobalt/trace/cobalt-posix.h
> index aa78efb..f764872 100644
> --- a/kernel/cobalt/trace/cobalt-posix.h
> +++ b/kernel/cobalt/trace/cobalt-posix.h
> @@ -748,6 +748,26 @@ DECLARE_EVENT_CLASS(cobalt_clock_timespec,
> )
> );
>
> +DECLARE_EVENT_CLASS(cobalt_clock_timespec64,
> + TP_PROTO(clockid_t clk_id, const struct timespec64 *val),
> + TP_ARGS(clk_id, val),
> +
> + TP_STRUCT__entry(
> + __field(clockid_t, clk_id)
> + __timespec_fields(val)
> + ),
> +
> + TP_fast_assign(
> + __entry->clk_id = clk_id;
> + __assign_timespec(val, val);
> + ),
> +
> + TP_printk("clock_id=%d timeval=(%ld.%09ld)",
> + __entry->clk_id,
> + __timespec_args(val)
> + )
> +);
> +
> DEFINE_EVENT(cobalt_clock_timespec, cobalt_clock_getres,
> TP_PROTO(clockid_t clk_id, const struct timespec *res),
> TP_ARGS(clk_id, res)
> @@ -758,8 +778,8 @@ DEFINE_EVENT(cobalt_clock_timespec, cobalt_clock_gettime,
> TP_ARGS(clk_id, time)
> );
>
> -DEFINE_EVENT(cobalt_clock_timespec, cobalt_clock_settime,
> - TP_PROTO(clockid_t clk_id, const struct timespec *time),
> +DEFINE_EVENT(cobalt_clock_timespec64, cobalt_clock_settime,
> + TP_PROTO(clockid_t clk_id, const struct timespec64 *time),
> TP_ARGS(clk_id, time)
> );
>
>
--
Siemens AG, T RDA IOT
Corporate Competence Center Embedded Linux
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH V2] y2038: replace timespec with timespec64 in clock_settime
2020-10-27 16:33 ` Jan Kiszka
@ 2020-10-30 8:50 ` chensong
2020-10-30 9:09 ` Jan Kiszka
0 siblings, 1 reply; 5+ messages in thread
From: chensong @ 2020-10-30 8:50 UTC (permalink / raw)
To: Jan Kiszka, xenomai
On 2020年10月28日 00:33, Jan Kiszka wrote:
> On 29.09.20 09:19, chensong wrote:
>> Upstream has used timespec64 to replace timespec inside kernel and
>> used __kernel_timespec to replace timespect in syscalls, therefore
>> we must keep aligned with upstream.
>>
>> clock_settime is a point to get started at, it involves 2 parts:
>> 1, syscall in ./include/trace/events/cobalt-posix.h for 64bits
>> 2, compat syscall in kernel/cobalt/posix/syscall32.c for 32bits
>> 3, define __kernel_timespec in wrapper.h for compatibility
>>
>> some new functions are implemented to keep the compatibility with
>> other syscalls which haven't been handled yet and will be removed
>> in the end of the work.
>>
>> Signed-off-by: chensong <chensong@tj.kylinos.cn>
>>
>> CC: Jan Kiszka <jan.kiszka@siemens.com>
>> CC: Henning Schild <henning.schild@siemens.com>
>> ---
>> include/cobalt/kernel/clock.h | 6 ++---
>> include/cobalt/kernel/compat.h | 3 +++
>> .../cobalt/include/asm-generic/xenomai/wrappers.h | 9 +++++++
>> kernel/cobalt/posix/clock.c | 28 +++++++++++++++++-----
>> kernel/cobalt/posix/clock.h | 15 ++++++++++--
>> kernel/cobalt/posix/compat.c | 11 +++++++++
>> kernel/cobalt/posix/syscall32.c | 6 ++---
>> kernel/cobalt/trace/cobalt-posix.h | 24 +++++++++++++++++--
>> 8 files changed, 86 insertions(+), 16 deletions(-)
>>
>> diff --git a/include/cobalt/kernel/clock.h b/include/cobalt/kernel/clock.h
>> index c26776a..cf9d5ec 100644
>> --- a/include/cobalt/kernel/clock.h
>> +++ b/include/cobalt/kernel/clock.h
>> @@ -52,7 +52,7 @@ struct xnclock {
>> xnticks_t (*read_raw)(struct xnclock *clock);
>> xnticks_t (*read_monotonic)(struct xnclock *clock);
>> int (*set_time)(struct xnclock *clock,
>> - const struct timespec *ts);
>> + const struct timespec64 *ts);
>> xnsticks_t (*ns_to_ticks)(struct xnclock *clock,
>> xnsticks_t ns);
>> xnsticks_t (*ticks_to_ns)(struct xnclock *clock,
>> @@ -213,7 +213,7 @@ static inline xnticks_t xnclock_read_monotonic(struct xnclock *clock)
>> }
>>
>> static inline int xnclock_set_time(struct xnclock *clock,
>> - const struct timespec *ts)
>> + const struct timespec64 *ts)
>> {
>> if (likely(clock == &nkclock))
>> return -EINVAL;
>> @@ -266,7 +266,7 @@ static inline xnticks_t xnclock_read_monotonic(struct xnclock *clock)
>> }
>>
>> static inline int xnclock_set_time(struct xnclock *clock,
>> - const struct timespec *ts)
>> + const struct timespec64 *ts)
>> {
>> /*
>> * There is no way to change the core clock's idea of time.
>> diff --git a/include/cobalt/kernel/compat.h b/include/cobalt/kernel/compat.h
>> index 05754cb..70e2c03 100644
>> --- a/include/cobalt/kernel/compat.h
>> +++ b/include/cobalt/kernel/compat.h
>> @@ -89,6 +89,9 @@ struct compat_rtdm_mmap_request {
>> int sys32_get_timespec(struct timespec *ts,
>> const struct compat_timespec __user *cts);
>>
>> +int sys64_get_timespec(struct timespec64 *ts,
>> + const struct compat_timespec __user *cts);
>> +
>> int sys32_put_timespec(struct compat_timespec __user *cts,
>> const struct timespec *ts);
>>
>> diff --git a/kernel/cobalt/include/asm-generic/xenomai/wrappers.h b/kernel/cobalt/include/asm-generic/xenomai/wrappers.h
>> index 23993dc..e5375de 100644
>> --- a/kernel/cobalt/include/asm-generic/xenomai/wrappers.h
>> +++ b/kernel/cobalt/include/asm-generic/xenomai/wrappers.h
>> @@ -156,4 +156,13 @@ devm_hwmon_device_register_with_groups(struct device *dev, const char *name,
>> #error "Xenomai/cobalt requires Linux kernel 3.10 or above"
>> #endif /* < 3.10 */
>>
>> +#if LINUX_VERSION_CODE < KERNEL_VERSION(4, 18, 0)
>> +typedef long long __kernel_time64_t;
>> +struct __kernel_timespec {
>> + __kernel_time64_t tv_sec; /* seconds */
>> + long long tv_nsec; /* nanoseconds */
>> +};
>> +#endif /* < 4.18 */
>> +
>> +
>> #endif /* _COBALT_ASM_GENERIC_WRAPPERS_H */
>> diff --git a/kernel/cobalt/posix/clock.c b/kernel/cobalt/posix/clock.c
>> index 561358e..c960d7b 100644
>> --- a/kernel/cobalt/posix/clock.c
>> +++ b/kernel/cobalt/posix/clock.c
>> @@ -194,7 +194,7 @@ COBALT_SYSCALL(clock_gettime, current,
>> return 0;
>> }
>>
>> -int __cobalt_clock_settime(clockid_t clock_id, const struct timespec *ts)
>> +int __cobalt_clock_settime(clockid_t clock_id, const struct timespec64 *ts)
>> {
>> int _ret, ret = 0;
>> xnticks_t now;
>> @@ -207,7 +207,7 @@ int __cobalt_clock_settime(clockid_t clock_id, const struct timespec *ts)
>> case CLOCK_REALTIME:
>> xnlock_get_irqsave(&nklock, s);
>> now = xnclock_read_realtime(&nkclock);
>> - xnclock_adjust(&nkclock, (xnsticks_t) (ts2ns(ts) - now));
>> + xnclock_adjust(&nkclock, (xnsticks_t) (ts2ns64(ts) - now));
>> xnlock_put_irqrestore(&nklock, s);
>> break;
>> default:
>> @@ -242,15 +242,31 @@ int __cobalt_clock_adjtime(clockid_t clock_id, struct timex *tx)
>> return 0;
>> }
>>
>> +static int __cobalt_get_timespec64(struct timespec64 *ts,
>> + const struct __kernel_timespec __user *uts)
>> +{
>> + struct __kernel_timespec kts;
>> + int ret;
>> +
>> + ret = cobalt_copy_from_user(&kts, uts, sizeof(kts));
>> + if (ret)
>> + return -EFAULT;
>> +
>> + ts->tv_sec = kts.tv_sec;
>> + ts->tv_nsec = kts.tv_nsec;
>> +
>> + return 0;
>> +
>> +}
>> COBALT_SYSCALL(clock_settime, current,
>> - (clockid_t clock_id, const struct timespec __user *u_ts))
>> + (clockid_t clock_id, const struct __kernel_timespec __user *u_ts))
>
> This is still a point where I see ABI breakage: On a 32-bit system,
> struct timespec was using a 32-bit tv_sec. Now with struct
> _kernel_timespec, this will become a 64-bit value. That is what we want
> for new applications being built after this patch. However, existing
> application - and that includes existing Xenomai libs built before this
> change (stable kernel ABI...) will continue to deliver 32-bit tv_sec
> here and break over this syscall change.
>
> To model such a change in an ABI-preserving way, we need some
> "clock_settime64" syscall that new userspace will pick by default, and
> an unmodified (those 2038-wise broken) clock_settime. Same for all other
> syscalls and drivers taking time_t directly or indirectly.
>
> Jan
how about this way:
we copy the idea from musl's clock_settime(./src/time/clock_settime.c)
which is:
#define IS32BIT(x) !((x)+0x80000000ULL>>32)
int clock_settime(clockid_t clk, const struct timespec *ts)
{
#ifdef SYS_clock_settime64
time_t s = ts->tv_sec;
long ns = ts->tv_nsec;
int r = -ENOSYS;
if (SYS_clock_settime == SYS_clock_settime64 || !IS32BIT(s))
r = __syscall(SYS_clock_settime64, clk,
((long long[]){s, ns}));
if (SYS_clock_settime == SYS_clock_settime64 || r!=-ENOSYS)
return __syscall_ret(r);
if (!IS32BIT(s))
return __syscall_ret(-ENOTSUP);
return syscall(SYS_clock_settime, clk, ((long[]){s, ns}));
#else
return syscall(SYS_clock_settime, clk, ts);
#endif
}
to ./lib/cobalt/clock.c, which is
COBALT_IMPL(int, clock_settime, (clockid_t clock_id, const struct
timespec *tp))
{
int ret;
ret = -XENOMAI_SYSCALL2(sc_cobalt_clock_settime, clock_id, tp);
if (ret) {
errno = ret;
return -1;
}
return 0;
}
in userspace, we use IS32BIT to check if tv_sec is 32bits, and switch it
to long long and pass to kernel space.
chensong
>> {
>> - struct timespec ts;
>> + struct timespec64 new_ts;
>>
>> - if (cobalt_copy_from_user(&ts, u_ts, sizeof(ts)))
>> + if (__cobalt_get_timespec64(&new_ts, u_ts))
>> return -EFAULT;
>>
>> - return __cobalt_clock_settime(clock_id, &ts);
>> + return __cobalt_clock_settime(clock_id, &new_ts);
>> }
>>
>> COBALT_SYSCALL(clock_adjtime, current,
>> diff --git a/kernel/cobalt/posix/clock.h b/kernel/cobalt/posix/clock.h
>> index 0b06b93..d7328b0 100644
>> --- a/kernel/cobalt/posix/clock.h
>> +++ b/kernel/cobalt/posix/clock.h
>> @@ -43,6 +43,16 @@ static inline xnticks_t ts2ns(const struct timespec *ts)
>> return nsecs;
>> }
>>
>> +static inline xnticks_t ts2ns64(const struct timespec64 *ts)
>> +{
>> + xnticks_t nsecs = ts->tv_nsec;
>> +
>> + if (ts->tv_sec)
>> + nsecs += (xnticks_t)ts->tv_sec * ONE_BILLION;
>> +
>> + return nsecs;
>> +}
>> +
>> static inline xnticks_t tv2ns(const struct timeval *tv)
>> {
>> xnticks_t nsecs = tv->tv_usec * 1000;
>> @@ -86,7 +96,7 @@ int __cobalt_clock_gettime(clockid_t clock_id,
>> struct timespec *ts);
>>
>> int __cobalt_clock_settime(clockid_t clock_id,
>> - const struct timespec *ts);
>> + const struct timespec64 *ts);
>>
>> int __cobalt_clock_adjtime(clockid_t clock_id,
>> struct timex *tx);
>> @@ -102,7 +112,8 @@ COBALT_SYSCALL_DECL(clock_gettime,
>> (clockid_t clock_id, struct timespec __user *u_ts));
>>
>> COBALT_SYSCALL_DECL(clock_settime,
>> - (clockid_t clock_id, const struct timespec __user *u_ts));
>> + (clockid_t clock_id,
>> + const struct __kernel_timespec __user *u_ts));
>>
>> COBALT_SYSCALL_DECL(clock_adjtime,
>> (clockid_t clock_id, struct timex __user *u_tx));
>> diff --git a/kernel/cobalt/posix/compat.c b/kernel/cobalt/posix/compat.c
>> index 17968bf..08aedcf 100644
>> --- a/kernel/cobalt/posix/compat.c
>> +++ b/kernel/cobalt/posix/compat.c
>> @@ -32,6 +32,17 @@ int sys32_get_timespec(struct timespec *ts,
>> }
>> EXPORT_SYMBOL_GPL(sys32_get_timespec);
>>
>> +int sys64_get_timespec(struct timespec64 *ts,
>> + const struct compat_timespec __user *cts)
>> +{
>> + return (cts == NULL ||
>> + !access_rok(cts, sizeof(*cts)) ||
>> + __xn_get_user(ts->tv_sec, &cts->tv_sec) ||
>> + __xn_get_user(ts->tv_nsec, &cts->tv_nsec)) ? -EFAULT : 0;
>> +}
>> +EXPORT_SYMBOL_GPL(sys64_get_timespec);
>> +
>> +
>> int sys32_put_timespec(struct compat_timespec __user *cts,
>> const struct timespec *ts)
>> {
>> diff --git a/kernel/cobalt/posix/syscall32.c b/kernel/cobalt/posix/syscall32.c
>> index faa7ef5..4b1dfcc 100644
>> --- a/kernel/cobalt/posix/syscall32.c
>> +++ b/kernel/cobalt/posix/syscall32.c
>> @@ -161,14 +161,14 @@ COBALT_SYSCALL32emu(clock_settime, current,
>> (clockid_t clock_id,
>> const struct compat_timespec __user *u_ts))
>> {
>> - struct timespec ts;
>> + struct timespec64 new_ts;
>> int ret;
>>
>> - ret = sys32_get_timespec(&ts, u_ts);
>> + ret = sys64_get_timespec(&new_ts, u_ts);
>> if (ret)
>> return ret;
>>
>> - return __cobalt_clock_settime(clock_id, &ts);
>> + return __cobalt_clock_settime(clock_id, &new_ts);
>> }
>>
>> COBALT_SYSCALL32emu(clock_adjtime, current,
>> diff --git a/kernel/cobalt/trace/cobalt-posix.h b/kernel/cobalt/trace/cobalt-posix.h
>> index aa78efb..f764872 100644
>> --- a/kernel/cobalt/trace/cobalt-posix.h
>> +++ b/kernel/cobalt/trace/cobalt-posix.h
>> @@ -748,6 +748,26 @@ DECLARE_EVENT_CLASS(cobalt_clock_timespec,
>> )
>> );
>>
>> +DECLARE_EVENT_CLASS(cobalt_clock_timespec64,
>> + TP_PROTO(clockid_t clk_id, const struct timespec64 *val),
>> + TP_ARGS(clk_id, val),
>> +
>> + TP_STRUCT__entry(
>> + __field(clockid_t, clk_id)
>> + __timespec_fields(val)
>> + ),
>> +
>> + TP_fast_assign(
>> + __entry->clk_id = clk_id;
>> + __assign_timespec(val, val);
>> + ),
>> +
>> + TP_printk("clock_id=%d timeval=(%ld.%09ld)",
>> + __entry->clk_id,
>> + __timespec_args(val)
>> + )
>> +);
>> +
>> DEFINE_EVENT(cobalt_clock_timespec, cobalt_clock_getres,
>> TP_PROTO(clockid_t clk_id, const struct timespec *res),
>> TP_ARGS(clk_id, res)
>> @@ -758,8 +778,8 @@ DEFINE_EVENT(cobalt_clock_timespec, cobalt_clock_gettime,
>> TP_ARGS(clk_id, time)
>> );
>>
>> -DEFINE_EVENT(cobalt_clock_timespec, cobalt_clock_settime,
>> - TP_PROTO(clockid_t clk_id, const struct timespec *time),
>> +DEFINE_EVENT(cobalt_clock_timespec64, cobalt_clock_settime,
>> + TP_PROTO(clockid_t clk_id, const struct timespec64 *time),
>> TP_ARGS(clk_id, time)
>> );
>>
>>
>
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH V2] y2038: replace timespec with timespec64 in clock_settime
2020-10-30 8:50 ` chensong
@ 2020-10-30 9:09 ` Jan Kiszka
2020-11-03 2:59 ` chensong
0 siblings, 1 reply; 5+ messages in thread
From: Jan Kiszka @ 2020-10-30 9:09 UTC (permalink / raw)
To: chensong, xenomai
On 30.10.20 09:50, chensong wrote:
>
>
> On 2020年10月28日 00:33, Jan Kiszka wrote:
>> On 29.09.20 09:19, chensong wrote:
>>> Upstream has used timespec64 to replace timespec inside kernel and
>>> used __kernel_timespec to replace timespect in syscalls, therefore
>>> we must keep aligned with upstream.
>>>
>>> clock_settime is a point to get started at, it involves 2 parts:
>>> 1, syscall in ./include/trace/events/cobalt-posix.h for 64bits
>>> 2, compat syscall in kernel/cobalt/posix/syscall32.c for 32bits
>>> 3, define __kernel_timespec in wrapper.h for compatibility
>>>
>>> some new functions are implemented to keep the compatibility with
>>> other syscalls which haven't been handled yet and will be removed
>>> in the end of the work.
>>>
>>> Signed-off-by: chensong <chensong@tj.kylinos.cn>
>>>
>>> CC: Jan Kiszka <jan.kiszka@siemens.com>
>>> CC: Henning Schild <henning.schild@siemens.com>
>>> ---
>>> include/cobalt/kernel/clock.h | 6 ++---
>>> include/cobalt/kernel/compat.h | 3 +++
>>> .../cobalt/include/asm-generic/xenomai/wrappers.h | 9 +++++++
>>> kernel/cobalt/posix/clock.c | 28
>>> +++++++++++++++++-----
>>> kernel/cobalt/posix/clock.h | 15 ++++++++++--
>>> kernel/cobalt/posix/compat.c | 11 +++++++++
>>> kernel/cobalt/posix/syscall32.c | 6 ++---
>>> kernel/cobalt/trace/cobalt-posix.h | 24
>>> +++++++++++++++++--
>>> 8 files changed, 86 insertions(+), 16 deletions(-)
>>>
>>> diff --git a/include/cobalt/kernel/clock.h
>>> b/include/cobalt/kernel/clock.h
>>> index c26776a..cf9d5ec 100644
>>> --- a/include/cobalt/kernel/clock.h
>>> +++ b/include/cobalt/kernel/clock.h
>>> @@ -52,7 +52,7 @@ struct xnclock {
>>> xnticks_t (*read_raw)(struct xnclock *clock);
>>> xnticks_t (*read_monotonic)(struct xnclock *clock);
>>> int (*set_time)(struct xnclock *clock,
>>> - const struct timespec *ts);
>>> + const struct timespec64 *ts);
>>> xnsticks_t (*ns_to_ticks)(struct xnclock *clock,
>>> xnsticks_t ns);
>>> xnsticks_t (*ticks_to_ns)(struct xnclock *clock,
>>> @@ -213,7 +213,7 @@ static inline xnticks_t
>>> xnclock_read_monotonic(struct xnclock *clock)
>>> }
>>>
>>> static inline int xnclock_set_time(struct xnclock *clock,
>>> - const struct timespec *ts)
>>> + const struct timespec64 *ts)
>>> {
>>> if (likely(clock == &nkclock))
>>> return -EINVAL;
>>> @@ -266,7 +266,7 @@ static inline xnticks_t
>>> xnclock_read_monotonic(struct xnclock *clock)
>>> }
>>>
>>> static inline int xnclock_set_time(struct xnclock *clock,
>>> - const struct timespec *ts)
>>> + const struct timespec64 *ts)
>>> {
>>> /*
>>> * There is no way to change the core clock's idea of time.
>>> diff --git a/include/cobalt/kernel/compat.h
>>> b/include/cobalt/kernel/compat.h
>>> index 05754cb..70e2c03 100644
>>> --- a/include/cobalt/kernel/compat.h
>>> +++ b/include/cobalt/kernel/compat.h
>>> @@ -89,6 +89,9 @@ struct compat_rtdm_mmap_request {
>>> int sys32_get_timespec(struct timespec *ts,
>>> const struct compat_timespec __user *cts);
>>>
>>> +int sys64_get_timespec(struct timespec64 *ts,
>>> + const struct compat_timespec __user *cts);
>>> +
>>> int sys32_put_timespec(struct compat_timespec __user *cts,
>>> const struct timespec *ts);
>>>
>>> diff --git a/kernel/cobalt/include/asm-generic/xenomai/wrappers.h
>>> b/kernel/cobalt/include/asm-generic/xenomai/wrappers.h
>>> index 23993dc..e5375de 100644
>>> --- a/kernel/cobalt/include/asm-generic/xenomai/wrappers.h
>>> +++ b/kernel/cobalt/include/asm-generic/xenomai/wrappers.h
>>> @@ -156,4 +156,13 @@ devm_hwmon_device_register_with_groups(struct
>>> device *dev, const char *name,
>>> #error "Xenomai/cobalt requires Linux kernel 3.10 or above"
>>> #endif /* < 3.10 */
>>>
>>> +#if LINUX_VERSION_CODE < KERNEL_VERSION(4, 18, 0)
>>> +typedef long long __kernel_time64_t;
>>> +struct __kernel_timespec {
>>> + __kernel_time64_t tv_sec; /* seconds */
>>> + long long tv_nsec; /* nanoseconds */
>>> +};
>>> +#endif /* < 4.18 */
>>> +
>>> +
>>> #endif /* _COBALT_ASM_GENERIC_WRAPPERS_H */
>>> diff --git a/kernel/cobalt/posix/clock.c b/kernel/cobalt/posix/clock.c
>>> index 561358e..c960d7b 100644
>>> --- a/kernel/cobalt/posix/clock.c
>>> +++ b/kernel/cobalt/posix/clock.c
>>> @@ -194,7 +194,7 @@ COBALT_SYSCALL(clock_gettime, current,
>>> return 0;
>>> }
>>>
>>> -int __cobalt_clock_settime(clockid_t clock_id, const struct timespec
>>> *ts)
>>> +int __cobalt_clock_settime(clockid_t clock_id, const struct
>>> timespec64 *ts)
>>> {
>>> int _ret, ret = 0;
>>> xnticks_t now;
>>> @@ -207,7 +207,7 @@ int __cobalt_clock_settime(clockid_t clock_id,
>>> const struct timespec *ts)
>>> case CLOCK_REALTIME:
>>> xnlock_get_irqsave(&nklock, s);
>>> now = xnclock_read_realtime(&nkclock);
>>> - xnclock_adjust(&nkclock, (xnsticks_t) (ts2ns(ts) - now));
>>> + xnclock_adjust(&nkclock, (xnsticks_t) (ts2ns64(ts) - now));
>>> xnlock_put_irqrestore(&nklock, s);
>>> break;
>>> default:
>>> @@ -242,15 +242,31 @@ int __cobalt_clock_adjtime(clockid_t clock_id,
>>> struct timex *tx)
>>> return 0;
>>> }
>>>
>>> +static int __cobalt_get_timespec64(struct timespec64 *ts,
>>> + const struct __kernel_timespec __user *uts)
>>> +{
>>> + struct __kernel_timespec kts;
>>> + int ret;
>>> +
>>> + ret = cobalt_copy_from_user(&kts, uts, sizeof(kts));
>>> + if (ret)
>>> + return -EFAULT;
>>> +
>>> + ts->tv_sec = kts.tv_sec;
>>> + ts->tv_nsec = kts.tv_nsec;
>>> +
>>> + return 0;
>>> +
>>> +}
>>> COBALT_SYSCALL(clock_settime, current,
>>> - (clockid_t clock_id, const struct timespec __user *u_ts))
>>> + (clockid_t clock_id, const struct __kernel_timespec __user *u_ts))
>>
>> This is still a point where I see ABI breakage: On a 32-bit system,
>> struct timespec was using a 32-bit tv_sec. Now with struct
>> _kernel_timespec, this will become a 64-bit value. That is what we want
>> for new applications being built after this patch. However, existing
>> application - and that includes existing Xenomai libs built before this
>> change (stable kernel ABI...) will continue to deliver 32-bit tv_sec
>> here and break over this syscall change.
>>
>> To model such a change in an ABI-preserving way, we need some
>> "clock_settime64" syscall that new userspace will pick by default, and
>> an unmodified (those 2038-wise broken) clock_settime. Same for all other
>> syscalls and drivers taking time_t directly or indirectly.
>>
>> Jan
>
> how about this way:
>
> we copy the idea from musl's clock_settime(./src/time/clock_settime.c)
> which is:
>
> #define IS32BIT(x) !((x)+0x80000000ULL>>32)
>
> int clock_settime(clockid_t clk, const struct timespec *ts)
> {
> #ifdef SYS_clock_settime64
> time_t s = ts->tv_sec;
> long ns = ts->tv_nsec;
> int r = -ENOSYS;
> if (SYS_clock_settime == SYS_clock_settime64 || !IS32BIT(s))
> r = __syscall(SYS_clock_settime64, clk,
> ((long long[]){s, ns}));
> if (SYS_clock_settime == SYS_clock_settime64 || r!=-ENOSYS)
> return __syscall_ret(r);
> if (!IS32BIT(s))
> return __syscall_ret(-ENOTSUP);
> return syscall(SYS_clock_settime, clk, ((long[]){s, ns}));
> #else
> return syscall(SYS_clock_settime, clk, ts);
> #endif
> }
>
> to ./lib/cobalt/clock.c, which is
>
> COBALT_IMPL(int, clock_settime, (clockid_t clock_id, const struct
> timespec *tp))
> {
> int ret;
>
> ret = -XENOMAI_SYSCALL2(sc_cobalt_clock_settime, clock_id, tp);
> if (ret) {
> errno = ret;
> return -1;
> }
>
> return 0;
> }
>
> in userspace, we use IS32BIT to check if tv_sec is 32bits, and switch it
> to long long and pass to kernel space.
But that does not address the problem that old userspace libs will pass
down 32-bit tv_sec, ie. will hand over smaller data structures. Also
musl expects that there is a SYS_clock_settime64.
At the same time, we won't have to address the case of running a newer
lib over an older kernel.
Jan
--
Siemens AG, T RDA IOT
Corporate Competence Center Embedded Linux
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH V2] y2038: replace timespec with timespec64 in clock_settime
2020-10-30 9:09 ` Jan Kiszka
@ 2020-11-03 2:59 ` chensong
0 siblings, 0 replies; 5+ messages in thread
From: chensong @ 2020-11-03 2:59 UTC (permalink / raw)
To: Jan Kiszka, xenomai
On 2020年10月30日 17:09, Jan Kiszka wrote:
> On 30.10.20 09:50, chensong wrote:
>>
>>
>> On 2020年10月28日 00:33, Jan Kiszka wrote:
>>> On 29.09.20 09:19, chensong wrote:
>>>> Upstream has used timespec64 to replace timespec inside kernel and
>>>> used __kernel_timespec to replace timespect in syscalls, therefore
>>>> we must keep aligned with upstream.
>>>>
>>>> clock_settime is a point to get started at, it involves 2 parts:
>>>> 1, syscall in ./include/trace/events/cobalt-posix.h for 64bits
>>>> 2, compat syscall in kernel/cobalt/posix/syscall32.c for 32bits
>>>> 3, define __kernel_timespec in wrapper.h for compatibility
>>>>
>>>> some new functions are implemented to keep the compatibility with
>>>> other syscalls which haven't been handled yet and will be removed
>>>> in the end of the work.
>>>>
>>>> Signed-off-by: chensong <chensong@tj.kylinos.cn>
>>>>
>>>> CC: Jan Kiszka <jan.kiszka@siemens.com>
>>>> CC: Henning Schild <henning.schild@siemens.com>
>>>> ---
>>>> include/cobalt/kernel/clock.h | 6 ++---
>>>> include/cobalt/kernel/compat.h | 3 +++
>>>> .../cobalt/include/asm-generic/xenomai/wrappers.h | 9 +++++++
>>>> kernel/cobalt/posix/clock.c | 28
>>>> +++++++++++++++++-----
>>>> kernel/cobalt/posix/clock.h | 15 ++++++++++--
>>>> kernel/cobalt/posix/compat.c | 11 +++++++++
>>>> kernel/cobalt/posix/syscall32.c | 6 ++---
>>>> kernel/cobalt/trace/cobalt-posix.h | 24
>>>> +++++++++++++++++--
>>>> 8 files changed, 86 insertions(+), 16 deletions(-)
>>>>
>>>> diff --git a/include/cobalt/kernel/clock.h
>>>> b/include/cobalt/kernel/clock.h
>>>> index c26776a..cf9d5ec 100644
>>>> --- a/include/cobalt/kernel/clock.h
>>>> +++ b/include/cobalt/kernel/clock.h
>>>> @@ -52,7 +52,7 @@ struct xnclock {
>>>> xnticks_t (*read_raw)(struct xnclock *clock);
>>>> xnticks_t (*read_monotonic)(struct xnclock *clock);
>>>> int (*set_time)(struct xnclock *clock,
>>>> - const struct timespec *ts);
>>>> + const struct timespec64 *ts);
>>>> xnsticks_t (*ns_to_ticks)(struct xnclock *clock,
>>>> xnsticks_t ns);
>>>> xnsticks_t (*ticks_to_ns)(struct xnclock *clock,
>>>> @@ -213,7 +213,7 @@ static inline xnticks_t
>>>> xnclock_read_monotonic(struct xnclock *clock)
>>>> }
>>>>
>>>> static inline int xnclock_set_time(struct xnclock *clock,
>>>> - const struct timespec *ts)
>>>> + const struct timespec64 *ts)
>>>> {
>>>> if (likely(clock == &nkclock))
>>>> return -EINVAL;
>>>> @@ -266,7 +266,7 @@ static inline xnticks_t
>>>> xnclock_read_monotonic(struct xnclock *clock)
>>>> }
>>>>
>>>> static inline int xnclock_set_time(struct xnclock *clock,
>>>> - const struct timespec *ts)
>>>> + const struct timespec64 *ts)
>>>> {
>>>> /*
>>>> * There is no way to change the core clock's idea of time.
>>>> diff --git a/include/cobalt/kernel/compat.h
>>>> b/include/cobalt/kernel/compat.h
>>>> index 05754cb..70e2c03 100644
>>>> --- a/include/cobalt/kernel/compat.h
>>>> +++ b/include/cobalt/kernel/compat.h
>>>> @@ -89,6 +89,9 @@ struct compat_rtdm_mmap_request {
>>>> int sys32_get_timespec(struct timespec *ts,
>>>> const struct compat_timespec __user *cts);
>>>>
>>>> +int sys64_get_timespec(struct timespec64 *ts,
>>>> + const struct compat_timespec __user *cts);
>>>> +
>>>> int sys32_put_timespec(struct compat_timespec __user *cts,
>>>> const struct timespec *ts);
>>>>
>>>> diff --git a/kernel/cobalt/include/asm-generic/xenomai/wrappers.h
>>>> b/kernel/cobalt/include/asm-generic/xenomai/wrappers.h
>>>> index 23993dc..e5375de 100644
>>>> --- a/kernel/cobalt/include/asm-generic/xenomai/wrappers.h
>>>> +++ b/kernel/cobalt/include/asm-generic/xenomai/wrappers.h
>>>> @@ -156,4 +156,13 @@ devm_hwmon_device_register_with_groups(struct
>>>> device *dev, const char *name,
>>>> #error "Xenomai/cobalt requires Linux kernel 3.10 or above"
>>>> #endif /* < 3.10 */
>>>>
>>>> +#if LINUX_VERSION_CODE < KERNEL_VERSION(4, 18, 0)
>>>> +typedef long long __kernel_time64_t;
>>>> +struct __kernel_timespec {
>>>> + __kernel_time64_t tv_sec; /* seconds */
>>>> + long long tv_nsec; /* nanoseconds */
>>>> +};
>>>> +#endif /* < 4.18 */
>>>> +
>>>> +
>>>> #endif /* _COBALT_ASM_GENERIC_WRAPPERS_H */
>>>> diff --git a/kernel/cobalt/posix/clock.c b/kernel/cobalt/posix/clock.c
>>>> index 561358e..c960d7b 100644
>>>> --- a/kernel/cobalt/posix/clock.c
>>>> +++ b/kernel/cobalt/posix/clock.c
>>>> @@ -194,7 +194,7 @@ COBALT_SYSCALL(clock_gettime, current,
>>>> return 0;
>>>> }
>>>>
>>>> -int __cobalt_clock_settime(clockid_t clock_id, const struct timespec
>>>> *ts)
>>>> +int __cobalt_clock_settime(clockid_t clock_id, const struct
>>>> timespec64 *ts)
>>>> {
>>>> int _ret, ret = 0;
>>>> xnticks_t now;
>>>> @@ -207,7 +207,7 @@ int __cobalt_clock_settime(clockid_t clock_id,
>>>> const struct timespec *ts)
>>>> case CLOCK_REALTIME:
>>>> xnlock_get_irqsave(&nklock, s);
>>>> now = xnclock_read_realtime(&nkclock);
>>>> - xnclock_adjust(&nkclock, (xnsticks_t) (ts2ns(ts) - now));
>>>> + xnclock_adjust(&nkclock, (xnsticks_t) (ts2ns64(ts) - now));
>>>> xnlock_put_irqrestore(&nklock, s);
>>>> break;
>>>> default:
>>>> @@ -242,15 +242,31 @@ int __cobalt_clock_adjtime(clockid_t clock_id,
>>>> struct timex *tx)
>>>> return 0;
>>>> }
>>>>
>>>> +static int __cobalt_get_timespec64(struct timespec64 *ts,
>>>> + const struct __kernel_timespec __user *uts)
>>>> +{
>>>> + struct __kernel_timespec kts;
>>>> + int ret;
>>>> +
>>>> + ret = cobalt_copy_from_user(&kts, uts, sizeof(kts));
>>>> + if (ret)
>>>> + return -EFAULT;
>>>> +
>>>> + ts->tv_sec = kts.tv_sec;
>>>> + ts->tv_nsec = kts.tv_nsec;
>>>> +
>>>> + return 0;
>>>> +
>>>> +}
>>>> COBALT_SYSCALL(clock_settime, current,
>>>> - (clockid_t clock_id, const struct timespec __user *u_ts))
>>>> + (clockid_t clock_id, const struct __kernel_timespec __user *u_ts))
>>>
>>> This is still a point where I see ABI breakage: On a 32-bit system,
>>> struct timespec was using a 32-bit tv_sec. Now with struct
>>> _kernel_timespec, this will become a 64-bit value. That is what we want
>>> for new applications being built after this patch. However, existing
>>> application - and that includes existing Xenomai libs built before this
>>> change (stable kernel ABI...) will continue to deliver 32-bit tv_sec
>>> here and break over this syscall change.
>>>
>>> To model such a change in an ABI-preserving way, we need some
>>> "clock_settime64" syscall that new userspace will pick by default, and
>>> an unmodified (those 2038-wise broken) clock_settime. Same for all other
>>> syscalls and drivers taking time_t directly or indirectly.
>>>
>>> Jan
>>
>> how about this way:
>>
>> we copy the idea from musl's clock_settime(./src/time/clock_settime.c)
>> which is:
>>
>> #define IS32BIT(x) !((x)+0x80000000ULL>>32)
>>
>> int clock_settime(clockid_t clk, const struct timespec *ts)
>> {
>> #ifdef SYS_clock_settime64
>> time_t s = ts->tv_sec;
>> long ns = ts->tv_nsec;
>> int r = -ENOSYS;
>> if (SYS_clock_settime == SYS_clock_settime64 || !IS32BIT(s))
>> r = __syscall(SYS_clock_settime64, clk,
>> ((long long[]){s, ns}));
>> if (SYS_clock_settime == SYS_clock_settime64 || r!=-ENOSYS)
>> return __syscall_ret(r);
>> if (!IS32BIT(s))
>> return __syscall_ret(-ENOTSUP);
>> return syscall(SYS_clock_settime, clk, ((long[]){s, ns}));
>> #else
>> return syscall(SYS_clock_settime, clk, ts);
>> #endif
>> }
>>
>> to ./lib/cobalt/clock.c, which is
>>
>> COBALT_IMPL(int, clock_settime, (clockid_t clock_id, const struct
>> timespec *tp))
>> {
>> int ret;
>>
>> ret = -XENOMAI_SYSCALL2(sc_cobalt_clock_settime, clock_id, tp);
>> if (ret) {
>> errno = ret;
>> return -1;
>> }
>>
>> return 0;
>> }
>>
>> in userspace, we use IS32BIT to check if tv_sec is 32bits, and switch it
>> to long long and pass to kernel space.
>
> But that does not address the problem that old userspace libs will pass
> down 32-bit tv_sec, ie. will hand over smaller data structures. Also
> musl expects that there is a SYS_clock_settime64.
>
> At the same time, we won't have to address the case of running a newer
> lib over an older kernel.
>
> Jan
point taken, i will submit a patch list with a new implemented
clock_settime64, above problems will be addressed.
chensong
>
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2020-11-03 2:59 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-09-29 7:19 [PATCH V2] y2038: replace timespec with timespec64 in clock_settime chensong
2020-10-27 16:33 ` Jan Kiszka
2020-10-30 8:50 ` chensong
2020-10-30 9:09 ` Jan Kiszka
2020-11-03 2:59 ` chensong
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.