Re: [PATCH v4 2/3] tools/accounting: factor out shared format_timespec() implementation

On Fri, Jul 24, 2026 at 03:07:00PM +0800, [email protected] wrote:
> From: Wang Yaxin <[email protected]>
> 
> The same __kernel_timespec formatting logic existed independently in
> both getdelays.c and delaytop.c with minor differences (strftime vs
> snprintf, __kernel_time64_t vs time_t).
> 
> Create a shared format_timespec.c/h with a canonical implementation
> (strftime + time_t), remove the static copies from both files, and
> link both programs against the common object.
> 
> Also simplify the Makefile with a pattern rule for %.o and a static
> pattern rule for the two programs that need format_timespec.o.
> 
> Signed-off-by: Wang Yaxin <[email protected]>
> Acked-by: Xu Xin <[email protected]>
> ---
>  tools/accounting/Makefile             | 12 ++++++-
>  tools/accounting/delaytop.c           | 39 ++------------------
>  tools/accounting/format_timespec.c    | 39 ++++++++++++++++++++
>  tools/accounting/format_timespec.h    |  9 +++++
>  tools/accounting/getdelays.c          | 32 ++---------------
>  tools/include/uapi/linux/time_types.h | 51 +++++++++++++++++++++++++++
>  6 files changed, 115 insertions(+), 67 deletions(-)
>  create mode 100644 tools/accounting/format_timespec.c
>  create mode 100644 tools/accounting/format_timespec.h
>  create mode 100644 tools/include/uapi/linux/time_types.h

(...)

> diff --git a/tools/include/uapi/linux/time_types.h b/tools/include/uapi/linux/time_types.h
> new file mode 100644
> index 000000000000..8abdca230a54
> --- /dev/null
> +++ b/tools/include/uapi/linux/time_types.h

The addition of a new tools/include/ header should be its own patch.

> @@ -0,0 +1,51 @@
> +/* SPDX-License-Identifier: GPL-2.0 WITH Linux-syscall-note */
> +#ifndef _TOOLS_UAPI_LINUX_TIME_TYPES_H
> +#define _TOOLS_UAPI_LINUX_TIME_TYPES_H
> +
> +#include <linux/types.h>
> +#include <asm/posix_types.h>

So the tools/include/ header are supposed to be platform independent and standalone.
But this relies on an asm/ header which itself is not part of tools/include/.
The addition of this header did not remove the dependency on system UAPI headers.

So why is this new header copy needed in its current form?

> +
> +/*
> + * Copied from include/uapi/linux/time_types.h

This should be obvious without a comment.

> + *
> + * Uses the arch-dependent __kernel_long_t rather than a hardcoded
> + * 'long' so that x32 (ILP32 on x86_64), where the kernel defines
> + * __kernel_long_t as 'long long', gets the correct struct layout.
> + *
> + * __kernel_long_t is provided by <asm/posix_types.h> - included
> + * below via <linux/types.h> on most systems, or directly here.

"below"? It is above.

But the whole comment is misplaced in my opinion.
If it doesn't exist in the original it should not be in the copy.

> + */
> +
> +struct __kernel_timespec {
> +	long long	tv_sec;
> +	long long	tv_nsec;
> +};
> +
> +struct __kernel_itimerspec {
> +	struct __kernel_timespec it_interval;
> +	struct __kernel_timespec it_value;
> +};
> +
> +#ifndef __kernel_old_timeval
> +struct __kernel_old_timeval {
> +	__kernel_long_t		tv_sec;
> +	__kernel_long_t		tv_usec;
> +};
> +#endif
> +
> +struct __kernel_old_timespec {
> +	__kernel_long_t		tv_sec;
> +	long			tv_nsec;
> +};
> +
> +struct __kernel_old_itimerval {
> +	struct __kernel_old_timeval it_interval;
> +	struct __kernel_old_timeval it_value;
> +};
> +
> +struct __kernel_sock_timeval {
> +	__s64 tv_sec;
> +	__s64 tv_usec;
> +};
> +
> +#endif /* _TOOLS_UAPI_LINUX_TIME_TYPES_H */
> -- 
> 2.27.0