On Tue, Mar 31, 2026 at 08:34:10AM -0700, Samuel Wu wrote: > Iterating through wakeup sources via sysfs or debugfs can be inefficient > or restricted. Introduce BPF kfuncs to allow high-performance and safe > in-kernel traversal of the wakeup_sources list.
What exactly is "inefficient"? I think you might have some numbers in your 0/2 patch, but putting it in here would be best. And who is going to be calling these functions, just ebpf scripts? > The new kfuncs include: > - bpf_wakeup_sources_get_head() to obtain the list head. > - bpf_wakeup_sources_read_lock/unlock() to manage the SRCU lock. Does this mean we can stop exporting wakeup_sources_read_lock() now? > For verifier safety, the underlying SRCU index is wrapped in an opaque > 'struct bpf_ws_lock' pointer. This enables the use of KF_ACQUIRE and > KF_RELEASE flags, allowing the BPF verifier to strictly enforce paired > lock/unlock cycles and prevent resource leaks. But it's an index, not a lock. Is this just a verifier thing? > > Signed-off-by: Samuel Wu <[email protected]> > --- > drivers/base/power/power.h | 7 ++++ > drivers/base/power/wakeup.c | 72 +++++++++++++++++++++++++++++++++++-- > 2 files changed, 77 insertions(+), 2 deletions(-) > > diff --git a/drivers/base/power/power.h b/drivers/base/power/power.h > index 922ed457db19..8823aceeac8b 100644 > --- a/drivers/base/power/power.h > +++ b/drivers/base/power/power.h > @@ -168,3 +168,10 @@ static inline void device_pm_init(struct device *dev) > device_pm_sleep_init(dev); > pm_runtime_init(dev); > } > + > +#ifdef CONFIG_BPF_SYSCALL > +struct bpf_ws_lock { }; An empty structure? This is just an int, so you are casting an int to a pointer? Can we make wakeup_sources_read_lock() actually use a structure instead to make this simpler? > +struct bpf_ws_lock *bpf_wakeup_sources_read_lock(void); > +void bpf_wakeup_sources_read_unlock(struct bpf_ws_lock *lock); > +void *bpf_wakeup_sources_get_head(void); > +#endif > diff --git a/drivers/base/power/wakeup.c b/drivers/base/power/wakeup.c > index b8e48a023bf0..8eda7d35d9cc 100644 > --- a/drivers/base/power/wakeup.c > +++ b/drivers/base/power/wakeup.c > @@ -1168,11 +1168,79 @@ static const struct file_operations > wakeup_sources_stats_fops = { > .release = seq_release_private, > }; > > -static int __init wakeup_sources_debugfs_init(void) > +#ifdef CONFIG_BPF_SYSCALL > +#include <linux/btf.h> > + > +__bpf_kfunc_start_defs(); > + > +/** > + * bpf_wakeup_sources_read_lock - Acquire the SRCU lock for wakeup sources > + * > + * The underlying SRCU lock returns an integer index. However, the BPF > verifier > + * requires a pointer (PTR_TO_BTF_ID) to strictly track the state of acquired > + * resources using KF_ACQUIRE and KF_RELEASE semantics. We use an opaque > + * structure pointer (struct bpf_ws_lock *) to satisfy the verifier while > + * safely encoding the integer index within the pointer address itself. > + * > + * Return: An opaque pointer encoding the SRCU lock index + 1 (to avoid > NULL). > + */ > +__bpf_kfunc struct bpf_ws_lock *bpf_wakeup_sources_read_lock(void) > +{ > + return (struct bpf_ws_lock *)(long)(wakeup_sources_read_lock() + 1); Why are you incrementing this by 1? > +} > + > +/** > + * bpf_wakeup_sources_read_unlock - Release the SRCU lock for wakeup sources > + * @lock: The opaque pointer returned by bpf_wakeup_sources_read_lock() > + * > + * The BPF verifier guarantees that @lock is a valid, unreleased pointer from > + * the acquire function. We decode the pointer back into the integer SRCU > index > + * by subtracting 1 and release the lock. > + */ > +__bpf_kfunc void bpf_wakeup_sources_read_unlock(struct bpf_ws_lock *lock) > +{ > + wakeup_sources_read_unlock((int)(long)lock - 1); Why decrementing by one? So it's really an int, but you are casting it to a pointer, incrementing it by one to make it a "fake" pointer value (i.e. unaligned mess), and then when unlocking casting the pointer back to an int, and then decrementing the value? This feels "odd" :( > +} > + > +/** > + * bpf_wakeup_sources_get_head - Get the head of the wakeup sources list > + * > + * Return: The head of the wakeup sources list. > + */ > +__bpf_kfunc void *bpf_wakeup_sources_get_head(void) > +{ > + return &wakeup_sources; > +} > + > +__bpf_kfunc_end_defs(); > + > +BTF_KFUNCS_START(wakeup_source_kfunc_ids) > +BTF_ID_FLAGS(func, bpf_wakeup_sources_read_lock, KF_ACQUIRE) > +BTF_ID_FLAGS(func, bpf_wakeup_sources_read_unlock, KF_RELEASE) > +BTF_ID_FLAGS(func, bpf_wakeup_sources_get_head) > +BTF_KFUNCS_END(wakeup_source_kfunc_ids) > + > +static const struct btf_kfunc_id_set wakeup_source_kfunc_set = { > + .owner = THIS_MODULE, This isn't a module. thanks, greg k-h
