Re: [PATCH] Make task_info.h structs more portable

[Samuel Thibault]

Applied, thanks!

Flavio Cruz, le mar. 06 déc. 2022 00:35:01 -0500, a ecrit:
> Changed vm_size_t to rpc_size_t so that both userland and kernel agree
> on the same size. Also changed the denominator for the maximum struct
> sizes to be integer_t to match the other declarations.
> 
> Introduced long_natural_t and rpc_long_natural_t to represent large
> counters. Replaced rpc_unsigned_long with rpc_long_natural_t.
> ---
>  doc/mach.texi                             | 18 ++++++------
>  i386/include/mach/i386/machine_types.defs | 35 ++++++++++++-----------
>  i386/include/mach/i386/vm_types.h         | 26 ++++++++---------
>  include/mach/task_info.h                  | 24 ++++++++--------
>  include/mach_debug/mach_debug_types.defs  | 10 +++----
>  include/mach_debug/slab_info.h            | 10 +++----
>  6 files changed, 63 insertions(+), 60 deletions(-)
> 
> diff --git a/doc/mach.texi b/doc/mach.texi
> index fecd097b..86a557cb 100644
> --- a/doc/mach.texi
> +++ b/doc/mach.texi
> @@ -5011,10 +5011,10 @@ suspend count for task
>  @item integer_t base_priority
>  base scheduling priority
>  
> -@item vm_size_t virtual_size
> +@item rpc_vm_size_t virtual_size
>  number of virtual pages
>  
> -@item vm_size_t resident_size
> +@item rpc_vm_size_t resident_size
>  number of resident pages
>  
>  @item time_value_t user_time
> @@ -5041,25 +5041,25 @@ provided it as the @var{task_info} parameter for the
>  following members:
>  
>  @table @code
> -@item natural_t faults
> +@item rpc_long_natural_t faults
>  number of page faults
>  
> -@item natural_t zero_fills
> +@item rpc_long_natural_t zero_fills
>  number of zero fill pages
>  
> -@item natural_t reactivations
> +@item rpc_long_natural_t reactivations
>  number of reactivated pages
>  
> -@item natural_t pageins
> +@item rpc_long_natural_t pageins
>  number of actual pageins
>  
> -@item natural_t cow_faults
> +@item rpc_long_natural_t cow_faults
>  number of copy-on-write faults
>  
> -@item natural_t messages_sent
> +@item rpc_long_natural_t messages_sent
>  number of messages sent
>  
> -@item natural_t messages_received
> +@item rpc_long_natural_t messages_received
>  number of messages received
>  @end table
>  @end deftp
> diff --git a/i386/include/mach/i386/machine_types.defs 
> b/i386/include/mach/i386/machine_types.defs
> index 0e94999b..f3de1e44 100755
> --- a/i386/include/mach/i386/machine_types.defs
> +++ b/i386/include/mach/i386/machine_types.defs
> @@ -38,14 +38,10 @@
>  
>  /*
>   * A natural_t is the type for the native
> - * integer type, e.g. 32 or 64 or.. whatever
> - * register size the machine has.  Unsigned, it is
> - * used for entities that might be either
> - * unsigned integers or pointers, and for
> - * type-casting between the two.
> - * For instance, the IPC system represents
> - * a port in user space as an integer and
> - * in kernel space as a pointer.
> + * unsigned integer type, usually 32 bits. It is suitable for
> + * most counters with a small chance of overflow.
> + * While historically natural_t was meant to be the same
> + * as a pointer, that is not the case here.
>   */
>  type natural_t = uint32_t;
>  
> @@ -59,17 +55,24 @@ type natural_t = uint32_t;
>  type integer_t = int32_t;
>  
>  /*
> - * unsigned long for kernel <-> userland interfaces size depends on the 
> architecture
> - * of both kernel and userland.
> + * A long_natural_t is a possibly larger unsigned integer type than 
> natural_t.
> + * Should be used instead of natural_t when we want the data to be less 
> subject
> + * to overflows.
>   */
> -#if defined(KERNEL) && defined(USER32)
> -type rpc_unsigned_long = uint32_t;
> -#else /* KERNEL and USER32 */
>  #if defined(__x86_64__)
> -type rpc_unsigned_long = uint64_t;
> -#else /* __x86_64__ */
> -type rpc_unsigned_long = uint32_t;
> +type long_natural_t = uint64_t;
> +#else
> +type long_natural_t = uint32_t;
>  #endif /* __x86_64__ */
> +
> +/*
> + * long_natural_t for kernel <-> userland interfaces as the size depends on 
> the
> + * architecture of both kernel and userland.
> + */
> +#if defined(KERNEL) && defined(USER32)
> +type rpc_long_natural_t = uint32_t;
> +#else /* KERNEL and USER32 */
> +type rpc_long_natural_t = long_natural_t;
>  #endif /* KERNEL_SERVER and USER32 */
>  
>  /*
> diff --git a/i386/include/mach/i386/vm_types.h 
> b/i386/include/mach/i386/vm_types.h
> index 9daaa15e..a50665c8 100644
> --- a/i386/include/mach/i386/vm_types.h
> +++ b/i386/include/mach/i386/vm_types.h
> @@ -45,18 +45,11 @@
>  
>  /*
>   * A natural_t is the type for the native
> - * integer type, e.g. 32 or 64 or.. whatever
> - * register size the machine has.  Unsigned, it is
> - * used for entities that might be either
> - * unsigned integers or pointers, and for
> - * type-casting between the two.
> - * For instance, the IPC system represents
> - * a port in user space as an integer and
> - * in kernel space as a pointer.
> + * unsigned integer type, usually 32 bits. It is suitable for
> + * most counters with a small chance of overflow.
> + * While historically natural_t was meant to be the same
> + * as a pointer, that is not the case here.
>   */
> -#ifdef __x86_64__
> -// unsigned long ?
> -#endif
>  typedef unsigned int natural_t;
>  
>  /*
> @@ -68,6 +61,13 @@ typedef unsigned int       natural_t;
>   */
>  typedef int          integer_t;
>  
> +/*
> + * A long_natural_t is a possibly larger unsigned integer type than 
> natural_t.
> + * Should be used instead of natural_t when we want the data to be less 
> subject
> + * to overflows.
> + */
> +typedef unsigned long long_natural_t;
> +
>  /*
>   * A vm_offset_t is a type-neutral pointer,
>   * e.g. an offset into a virtual memory space.
> @@ -116,14 +116,14 @@ static inline __mach_uint32_t 
> convert_vm_to_user(__mach_uint64_t kaddr)
>      assert(kaddr <= 0xFFFFFFFF);
>      return (__mach_uint32_t)kaddr;
>  }
> -typedef __mach_uint32_t rpc_unsigned_long;
> +typedef __mach_uint32_t rpc_long_natural_t;
>  #else /* MACH_KERNEL */
>  typedef vm_offset_t  rpc_vm_address_t;
>  typedef vm_offset_t  rpc_vm_offset_t;
>  typedef vm_size_t    rpc_vm_size_t;
>  #define convert_vm_to_user null_conversion
>  #define convert_vm_from_user null_conversion
> -typedef unsigned long rpc_unsigned_long;
> +typedef long_natural_t rpc_long_natural_t;
>  #endif /* MACH_KERNEL */
>  
>  #endif       /* __ASSEMBLER__ */
> diff --git a/include/mach/task_info.h b/include/mach/task_info.h
> index 5607178c..3aaa7cd6 100644
> --- a/include/mach/task_info.h
> +++ b/include/mach/task_info.h
> @@ -54,8 +54,8 @@ typedef     integer_t       task_info_data_t[TASK_INFO_MAX];
>  struct task_basic_info {
>       integer_t       suspend_count;  /* suspend count for task */
>       integer_t       base_priority;  /* base scheduling priority */
> -     vm_size_t       virtual_size;   /* number of virtual pages */
> -     vm_size_t       resident_size;  /* number of resident pages */
> +     rpc_vm_size_t   virtual_size;   /* number of virtual pages */
> +     rpc_vm_size_t   resident_size;  /* number of resident pages */
>       time_value_t    user_time;      /* total user run time for
>                                          terminated threads */
>       time_value_t    system_time;    /* total system run time for
> @@ -66,24 +66,24 @@ struct task_basic_info {
>  typedef struct task_basic_info               task_basic_info_data_t;
>  typedef struct task_basic_info               *task_basic_info_t;
>  #define      TASK_BASIC_INFO_COUNT   \
> -             (sizeof(task_basic_info_data_t) / sizeof(natural_t))
> +             (sizeof(task_basic_info_data_t) / sizeof(integer_t))
>  
>  
>  #define      TASK_EVENTS_INFO        2       /* various event counts */
>  
>  struct task_events_info {
> -     natural_t               faults;         /* number of page faults */
> -     natural_t               zero_fills;     /* number of zero fill pages */
> -     natural_t               reactivations;  /* number of reactivated pages 
> */
> -     natural_t               pageins;        /* number of actual pageins */
> -     natural_t               cow_faults;     /* number of copy-on-write 
> faults */
> -     natural_t               messages_sent;  /* number of messages sent */
> -     natural_t               messages_received; /* number of messages 
> received */
> +     rpc_long_natural_t      faults;         /* number of page faults */
> +     rpc_long_natural_t      zero_fills;     /* number of zero fill pages */
> +     rpc_long_natural_t      reactivations;  /* number of reactivated pages 
> */
> +     rpc_long_natural_t      pageins;        /* number of actual pageins */
> +     rpc_long_natural_t      cow_faults;     /* number of copy-on-write 
> faults */
> +     rpc_long_natural_t      messages_sent;  /* number of messages sent */
> +     rpc_long_natural_t      messages_received; /* number of messages 
> received */
>  };
>  typedef struct task_events_info              task_events_info_data_t;
>  typedef struct task_events_info              *task_events_info_t;
>  #define      TASK_EVENTS_INFO_COUNT  \
> -             (sizeof(task_events_info_data_t) / sizeof(natural_t))
> +             (sizeof(task_events_info_data_t) / sizeof(integer_t))
>  
>  #define      TASK_THREAD_TIMES_INFO  3       /* total times for live threads 
> -
>                                          only accurate if suspended */
> @@ -98,7 +98,7 @@ struct task_thread_times_info {
>  typedef struct task_thread_times_info        task_thread_times_info_data_t;
>  typedef struct task_thread_times_info        *task_thread_times_info_t;
>  #define      TASK_THREAD_TIMES_INFO_COUNT    \
> -             (sizeof(task_thread_times_info_data_t) / sizeof(natural_t))
> +             (sizeof(task_thread_times_info_data_t) / sizeof(integer_t))
>  
>  /*
>   * Flavor definitions for task_ras_control
> diff --git a/include/mach_debug/mach_debug_types.defs 
> b/include/mach_debug/mach_debug_types.defs
> index c138dc40..fd940384 100644
> --- a/include/mach_debug/mach_debug_types.defs
> +++ b/include/mach_debug/mach_debug_types.defs
> @@ -42,11 +42,11 @@ type cache_info_t = struct {
>     rpc_vm_size_t align;
>     rpc_vm_size_t buf_size;
>     rpc_vm_size_t slab_size;
> -   rpc_unsigned_long bufs_per_slab;
> -   rpc_unsigned_long nr_objs;
> -   rpc_unsigned_long nr_bufs;
> -   rpc_unsigned_long nr_slabs;
> -   rpc_unsigned_long nr_free_slabs;
> +   rpc_long_natural_t bufs_per_slab;
> +   rpc_long_natural_t nr_objs;
> +   rpc_long_natural_t nr_bufs;
> +   rpc_long_natural_t nr_slabs;
> +   rpc_long_natural_t nr_free_slabs;
>     cache_name_t name;
>  };
>  type cache_info_array_t = array[] of cache_info_t;
> diff --git a/include/mach_debug/slab_info.h b/include/mach_debug/slab_info.h
> index 19a87307..0f6b5a2c 100644
> --- a/include/mach_debug/slab_info.h
> +++ b/include/mach_debug/slab_info.h
> @@ -43,11 +43,11 @@ typedef struct cache_info {
>       rpc_vm_size_t align;
>       rpc_vm_size_t buf_size;
>       rpc_vm_size_t slab_size;
> -     rpc_unsigned_long bufs_per_slab;
> -     rpc_unsigned_long nr_objs;
> -     rpc_unsigned_long nr_bufs;
> -     rpc_unsigned_long nr_slabs;
> -     rpc_unsigned_long nr_free_slabs;
> +     rpc_long_natural_t bufs_per_slab;
> +     rpc_long_natural_t nr_objs;
> +     rpc_long_natural_t nr_bufs;
> +     rpc_long_natural_t nr_slabs;
> +     rpc_long_natural_t nr_free_slabs;
>       char name[CACHE_NAME_MAX_LEN];
>  } cache_info_t;
>  
> -- 
> 2.37.2
> 
> 

-- 
Samuel
---
Pour une évaluation indépendante, transparente et rigoureuse !
Je soutiens la Commission d'Évaluation de l'Inria.