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.
