Re: Feedback Request: Standardizing Time Helpers

On Tue, Sep 30, 2025 at 10:08 AM Gleb Smirnoff <glebius@freebsd.org> wrote:

> Forwarding to hackers@ since Nick isn't subscribed and the original email
> didn't reach.
>
> On Tue, Sep 30, 2025 at 10:10:14AM -0400, Nick Banks wrote:
> N> Hello All,
> N>
> N> For the last few weeks I've been refactoring the HPTS code (tcp_hpts.c),
> N> with one ultimate goal of eliminating the hard dependencies with TCP so
> the
> N> HPTS code may be used in the future for other high performance timing
> N> scenarios.
> N>
> N> My latest focus is on various constants and macros tcp_hpts.h for
> N> converting between different time units and structures. For instance:
> N>
> N> #define HPTS_USEC_IN_SEC 1000000
> N> #define HPTS_MSEC_IN_SEC 1000
> N> #define HPTS_USEC_IN_MSEC 1000
> N>
> N> static inline uint32_t
> N> tcp_tv_to_usec(const struct timeval *sv)
> N> {
> N> return ((uint32_t) ((sv->tv_sec * HPTS_USEC_IN_SEC) + sv->tv_usec));
> N> }
> N>
> N> static inline uint32_t
> N> tcp_tv_to_msec(const struct timeval *sv)
> N> {
> N> return ((uint32_t) ((sv->tv_sec * HPTS_MSEC_IN_SEC) + (sv->tv_usec/
> N> HPTS_USEC_IN_MSEC)));
> N> }
> N>
> N> static inline uint64_t
> N> tcp_tv_to_lusec(const struct timeval *sv)
> N> {
> N> return ((uint64_t)((sv->tv_sec * HPTS_USEC_IN_SEC) + sv->tv_usec));
> N> }
> N>
> N> My goal is to standardize these and move them out of the HPTS & TCP
> code,
> N> if possible. One possible location could be time.h where many of the
> N> related dependencies are already. But, depending on how we move
> forward, we
> N> obviously need to be careful about collisions with pre-existing user
> mode
> N> code. So, perhaps the new defines could be wrapped with _KERNEL and some
> N> opt-in flag (or possibly __BSD_VISIBLE), such as what I have below?
> N>
> N> #if defined(_KERNEL) || defined(__BSD_VISIBLE)
> N> /*
> N> * Time unit conversion constants
> N> */
> N> #define USEC_IN_SEC 1000000L /* microseconds per second */
> N> #define MSEC_IN_SEC 1000L /* milliseconds per second */
> N> #define USEC_IN_MSEC 1000L /* microseconds per millisecond */
> N>
> N> /*
> N> * Time unit conversion macros
> N> */
> N> #define USEC_TO_MSEC(x) ((x) / USEC_IN_MSEC) /* microseconds to
> N> milliseconds */
> N> #define MSEC_TO_USEC(x) ((x) * USEC_IN_MSEC) /* milliseconds to
> N> microseconds */
> N> #define USEC_TO_SEC(x) ((x) / USEC_IN_SEC) /* microseconds to seconds */
> N> #define SEC_TO_USEC(x) ((x) * USEC_IN_SEC) /* seconds to microseconds */
> N> #define USEC64_TO_MSEC(x) ((uint32_t)((x) / USEC_IN_MSEC)) /* 64-bit
> usec
> N> timestamp to msec */
> N>
> N> /*
> N> * Timeval conversion macros
> N> */
> N> #define timeval_to_usec(tv) ((uint32_t)(((tv)->tv_sec * USEC_IN_SEC) +
> N> (tv)->tv_usec))
> N> #define timeval_to_msec(tv) ((uint32_t)(((tv)->tv_sec * MSEC_IN_SEC) +
> N> ((tv)->tv_usec / USEC_IN_MSEC)))
> N> #define timeval_to_usec64(tv) ((uint64_t)(((tv)->tv_sec * USEC_IN_SEC) +
> N> (tv)->tv_usec))
> N> #endif /* _KERNEL || __BSD_VISIBLE */
> N>
> N> I'd appreciate any suggestion on how best to proceed here. Is there an
> N> appetite to have a common place for these kinds of definitions, outside
> of
> N> networking?
>

So what's wrong with the stuff we define in sys/time.h that does these
conversions (with more accuracy, but it's going from base 10 fractions to
base 2 fractions effectively) for the sbintime_t stuff? This looks largely
duplicative of that stuff, or at least complementary...

For example:
static __inline uint64_t
bintime2ns(const struct bintime *_bt)
{
        uint64_t ret;

        ret = (uint64_t)(_bt->sec) * (uint64_t)1000000000;
        ret += __utime64_scale64_floor(
            _bt->frac, 1000000000, 1ULL << 32) >> 32;
        return (ret);
}

looks similar to the above conversions.  Why not just expand those?

Warner