* [PATCH 2/5] cobalt/kernel: introduce old_timespec32 to clock_settime
@ 2020-11-03 3:04 chensong
2020-11-10 10:14 ` Jan Kiszka
0 siblings, 1 reply; 5+ messages in thread
From: chensong @ 2020-11-03 3:04 UTC (permalink / raw)
To: xenomai, jan.kiszka, henning.schild
Replace timespec with old_timespec32 as the parameter passed from
processes with 32bit timespec in clock_settime.
Also, replace timespec with timespec64 inside clock_settime.
Signed-off-by: chensong <chensong@tj.kylinos.cn>
---
include/cobalt/kernel/clock.h | 6 +++---
kernel/cobalt/posix/clock.c | 14 +++++++++-----
kernel/cobalt/posix/clock.h | 15 +++++++++++++--
kernel/cobalt/trace/cobalt-posix.h | 24 ++++++++++++++++++++++--
4 files changed, 47 insertions(+), 12 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/kernel/cobalt/posix/clock.c b/kernel/cobalt/posix/clock.c
index 561358e..d169ffd 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:
@@ -243,14 +243,18 @@ int __cobalt_clock_adjtime(clockid_t clock_id, struct timex *tx)
}
COBALT_SYSCALL(clock_settime, current,
- (clockid_t clock_id, const struct timespec __user *u_ts))
+ (clockid_t clock_id, const struct old_timespec32 __user *u_ts))
{
- struct timespec ts;
+ struct old_timespec32 ts;
+ struct timespec64 ts64;
if (cobalt_copy_from_user(&ts, u_ts, sizeof(ts)))
return -EFAULT;
- return __cobalt_clock_settime(clock_id, &ts);
+ ts64.tv_sec = ts.tv_sec;
+ ts64.tv_nsec = ts.tv_nsec;
+
+ return __cobalt_clock_settime(clock_id, &ts64);
}
COBALT_SYSCALL(clock_adjtime, current,
diff --git a/kernel/cobalt/posix/clock.h b/kernel/cobalt/posix/clock.h
index 0b06b93..bcc608e 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 old_timespec32 __user *u_ts));
COBALT_SYSCALL_DECL(clock_adjtime,
(clockid_t clock_id, struct timex __user *u_tx));
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 2/5] cobalt/kernel: introduce old_timespec32 to clock_settime
2020-11-03 3:04 [PATCH 2/5] cobalt/kernel: introduce old_timespec32 to clock_settime chensong
@ 2020-11-10 10:14 ` Jan Kiszka
2020-11-11 3:21 ` chensong
0 siblings, 1 reply; 5+ messages in thread
From: Jan Kiszka @ 2020-11-10 10:14 UTC (permalink / raw)
To: chensong, xenomai, henning.schild
On 03.11.20 04:04, chensong wrote:
> Replace timespec with old_timespec32 as the parameter passed from
> processes with 32bit timespec in clock_settime.
>
> Also, replace timespec with timespec64 inside clock_settime.
>
> Signed-off-by: chensong <chensong@tj.kylinos.cn>
> ---
> include/cobalt/kernel/clock.h | 6 +++---
> kernel/cobalt/posix/clock.c | 14 +++++++++-----
> kernel/cobalt/posix/clock.h | 15 +++++++++++++--
> kernel/cobalt/trace/cobalt-posix.h | 24 ++++++++++++++++++++++--
> 4 files changed, 47 insertions(+), 12 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/kernel/cobalt/posix/clock.c b/kernel/cobalt/posix/clock.c
> index 561358e..d169ffd 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:
> @@ -243,14 +243,18 @@ int __cobalt_clock_adjtime(clockid_t clock_id, struct timex *tx)
> }
>
> COBALT_SYSCALL(clock_settime, current,
> - (clockid_t clock_id, const struct timespec __user *u_ts))
> + (clockid_t clock_id, const struct old_timespec32 __user *u_ts))
> {
> - struct timespec ts;
> + struct old_timespec32 ts;
> + struct timespec64 ts64;
>
> if (cobalt_copy_from_user(&ts, u_ts, sizeof(ts)))
> return -EFAULT;
>
> - return __cobalt_clock_settime(clock_id, &ts);
> + ts64.tv_sec = ts.tv_sec;
> + ts64.tv_nsec = ts.tv_nsec;
> +
> + return __cobalt_clock_settime(clock_id, &ts64);
> }
>
> COBALT_SYSCALL(clock_adjtime, current,
> diff --git a/kernel/cobalt/posix/clock.h b/kernel/cobalt/posix/clock.h
> index 0b06b93..bcc608e 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 old_timespec32 __user *u_ts));
>
> COBALT_SYSCALL_DECL(clock_adjtime,
> (clockid_t clock_id, struct timex __user *u_tx));
> 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,
I don't think we need to rename this event. We just now ensure that it
can now handle timespec64. If that came from a legacy 32-bit or a new
64-bit syscall will be traced via the cobalt syscall event.
> + TP_PROTO(clockid_t clk_id, const struct timespec64 *time),
> TP_ARGS(clk_id, time)
> );
>
>
Jan
--
Siemens AG, T RDA IOT
Corporate Competence Center Embedded Linux
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH 2/5] cobalt/kernel: introduce old_timespec32 to clock_settime
2020-11-10 10:14 ` Jan Kiszka
@ 2020-11-11 3:21 ` chensong
2020-11-11 7:23 ` Jan Kiszka
0 siblings, 1 reply; 5+ messages in thread
From: chensong @ 2020-11-11 3:21 UTC (permalink / raw)
To: Jan Kiszka, xenomai, henning.schild
On 2020年11月10日 18:14, Jan Kiszka wrote:
>
> On 03.11.20 04:04, chensong wrote:
>> Replace timespec with old_timespec32 as the parameter passed from
>> processes with 32bit timespec in clock_settime.
>>
>> Also, replace timespec with timespec64 inside clock_settime.
>>
>> Signed-off-by: chensong <chensong@tj.kylinos.cn>
>> ---
>> include/cobalt/kernel/clock.h | 6 +++---
>> kernel/cobalt/posix/clock.c | 14 +++++++++-----
>> kernel/cobalt/posix/clock.h | 15 +++++++++++++--
>> kernel/cobalt/trace/cobalt-posix.h | 24 ++++++++++++++++++++++--
>> 4 files changed, 47 insertions(+), 12 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/kernel/cobalt/posix/clock.c b/kernel/cobalt/posix/clock.c
>> index 561358e..d169ffd 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:
>> @@ -243,14 +243,18 @@ int __cobalt_clock_adjtime(clockid_t clock_id, struct timex *tx)
>> }
>>
>> COBALT_SYSCALL(clock_settime, current,
>> - (clockid_t clock_id, const struct timespec __user *u_ts))
>> + (clockid_t clock_id, const struct old_timespec32 __user *u_ts))
>> {
>> - struct timespec ts;
>> + struct old_timespec32 ts;
>> + struct timespec64 ts64;
>>
>> if (cobalt_copy_from_user(&ts, u_ts, sizeof(ts)))
>> return -EFAULT;
>>
>> - return __cobalt_clock_settime(clock_id, &ts);
>> + ts64.tv_sec = ts.tv_sec;
>> + ts64.tv_nsec = ts.tv_nsec;
>> +
>> + return __cobalt_clock_settime(clock_id, &ts64);
>> }
>>
>> COBALT_SYSCALL(clock_adjtime, current,
>> diff --git a/kernel/cobalt/posix/clock.h b/kernel/cobalt/posix/clock.h
>> index 0b06b93..bcc608e 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 old_timespec32 __user *u_ts));
>>
>> COBALT_SYSCALL_DECL(clock_adjtime,
>> (clockid_t clock_id, struct timex __user *u_tx));
>> 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,
>
> I don't think we need to rename this event. We just now ensure that it
> can now handle timespec64. If that came from a legacy 32-bit or a new
> 64-bit syscall will be traced via the cobalt syscall event.
>
I defined a new event class cobalt_clock_timespec64 specific for
timespec64, it will hit a compiling mistake without it, here is the
information:
kernel/xenomai/posix/clock.c:219:39: error: passing argument 2 of
‘trace_cobalt_clock_settime’ from incompatible pointer type
[-Werror=incompatible-pointer-types]
trace_cobalt_clock_settime(clock_id, ts);
my idea is defining a new event class cobalt_clock_timespec64, and every
trace event(clock_settime, clock_gettime and clock_getres) goes to it
and the original event class,cobalt_clock_timespec, will be removed
eventually.
if i understand you correctly(If that came from a legacy 32-bit or a new
64-bit syscall will be traced via the cobalt syscall event), i just need
to remove trace_cobalt_clock_settime from __cobalt_clock_settime?
chensong
>> + TP_PROTO(clockid_t clk_id, const struct timespec64 *time),
>> TP_ARGS(clk_id, time)
>> );
>>
>>
>
> Jan
>
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH 2/5] cobalt/kernel: introduce old_timespec32 to clock_settime
2020-11-11 3:21 ` chensong
@ 2020-11-11 7:23 ` Jan Kiszka
2020-11-11 9:41 ` chensong
0 siblings, 1 reply; 5+ messages in thread
From: Jan Kiszka @ 2020-11-11 7:23 UTC (permalink / raw)
To: chensong, xenomai, henning.schild
On 11.11.20 04:21, chensong wrote:
>
>
> On 2020年11月10日 18:14, Jan Kiszka wrote:
>>
>> On 03.11.20 04:04, chensong wrote:
>>> Replace timespec with old_timespec32 as the parameter passed from
>>> processes with 32bit timespec in clock_settime.
>>>
>>> Also, replace timespec with timespec64 inside clock_settime.
>>>
>>> Signed-off-by: chensong <chensong@tj.kylinos.cn>
>>> ---
>>> include/cobalt/kernel/clock.h | 6 +++---
>>> kernel/cobalt/posix/clock.c | 14 +++++++++-----
>>> kernel/cobalt/posix/clock.h | 15 +++++++++++++--
>>> kernel/cobalt/trace/cobalt-posix.h | 24 ++++++++++++++++++++++--
>>> 4 files changed, 47 insertions(+), 12 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/kernel/cobalt/posix/clock.c b/kernel/cobalt/posix/clock.c
>>> index 561358e..d169ffd 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:
>>> @@ -243,14 +243,18 @@ int __cobalt_clock_adjtime(clockid_t clock_id,
>>> struct timex *tx)
>>> }
>>>
>>> COBALT_SYSCALL(clock_settime, current,
>>> - (clockid_t clock_id, const struct timespec __user *u_ts))
>>> + (clockid_t clock_id, const struct old_timespec32 __user
>>> *u_ts))
>>> {
>>> - struct timespec ts;
>>> + struct old_timespec32 ts;
>>> + struct timespec64 ts64;
>>>
>>> if (cobalt_copy_from_user(&ts, u_ts, sizeof(ts)))
>>> return -EFAULT;
>>>
>>> - return __cobalt_clock_settime(clock_id, &ts);
>>> + ts64.tv_sec = ts.tv_sec;
>>> + ts64.tv_nsec = ts.tv_nsec;
>>> +
>>> + return __cobalt_clock_settime(clock_id, &ts64);
>>> }
>>>
>>> COBALT_SYSCALL(clock_adjtime, current,
>>> diff --git a/kernel/cobalt/posix/clock.h b/kernel/cobalt/posix/clock.h
>>> index 0b06b93..bcc608e 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 old_timespec32 __user *u_ts));
>>>
>>> COBALT_SYSCALL_DECL(clock_adjtime,
>>> (clockid_t clock_id, struct timex __user *u_tx));
>>> 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,
>>
>> I don't think we need to rename this event. We just now ensure that it
>> can now handle timespec64. If that came from a legacy 32-bit or a new
>> 64-bit syscall will be traced via the cobalt syscall event.
>>
> I defined a new event class cobalt_clock_timespec64 specific for
> timespec64, it will hit a compiling mistake without it, here is the
> information:
>
> kernel/xenomai/posix/clock.c:219:39: error: passing argument 2 of
> ‘trace_cobalt_clock_settime’ from incompatible pointer type
> [-Werror=incompatible-pointer-types]
> trace_cobalt_clock_settime(clock_id, ts);
>
> my idea is defining a new event class cobalt_clock_timespec64, and every
> trace event(clock_settime, clock_gettime and clock_getres) goes to it
> and the original event class,cobalt_clock_timespec, will be removed
> eventually.
>
> if i understand you correctly(If that came from a legacy 32-bit or a new
> 64-bit syscall will be traced via the cobalt syscall event), i just need
> to remove trace_cobalt_clock_settime from __cobalt_clock_settime?
>
I didn't look at the details yet, but the idea is having a common
settime64 path where also the trace event is issued. The 32-bit entry
path does a conversion first in order to use the common implementation.
And whether we were taking the 32 or the 64-bit path can be looked up in
the traces by checking the cobalt syscall entry event.
Jan
--
Siemens AG, T RDA IOT
Corporate Competence Center Embedded Linux
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH 2/5] cobalt/kernel: introduce old_timespec32 to clock_settime
2020-11-11 7:23 ` Jan Kiszka
@ 2020-11-11 9:41 ` chensong
0 siblings, 0 replies; 5+ messages in thread
From: chensong @ 2020-11-11 9:41 UTC (permalink / raw)
To: Jan Kiszka, xenomai, henning.schild
On 2020年11月11日 15:23, Jan Kiszka wrote:
> On 11.11.20 04:21, chensong wrote:
>>
>>
>> On 2020年11月10日 18:14, Jan Kiszka wrote:
>>>
>>> On 03.11.20 04:04, chensong wrote:
>>>> Replace timespec with old_timespec32 as the parameter passed from
>>>> processes with 32bit timespec in clock_settime.
>>>>
>>>> Also, replace timespec with timespec64 inside clock_settime.
>>>>
>>>> Signed-off-by: chensong <chensong@tj.kylinos.cn>
>>>> ---
>>>> include/cobalt/kernel/clock.h | 6 +++---
>>>> kernel/cobalt/posix/clock.c | 14 +++++++++-----
>>>> kernel/cobalt/posix/clock.h | 15 +++++++++++++--
>>>> kernel/cobalt/trace/cobalt-posix.h | 24 ++++++++++++++++++++++--
>>>> 4 files changed, 47 insertions(+), 12 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/kernel/cobalt/posix/clock.c b/kernel/cobalt/posix/clock.c
>>>> index 561358e..d169ffd 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:
>>>> @@ -243,14 +243,18 @@ int __cobalt_clock_adjtime(clockid_t clock_id,
>>>> struct timex *tx)
>>>> }
>>>>
>>>> COBALT_SYSCALL(clock_settime, current,
>>>> - (clockid_t clock_id, const struct timespec __user *u_ts))
>>>> + (clockid_t clock_id, const struct old_timespec32 __user
>>>> *u_ts))
>>>> {
>>>> - struct timespec ts;
>>>> + struct old_timespec32 ts;
>>>> + struct timespec64 ts64;
>>>>
>>>> if (cobalt_copy_from_user(&ts, u_ts, sizeof(ts)))
>>>> return -EFAULT;
>>>>
>>>> - return __cobalt_clock_settime(clock_id, &ts);
>>>> + ts64.tv_sec = ts.tv_sec;
>>>> + ts64.tv_nsec = ts.tv_nsec;
>>>> +
>>>> + return __cobalt_clock_settime(clock_id, &ts64);
>>>> }
>>>>
>>>> COBALT_SYSCALL(clock_adjtime, current,
>>>> diff --git a/kernel/cobalt/posix/clock.h b/kernel/cobalt/posix/clock.h
>>>> index 0b06b93..bcc608e 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 old_timespec32 __user *u_ts));
>>>>
>>>> COBALT_SYSCALL_DECL(clock_adjtime,
>>>> (clockid_t clock_id, struct timex __user *u_tx));
>>>> 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,
>>>
>>> I don't think we need to rename this event. We just now ensure that it
>>> can now handle timespec64. If that came from a legacy 32-bit or a new
>>> 64-bit syscall will be traced via the cobalt syscall event.
>>>
>> I defined a new event class cobalt_clock_timespec64 specific for
>> timespec64, it will hit a compiling mistake without it, here is the
>> information:
>>
>> kernel/xenomai/posix/clock.c:219:39: error: passing argument 2 of
>> ‘trace_cobalt_clock_settime’ from incompatible pointer type
>> [-Werror=incompatible-pointer-types]
>> trace_cobalt_clock_settime(clock_id, ts);
>>
>> my idea is defining a new event class cobalt_clock_timespec64, and every
>> trace event(clock_settime, clock_gettime and clock_getres) goes to it
>> and the original event class,cobalt_clock_timespec, will be removed
>> eventually.
>>
>> if i understand you correctly(If that came from a legacy 32-bit or a new
>> 64-bit syscall will be traced via the cobalt syscall event), i just need
>> to remove trace_cobalt_clock_settime from __cobalt_clock_settime?
>>
>
> I didn't look at the details yet, but the idea is having a common
> settime64 path where also the trace event is issued. The 32-bit entry
> path does a conversion first in order to use the common implementation.
> And whether we were taking the 32 or the 64-bit path can be looked up in
> the traces by checking the cobalt syscall entry event.
>
> Jan
32bit timespec is only used in interface level, internally,
clock_settime for 32bit also uses timespec64 as well as clock_settime64.
It works this way:
ts64.tv_sec = ts.tv_sec
ts64.tv_nsec = ts.tv_nsec
as a result, trace_cobalt_clock_settime will accept timespec64 instead
of timespec.
The code in cobalt-posix.h looks redundant only because of clock_gettime
and clock_getres. It will look clean as it used to be after i finish
"clock_gettime and clock_getres".
There is no impact, what do you think?
chensong
>
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2020-11-11 9:41 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-11-03 3:04 [PATCH 2/5] cobalt/kernel: introduce old_timespec32 to clock_settime chensong
2020-11-10 10:14 ` Jan Kiszka
2020-11-11 3:21 ` chensong
2020-11-11 7:23 ` Jan Kiszka
2020-11-11 9:41 ` 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.