Re: [PATCH gnumach] Update task_basic_info and thread_basic_info to include time_value64_t data.

[Samuel Thibault]

Hello,

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)

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
> 
>