On Tue, Apr 25, 2023 at 4:11 PM Samuel Thibault <[email protected]>
wrote:
> Flavio Cruz, le mar. 25 avril 2023 00:07:46 -0400, a ecrit:
> > On Mon, Apr 17, 2023 at 11:49:46AM +0200, Samuel Thibault wrote:
> > > Is this really needed? Since rpc_time_value_t will already be 64bit on
> > > 64bit platforms.
> > >
> > > (I don't hope to bring 64bit time to 32bit Hurd)
> >
> > time_value64_t is slightly better than time_value_t since it is
> > future-proofed to provide nanosecond precision while time_value_t is
> limited
> > to microseconds.
>
> Ok, but none of the clock(), getrusage() and times() functions expose
> nano-second precision. If we can avoid complexity when we don't actually
> need it, I'd rather avoid it :)
>
Sounds fair, but doesn't clock_gettime use struct timespec? According to
the man pages, it uses tv_nsec but then again if we don't ever intend to
change the resolution of the timer, it's not worth changing the RPCs anyway.
> Samuel
>
> > > Flavio Cruz, le lun. 17 avril 2023 00:46:36 -0400, a ecrit:
> > > > RPCs remain compatible with existing clients but if they know about
> the
> > > > new size then we will populate the new fields.
> > > > ---
> > > > include/mach/task_info.h | 8 ++++++++
> > > > include/mach/thread_info.h | 6 ++++++
> > > > kern/task.c | 16 +++++++++++-----
> > > > kern/thread.c | 16 ++++++++++++----
> > > > 4 files changed, 37 insertions(+), 9 deletions(-)
> > > >
> > > > diff --git a/include/mach/task_info.h b/include/mach/task_info.h
> > > > index f448ee04..2631b04e 100644
> > > > --- a/include/mach/task_info.h
> > > > +++ b/include/mach/task_info.h
> > > > @@ -56,11 +56,19 @@ struct task_basic_info {
> > > > integer_t base_priority; /* base scheduling priority */
> > > > rpc_vm_size_t virtual_size; /* number of virtual pages */
> > > > rpc_vm_size_t resident_size; /* number of resident pages */
> > > > + /* Deprecated, please use user_time64 */
> > > > rpc_time_value_t user_time; /* total user run time for
> > > > terminated threads */
> > > > + /* Deprecated, please use system_time64 */
> > > > rpc_time_value_t system_time; /* total system run time
> for
> > > > terminated threads */
> > > > + /* Deprecated, please use creation_time64 */
> > > > rpc_time_value_t creation_time; /* creation time stamp */
> > > > + time_value64_t user_time64; /* total user run time for
> > > > + terminated threads */
> > > > + time_value64_t system_time64; /* total system run time
> for
> > > > + terminated threads */
> > > > + time_value64_t creation_time64; /* creation time
> stamp */
> > > > };
> > > >
> > > > typedef struct task_basic_info task_basic_info_data_t;
> > > > diff --git a/include/mach/thread_info.h b/include/mach/thread_info.h
> > > > index 46c1ceca..4f322e0a 100644
> > > > --- a/include/mach/thread_info.h
> > > > +++ b/include/mach/thread_info.h
> > > > @@ -55,7 +55,9 @@ typedef integer_t
> thread_info_data_t[THREAD_INFO_MAX];
> > > > #define THREAD_BASIC_INFO 1 /* basic
> information */
> > > >
> > > > struct thread_basic_info {
> > > > + /* Deprecated, please use user_time64 */
> > > > rpc_time_value_t user_time; /* user run time */
> > > > + /* Deprecated, please use system_time64 */
> > > > rpc_time_value_t system_time; /* system run time */
> > > > integer_t cpu_usage; /* scaled cpu usage percentage */
> > > > integer_t base_priority; /* base scheduling priority */
> > > > @@ -65,7 +67,11 @@ struct thread_basic_info {
> > > > integer_t suspend_count; /* suspend count for thread */
> > > > integer_t sleep_time; /* number of seconds that thread
> > > > has been sleeping */
> > > > + /* Deprecated, please use creation_time64 */
> > > > rpc_time_value_t creation_time; /* time stamp of creation
> */
> > > > + time_value64_t user_time64; /* user run time */
> > > > + time_value64_t system_time64; /* system run time */
> > > > + time_value64_t creation_time64; /* time stamp of creation
> */
> > > > };
> > > >
> > > > typedef struct thread_basic_info thread_basic_info_data_t;
> > > > diff --git a/kern/task.c b/kern/task.c
> > > > index 65191f5d..9492b448 100644
> > > > --- a/kern/task.c
> > > > +++ b/kern/task.c
> > > > @@ -787,13 +787,13 @@ kern_return_t task_info(
> > > > {
> > > > task_basic_info_t basic_info;
> > > >
> > > > - /* Allow *task_info_count to be two words smaller than
> > > > - the usual amount, because creation_time is a new member
> > > > - that some callers might not know about. */
> > > > + /* Allow *task_info_count to be smaller than the provided
> amount
> > > > + * that does not contain the new time_value64_t fields as
> some
> > > > + * callers might not know about them yet. */
> > > >
> > > > - if (*task_info_count < TASK_BASIC_INFO_COUNT - 2) {
> > > > + if (*task_info_count <
> > > > + TASK_BASIC_INFO_COUNT - 3 *
> sizeof(time_value64_t)/sizeof(integer_t))
> > > > return KERN_INVALID_ARGUMENT;
> > > > - }
> > > >
> > > > basic_info = (task_basic_info_t) task_info_out;
> > > >
> > > > @@ -813,6 +813,12 @@ kern_return_t task_info(
> > > > time_value64_t creation_time64;
> > > > read_time_stamp(&task->creation_time, &creation_time64);
> > > > TIME_VALUE64_TO_TIME_VALUE(&creation_time64,
> &basic_info->creation_time);
> > > > + if (*task_info_count == TASK_BASIC_INFO_COUNT) {
> > > > + /* Copy new time_value64_t fields */
> > > > + basic_info->user_time64 = task->total_user_time;
> > > > + basic_info->system_time64 = task->total_system_time;
> > > > + basic_info->creation_time64 = creation_time64;
> > > > + }
> > > > task_unlock(task);
> > > >
> > > > if (*task_info_count > TASK_BASIC_INFO_COUNT)
> > > > diff --git a/kern/thread.c b/kern/thread.c
> > > > index 392d38f8..20c11024 100644
> > > > --- a/kern/thread.c
> > > > +++ b/kern/thread.c
> > > > @@ -1501,11 +1501,12 @@ kern_return_t thread_info(
> > > > if (flavor == THREAD_BASIC_INFO) {
> > > > thread_basic_info_t basic_info;
> > > >
> > > > - /* Allow *thread_info_count to be one smaller than the
> > > > - usual amount, because creation_time is a new member
> > > > - that some callers might not know about. */
> > > > + /* Allow *thread_info_count to be smaller than the provided
> amount
> > > > + * that does not contain the new time_value64_t fields as some
> > > > + * callers might not know about them yet. */
> > > >
> > > > - if (*thread_info_count < THREAD_BASIC_INFO_COUNT - 1)
> > > > + if (*thread_info_count <
> > > > + THREAD_BASIC_INFO_COUNT - 3 *
> sizeof(time_value64_t)/sizeof(natural_t))
> > > > return KERN_INVALID_ARGUMENT;
> > > >
> > > > basic_info = (thread_basic_info_t) thread_info_out;
> > > > @@ -1533,6 +1534,13 @@ kern_return_t thread_info(
> > > > read_time_stamp(&thread->creation_time, &creation_time);
> > > > TIME_VALUE64_TO_TIME_VALUE(&creation_time,
> &basic_info->creation_time);
> > > >
> > > > + if (*thread_info_count == THREAD_BASIC_INFO_COUNT) {
> > > > + /* Copy new time_value64_t fields */
> > > > + basic_info->user_time64 = user_time;
> > > > + basic_info->system_time64 = user_time;
> > > > + basic_info->creation_time64 = creation_time;
> > > > + }
> > > > +
> > > > /*
> > > > * To calculate cpu_usage, first correct for timer rate,
> > > > * then for 5/8 ageing. The correction factor [3/5] is
> > > > --
> > > > 2.39.2
> > > >
> > > >
> >
>
> --
> Samuel
> ---
> Pour une évaluation indépendante, transparente et rigoureuse !
> Je soutiens la Commission d'Évaluation de l'Inria.
>
