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

[Samuel Thibault]

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

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.