Applied, thanks!
Flavio Cruz, le jeu. 19 janv. 2023 14:21:38 -0500, a ecrit:
> Also updated the mapped time to support the new 64-bit time while
> keeping compatible with the user land programs currently using it so
> they can be migrated in parallel.
> ---
> On Mon, Jan 16, 2023 at 11:37:47PM +0100, Samuel Thibault wrote:
> > Flavio Cruz, le ven. 13 janv. 2023 03:27:05 -0500, a ecrit:
> > > #define time_value64_assert(val) \
> > > - assert(0 <= (val)->nanoseconds && (val)->nanoseconds < TIME_NANOS_MAX);
> > > + assert(0 <= (val).nanoseconds && (val).nanoseconds < TIME_NANOS_MAX);
> > [...]
> > > +#define time_value64_add_nanos(val, nanos) { \
> > > + time_value64_assert(*val); \
> > [...]
> >
> > Macros such as TIMEVAL_TO_TIMESPEC etc. do always take pointers, so
> > better keep coherent with them.
>
> Done.
>
> >
> > > -record_time_stamp (time_value_t *stamp)
> > > +record_time_stamp(time_value_t *stamp)
> > > {
> > > - read_mapped_time(stamp);
> > > - time_value_add(stamp, &clock_boottime_offset);
> > > + time_value64_t stamp64;
> > > + read_mapped_time(&stamp64);
> > > + time_value64_add(&stamp64, clock_boottime_offset);
> > > + stamp->seconds = stamp64.seconds;
> > > + stamp->microseconds = stamp64.nanoseconds / 1000;
> >
> > There are quite a few such conversions, perhaps introduce a macro to
> > convert between time_value_t and time_value64_t?
>
> Created two new macros for the translations.
>
> >
> > Samuel
>
> doc/mach.texi | 42 ++++++++++++---------
> include/mach/mach_host.defs | 8 ++++
> include/mach/mach_types.defs | 5 +++
> include/mach/time_value.h | 44 +++++++++++++++-------
> kern/mach_clock.c | 73 ++++++++++++++++++++++++------------
> kern/mach_clock.h | 2 +-
> 6 files changed, 118 insertions(+), 56 deletions(-)
>
> diff --git a/doc/mach.texi b/doc/mach.texi
> index 86a557cb..53026d0b 100644
> --- a/doc/mach.texi
> +++ b/doc/mach.texi
> @@ -5612,39 +5612,39 @@ necessarily null-terminated.
> @node Host Time
> @section Host Time
> -@deftp {Data type} time_value_t
> +@deftp {Data type} time_value64_t
> This is the representation of a time in Mach. It is a @code{struct
> -time_value} and consists of the following members:
> +time_value64} and consists of the following members:
> @table @code
> -@item integer_t seconds
> +@item int64_t seconds
> The number of seconds.
> -@item integer_t microseconds
> -The number of microseconds.
> +@item int64_t nanoseconds
> +The number of nanoseconds.
> @end table
> @end deftp
> -The number of microseconds should always be smaller than
> -@code{TIME_MICROS_MAX} (100000). A time with this property is
> +The number of nanoseconds should always be smaller than
> +@code{TIME_NANOS_MAX} (100000000). A time with this property is
> @dfn{normalized}. Normalized time values can be manipulated with the
> following macros:
> -@defmac time_value_add_usec (@w{time_value_t *@var{val}}, @w{integer_t
> *@var{micros}})
> -Add @var{micros} microseconds to @var{val}. If @var{val} is normalized
> -and @var{micros} smaller than @code{TIME_MICROS_MAX}, @var{val} will be
> +@defmac time_value64_add_nanos (@w{time_value64_t *@var{val}}, @w{int64_t
> *@var{nanos}})
> +Add @var{nanos} nanoseconds to @var{val}. If @var{val} is normalized
> +and @var{nanos} smaller than @code{TIME_NANOS_MAX}, @var{val} will be
> normalized afterwards.
> @end defmac
> -@defmac time_value_add (@w{time_value_t *@var{result}}, @w{time_value_t
> *@var{addend}})
> +@defmac time_value64_add (@w{time_value64_t *@var{result}},
> @w{time_value64_t *@var{addend}})
> Add the values in @var{addend} to @var{result}. If both are normalized,
> @var{result} will be normalized afterwards.
> @end defmac
> -A variable of type @code{time_value_t} can either represent a duration
> +A variable of type @code{time_value64_t} can either represent a duration
> or a fixed point in time. In the latter case, it shall be interpreted as
> -the number of seconds and microseconds after the epoch 1. Jan 1970.
> +the number of seconds and nanoseconds after the epoch 1. Jan 1970.
> -@deftypefun kern_return_t host_get_time (@w{host_t @var{host}},
> @w{time_value_t *@var{current_time}})
> +@deftypefun kern_return_t host_get_time64 (@w{host_t @var{host}},
> @w{time_value64_t *@var{current_time}})
> Get the current time as seen by @var{host}. On success, the time passed
> since the epoch is returned in @var{current_time}.
> @end deftypefun
> @@ -5675,6 +5675,14 @@ The number of microseconds.
> @item integer_t check_seconds
> This is a copy of the seconds value, which must be checked to protect
> +against a race condition when reading out the two time values. This
> +should only be used when getting the 32 bit version of @code{time_value64_t}.
> +
> +@item time_value64_t time_value
> +The current time.
> +
> +@item int64_t check_seconds64
> +This is a copy of the seconds value in @var{time_value}, which must be
> checked to protect
> against a race condition when reading out the two time values.
> @end table
> @end deftp
> @@ -5686,12 +5694,12 @@ mapped-time interface:
> @example
> do @{
> - secs = mtime->seconds;
> + secs = mtime->time_value.seconds;
> __sync_synchronize();
> - usecs = mtime->microseconds;
> + nanos = mtime->time_value.nanoseconds;
> __sync_synchronize();
> @}
> -while (secs != mtime->check_seconds);
> +while (secs != mtime->check_seconds64);
> @end example
> diff --git a/include/mach/mach_host.defs b/include/mach/mach_host.defs
> index 28439a01..0674c859 100644
> --- a/include/mach/mach_host.defs
> +++ b/include/mach/mach_host.defs
> @@ -352,3 +352,11 @@ routine processor_control(
> routine host_get_boot_info(
> host_priv : host_priv_t;
> out boot_info : kernel_boot_info_t);
> +
> +/*
> + * Get the time on this host.
> + * Available to all.
> + */
> +routine host_get_time64(
> + host : host_t;
> + out current_time : time_value64_t);
> diff --git a/include/mach/mach_types.defs b/include/mach/mach_types.defs
> index a98e5c67..8f22137a 100644
> --- a/include/mach/mach_types.defs
> +++ b/include/mach/mach_types.defs
> @@ -272,6 +272,11 @@ type time_value_t = rpc_time_value_t
> #endif
> ;
> +type time_value64_t = struct {
> + int64_t seconds;
> + int64_t nanoseconds;
> +};
> +
> type emulation_vector_t = ^array[] of vm_offset_t;
> type rpc_signature_info_t = array[*:1024] of int;
> diff --git a/include/mach/time_value.h b/include/mach/time_value.h
> index 9ecdd28e..3a816b22 100644
> --- a/include/mach/time_value.h
> +++ b/include/mach/time_value.h
> @@ -98,23 +98,23 @@ 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) \
> +#define time_value64_add_nanos(val, nanos) { \
> + time_value64_assert(val); \
> + if (((val)->nanoseconds += (nanos)) \
> >= TIME_NANOS_MAX) { \
> (val)->nanoseconds -= TIME_NANOS_MAX; \
> (val)->seconds++; \
> } \
> - time_value64_assert(val); \
> + time_value64_assert(val); \
> }
> -#define time_value_sub_usec(val, micros) { \
> - time_value_assert(val); \
> - if (((val)->microseconds -= (micros)) < 0) { \
> - (val)->microseconds += TIME_MICROS_MAX; \
> +#define time_value64_sub_nanos(val, nanos) { \
> + time_value64_assert(val); \
> + if (((val)->nanoseconds -= (nanos)) < 0) { \
> + (val)->nanoseconds += TIME_NANOS_MAX; \
> (val)->seconds--; \
> } \
> - time_value_assert(val); \
> + time_value64_assert(val); \
> }
> #define time_value_add(result, addend) { \
> @@ -123,12 +123,28 @@ static __inline__ time_value_t
> convert_time_value_from_user(rpc_time_value_t tv)
> time_value_add_usec(result, (addend)->microseconds); \
> }
> -#define time_value_sub(result, subtrahend) { \
> - time_value_assert(subtrahend); \
> - (result)->seconds -= (subtrahend)->seconds; \
> - time_value_sub_usec(result, (subtrahend)->microseconds); \
> +#define time_value64_add(result, addend) {
> \
> + time_value64_assert(addend);
> \
> + (result)->seconds += (addend)->seconds;
> \
> + time_value64_add_nanos(result, (addend)->nanoseconds); \
> }
> +#define time_value64_sub(result, subtrahend) {
> \
> + time_value64_assert(subtrahend);
> \
> + (result)->seconds -= (subtrahend)->seconds;
> \
> + time_value64_sub_nanos(result, (subtrahend)->nanoseconds); \
> + }
> +
> +#define TIME_VALUE64_TO_TIME_VALUE(tv64, tv) do { \
> + (tv)->seconds = (tv64)->seconds;
> \
> + (tv)->microseconds = (tv64)->nanoseconds / 1000; \
> +} while(0)
> +
> +#define TIME_VALUE_TO_TIME_VALUE64(tv, tv64) do { \
> + (tv64)->seconds = (tv)->seconds;
> \
> + (tv64)->nanoseconds = (tv)->microseconds * 1000; \
> +} while(0)
> +
> /*
> * Time value available through the mapped-time interface.
> * Read this mapped value with
> @@ -144,6 +160,8 @@ typedef struct mapped_time_value {
> integer_t seconds;
> integer_t microseconds;
> integer_t check_seconds;
> + struct time_value64 time_value;
> + int64_t check_seconds64;
> } mapped_time_value_t;
> /* Macros for converting between struct timespec and time_value_t. */
> diff --git a/kern/mach_clock.c b/kern/mach_clock.c
> index 7383684b..06688fea 100644
> --- a/kern/mach_clock.c
> +++ b/kern/mach_clock.c
> @@ -96,21 +96,24 @@ volatile mapped_time_value_t *mtime = 0;
> MACRO_BEGIN \
> if (mtime != 0) { \
> mtime->check_seconds = (time)->seconds; \
> + mtime->check_seconds64 = (time)->seconds; \
> __sync_synchronize(); \
> mtime->microseconds = (time)->nanoseconds / 1000; \
> + mtime->time_value.nanoseconds = (time)->nanoseconds;
> \
> __sync_synchronize(); \
> mtime->seconds = (time)->seconds; \
> + mtime->time_value.seconds = (time)->seconds;
> \
> } \
> MACRO_END
> -#define read_mapped_time(time) \
> -MACRO_BEGIN \
> - do { \
> - time->seconds = mtime->seconds; \
> - __sync_synchronize(); \
> - time->microseconds = mtime->microseconds; \
> - __sync_synchronize(); \
> - } while (time->seconds != mtime->check_seconds); \
> +#define read_mapped_time(time)
> \
> +MACRO_BEGIN \
> + do { \
> + (time)->seconds = mtime->time_value.seconds; \
> + __sync_synchronize(); \
> + (time)->nanoseconds = mtime->time_value.nanoseconds; \
> + __sync_synchronize(); \
> + } while ((time)->seconds != mtime->check_seconds64); \
> MACRO_END
> decl_simple_lock_data(, timer_lock) /* lock for ... */
> @@ -225,7 +228,7 @@ void clock_interrupt(
> * Increment the time-of-day clock.
> */
> if (timedelta == 0) {
> - time_value64_add_usec(&time, usec);
> + time_value64_add_nanos(&time, usec * 1000);
> }
> else {
> int delta;
> @@ -246,7 +249,7 @@ void clock_interrupt(
> delta = usec + tickdelta;
> timedelta -= tickdelta;
> }
> - time_value64_add_usec(&time, delta);
> + time_value64_add_nanos(&time, delta * 1000);
> }
> update_mapped_time(&time);
> @@ -390,7 +393,7 @@ void init_timeout(void)
> * the boot-time clock by storing the difference to the real-time
> * clock.
> */
> -struct time_value clock_boottime_offset;
> +struct time_value64 clock_boottime_offset;
> /*
> * Update the offset of the boot-time clock from the real-time clock.
> @@ -398,11 +401,11 @@ struct time_value clock_boottime_offset;
> * This function must be called at SPLHIGH.
> */
> static void
> -clock_boottime_update(struct time_value *new_time)
> +clock_boottime_update(const struct time_value64 *new_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);
> + struct time_value64 delta = time;
> + time_value64_sub(&delta, new_time);
> + time_value64_add(&clock_boottime_offset, &delta);
> }
> /*
> @@ -410,10 +413,12 @@ clock_boottime_update(struct time_value *new_time)
> * frame.
> */
> void
> -record_time_stamp (time_value_t *stamp)
> +record_time_stamp(time_value_t *stamp)
> {
> - read_mapped_time(stamp);
> - time_value_add(stamp, &clock_boottime_offset);
> + time_value64_t stamp64;
> + read_mapped_time(&stamp64);
> + time_value64_add(&stamp64, &clock_boottime_offset);
> + TIME_VALUE64_TO_TIME_VALUE(&stamp64, stamp);
> }
> /*
> @@ -421,18 +426,35 @@ record_time_stamp (time_value_t *stamp)
> * real-time clock frame.
> */
> void
> -read_time_stamp (time_value_t *stamp, time_value_t *result)
> +read_time_stamp (const time_value_t *stamp, time_value_t *result)
> {
> - *result = *stamp;
> - time_value_sub(result, &clock_boottime_offset);
> + time_value64_t result64;
> + TIME_VALUE_TO_TIME_VALUE64(stamp, &result64);
> + time_value64_sub(&result64, &clock_boottime_offset);
> + TIME_VALUE64_TO_TIME_VALUE(&result64, result);
> }
> /*
> - * Read the time.
> + * Read the time (deprecated version).
> */
> kern_return_t
> host_get_time(const host_t host, time_value_t *current_time)
> +{
> + if (host == HOST_NULL)
> + return(KERN_INVALID_HOST);
> +
> + time_value64_t current_time64;
> + read_mapped_time(¤t_time64);
> + TIME_VALUE64_TO_TIME_VALUE(¤t_time64, current_time);
> + return (KERN_SUCCESS);
> +}
> +
> +/*
> + * Read the time.
> + */
> +kern_return_t
> +host_get_time64(const host_t host, time_value64_t *current_time)
> {
> if (host == HOST_NULL)
> return(KERN_INVALID_HOST);
> @@ -462,9 +484,10 @@ host_set_time(const host_t host, time_value_t new_time)
> #endif /* NCPUS > 1 */
> s = splhigh();
> - clock_boottime_update(&new_time);
> - time.seconds = new_time.seconds;
> - time.nanoseconds = new_time.microseconds * 1000;
> + time_value64_t new_time64;
> + TIME_VALUE_TO_TIME_VALUE64(&new_time, &new_time64);
> + clock_boottime_update(&new_time64);
> + time = new_time64;
> update_mapped_time(&time);
> resettodr();
> splx(s);
> diff --git a/kern/mach_clock.h b/kern/mach_clock.h
> index 903492e6..2da311d1 100644
> --- a/kern/mach_clock.h
> +++ b/kern/mach_clock.h
> @@ -98,7 +98,7 @@ extern void record_time_stamp (time_value_t *stamp);
> * Read a timestamp in STAMP into RESULT. Returns values in the
> * real-time clock frame.
> */
> -extern void read_time_stamp (time_value_t *stamp, time_value_t *result);
> +extern void read_time_stamp (const time_value_t *stamp, time_value_t
> *result);
> extern kern_return_t host_get_time(
> host_t host,
> --
> 2.39.0
>
--
Samuel
---
Pour une évaluation indépendante, transparente et rigoureuse !
Je soutiens la Commission d'Évaluation de l'Inria.
