[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH] Introduce time_value64_t to keep track of real time in the k
|
From: |
Samuel Thibault |
|
Subject: |
Re: [PATCH] Introduce time_value64_t to keep track of real time in the kernel |
|
Date: |
Tue, 3 Jan 2023 00:46:06 +0100 |
|
User-agent: |
NeoMutt/20170609 (1.8.3) |
Applied, thanks!
Flavio Cruz, le lun. 02 janv. 2023 18:15:15 -0500, a ecrit:
> time_value64_t uses int64_t to track seconds and nanoseconds and hence
> is Y2038 proof. It does not have nano second resolution but it could
> be provided in the future.
>
> Removed include/sys/time.h as it remaps time_value_t into timeval which
> can create confusion.
>
> The timestamp from keyboard and mouse events is no longer set and
> replaced with rpc_time_value for better compatibility.
> ---
> On Wed, Dec 28, 2022 at 08:39:49PM +0100, Samuel Thibault wrote:
> > Hello,
> >
> > Flavio Cruz, le mer. 28 déc. 2022 14:11:35 -0500, a ecrit:
> > > The timestamp from keyboard and mouse events was removed. It does not
> > > seem to be needed anywhere.
> >
> > But we cannot just drop it like that since the hurd console uses these
> > events.
>
> That's a good point. I have changed the time field to use the type
> rpc_time_value since that will work well with a 64 bit kernel but marked it
> as unused. There's likely more situations where we are sharing data
> structures with the user space that need to be fixed.
>
>
> Makefile.am | 2 +-
> Makefrag.am | 2 +-
> i386/i386at/com.c | 1 -
> i386/i386at/kd.h | 8 ++++--
> i386/i386at/kd_event.c | 4 ++-
> i386/i386at/kd_mouse.c | 5 ++--
> i386/i386at/kd_queue.c | 2 +-
> i386/i386at/lpr.c | 1 -
> i386/i386at/model_dep.c | 5 ++--
> i386/i386at/rtc.c | 7 ++----
> include/mach/time_value.h | 27 +++++++++++++++++++-
> include/sys/time.h | 53 ---------------------------------------
> kern/mach_clock.c | 30 +++++++++++-----------
> kern/mach_clock.h | 1 +
> 14 files changed, 61 insertions(+), 87 deletions(-)
> delete mode 100644 include/sys/time.h
>
> diff --git a/Makefile.am b/Makefile.am
> index 6e629e87..7b7247c5 100644
> --- a/Makefile.am
> +++ b/Makefile.am
> @@ -169,7 +169,7 @@ noinst_PROGRAMS += \
> gnumach.o
> # This is the list of routines we use from libgcc.
> -libgcc_routines := udivdi3 __udivdi3 __udivmoddi4 __umoddi3 __divdi3
> __moddi3 __ffsdi2
> +libgcc_routines := udivdi3 __udivdi3 __udivmoddi4 __umoddi3 __divdi3
> __divmoddi4 __moddi3 __ffsdi2
> # References generated by ld.
> ld_magic_routines := __rel_iplt_start __rel_iplt_end __rela_iplt_start
> __rela_iplt_end _START etext _edata _end
> gnumach-undef: gnumach.$(OBJEXT)
> diff --git a/Makefrag.am b/Makefrag.am
> index 2840234f..c778ed80 100644
> --- a/Makefrag.am
> +++ b/Makefrag.am
> @@ -453,7 +453,7 @@ include_mach_debug_HEADERS = \
> # Other headers for the distribution. We don't install these, because the
> # GNU C library has correct versions for users to use.
> -# other-sys-headers := types.h time.h reboot.h ioctl.h
> +# other-sys-headers := types.h reboot.h ioctl.h
> # other-mach-headers := mig_support.h mach_traps.h error.h
> # other-headers := alloca.h
> diff --git a/i386/i386at/com.c b/i386/i386at/com.c
> index c63c30a4..a6d6f701 100644
> --- a/i386/i386at/com.c
> +++ b/i386/i386at/com.c
> @@ -33,7 +33,6 @@
> #include <sys/types.h>
> #include <kern/printf.h>
> #include <kern/mach_clock.h>
> -#include <sys/time.h>
> #include <device/conf.h>
> #include <device/device_types.h>
> #include <device/tty.h>
> diff --git a/i386/i386at/kd.h b/i386/i386at/kd.h
> index 6f425ae9..cfa7819e 100644
> --- a/i386/i386at/kd.h
> +++ b/i386/i386at/kd.h
> @@ -74,7 +74,6 @@ WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
> #include <sys/ioctl.h>
> #include <mach/boolean.h>
> #include <sys/types.h>
> -#include <sys/time.h>
> #include <device/cons.h>
> #include <device/io_req.h>
> #include <device/buf.h>
> @@ -672,7 +671,12 @@ struct mouse_motion {
> typedef struct {
> kev_type type; /* see below */
> - struct timeval time; /* timestamp */
> + /*
> + * This is not used anymore but is kept for backwards compatibility.
> + * Note the use of rpc_time_value to ensure compatibility for a 64 bit
> kernel and
> + * 32 bit user land.
> + */
> + struct rpc_time_value unused_time; /* timestamp*/
> union { /* value associated with event */
> boolean_t up; /* MOUSE_LEFT .. MOUSE_RIGHT */
> Scancode sc; /* KEYBD_EVENT */
> diff --git a/i386/i386at/kd_event.c b/i386/i386at/kd_event.c
> index 5b1f7098..25a0ef35 100644
> --- a/i386/i386at/kd_event.c
> +++ b/i386/i386at/kd_event.c
> @@ -275,7 +275,9 @@ kd_enqsc(Scancode sc)
> kd_event ev;
> ev.type = KEYBD_EVENT;
> - ev.time = time;
> + /* Not used but we set it to avoid garbage */
> + ev.unused_time.seconds = 0;
> + ev.unused_time.microseconds = 0;
> ev.value.sc = sc;
> kbd_enqueue(&ev);
> }
> diff --git a/i386/i386at/kd_mouse.c b/i386/i386at/kd_mouse.c
> index 3af21273..b001dbed 100644
> --- a/i386/i386at/kd_mouse.c
> +++ b/i386/i386at/kd_mouse.c
> @@ -751,7 +751,9 @@ mouse_moved(struct mouse_motion where)
> kd_event ev;
> ev.type = MOUSE_MOTION;
> - ev.time = time;
> + /* Not used but we set it to avoid garbage */
> + ev.unused_time.seconds = 0;
> + ev.unused_time.microseconds = 0;
> ev.value.mmotion = where;
> mouse_enqueue(&ev);
> }
> @@ -767,7 +769,6 @@ mouse_button(
> kd_event ev;
> ev.type = which;
> - ev.time = time;
> ev.value.up = (direction == MOUSE_UP) ? TRUE : FALSE;
> mouse_enqueue(&ev);
> }
> diff --git a/i386/i386at/kd_queue.c b/i386/i386at/kd_queue.c
> index 78035b18..ab399cd8 100644
> --- a/i386/i386at/kd_queue.c
> +++ b/i386/i386at/kd_queue.c
> @@ -88,7 +88,7 @@ kdq_put(kd_event_queue *q, kd_event *ev)
> kd_event *qp = q->events + q->firstfree;
> qp->type = ev->type;
> - qp->time = ev->time;
> + qp->unused_time = ev->unused_time;
> qp->value = ev->value;
> q->firstfree = q_next(q->firstfree);
> }
> diff --git a/i386/i386at/lpr.c b/i386/i386at/lpr.c
> index 7cef127c..3939af54 100644
> --- a/i386/i386at/lpr.c
> +++ b/i386/i386at/lpr.c
> @@ -34,7 +34,6 @@
> #include <sys/types.h>
> #include <kern/printf.h>
> #include <kern/mach_clock.h>
> -#include <sys/time.h>
> #include <device/conf.h>
> #include <device/device_types.h>
> #include <device/tty.h>
> diff --git a/i386/i386at/model_dep.c b/i386/i386at/model_dep.c
> index f677e134..9ccc7551 100644
> --- a/i386/i386at/model_dep.c
> +++ b/i386/i386at/model_dep.c
> @@ -51,7 +51,6 @@
> #include <kern/printf.h>
> #include <kern/startup.h>
> #include <kern/smp.h>
> -#include <sys/time.h>
> #include <sys/types.h>
> #include <vm/vm_page.h>
> #include <i386/fpu.h>
> @@ -705,12 +704,12 @@ startrtclock(void)
> void
> inittodr(void)
> {
> - time_value_t new_time;
> + time_value64_t new_time;
> uint64_t newsecs;
> (void) readtodc(&newsecs);
> new_time.seconds = newsecs;
> - new_time.microseconds = 0;
> + new_time.nanoseconds = 0;
> {
> spl_t s = splhigh();
> diff --git a/i386/i386at/rtc.c b/i386/i386at/rtc.c
> index b2068416..1930beb0 100644
> --- a/i386/i386at/rtc.c
> +++ b/i386/i386at/rtc.c
> @@ -47,7 +47,6 @@ WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
> */
> #include <sys/types.h>
> -#include <sys/time.h>
> #include <kern/mach_clock.h>
> #include <kern/printf.h>
> #include <i386/machspl.h>
> @@ -107,8 +106,6 @@ rtcput(struct rtc_st *st)
> }
> -extern struct timeval time;
> -
> static int month[12] = {31, 28, 31, 30, 31, 30, 31, 31, 30, 31, 30, 31};
> static int
> @@ -213,13 +210,13 @@ writetodc(void)
> splx(ospl);
> diff = 0;
> - n = (time.tv_sec - diff) % (3600 * 24); /* hrs+mins+secs */
> + n = (time.seconds - diff) % (3600 * 24); /* hrs+mins+secs */
> rtclk.rtc_sec = dectohexdec(n%60);
> n /= 60;
> rtclk.rtc_min = dectohexdec(n%60);
> rtclk.rtc_hr = dectohexdec(n/60);
> - n = (time.tv_sec - diff) / (3600 * 24); /* days */
> + n = (time.seconds - diff) / (3600 * 24); /* days */
> rtclk.rtc_dow = (n + 4) % 7; /* 1/1/70 is Thursday */
> /* Epoch shall be 1970 January 1st */
> diff --git a/include/mach/time_value.h b/include/mach/time_value.h
> index f9b977c8..e660daad 100644
> --- a/include/mach/time_value.h
> +++ b/include/mach/time_value.h
> @@ -41,7 +41,8 @@ struct rpc_time_value {
> typedef struct rpc_time_value rpc_time_value_t;
> /*
> - * Time value used by kernel.
> + * Time value used by kernel interfaces. Ideally they should be migrated
> + * to use time_value64 below.
> */
> struct time_value {
> long_integer_t seconds;
> @@ -49,6 +50,16 @@ struct time_value {
> };
> typedef struct time_value time_value_t;
> +/*
> + * Time value used internally by the kernel that uses 64 bits to track
> seconds
> + * and nanoseconds. Note that the current resolution is only microseconds.
> + */
> +struct time_value64 {
> + int64_t seconds;
> + int64_t nanoseconds;
> +};
> +typedef struct time_value64 time_value64_t;
> +
> /**
> * Functions used by Mig to perform user to kernel conversion and vice-versa.
> * We only do this because we may run a 64 bit kernel with a 32 bit user
> space.
> @@ -69,10 +80,14 @@ static __inline__ time_value_t
> convert_time_value_from_user(rpc_time_value_t tv)
> * are normalized (microseconds <= 999999).
> */
> #define TIME_MICROS_MAX (1000000)
> +#define TIME_NANOS_MAX (1000000000)
> #define time_value_assert(val) \
> assert(0 <= (val)->microseconds && (val)->microseconds < TIME_MICROS_MAX);
> +#define time_value64_assert(val) \
> + assert(0 <= (val)->nanoseconds && (val)->nanoseconds < TIME_NANOS_MAX);
> +
> #define time_value_add_usec(val, micros) { \
> time_value_assert(val); \
> if (((val)->microseconds += (micros)) \
> @@ -83,6 +98,16 @@ static __inline__ time_value_t
> convert_time_value_from_user(rpc_time_value_t tv)
> time_value_assert(val); \
> }
> +#define time_value64_add_usec(val, micros) { \
> + time_value64_assert(val); \
> + if (((val)->nanoseconds += (micros) * 1000) \
> + >= TIME_NANOS_MAX) { \
> + (val)->nanoseconds -= TIME_NANOS_MAX; \
> + (val)->seconds++; \
> + } \
> + time_value64_assert(val); \
> +}
> +
> #define time_value_sub_usec(val, micros) { \
> time_value_assert(val); \
> if (((val)->microseconds -= (micros)) < 0) { \
> diff --git a/include/sys/time.h b/include/sys/time.h
> deleted file mode 100644
> index de97d325..00000000
> --- a/include/sys/time.h
> +++ /dev/null
> @@ -1,53 +0,0 @@
> -/*
> - * Mach Operating System
> - * Copyright (c) 1991 Carnegie Mellon University
> - * All Rights Reserved.
> - *
> - * Permission to use, copy, modify and distribute this software and its
> - * documentation is hereby granted, provided that both the copyright
> - * notice and this permission notice appear in all copies of the
> - * software, derivative works or modified versions, and any portions
> - * thereof, and that both notices appear in supporting documentation.
> - *
> - * CARNEGIE MELLON ALLOWS FREE USE OF THIS SOFTWARE IN ITS "AS IS"
> - * CONDITION. CARNEGIE MELLON DISCLAIMS ANY LIABILITY OF ANY KIND FOR
> - * ANY DAMAGES WHATSOEVER RESULTING FROM THE USE OF THIS SOFTWARE.
> - *
> - * Carnegie Mellon requests users of this software to return to
> - *
> - * Software Distribution Coordinator or Software.Distribution@CS.CMU.EDU
> - * School of Computer Science
> - * Carnegie Mellon University
> - * Pittsburgh PA 15213-3890
> - *
> - * any improvements or extensions that they make and grant Carnegie Mellon
> rights
> - * to redistribute these changes.
> - */
> -/*
> - * Time-keeper for kernel IO devices.
> - *
> - * May or may not have any relation to wall-clock time.
> - */
> -
> -#ifndef _MACH_SA_SYS_TIME_H_
> -#define _MACH_SA_SYS_TIME_H_
> -
> -#include <mach/time_value.h>
> -
> -extern time_value_t time;
> -
> -/*
> - * Definitions to keep old code happy.
> - */
> -#define timeval_t time_value_t
> -#define timeval time_value
> -#define tv_sec seconds
> -#define tv_usec microseconds
> -
> -#define timerisset(tvp) ((tvp)->tv_sec || (tvp)->tv_usec)
> -#define timercmp(tvp, uvp, cmp) \
> - ((tvp)->tv_sec cmp (uvp)->tv_sec || \
> - (tvp)->tv_sec == (uvp)->tv_sec && (tvp)->tv_usec cmp (uvp)->tv_usec)
> -#define timerclear(tvp) (tvp)->tv_sec = (tvp)->tv_usec = 0
> -
> -#endif /* _MACH_SA_SYS_TIME_H_ */
> diff --git a/kern/mach_clock.c b/kern/mach_clock.c
> index ad986c64..7383684b 100644
> --- a/kern/mach_clock.c
> +++ b/kern/mach_clock.c
> @@ -55,7 +55,6 @@
> #include <kern/timer.h>
> #include <kern/priority.h>
> #include <vm/vm_kern.h>
> -#include <sys/time.h>
> #include <machine/mach_param.h> /* HZ */
> #include <machine/machspl.h>
> #include <machine/model_dep.h>
> @@ -66,7 +65,7 @@
> int hz = HZ; /* number of ticks per second */
> int tick = (1000000 / HZ); /* number of usec per tick */
> -time_value_t time = { 0, 0 }; /* time since bootup (uncorrected) */
> +time_value64_t time = { 0, 0 }; /* time since bootup
> (uncorrected) */
> unsigned long elapsed_ticks = 0; /* ticks elapsed since bootup */
> int timedelta = 0;
> @@ -93,15 +92,15 @@ unsigned bigadj = 1000000; /* adjust 10*tickadj if
> adjustment
> volatile mapped_time_value_t *mtime = 0;
> -#define update_mapped_time(time) \
> -MACRO_BEGIN \
> - if (mtime != 0) { \
> - mtime->check_seconds = (time)->seconds; \
> - __sync_synchronize(); \
> - mtime->microseconds = (time)->microseconds; \
> - __sync_synchronize(); \
> - mtime->seconds = (time)->seconds; \
> - } \
> +#define update_mapped_time(time) \
> +MACRO_BEGIN \
> + if (mtime != 0) { \
> + mtime->check_seconds = (time)->seconds; \
> + __sync_synchronize(); \
> + mtime->microseconds = (time)->nanoseconds / 1000; \
> + __sync_synchronize(); \
> + mtime->seconds = (time)->seconds; \
> + } \
> MACRO_END
> #define read_mapped_time(time) \
> @@ -226,7 +225,7 @@ void clock_interrupt(
> * Increment the time-of-day clock.
> */
> if (timedelta == 0) {
> - time_value_add_usec(&time, usec);
> + time_value64_add_usec(&time, usec);
> }
> else {
> int delta;
> @@ -247,7 +246,7 @@ void clock_interrupt(
> delta = usec + tickdelta;
> timedelta -= tickdelta;
> }
> - time_value_add_usec(&time, delta);
> + time_value64_add_usec(&time, delta);
> }
> update_mapped_time(&time);
> @@ -401,7 +400,7 @@ struct time_value clock_boottime_offset;
> static void
> clock_boottime_update(struct time_value *new_time)
> {
> - struct time_value delta = time;
> + struct time_value delta = {.seconds = time.seconds, .microseconds =
> time.nanoseconds / 1000};
> time_value_sub(&delta, new_time);
> time_value_add(&clock_boottime_offset, &delta);
> }
> @@ -464,7 +463,8 @@ host_set_time(const host_t host, time_value_t new_time)
> s = splhigh();
> clock_boottime_update(&new_time);
> - time = new_time;
> + time.seconds = new_time.seconds;
> + time.nanoseconds = new_time.microseconds * 1000;
> update_mapped_time(&time);
> resettodr();
> splx(s);
> diff --git a/kern/mach_clock.h b/kern/mach_clock.h
> index 977b43be..903492e6 100644
> --- a/kern/mach_clock.h
> +++ b/kern/mach_clock.h
> @@ -40,6 +40,7 @@ extern unsigned long elapsed_ticks; /* number of
> ticks elapsed since bootup */
> extern int hz; /* number of ticks per second */
> extern int tick; /* number of usec per tick */
> +extern time_value64_t time; /* time since bootup
> (uncorrected) */
> typedef void timer_func_t(void *);
> --
> 2.37.2
>
--
Samuel
---
Pour une évaluation indépendante, transparente et rigoureuse !
Je soutiens la Commission d'Évaluation de l'Inria.