[PATCH v3 2/4] printrusage: Correctly print the long types from the rusage struct

[Dmitry V. Levin]

On Fri, Mar 06, 2020 at 06:22:21PM -0800, Alistair Francis wrote:
> The kernel's rusage struct uses the kernel's __kernel_old_timeval which
> means that even for 32-bit archs with 64-bit time_t (like RV32) the time
> values are 32-bit.
> 
> There are two possible options here:
>  1. Use the glibc getrusage() function. Which takes a struct rusage with
>     a 64-bit time_t but does a syscall with a 32-bit time_t and handles
>     the conversion internally.
>  2. Specify our own rusage struct that uses a long for the time.
> 
> This patch fixes the failure useing option 2 above as we then get to
> call the raw syscall.
> 
> Signed-off-by: Alistair Francis <alistair.francis at wdc.com>
> ---
>  print_timeval.c   | 13 +++++++++++++
>  printrusage.c     |  4 ++--
>  tests/getrusage.c | 28 ++++++++++++++++++++++++++--
>  3 files changed, 41 insertions(+), 4 deletions(-)
> 
> diff --git a/print_timeval.c b/print_timeval.c
> index ff87b27b..4cc400e1 100644
> --- a/print_timeval.c
> +++ b/print_timeval.c
> @@ -7,6 +7,7 @@
>   */
>  
>  #include "defs.h"
> +#include <linux/time_types.h>

We cannot rely on this header because it's too new: it was introduced by
commit v5.1-rc1~178^2~227^2~1 while strace still builds with kernel
headers many years older than v5.1.

In other words, we have to provide a correct definition of the structure
we need without any help from kernel headers.

>  #include DEF_MPERS_TYPE(timeval_t)
>  
> @@ -25,6 +26,13 @@ print_timeval_t(const timeval_t *t)
>  		zero_extend_signed_to_ull(t->tv_usec));
>  }
>  
> +static void
> +print_old_timeval_t(const struct __kernel_old_timeval *t)
> +{
> +	tprintf(timeval_fmt, (long long) t->tv_sec,
> +		zero_extend_signed_to_ull(t->tv_usec));
> +}

Yet again, I don't understand why do we need yet another printer.

Since there are no architectures where kernel definitions of struct timeval
and struct __kernel_old_timeval wouldn't match, print_timeval_t should be
enough if timeval_t is defined correctly.

In other words, if you need print_old_timeval_t, then print_timeval_t
is already broken.

> +
>  static void
>  print_timeval_t_utime(const timeval_t *t)
>  {
> @@ -38,6 +46,11 @@ MPERS_PRINTER_DECL(void, print_struct_timeval, const void *arg)
>  	print_timeval_t(arg);
>  }
>  
> +MPERS_PRINTER_DECL(void, print_struct_old_timeval, const void *arg)
> +{
> +	print_old_timeval_t(arg);
> +}
> +
>  MPERS_PRINTER_DECL(bool, print_struct_timeval_data_size,
>  		   const void *arg, const size_t size)
>  {
> diff --git a/printrusage.c b/printrusage.c
> index 7cfeb7a0..04da400a 100644
> --- a/printrusage.c
> +++ b/printrusage.c
> @@ -51,9 +51,9 @@ MPERS_PRINTER_DECL(void, printrusage,
>  		return;
>  
>  	tprints("{ru_utime=");
> -	MPERS_FUNC_NAME(print_struct_timeval)(&ru.ru_utime);
> +	MPERS_FUNC_NAME(print_struct_old_timeval)(&ru.ru_utime);
>  	tprints(", ru_stime=");
> -	MPERS_FUNC_NAME(print_struct_timeval)(&ru.ru_stime);
> +	MPERS_FUNC_NAME(print_struct_old_timeval)(&ru.ru_stime);
>  	if (abbrev(tcp)) {
>  		tprints(", ...");
>  	} else {
> diff --git a/tests/getrusage.c b/tests/getrusage.c
> index b046fde2..cb2c4835 100644
> --- a/tests/getrusage.c
> +++ b/tests/getrusage.c
> @@ -21,8 +21,32 @@
>  # include "xlat.h"
>  # include "xlat/usagewho.h"
>  
> +struct kernel_old_timeval {
> +	kernel_long_t	tv_sec;
> +	long		tv_usec;
> +};
> +
> +struct rusage_t {
> +	struct kernel_old_timeval ru_utime;
> +	struct kernel_old_timeval ru_stime;
> +	kernel_long_t	ru_maxrss;
> +	kernel_long_t	ru_ixrss;
> +	kernel_long_t	ru_idrss;
> +	kernel_long_t	ru_isrss;
> +	kernel_long_t	ru_minflt;
> +	kernel_long_t	ru_majflt;
> +	kernel_long_t	ru_nswap;
> +	kernel_long_t	ru_inblock;
> +	kernel_long_t	ru_oublock;
> +	kernel_long_t	ru_msgsnd;
> +	kernel_long_t	ru_msgrcv;
> +	kernel_long_t	ru_nsignals;
> +	kernel_long_t	ru_nvcsw;
> +	kernel_long_t	ru_nivcsw;
> +};

Could we use <linux/resource.h> instead of <sys/resource.h> in this test?


-- 
ldv