Re: [PATCH bpf-next v3 0/6] Introduce KF_FORBID_SLEEP modifier for acquire/release kfuncs

[Eduard Zingerman <eddyz87@gmail.com>]

On Tue, 2026-02-24 at 19:51 +0000, Puranjay Mohan wrote:
> On Tue, Feb 24, 2026 at 6:55 PM Eduard Zingerman <eddyz87@gmail.com> wrote:
> > 
> > On Tue, 2026-02-24 at 10:00 -0800, Alexei Starovoitov wrote:
> > > On Tue, Feb 24, 2026 at 3:24 AM Puranjay Mohan <puranjay12@gmail.com> wrote:
> > > > 
> > > > On Tue, Feb 24, 2026 at 1:49 AM Alexei Starovoitov
> > > > <alexei.starovoitov@gmail.com> wrote:
> > > > > 
> > > > > On Mon, Feb 23, 2026 at 9:48 AM Puranjay Mohan <puranjay@kernel.org> wrote:
> > > > > > 
> > > > > > 
> > > > > > Iterator KF_ACQUIRE support is not useful on its own right now, but it
> > > > > > becomes the foundation for KF_FORBID_SLEEP: an iterator whose _next is
> > > > > > annotated with KF_ACQUIRE | KF_FORBID_SLEEP can now express "holding this
> > > > > > pointer forbids sleeping; calling _release invalidates the pointer and
> > > > > > re-enables sleeping."
> > > > > > 
> > > > > 
> > > > > The name of the flag and semantics bothers me a bit.
> > > > > How about we call it KF_ACQUIRE_LOCK and make it exclusive vs KF_ACQUIRE.
> > > > > And document it like:
> > > > > KF_ACQUIRE -> acquires a resource via reference counting
> > > > > KF_ACQUIRE_LOCK -> acquires a resource by locking it
> > > > > 
> > > > > and to match it I would do:
> > > > > KF_RELEASE -> releases a resource by decrementing refcount
> > > > > KF_RELEASE_LOCK -> releases a resource by unlocking it.
> > > > > 
> > > > > When lock is taken it's typically prohibited to sleep and do
> > > > > other things. So I feel such flags would describe what's
> > > > > going on underneath more accurately.
> > > > > 
> > > > > Thoughts?
> > > > 
> > > > I am fine with this except, I think we should still keep the
> > > > KF_RELEASE a single release mechanism that releases according to how
> > > > the reference was taken (refcount/lock). There is nothing wrong with
> > > > separate KF_RELEASE and KF_RELEASE_LOCK, except we would have to
> > > > reject all wrong usages like KF_ACQUIRE -> KF_RELEASE_LOCK,
> > > > KF_ACQUIRE_LOCK -> KF_RELEASE.
> > > 
> > > yes. Is this too complex or what's the concern?
> > > 
> > > > I am fine with both but prefer the
> > > > common KF_RELEASE. Let me know which one you like and I will go ahead
> > > > with it.
> > > 
> > > Magic KF_RELEASE simplifies things a bit. I don't mind, I guess,
> > > but would like to hear a 3rd opinion.
> > 
> > At the moment verifier tracks the following locking/non-sleepable
> > resources:
> > - active_irq_id        - nesting allowed, forbids sleep, special stack slot acquire/release logic.
> > - active_rcu_locks     - nesting allowed, forbids sleep, used by in in_rcu_cs(),
> >                          no special acquire/release logic.
> > - active_preempt_locks - nesting allowed, forbids sleep, no special acquire/release logic.
> > - active_locks         - nesting disallowed, forbids sleep, no special acquire/release logic.
> > 
> > Currently verifier checks what kfunc/helper is called and changes
> > internal state accordingly. It might be possible to slice the above
> > into a set of "generic" flags / feats, but it requires some thought.
> > 
> > Maybe explore this as a separate series?
> 
> Eduard,
> 
> Are you fine with making KF_ACQUIRE | KF_FORBIDS_SLEEP as a new flag
> called KF_ACQUIRE_LOCK
> and then keeping the common KF_RELEASE that will release according to
> how it was acquired?

Idk, I'd keep it KF_FORBIDS_SLEEP for now and rename later.
W/o the discussed refactoring ACQUIRE_LOCK is a bit confusing, imo.