On Feb 23 2024, Eduard Zingerman wrote: > On Wed, 2024-02-21 at 17:25 +0100, Benjamin Tissoires wrote: > > [...] > > > @@ -1282,7 +1333,7 @@ BPF_CALL_3(bpf_timer_start, struct bpf_timer_kern *, timer, u64, nsecs, u64, fla > > > > if (in_nmi()) > > return -EOPNOTSUPP; > > - if (flags & ~(BPF_F_TIMER_ABS | BPF_F_TIMER_CPU_PIN)) > > + if (flags & ~(BPF_F_TIMER_ABS | BPF_F_TIMER_CPU_PIN | BPF_F_TIMER_SLEEPABLE)) > > return -EINVAL; > > __bpf_spin_lock_irqsave(&timer->lock); > > t = timer->timer; > > @@ -1299,7 +1350,10 @@ BPF_CALL_3(bpf_timer_start, struct bpf_timer_kern *, timer, u64, nsecs, u64, fla > > if (flags & BPF_F_TIMER_CPU_PIN) > > mode |= HRTIMER_MODE_PINNED; > > > > - hrtimer_start(&t->timer, ns_to_ktime(nsecs), mode); > > + if (flags & BPF_F_TIMER_SLEEPABLE) > > + schedule_work(&t->work); > > + else > > + hrtimer_start(&t->timer, ns_to_ktime(nsecs), mode); > > It looks like nsecs is simply ignored for sleepable timers. > Should this be hrtimer_start() that waits nsecs and schedules work, > or schedule_delayed_work()? (but it takes delay in jiffies, which is > probably too coarse). Sorry if I miss something. Yeah, I agree it's confusing, but as mentioned by Toke in his reply, we should return -EINVAL if a timer value is provided (for now). Alexei mentioned[0] that he didn't want to mix delays in hrtimers with workqueue as they are non deterministic. So AFAIU, I should add the only guarantee we can provide: a sleepable context, and proper delays in sleepable contexts will be added once we have a better workqueue selection available. Cheers, Benjamin [0] https://lore.kernel.org/bpf/CAO-hwJKz+eRA+BFLANTrEqz2jQAOANTE3c7eqNJ6wDqJR7jMiQ@xxxxxxxxxxxxxx/T/#md15e431cbcddec9fcaddf1c305234523ed26f7ce
