Re: [PATCH v2] sysdeps: Add 'get_fast_jitter' interace in fast-jitter.h

["H.J. Lu" <hjl.tools@gmail.com>]

On Wed, Apr 27, 2022 at 1:13 PM Noah Goldstein <goldstein.w.n@gmail.com> wrote:
>
> 'get_fast_jitter' is meant to be used purely for performance
> purposes. In all cases it's used it should be acceptable to get no
> randomness (see default case). An example use case is in setting
> jitter for retries between threads at a lock. There is a
> performance benefit to having jitter, but only if the jitter can
> be generated very quickly and ultimately there is no serious issue
> if no jitter is generated.
>
> The implementation generally uses 'HP_TIMING_NOW' iff it is
> inlined (avoid any potential syscall paths).
> ---
>  sysdeps/generic/fast-jitter.h | 42 +++++++++++++++++++++++++++++++++++
>  1 file changed, 42 insertions(+)
>  create mode 100644 sysdeps/generic/fast-jitter.h
>
> diff --git a/sysdeps/generic/fast-jitter.h b/sysdeps/generic/fast-jitter.h
> new file mode 100644
> index 0000000000..4dd53e3475
> --- /dev/null
> +++ b/sysdeps/generic/fast-jitter.h
> @@ -0,0 +1,42 @@
> +/* Fallback for fast jitter just return 0.
> +   Copyright (C) 2019-2022 Free Software Foundation, Inc.
> +   This file is part of the GNU C Library.
> +
> +   The GNU C Library is free software; you can redistribute it and/or
> +   modify it under the terms of the GNU Lesser General Public
> +   License as published by the Free Software Foundation; either
> +   version 2.1 of the License, or (at your option) any later version.
> +
> +   The GNU C Library is distributed in the hope that it will be useful,
> +   but WITHOUT ANY WARRANTY; without even the implied warranty of
> +   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> +   Lesser General Public License for more details.
> +
> +   You should have received a copy of the GNU Lesser General Public
> +   License along with the GNU C Library; if not, see
> +   <https://www.gnu.org/licenses/>.  */
> +
> +#ifndef _FAST_JITTER_H
> +# define _FAST_JITTER_H
> +
> +# include <stdint.h>
> +# include <hp-timing.h>
> +
> +/* Baseline just return 0.  We could create jitter using a clock or
> +   'random_bits' but that may imply a syscall and the goal of
> +   'get_fast_jitter' is minimal overhead "randomness" when such
> +   randomness helps performance.  Adding high overhead the function
> +   defeats the purpose.  */
> +static inline uint32_t
> +get_fast_jitter (void)
> +{
> +# if HP_TIMING_INLINE
> +  hp_timing_t jitter;
> +  HP_TIMING_NOW (jitter);
> +  return (uint32_t) jitter;
> +# else
> +  return 0;
> +# endif
> +}
> +
> +#endif
> --
> 2.25.1
>

LGTM.    An arch can provide a better one if needed.

Reviewed-by: H.J. Lu <hjl.tools@gmail.com>

Thanks.

-- 
H.J.