Re: [RESEND PATCH] bpf: Add support for absolute value BPF timers

[Alexei Starovoitov]

On Tue, Feb 21, 2023 at 7:21 AM Tero Kristo <tero.kristo@xxxxxxxxxxxxxxx> wrote:
>
> Add a new flag BPF_F_TIMER_ABS that can be passed to bpf_timer_start()
> to start an absolute value timer instead of the default relative value.
> This makes the timer expire at an exact point in time, instead of a time
> with latencies and jitter induced by both the BPF and timer subsystems.
> This is useful e.g. in certain time sensitive profiling cases, where we
> need a timer to expire at an exact point in time.

"certain time sensitive profiling cases" is too vague.

Please precisely describe the use case.

> Signed-off-by: Tero Kristo <tero.kristo@xxxxxxxxxxxxxxx>
> ---
>  include/uapi/linux/bpf.h | 15 +++++++++++++++
>  kernel/bpf/helpers.c     | 11 +++++++++--
>  2 files changed, 24 insertions(+), 2 deletions(-)
>
> diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
> index 464ca3f01fe7..7f5b71847984 100644
> --- a/include/uapi/linux/bpf.h
> +++ b/include/uapi/linux/bpf.h
> @@ -4951,6 +4951,12 @@ union bpf_attr {
>   *             different maps if key/value layout matches across maps.
>   *             Every bpf_timer_set_callback() can have different callback_fn.
>   *
> + *             *flags* can be one of:
> + *
> + *             **BPF_F_TIMER_ABS**
> + *                     Start the timer in absolute expire value instead of the
> + *                     default relative one.
> + *
>   *     Return
>   *             0 on success.
>   *             **-EINVAL** if *timer* was not initialized with bpf_timer_init() earlier
> @@ -7050,4 +7056,13 @@ struct bpf_core_relo {
>         enum bpf_core_relo_kind kind;
>  };
>
> +/*
> + * Flags to control bpf_timer_start() behaviour.
> + *     - BPF_F_TIMER_ABS: Timeout passed is absolute time, by default it is
> + *       relative to current time.
> + */
> +enum {
> +       BPF_F_TIMER_ABS = (1ULL << 0),
> +};
> +
>  #endif /* _UAPI__LINUX_BPF_H__ */
> diff --git a/kernel/bpf/helpers.c b/kernel/bpf/helpers.c
> index af30c6cbd65d..924849d89828 100644
> --- a/kernel/bpf/helpers.c
> +++ b/kernel/bpf/helpers.c
> @@ -1253,10 +1253,11 @@ BPF_CALL_3(bpf_timer_start, struct bpf_timer_kern *, timer, u64, nsecs, u64, fla
>  {
>         struct bpf_hrtimer *t;
>         int ret = 0;
> +       enum hrtimer_mode mode;
>
>         if (in_nmi())
>                 return -EOPNOTSUPP;
> -       if (flags)
> +       if (flags > BPF_F_TIMER_ABS)
>                 return -EINVAL;
>         __bpf_spin_lock_irqsave(&timer->lock);
>         t = timer->timer;
> @@ -1264,7 +1265,13 @@ BPF_CALL_3(bpf_timer_start, struct bpf_timer_kern *, timer, u64, nsecs, u64, fla
>                 ret = -EINVAL;
>                 goto out;
>         }
> -       hrtimer_start(&t->timer, ns_to_ktime(nsecs), HRTIMER_MODE_REL_SOFT);
> +
> +       if (flags & BPF_F_TIMER_ABS)
> +               mode = HRTIMER_MODE_ABS_SOFT;
> +       else
> +               mode = HRTIMER_MODE_REL_SOFT;
> +
> +       hrtimer_start(&t->timer, ns_to_ktime(nsecs), mode);

The patch looks fine, but please add a selftest for new functionality
in the 2nd patch and resend them together.