Linux Kernel Selftest development
 help / color / mirror / Atom feed
* [PATCH bpf-next v5 0/2] Add wakeup_source iterators
@ 2026-02-25 21:08 Samuel Wu
  2026-02-25 21:08 ` [PATCH bpf-next v5 1/2] bpf: Add wakeup_source iterator Samuel Wu
                   ` (2 more replies)
  0 siblings, 3 replies; 11+ messages in thread
From: Samuel Wu @ 2026-02-25 21:08 UTC (permalink / raw)
  To: Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko,
	Martin KaFai Lau, Eduard Zingerman, Song Liu, Yonghong Song,
	John Fastabend, KP Singh, Stanislav Fomichev, Hao Luo, Jiri Olsa,
	Shuah Khan
  Cc: Samuel Wu, kernel-team, linux-kernel, bpf, linux-kselftest

This patch series introduces BPF iterators for wakeup_source, enabling
BPF programs to efficiently traverse a device's wakeup sources.

Currently, inspecting wakeup sources typically involves reading interfaces
like /sys/class/wakeup/* or debugfs. The repeated syscalls to query the
sysfs nodes is inefficient, as there can be hundreds of wakeup_sources, and
each wakeup source have multiple stats, with one sysfs node per stat.
debugfs is unstable and insecure.

The iterators utilize pre-existing wakeup_sources_walk_* functions to
traverse over the SRCU that backs the list of wakeup_sources.

Changes in v5:
 - Add CORE definitions for *__local variables to fix s390 build per bpf-ci
 - v4 link: https://lore.kernel.org/all/20260211235055.239024-1-wusamuel@google.com/

Changes in v4:
 - Proper init of variables in ws_iter_check_sleep_times() test per bpf-ci
 - Remove PM patch since it's already part of rebase
 - v3 link: https://lore.kernel.org/all/20260124012133.2451708-1-wusamuel@google.com/

Changes in v3:
 - Update wakeup_sources_walk_start() to handle an empty list per bpf-ci
 - Simplify read of a struct's field in BPF program selftest per Andrii
 - Drop open coded iterators for wakeup_sources
 - Fix condition from !get_ws_iter_stream to get_ws_iter_stream in selftest
 - Read event_count instead of wakeup_count in selftest
 - v2 link: https://lore.kernel.org/all/20260108225523.3268383-1-wusamuel@google.com/

Changes in v2:
 - Guard BPF Makefile with CONFIG_PM_SLEEP to fix build errors
 - Update copyright from 2025 to 2026
 - v1 link: https://lore.kernel.org/all/20251204025003.3162056-1-wusamuel@google.com/


Samuel Wu (2):
  bpf: Add wakeup_source iterator
  selftests/bpf: Add tests for wakeup_sources

 kernel/bpf/Makefile                           |   3 +
 kernel/bpf/wakeup_source_iter.c               | 103 ++++++
 tools/testing/selftests/bpf/config            |   1 +
 .../bpf/prog_tests/wakeup_source_iter.c       | 300 ++++++++++++++++++
 .../selftests/bpf/progs/wakeup_source_iter.c  |  80 +++++
 5 files changed, 487 insertions(+)
 create mode 100644 kernel/bpf/wakeup_source_iter.c
 create mode 100644 tools/testing/selftests/bpf/prog_tests/wakeup_source_iter.c
 create mode 100644 tools/testing/selftests/bpf/progs/wakeup_source_iter.c

-- 
2.53.0.473.g4a7958ca14-goog


^ permalink raw reply	[flat|nested] 11+ messages in thread

* [PATCH bpf-next v5 1/2] bpf: Add wakeup_source iterator
  2026-02-25 21:08 [PATCH bpf-next v5 0/2] Add wakeup_source iterators Samuel Wu
@ 2026-02-25 21:08 ` Samuel Wu
  2026-02-25 22:03   ` bot+bpf-ci
  2026-02-25 21:08 ` [PATCH bpf-next v5 2/2] selftests/bpf: Add tests for wakeup_sources Samuel Wu
  2026-03-05  1:31 ` [PATCH bpf-next v5 0/2] Add wakeup_source iterators Samuel Wu
  2 siblings, 1 reply; 11+ messages in thread
From: Samuel Wu @ 2026-02-25 21:08 UTC (permalink / raw)
  To: Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko,
	Martin KaFai Lau, Eduard Zingerman, Song Liu, Yonghong Song,
	John Fastabend, KP Singh, Stanislav Fomichev, Hao Luo, Jiri Olsa,
	Shuah Khan
  Cc: Samuel Wu, kernel-team, linux-kernel, bpf, linux-kselftest

Add a BPF iterator for traversing through wakeup_sources.

Setup iterators to traverse through a SRCUs of wakeup_sources. This is a
more elegant and efficient traversal than going through the options
today, such as at /sys/class/wakeup, or through debugfs.

Signed-off-by: Samuel Wu <wusamuel@google.com>
---
 kernel/bpf/Makefile             |   3 +
 kernel/bpf/wakeup_source_iter.c | 103 ++++++++++++++++++++++++++++++++
 2 files changed, 106 insertions(+)
 create mode 100644 kernel/bpf/wakeup_source_iter.c

diff --git a/kernel/bpf/Makefile b/kernel/bpf/Makefile
index 79cf22860a99..1259373298e1 100644
--- a/kernel/bpf/Makefile
+++ b/kernel/bpf/Makefile
@@ -66,6 +66,9 @@ obj-$(CONFIG_BPF_SYSCALL) += kmem_cache_iter.o
 ifeq ($(CONFIG_DMA_SHARED_BUFFER),y)
 obj-$(CONFIG_BPF_SYSCALL) += dmabuf_iter.o
 endif
+ifeq ($(CONFIG_PM_SLEEP),y)
+obj-$(CONFIG_BPF_SYSCALL) += wakeup_source_iter.o
+endif
 
 CFLAGS_REMOVE_percpu_freelist.o = $(CC_FLAGS_FTRACE)
 CFLAGS_REMOVE_bpf_lru_list.o = $(CC_FLAGS_FTRACE)
diff --git a/kernel/bpf/wakeup_source_iter.c b/kernel/bpf/wakeup_source_iter.c
new file mode 100644
index 000000000000..ab83d212a1f9
--- /dev/null
+++ b/kernel/bpf/wakeup_source_iter.c
@@ -0,0 +1,103 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/* Copyright (c) 2026 Google LLC */
+#include <linux/bpf.h>
+#include <linux/btf_ids.h>
+#include <linux/kernel.h>
+#include <linux/pm_wakeup.h>
+#include <linux/seq_file.h>
+
+struct bpf_iter__wakeup_source {
+	__bpf_md_ptr(struct bpf_iter_meta *, meta);
+	__bpf_md_ptr(struct wakeup_source *, wakeup_source);
+};
+
+static void *wakeup_source_iter_seq_start(struct seq_file *seq, loff_t *pos)
+{
+	int *srcuidx = seq->private;
+	struct wakeup_source *ws;
+	loff_t i;
+
+	*srcuidx = wakeup_sources_read_lock();
+
+	ws = wakeup_sources_walk_start();
+	for (i = 0; ws && i < *pos; i++)
+		ws = wakeup_sources_walk_next(ws);
+
+	return ws;
+}
+
+static void *wakeup_source_iter_seq_next(struct seq_file *seq, void *v, loff_t *pos)
+{
+	struct wakeup_source *ws = v;
+
+	++*pos;
+
+	return wakeup_sources_walk_next(ws);
+}
+
+static void wakeup_source_iter_seq_stop(struct seq_file *seq, void *v)
+{
+	int *srcuidx = seq->private;
+
+	if (*srcuidx >= 0)
+		wakeup_sources_read_unlock(*srcuidx);
+	*srcuidx = -1;
+}
+
+static int __wakeup_source_seq_show(struct seq_file *seq, void *v, bool in_stop)
+{
+	struct bpf_iter_meta meta = {
+		.seq = seq,
+	};
+	struct bpf_iter__wakeup_source ctx = {
+		.meta = &meta,
+		.wakeup_source = v,
+	};
+	struct bpf_prog *prog = bpf_iter_get_info(&meta, in_stop);
+
+	if (prog)
+		return bpf_iter_run_prog(prog, &ctx);
+
+	return 0;
+}
+
+static int wakeup_source_iter_seq_show(struct seq_file *seq, void *v)
+{
+	return __wakeup_source_seq_show(seq, v, false);
+}
+
+static const struct seq_operations wakeup_source_iter_seq_ops = {
+	.start	= wakeup_source_iter_seq_start,
+	.next	= wakeup_source_iter_seq_next,
+	.stop	= wakeup_source_iter_seq_stop,
+	.show	= wakeup_source_iter_seq_show,
+};
+
+static const struct bpf_iter_seq_info wakeup_source_iter_seq_info = {
+	.seq_ops		= &wakeup_source_iter_seq_ops,
+	.seq_priv_size		= sizeof(int),
+};
+
+static struct bpf_iter_reg bpf_wakeup_source_reg_info = {
+	.target			= "wakeup_source",
+	.ctx_arg_info_size	= 1,
+	.ctx_arg_info		= {
+		{
+			offsetof(struct bpf_iter__wakeup_source, wakeup_source),
+			PTR_TO_BTF_ID_OR_NULL
+		},
+	},
+	.seq_info		= &wakeup_source_iter_seq_info,
+};
+
+DEFINE_BPF_ITER_FUNC(wakeup_source, struct bpf_iter_meta *meta,
+		     struct wakeup_source *wakeup_source)
+BTF_ID_LIST_SINGLE(bpf_wakeup_source_btf_id, struct, wakeup_source)
+
+static int __init wakeup_source_iter_init(void)
+{
+	bpf_wakeup_source_reg_info.ctx_arg_info[0].btf_id = bpf_wakeup_source_btf_id[0];
+	return bpf_iter_reg_target(&bpf_wakeup_source_reg_info);
+}
+
+late_initcall(wakeup_source_iter_init);
-- 
2.53.0.473.g4a7958ca14-goog


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH bpf-next v5 2/2] selftests/bpf: Add tests for wakeup_sources
  2026-02-25 21:08 [PATCH bpf-next v5 0/2] Add wakeup_source iterators Samuel Wu
  2026-02-25 21:08 ` [PATCH bpf-next v5 1/2] bpf: Add wakeup_source iterator Samuel Wu
@ 2026-02-25 21:08 ` Samuel Wu
  2026-03-05  1:31 ` [PATCH bpf-next v5 0/2] Add wakeup_source iterators Samuel Wu
  2 siblings, 0 replies; 11+ messages in thread
From: Samuel Wu @ 2026-02-25 21:08 UTC (permalink / raw)
  To: Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko,
	Martin KaFai Lau, Eduard Zingerman, Song Liu, Yonghong Song,
	John Fastabend, KP Singh, Stanislav Fomichev, Hao Luo, Jiri Olsa,
	Shuah Khan
  Cc: Samuel Wu, kernel-team, linux-kernel, bpf, linux-kselftest

Sets up the framework to test wakeup_sources iterators using BPF, and
adds a few basic tests.

Adds several helper functions that for grabbing and releasing a
wakelock, abstracting out key functions to setup a framework for testing
wakeup_sources.

Additionally, adds 3 tests:
1. check_active_count: Checks that stats related to active_count are
   properly set after several lock/unlock cycles
2. check_sleep_times: Checks that time accounting related to sleep are
   properly calculated
3. check_no_infinite_reads: Checks that the iterator traversal returns
   NULL at the end

Signed-off-by: Samuel Wu <wusamuel@google.com>
---
 tools/testing/selftests/bpf/config            |   1 +
 .../bpf/prog_tests/wakeup_source_iter.c       | 300 ++++++++++++++++++
 .../selftests/bpf/progs/wakeup_source_iter.c  |  80 +++++
 3 files changed, 381 insertions(+)
 create mode 100644 tools/testing/selftests/bpf/prog_tests/wakeup_source_iter.c
 create mode 100644 tools/testing/selftests/bpf/progs/wakeup_source_iter.c

diff --git a/tools/testing/selftests/bpf/config b/tools/testing/selftests/bpf/config
index 24855381290d..75e8542ef111 100644
--- a/tools/testing/selftests/bpf/config
+++ b/tools/testing/selftests/bpf/config
@@ -111,6 +111,7 @@ CONFIG_IP6_NF_IPTABLES=y
 CONFIG_IP6_NF_FILTER=y
 CONFIG_NF_NAT=y
 CONFIG_PACKET=y
+CONFIG_PM_WAKELOCKS=y
 CONFIG_RC_CORE=y
 CONFIG_SAMPLES=y
 CONFIG_SAMPLE_LIVEPATCH=m
diff --git a/tools/testing/selftests/bpf/prog_tests/wakeup_source_iter.c b/tools/testing/selftests/bpf/prog_tests/wakeup_source_iter.c
new file mode 100644
index 000000000000..40e47157a6a0
--- /dev/null
+++ b/tools/testing/selftests/bpf/prog_tests/wakeup_source_iter.c
@@ -0,0 +1,300 @@
+// SPDX-License-Identifier: GPL-2.0
+/* Copyright (c) 2026 Google LLC */
+
+#include <test_progs.h>
+#include <bpf/btf.h>
+#include <bpf/libbpf.h>
+#include "wakeup_source_iter.skel.h"
+
+#include <fcntl.h>
+#include <stdbool.h>
+#include <stdio.h>
+#include <stdlib.h>
+#include <string.h>
+#include <unistd.h>
+
+
+/* Sleep for 10ms to ensure active time is > 0 after converting ns to ms*/
+#define TEST_SLEEP_US 10000
+#define TEST_SLEEP_MS (TEST_SLEEP_US / 1000)
+#define WAKEUP_SOURCE_NAME_LEN 32
+
+static const char test_ws_name[] = "bpf_selftest_ws";
+static bool test_ws_created;
+
+/*
+ * Creates a new wakeup source by writing to /sys/power/wake_lock.
+ * This lock persists until explicitly unlocked.
+ */
+static int lock_ws(const char *name)
+{
+	int fd;
+	ssize_t bytes;
+
+	fd = open("/sys/power/wake_lock", O_WRONLY);
+	if (!ASSERT_OK_FD(fd, "open /sys/power/wake_lock"))
+		return -1;
+
+	bytes = write(fd, name, strlen(name));
+	close(fd);
+	if (!ASSERT_EQ(bytes, strlen(name), "write to wake_lock"))
+		return -1;
+
+	return 0;
+}
+
+/*
+ * Destroys the ws by writing the same name to /sys/power/wake_unlock.
+ */
+static void unlock_ws(const char *name)
+{
+	int fd;
+
+	fd = open("/sys/power/wake_unlock", O_WRONLY);
+	if (!ASSERT_OK_FD(fd, "open /sys/power/wake_unlock"))
+		goto cleanup;
+
+	write(fd, name, strlen(name));
+
+cleanup:
+	if (fd)
+		close(fd);
+}
+
+/*
+ * Setups for testing ws iterators. Will run once prior to suite of tests.
+ */
+static int setup_test_ws(void)
+{
+	if (lock_ws(test_ws_name))
+		return -1;
+	test_ws_created = true;
+
+	return 0;
+}
+
+/*
+ * Tears down and cleanups testing ws iterators. WIll run once after the suite
+ * of tests.
+ */
+static void teardown_test_ws(void)
+{
+	if (!test_ws_created)
+		return;
+	unlock_ws(test_ws_name);
+	test_ws_created = false;
+}
+
+struct WakeupSourceInfo {
+	char name[WAKEUP_SOURCE_NAME_LEN];
+	unsigned long active_count;
+	long active_time_ms;
+	unsigned long event_count;
+	unsigned long expire_count;
+	long last_change_ms;
+	long max_time_ms;
+	long prevent_sleep_time_ms;
+	long total_time_ms;
+	unsigned long wakeup_count;
+};
+
+/*
+ * Reads and parses one wakeup_source record from the iterator file.
+ * A record is a single space-delimited line.
+ * Returns true on success, false on EOF. Asserts internally on errors.
+ */
+static bool read_ws_info(FILE *iter_file, struct WakeupSourceInfo *ws_info,
+			 char **line)
+{
+	size_t linesize;
+	int items;
+
+	if (getline(line, &linesize, iter_file) == -1)
+		return false;
+
+	(*line)[strcspn(*line, "\n")] = 0;
+
+	items = sscanf(*line, "%s %lu %ld %lu %lu %ld %ld %ld %ld %lu",
+		       ws_info->name, &ws_info->active_count,
+		       &ws_info->active_time_ms, &ws_info->event_count,
+		       &ws_info->expire_count, &ws_info->last_change_ms,
+		       &ws_info->max_time_ms, &ws_info->prevent_sleep_time_ms,
+		       &ws_info->total_time_ms, &ws_info->wakeup_count);
+
+	if (!ASSERT_EQ(items, 10, "read wakeup source info"))
+		return false;
+
+	if (!ASSERT_LT(strlen(ws_info->name), WAKEUP_SOURCE_NAME_LEN,
+		       "name length"))
+		return false;
+
+	return true;
+}
+
+static int get_ws_iter_stream(struct wakeup_source_iter *skel, int *iter_fd,
+			      FILE **iter_file)
+{
+	*iter_fd = bpf_iter_create(
+			bpf_link__fd(skel->links.wakeup_source_collector));
+	if (!ASSERT_OK_FD(*iter_fd, "iter_create"))
+		return -1;
+
+	*iter_file = fdopen(*iter_fd, "r");
+	if (!ASSERT_OK_PTR(*iter_file, "fdopen"))
+		return -1;
+
+	return 0;
+}
+
+static void subtest_ws_iter_check_active_count(struct wakeup_source_iter *skel)
+{
+	static const char subtest_ws_name[] = "bpf_selftest_ws_active_count";
+	const int lock_unlock_cycles = 5;
+	struct WakeupSourceInfo ws_info;
+	char *line = NULL;
+	bool found_ws = false;
+	FILE *iter_file = NULL;
+	int iter_fd = -1;
+	int i;
+
+	for (i = 0; i < lock_unlock_cycles; i++) {
+		if (!ASSERT_OK(lock_ws(subtest_ws_name), "lock_ws"))
+			goto cleanup;
+		unlock_ws(subtest_ws_name);
+	}
+
+	if (get_ws_iter_stream(skel, &iter_fd, &iter_file))
+		goto cleanup;
+
+	while (read_ws_info(iter_file, &ws_info, &line)) {
+		if (strcmp(ws_info.name, subtest_ws_name) == 0) {
+			found_ws = true;
+			ASSERT_EQ(ws_info.active_count, lock_unlock_cycles,
+				  "active_count check");
+			ASSERT_EQ(ws_info.event_count, lock_unlock_cycles,
+				  "event_count check");
+			break;
+		}
+	}
+
+	ASSERT_TRUE(found_ws, "found active_count test ws");
+
+	free(line);
+cleanup:
+	if (iter_file)
+		fclose(iter_file);
+	else if (iter_fd >= 0)
+		close(iter_fd);
+}
+
+static void subtest_ws_iter_check_sleep_times(struct wakeup_source_iter *skel)
+{
+	bool found_test_ws = false;
+	struct WakeupSourceInfo ws_info;
+	char *line = NULL;
+	FILE *iter_file = NULL;
+	int iter_fd = -1;
+
+	if (get_ws_iter_stream(skel, &iter_fd, &iter_file))
+		goto cleanup;
+
+	while (read_ws_info(iter_file, &ws_info, &line)) {
+		if (strcmp(ws_info.name, test_ws_name) == 0) {
+			found_test_ws = true;
+			ASSERT_GT(ws_info.last_change_ms, 0,
+				  "Expected non-zero last change");
+			ASSERT_GE(ws_info.active_time_ms, TEST_SLEEP_MS,
+				  "Expected active time >= TEST_SLEEP_MS");
+			ASSERT_GE(ws_info.max_time_ms, TEST_SLEEP_MS,
+				  "Expected max time >= TEST_SLEEP_MS");
+			ASSERT_GE(ws_info.total_time_ms, TEST_SLEEP_MS,
+				  "Expected total time >= TEST_SLEEP_MS");
+			break;
+		}
+	}
+
+	ASSERT_TRUE(found_test_ws, "found_test_ws");
+
+	free(line);
+cleanup:
+	if (iter_file)
+		fclose(iter_file);
+	else if (iter_fd >= 0)
+		close(iter_fd);
+}
+
+static void subtest_ws_iter_check_no_infinite_reads(
+		struct wakeup_source_iter *skel)
+{
+	int iter_fd;
+	char buf[256];
+
+	iter_fd = bpf_iter_create(bpf_link__fd(skel->links.wakeup_source_collector));
+	if (!ASSERT_OK_FD(iter_fd, "iter_create"))
+		return;
+
+	while (read(iter_fd, buf, sizeof(buf)) > 0)
+		;
+
+	/* Final read should return 0 */
+	ASSERT_EQ(read(iter_fd, buf, sizeof(buf)), 0, "read");
+
+	close(iter_fd);
+}
+
+void test_wakeup_source_iter(void)
+{
+	struct wakeup_source_iter *skel = NULL;
+	struct btf *vmlinux_btf;
+	int btf_id;
+
+	if (geteuid() != 0) {
+		fprintf(stderr,
+			"Skipping wakeup_source_iter test, requires root\n");
+		test__skip();
+		return;
+	}
+
+	skel = wakeup_source_iter__open();
+	if (!ASSERT_OK_PTR(skel, "wakeup_source_iter__open"))
+		return;
+
+	vmlinux_btf = btf__load_vmlinux_btf();
+	if (!vmlinux_btf)
+		return;
+
+	btf_id = btf__find_by_name_kind(vmlinux_btf, "bpf_iter__wakeup_source", BTF_KIND_STRUCT);
+	btf__free(vmlinux_btf);
+
+	if (btf_id <= 0) {
+		test__skip();
+		goto destroy;
+	}
+
+	if (!ASSERT_OK(wakeup_source_iter__load(skel), "iter__load"))
+		goto destroy;
+
+	if (!ASSERT_OK(setup_test_ws(), "setup_test_ws"))
+		goto teardown;
+
+	if (!ASSERT_OK(wakeup_source_iter__attach(skel), "skel_attach"))
+		goto teardown;
+
+	/*
+	 * Sleep on O(ms) to ensure that time stats' resolution isn't lost when
+	 * converting from ns to ms
+	 */
+	usleep(TEST_SLEEP_US);
+
+	if (test__start_subtest("active_count"))
+		subtest_ws_iter_check_active_count(skel);
+	if (test__start_subtest("sleep_times"))
+		subtest_ws_iter_check_sleep_times(skel);
+	if (test__start_subtest("no_infinite_reads"))
+		subtest_ws_iter_check_no_infinite_reads(skel);
+
+teardown:
+	teardown_test_ws();
+destroy:
+	wakeup_source_iter__destroy(skel);
+}
diff --git a/tools/testing/selftests/bpf/progs/wakeup_source_iter.c b/tools/testing/selftests/bpf/progs/wakeup_source_iter.c
new file mode 100644
index 000000000000..d770efc3bd0d
--- /dev/null
+++ b/tools/testing/selftests/bpf/progs/wakeup_source_iter.c
@@ -0,0 +1,80 @@
+// SPDX-License-Identifier: GPL-2.0
+/* Copyright (c) 2026 Google LLC */
+#include <vmlinux.h>
+#include <bpf/bpf_core_read.h>
+#include <bpf/bpf_helpers.h>
+
+#define NSEC_PER_MS 1000000UL
+#define WAKEUP_SOURCE_NAME_LEN 32
+
+char _license[] SEC("license") = "GPL";
+
+struct wakeup_source___local {
+	const char *name;
+	unsigned long active_count;
+	unsigned long event_count;
+	unsigned long wakeup_count;
+	unsigned long expire_count;
+	ktime_t last_time;
+	ktime_t max_time;
+	ktime_t total_time;
+	ktime_t start_prevent_time;
+	ktime_t prevent_sleep_time;
+	bool active:1;
+	bool autosleep_enabled:1;
+} __attribute__((preserve_access_index));
+
+struct bpf_iter__wakeup_source___local {
+	struct bpf_iter_meta *meta;
+	struct wakeup_source___local *wakeup_source;
+} __attribute__((preserve_access_index));
+
+SEC("iter/wakeup_source")
+int wakeup_source_collector(struct bpf_iter__wakeup_source___local *ctx)
+{
+	const struct wakeup_source___local *ws = ctx->wakeup_source;
+	struct seq_file *seq = ctx->meta->seq;
+	char name[WAKEUP_SOURCE_NAME_LEN] = {'\0'};
+	const char *pname;
+	bool active, autosleep_enable;
+	s64 active_time, curr_time, max_time, prevent_sleep_time, total_time;
+
+	if (!ws)
+		return 0;
+
+	active = BPF_CORE_READ_BITFIELD_PROBED(ws, active);
+	autosleep_enable = BPF_CORE_READ_BITFIELD_PROBED(ws, autosleep_enabled);
+	if (bpf_core_read(&pname, sizeof(pname), &ws->name) ||
+	    bpf_probe_read_kernel_str(name, sizeof(name), pname) < 0)
+		return 0;
+
+	active_time = 0;
+	curr_time = bpf_ktime_get_ns();
+	max_time = ws->max_time;
+	prevent_sleep_time = ws->prevent_sleep_time;
+	total_time = ws->total_time;
+
+	if (active) {
+		active_time = curr_time - ws->last_time;
+		total_time += active_time;
+		if (active_time > max_time)
+			max_time = active_time;
+		if (autosleep_enable)
+			prevent_sleep_time +=
+				curr_time - ws->start_prevent_time;
+	}
+
+	BPF_SEQ_PRINTF(seq,
+		       "%s %lu %ld %lu %lu %ld %ld %ld %ld %lu\n",
+		       name,
+		       ws->active_count,
+		       active_time / NSEC_PER_MS,
+		       ws->event_count,
+		       ws->expire_count,
+		       ws->last_time / NSEC_PER_MS,
+		       max_time / NSEC_PER_MS,
+		       prevent_sleep_time / NSEC_PER_MS,
+		       total_time / NSEC_PER_MS,
+		       ws->wakeup_count);
+	return 0;
+}
-- 
2.53.0.473.g4a7958ca14-goog


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* Re: [PATCH bpf-next v5 1/2] bpf: Add wakeup_source iterator
  2026-02-25 21:08 ` [PATCH bpf-next v5 1/2] bpf: Add wakeup_source iterator Samuel Wu
@ 2026-02-25 22:03   ` bot+bpf-ci
  2026-02-25 22:35     ` Samuel Wu
  0 siblings, 1 reply; 11+ messages in thread
From: bot+bpf-ci @ 2026-02-25 22:03 UTC (permalink / raw)
  To: wusamuel, ast, daniel, andrii, martin.lau, eddyz87, song,
	yonghong.song, john.fastabend, kpsingh, sdf, haoluo, jolsa, shuah
  Cc: wusamuel, kernel-team, linux-kernel, bpf, linux-kselftest, ast,
	andrii, daniel, martin.lau, eddyz87, yonghong.song, clm,
	ihor.solodrai

[-- Attachment #1: Type: text/plain, Size: 2079 bytes --]

> diff --git a/kernel/bpf/wakeup_source_iter.c b/kernel/bpf/wakeup_source_iter.c
> --- /dev/null
> +++ b/kernel/bpf/wakeup_source_iter.c

[ ... ]

> +static void wakeup_source_iter_seq_stop(struct seq_file *seq, void *v)
> +{
> +	int *srcuidx = seq->private;
> +
> +	if (*srcuidx >= 0)
> +		wakeup_sources_read_unlock(*srcuidx);
> +	*srcuidx = -1;
> +}

Should wakeup_source_iter_seq_stop() call
__wakeup_source_seq_show(seq, v, true) when v is NULL, before
releasing the SRCU lock?

bpf_seq_read() in kernel/bpf/bpf_iter.c has this at the stop
label:

    /* bpf program called if !p */
    seq->op->stop(seq, p);

The other BPF iterators follow this contract:

    bpf_map_seq_stop():
        if (!v)
            (void)__bpf_map_seq_show(seq, v, true);

    task_seq_stop():
        if (!v)
            (void)__task_seq_show(seq, v, true);

The same pattern appears in bpf_prog_seq_stop(),
bpf_link_seq_stop(), task_file_seq_stop(), and
task_vma_seq_stop().

Without the in_stop callback, BPF programs attached to this
iterator cannot detect when iteration has finished, which
prevents finalization such as outputting aggregated results.

The __wakeup_source_seq_show() function already accepts the
in_stop parameter but is never called with true:

> +static int __wakeup_source_seq_show(struct seq_file *seq, void *v, bool in_stop)
> +{

[ ... ]

> +	struct bpf_prog *prog = bpf_iter_get_info(&meta, in_stop);

[ ... ]

> +}


---
AI reviewed your patch. Please fix the bug or email reply why it's not a bug.
See: https://github.com/kernel-patches/vmtest/blob/master/ci/claude/README.md

CI run summary: https://github.com/kernel-patches/bpf/actions/runs/22416722500

AI-authorship-score: low
AI-authorship-explanation: The code follows standard kernel BPF iterator patterns with no unusual characteristics suggesting AI generation.
issues-found: 1
issue-severity-score: low
issue-severity-explanation: Missing end-of-iteration callback is an API consistency issue that prevents BPF programs from detecting iteration completion, but does not cause crashes or data corruption.

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH bpf-next v5 1/2] bpf: Add wakeup_source iterator
  2026-02-25 22:03   ` bot+bpf-ci
@ 2026-02-25 22:35     ` Samuel Wu
  0 siblings, 0 replies; 11+ messages in thread
From: Samuel Wu @ 2026-02-25 22:35 UTC (permalink / raw)
  To: bot+bpf-ci
  Cc: ast, daniel, andrii, martin.lau, eddyz87, song, yonghong.song,
	john.fastabend, kpsingh, sdf, haoluo, jolsa, shuah, kernel-team,
	linux-kernel, bpf, linux-kselftest, martin.lau, clm,
	ihor.solodrai

On Wed, Feb 25, 2026 at 2:03 PM <bot+bpf-ci@kernel.org> wrote:
>
> > diff --git a/kernel/bpf/wakeup_source_iter.c b/kernel/bpf/wakeup_source_iter.c
> > --- /dev/null
> > +++ b/kernel/bpf/wakeup_source_iter.c
>
> [ ... ]
>
> > +static void wakeup_source_iter_seq_stop(struct seq_file *seq, void *v)
> > +{
> > +     int *srcuidx = seq->private;
> > +
> > +     if (*srcuidx >= 0)
> > +             wakeup_sources_read_unlock(*srcuidx);
> > +     *srcuidx = -1;
> > +}
>
> Should wakeup_source_iter_seq_stop() call
> __wakeup_source_seq_show(seq, v, true) when v is NULL, before
> releasing the SRCU lock?

This is the same response as in v3 and v4, but I don't think it's
necessary here since we don't need any additional output after
finalization, such as printing a footer.

I understand it that bpf-next is now open for processing since the
merge window is closed, so unless a human reviewer has a different
opinion on this I'll ignore this AI feedback for now.

> [ ... ]

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH bpf-next v5 0/2] Add wakeup_source iterators
  2026-02-25 21:08 [PATCH bpf-next v5 0/2] Add wakeup_source iterators Samuel Wu
  2026-02-25 21:08 ` [PATCH bpf-next v5 1/2] bpf: Add wakeup_source iterator Samuel Wu
  2026-02-25 21:08 ` [PATCH bpf-next v5 2/2] selftests/bpf: Add tests for wakeup_sources Samuel Wu
@ 2026-03-05  1:31 ` Samuel Wu
  2026-03-06 23:28   ` Kumar Kartikeya Dwivedi
  2 siblings, 1 reply; 11+ messages in thread
From: Samuel Wu @ 2026-03-05  1:31 UTC (permalink / raw)
  To: Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko,
	Martin KaFai Lau, Eduard Zingerman, Song Liu, Yonghong Song,
	John Fastabend, KP Singh, Stanislav Fomichev, Hao Luo, Jiri Olsa,
	Shuah Khan
  Cc: kernel-team, linux-kernel, bpf, linux-kselftest

On Wed, Feb 25, 2026 at 1:08 PM Samuel Wu <wusamuel@google.com> wrote:
>
> This patch series introduces BPF iterators for wakeup_source, enabling
> BPF programs to efficiently traverse a device's wakeup sources.
>
> Currently, inspecting wakeup sources typically involves reading interfaces
> like /sys/class/wakeup/* or debugfs. The repeated syscalls to query the
> sysfs nodes is inefficient, as there can be hundreds of wakeup_sources, and
> each wakeup source have multiple stats, with one sysfs node per stat.
> debugfs is unstable and insecure.
>
> The iterators utilize pre-existing wakeup_sources_walk_* functions to
> traverse over the SRCU that backs the list of wakeup_sources.
>
> Changes in v5:
>  - Add CORE definitions for *__local variables to fix s390 build per bpf-ci
>  - v4 link: https://lore.kernel.org/all/20260211235055.239024-1-wusamuel@google.com/
>
> Changes in v4:
>  - Proper init of variables in ws_iter_check_sleep_times() test per bpf-ci
>  - Remove PM patch since it's already part of rebase
>  - v3 link: https://lore.kernel.org/all/20260124012133.2451708-1-wusamuel@google.com/
>
> Changes in v3:
>  - Update wakeup_sources_walk_start() to handle an empty list per bpf-ci
>  - Simplify read of a struct's field in BPF program selftest per Andrii
>  - Drop open coded iterators for wakeup_sources
>  - Fix condition from !get_ws_iter_stream to get_ws_iter_stream in selftest
>  - Read event_count instead of wakeup_count in selftest
>  - v2 link: https://lore.kernel.org/all/20260108225523.3268383-1-wusamuel@google.com/
>
> Changes in v2:
>  - Guard BPF Makefile with CONFIG_PM_SLEEP to fix build errors
>  - Update copyright from 2025 to 2026
>  - v1 link: https://lore.kernel.org/all/20251204025003.3162056-1-wusamuel@google.com/
>
>
> Samuel Wu (2):
>   bpf: Add wakeup_source iterator
>   selftests/bpf: Add tests for wakeup_sources
>
>  kernel/bpf/Makefile                           |   3 +
>  kernel/bpf/wakeup_source_iter.c               | 103 ++++++
>  tools/testing/selftests/bpf/config            |   1 +
>  .../bpf/prog_tests/wakeup_source_iter.c       | 300 ++++++++++++++++++
>  .../selftests/bpf/progs/wakeup_source_iter.c  |  80 +++++
>  5 files changed, 487 insertions(+)
>  create mode 100644 kernel/bpf/wakeup_source_iter.c
>  create mode 100644 tools/testing/selftests/bpf/prog_tests/wakeup_source_iter.c
>  create mode 100644 tools/testing/selftests/bpf/progs/wakeup_source_iter.c
>
> --
> 2.53.0.473.g4a7958ca14-goog
>

Hi Andrii, any thoughts on this patchset? I've addressed your
suggestion from v2 of this patchset regarding selftest, but nothing
fundamental has changed since then.

Thanks!
Sam

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH bpf-next v5 0/2] Add wakeup_source iterators
  2026-03-05  1:31 ` [PATCH bpf-next v5 0/2] Add wakeup_source iterators Samuel Wu
@ 2026-03-06 23:28   ` Kumar Kartikeya Dwivedi
  2026-03-07 20:17     ` Kumar Kartikeya Dwivedi
  0 siblings, 1 reply; 11+ messages in thread
From: Kumar Kartikeya Dwivedi @ 2026-03-06 23:28 UTC (permalink / raw)
  To: Samuel Wu
  Cc: Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko,
	Martin KaFai Lau, Eduard Zingerman, Song Liu, Yonghong Song,
	John Fastabend, KP Singh, Stanislav Fomichev, Hao Luo, Jiri Olsa,
	Shuah Khan, kernel-team, linux-kernel, bpf, linux-kselftest

On Thu, 5 Mar 2026 at 02:31, Samuel Wu <wusamuel@google.com> wrote:
>
> On Wed, Feb 25, 2026 at 1:08 PM Samuel Wu <wusamuel@google.com> wrote:
> >
> > This patch series introduces BPF iterators for wakeup_source, enabling
> > BPF programs to efficiently traverse a device's wakeup sources.
> >
> > Currently, inspecting wakeup sources typically involves reading interfaces
> > like /sys/class/wakeup/* or debugfs. The repeated syscalls to query the
> > sysfs nodes is inefficient, as there can be hundreds of wakeup_sources, and
> > each wakeup source have multiple stats, with one sysfs node per stat.
> > debugfs is unstable and insecure.
> >
> > The iterators utilize pre-existing wakeup_sources_walk_* functions to
> > traverse over the SRCU that backs the list of wakeup_sources.
> >
> > Changes in v5:
> >  - Add CORE definitions for *__local variables to fix s390 build per bpf-ci
> >  - v4 link: https://lore.kernel.org/all/20260211235055.239024-1-wusamuel@google.com/
> >
> > Changes in v4:
> >  - Proper init of variables in ws_iter_check_sleep_times() test per bpf-ci
> >  - Remove PM patch since it's already part of rebase
> >  - v3 link: https://lore.kernel.org/all/20260124012133.2451708-1-wusamuel@google.com/
> >
> > Changes in v3:
> >  - Update wakeup_sources_walk_start() to handle an empty list per bpf-ci
> >  - Simplify read of a struct's field in BPF program selftest per Andrii
> >  - Drop open coded iterators for wakeup_sources
> >  - Fix condition from !get_ws_iter_stream to get_ws_iter_stream in selftest
> >  - Read event_count instead of wakeup_count in selftest
> >  - v2 link: https://lore.kernel.org/all/20260108225523.3268383-1-wusamuel@google.com/
> >
> > Changes in v2:
> >  - Guard BPF Makefile with CONFIG_PM_SLEEP to fix build errors
> >  - Update copyright from 2025 to 2026
> >  - v1 link: https://lore.kernel.org/all/20251204025003.3162056-1-wusamuel@google.com/
> >
> >
> > Samuel Wu (2):
> >   bpf: Add wakeup_source iterator
> >   selftests/bpf: Add tests for wakeup_sources
> >
> >  kernel/bpf/Makefile                           |   3 +
> >  kernel/bpf/wakeup_source_iter.c               | 103 ++++++
> >  tools/testing/selftests/bpf/config            |   1 +
> >  .../bpf/prog_tests/wakeup_source_iter.c       | 300 ++++++++++++++++++
> >  .../selftests/bpf/progs/wakeup_source_iter.c  |  80 +++++
> >  5 files changed, 487 insertions(+)
> >  create mode 100644 kernel/bpf/wakeup_source_iter.c
> >  create mode 100644 tools/testing/selftests/bpf/prog_tests/wakeup_source_iter.c
> >  create mode 100644 tools/testing/selftests/bpf/progs/wakeup_source_iter.c
> >
> > --
> > 2.53.0.473.g4a7958ca14-goog
> >
>
> Hi Andrii, any thoughts on this patchset? I've addressed your
> suggestion from v2 of this patchset regarding selftest, but nothing
> fundamental has changed since then.

I think it's still unclear why this cannot be achieved by exploring
what Andrii proposed initially in [0], i.e. by using existing
primitives and adding extra kfuncs where necessary to solve
bottlenecks, and I didn't see any indication where it was explored
sufficiently.

I think all we need is the wakeup source read_lock()/unlock() APIs as
KF_ACQUIRE/KF_RELEASE kfuncs. Just need to ensure we pair each lock
with unlock by passing the index, which can be an opaque value
returned from the kfunc (e.g. dummy empty struct pointer encoding the
index). Then you can use bpf_for() to open-code the list iteration.
The iterator would become unnecessary, and everything can be done from
a syscall program's BPF_PROG_RUN invocation. I didn't
check how we can get the pointer to the wakeup_sources LIST_HEAD, but
you could try several options (__ksym,
bpf_kallsyms_lookup_name+bpf_core_cast and then walk it, or return
pointer from a new kfunc and walk it using bpf_for()+bpf_core_cast()).

Also, how much speed up does this really provide? It is hard to
understand the motivation from the current cover letter. Is the
bottleneck due to opening multiple files in sysfs? You will still have
to read text and parse text from the iterator's output in your current
solution. Wouldn't it be faster if you just used a syscall program,
and used binary output (using a ring buffer) to stream data to user
space?

  [0]: https://lore.kernel.org/all/CAEf4BzZv9kCAmX0Fo4kR8qh9mu5NB-wLDmNAK_R=VzcxL7VPpQ@mail.gmail.com/

>
> Thanks!
> Sam
>

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH bpf-next v5 0/2] Add wakeup_source iterators
  2026-03-06 23:28   ` Kumar Kartikeya Dwivedi
@ 2026-03-07 20:17     ` Kumar Kartikeya Dwivedi
  2026-03-11  0:23       ` Samuel Wu
  0 siblings, 1 reply; 11+ messages in thread
From: Kumar Kartikeya Dwivedi @ 2026-03-07 20:17 UTC (permalink / raw)
  To: Samuel Wu, Paul E . McKenney
  Cc: Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko,
	Martin KaFai Lau, Eduard Zingerman, Song Liu, Yonghong Song,
	John Fastabend, KP Singh, Stanislav Fomichev, Hao Luo, Jiri Olsa,
	Shuah Khan, kernel-team, linux-kernel, bpf, linux-kselftest

On Sat, 7 Mar 2026 at 00:28, Kumar Kartikeya Dwivedi <memxor@gmail.com> wrote:
>
> On Thu, 5 Mar 2026 at 02:31, Samuel Wu <wusamuel@google.com> wrote:
> >
> > On Wed, Feb 25, 2026 at 1:08 PM Samuel Wu <wusamuel@google.com> wrote:
> > >
> > > This patch series introduces BPF iterators for wakeup_source, enabling
> > > BPF programs to efficiently traverse a device's wakeup sources.
> > >
> > > Currently, inspecting wakeup sources typically involves reading interfaces
> > > like /sys/class/wakeup/* or debugfs. The repeated syscalls to query the
> > > sysfs nodes is inefficient, as there can be hundreds of wakeup_sources, and
> > > each wakeup source have multiple stats, with one sysfs node per stat.
> > > debugfs is unstable and insecure.
> > >
> > > The iterators utilize pre-existing wakeup_sources_walk_* functions to
> > > traverse over the SRCU that backs the list of wakeup_sources.
> > >
> > > Changes in v5:
> > >  - Add CORE definitions for *__local variables to fix s390 build per bpf-ci
> > >  - v4 link: https://lore.kernel.org/all/20260211235055.239024-1-wusamuel@google.com/
> > >
> > > Changes in v4:
> > >  - Proper init of variables in ws_iter_check_sleep_times() test per bpf-ci
> > >  - Remove PM patch since it's already part of rebase
> > >  - v3 link: https://lore.kernel.org/all/20260124012133.2451708-1-wusamuel@google.com/
> > >
> > > Changes in v3:
> > >  - Update wakeup_sources_walk_start() to handle an empty list per bpf-ci
> > >  - Simplify read of a struct's field in BPF program selftest per Andrii
> > >  - Drop open coded iterators for wakeup_sources
> > >  - Fix condition from !get_ws_iter_stream to get_ws_iter_stream in selftest
> > >  - Read event_count instead of wakeup_count in selftest
> > >  - v2 link: https://lore.kernel.org/all/20260108225523.3268383-1-wusamuel@google.com/
> > >
> > > Changes in v2:
> > >  - Guard BPF Makefile with CONFIG_PM_SLEEP to fix build errors
> > >  - Update copyright from 2025 to 2026
> > >  - v1 link: https://lore.kernel.org/all/20251204025003.3162056-1-wusamuel@google.com/
> > >
> > >
> > > Samuel Wu (2):
> > >   bpf: Add wakeup_source iterator
> > >   selftests/bpf: Add tests for wakeup_sources
> > >
> > >  kernel/bpf/Makefile                           |   3 +
> > >  kernel/bpf/wakeup_source_iter.c               | 103 ++++++
> > >  tools/testing/selftests/bpf/config            |   1 +
> > >  .../bpf/prog_tests/wakeup_source_iter.c       | 300 ++++++++++++++++++
> > >  .../selftests/bpf/progs/wakeup_source_iter.c  |  80 +++++
> > >  5 files changed, 487 insertions(+)
> > >  create mode 100644 kernel/bpf/wakeup_source_iter.c
> > >  create mode 100644 tools/testing/selftests/bpf/prog_tests/wakeup_source_iter.c
> > >  create mode 100644 tools/testing/selftests/bpf/progs/wakeup_source_iter.c
> > >
> > > --
> > > 2.53.0.473.g4a7958ca14-goog
> > >
> >
> > Hi Andrii, any thoughts on this patchset? I've addressed your
> > suggestion from v2 of this patchset regarding selftest, but nothing
> > fundamental has changed since then.
>
> I think it's still unclear why this cannot be achieved by exploring
> what Andrii proposed initially in [0], i.e. by using existing
> primitives and adding extra kfuncs where necessary to solve
> bottlenecks, and I didn't see any indication where it was explored
> sufficiently.
>
> I think all we need is the wakeup source read_lock()/unlock() APIs as
> KF_ACQUIRE/KF_RELEASE kfuncs. Just need to ensure we pair each lock
> with unlock by passing the index, which can be an opaque value
> returned from the kfunc (e.g. dummy empty struct pointer encoding the
> index). Then you can use bpf_for() to open-code the list iteration.

I had a longer discussion with Paul on this. When having multiple open
SRCU sections, mixing nested calls doesn't flatten the protection
scopes and leads to all kinds of issues, an example of which is in
[0]. That said, in this case, the verifier isn't tracking what the
underlying SRCU section protects or isn't even aware of it, all it
needs to ensure is that section is closed eventually, out-of-order
unlocks are ok, it would just exist to allow looking at the list in a
stable fashion using safe bpf_core_cast() accesses.

So, I just wanted to clarify why I think this should be perfectly ok.

  [0]: https://www.kernel.org/pub/linux/kernel/people/paulmck/Answers/RCU/SRCUhoha.html

(if you are like me, please follow the link at the bottom of [0]
before feeling good about understanding the example).

> The iterator would become unnecessary, and everything can be done from
> a syscall program's BPF_PROG_RUN invocation. I didn't
> check how we can get the pointer to the wakeup_sources LIST_HEAD, but
> you could try several options (__ksym,
> bpf_kallsyms_lookup_name+bpf_core_cast and then walk it, or return
> pointer from a new kfunc and walk it using bpf_for()+bpf_core_cast()).
>
> Also, how much speed up does this really provide? It is hard to
> understand the motivation from the current cover letter. Is the
> bottleneck due to opening multiple files in sysfs? You will still have
> to read text and parse text from the iterator's output in your current
> solution. Wouldn't it be faster if you just used a syscall program,
> and used binary output (using a ring buffer) to stream data to user
> space?
>
>   [0]: https://lore.kernel.org/all/CAEf4BzZv9kCAmX0Fo4kR8qh9mu5NB-wLDmNAK_R=VzcxL7VPpQ@mail.gmail.com/
>
> >
> > Thanks!
> > Sam
> >

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH bpf-next v5 0/2] Add wakeup_source iterators
  2026-03-07 20:17     ` Kumar Kartikeya Dwivedi
@ 2026-03-11  0:23       ` Samuel Wu
  2026-03-11  9:06         ` Kumar Kartikeya Dwivedi
  0 siblings, 1 reply; 11+ messages in thread
From: Samuel Wu @ 2026-03-11  0:23 UTC (permalink / raw)
  To: Kumar Kartikeya Dwivedi
  Cc: Paul E . McKenney, Alexei Starovoitov, Daniel Borkmann,
	Andrii Nakryiko, Martin KaFai Lau, Eduard Zingerman, Song Liu,
	Yonghong Song, John Fastabend, KP Singh, Stanislav Fomichev,
	Hao Luo, Jiri Olsa, Shuah Khan, kernel-team, linux-kernel, bpf,
	linux-kselftest

On Sat, Mar 7, 2026 at 12:17 PM Kumar Kartikeya Dwivedi
<memxor@gmail.com> wrote:
>
> On Sat, 7 Mar 2026 at 00:28, Kumar Kartikeya Dwivedi <memxor@gmail.com> wrote:

[...]

> > I think it's still unclear why this cannot be achieved by exploring
> > what Andrii proposed initially in [0], i.e. by using existing
> > primitives and adding extra kfuncs where necessary to solve
> > bottlenecks, and I didn't see any indication where it was explored
> > sufficiently.
> >
This is my bad, I misinterpreted/misunderstood Andrii's original
proposal to apply only to open-coded BPF iterators, hence the follow
up patches with them removed. However, I don't disagree this can be
achieved via open-coding. Both proposals are viable to traverse wakeup
sources via BPF, however I see two distinct advantages of the BPF
iterator:

1. Iterators provide a stable kernel interface that decouples the BPF
code from the kernel's internal data structures and refactors
2. If wakeup sources are accessible via BPF, the next step should be
to deprecate the existing interfaces for wakeup sources, such as the
sysfs and debugfs stats. In that case, a text-based BPF iterator
output will provide less friction during the transition.

> > I think all we need is the wakeup source read_lock()/unlock() APIs as
> > KF_ACQUIRE/KF_RELEASE kfuncs. Just need to ensure we pair each lock
> > with unlock by passing the index, which can be an opaque value
> > returned from the kfunc (e.g. dummy empty struct pointer encoding the
> > index). Then you can use bpf_for() to open-code the list iteration.
>
> I had a longer discussion with Paul on this. When having multiple open
> SRCU sections, mixing nested calls doesn't flatten the protection
> scopes and leads to all kinds of issues, an example of which is in
> [0]. That said, in this case, the verifier isn't tracking what the
> underlying SRCU section protects or isn't even aware of it, all it
> needs to ensure is that section is closed eventually, out-of-order
> unlocks are ok, it would just exist to allow looking at the list in a
> stable fashion using safe bpf_core_cast() accesses.
>
> So, I just wanted to clarify why I think this should be perfectly ok.
>
>   [0]: https://www.kernel.org/pub/linux/kernel/people/paulmck/Answers/RCU/SRCUhoha.html
>
> (if you are like me, please follow the link at the bottom of [0]
> before feeling good about understanding the example).

Appreciate the thoughtful feedback from you and Paul. The example was
nuanced but well-explained by the post, however, a key difference I
see is that the BPF iterator solution creates continuous locking
segments during traversal, as opposed to hand-over-hand locking in the
example provided. I could be missing something fundamental here- do
you mind elaborating?

>
> > The iterator would become unnecessary, and everything can be done from
> > a syscall program's BPF_PROG_RUN invocation. I didn't
> > check how we can get the pointer to the wakeup_sources LIST_HEAD, but
> > you could try several options (__ksym,
> > bpf_kallsyms_lookup_name+bpf_core_cast and then walk it, or return
> > pointer from a new kfunc and walk it using bpf_for()+bpf_core_cast()).
> >

Getting the static variable is key to the open-coded approach, but a
clean way forward isn't obvious to me. Passing around raw addresses
seems brittle and can raise security concerns. The wrapper kfunc to
get the static address might be the best option, but it feels like it
violates the power subsystem's encapsulation. The iterator approach
abstracts all of this cleanly.

> > Also, how much speed up does this really provide? It is hard to
> > understand the motivation from the current cover letter. Is the
> > bottleneck due to opening multiple files in sysfs? You will still have
> > to read text and parse text from the iterator's output in your current
> > solution. Wouldn't it be faster if you just used a syscall program,
> > and used binary output (using a ring buffer) to stream data to user
> > space?

With the BPF iterator approach on a Pixel 6 in a quiescent state, I'm
seeing a 10x speedup when traversing wakeup sources; the speedup would
likely be even greater on a loaded system.

As alluded to earlier, the next step is to remove the
/sys/class/wakeup node and debugfs nodes. My apologies for not
explicitly listing this in the cover letter, but ultimately this would
save memory by removing those nodes, eliminate work by not creating
the nodes, and improve performance with a faster traversal, all while
transparently replacing legacy interfaces.

Thanks!
Sam

[...]

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH bpf-next v5 0/2] Add wakeup_source iterators
  2026-03-11  0:23       ` Samuel Wu
@ 2026-03-11  9:06         ` Kumar Kartikeya Dwivedi
  2026-03-12 17:13           ` Samuel Wu
  0 siblings, 1 reply; 11+ messages in thread
From: Kumar Kartikeya Dwivedi @ 2026-03-11  9:06 UTC (permalink / raw)
  To: Samuel Wu
  Cc: Paul E . McKenney, Alexei Starovoitov, Daniel Borkmann,
	Andrii Nakryiko, Martin KaFai Lau, Eduard Zingerman, Song Liu,
	Yonghong Song, John Fastabend, KP Singh, Stanislav Fomichev,
	Hao Luo, Jiri Olsa, Shuah Khan, kernel-team, linux-kernel, bpf,
	linux-kselftest

On Wed, 11 Mar 2026 at 01:23, Samuel Wu <wusamuel@google.com> wrote:
>
> On Sat, Mar 7, 2026 at 12:17 PM Kumar Kartikeya Dwivedi
> <memxor@gmail.com> wrote:
> >
> > On Sat, 7 Mar 2026 at 00:28, Kumar Kartikeya Dwivedi <memxor@gmail.com> wrote:
>
> [...]
>
> > > I think it's still unclear why this cannot be achieved by exploring
> > > what Andrii proposed initially in [0], i.e. by using existing
> > > primitives and adding extra kfuncs where necessary to solve
> > > bottlenecks, and I didn't see any indication where it was explored
> > > sufficiently.
> > >
> This is my bad, I misinterpreted/misunderstood Andrii's original
> proposal to apply only to open-coded BPF iterators, hence the follow
> up patches with them removed. However, I don't disagree this can be
> achieved via open-coding. Both proposals are viable to traverse wakeup
> sources via BPF, however I see two distinct advantages of the BPF
> iterator:
>
> 1. Iterators provide a stable kernel interface that decouples the BPF
> code from the kernel's internal data structures and refactors

I think this is my concern; committing to a stable interface that
we'll have to maintain forever.
It's a narrow use case that can be better served by dedicated kfuncs
and rolling your own solution.
There will be no end to the number of iterators if we keep making one
for every linked list in the kernel.

It would be different if the synchronization scheme for the
wakeup_sources list was really complicated and hard to express inline
in the verifier, but that is not the case.

> 2. If wakeup sources are accessible via BPF, the next step should be
> to deprecate the existing interfaces for wakeup sources, such as the
> sysfs and debugfs stats. In that case, a text-based BPF iterator
> output will provide less friction during the transition.
>

I would agree if this was a one-to-one conversion, where you just
replace the directory for look up.
But you'll be reading from one file vs several, so it's already a
non-trivial change in the consumer.
At that point, the conversion burden doesn't have the same weight in
deciding whether to stick with textual representation.

> > > I think all we need is the wakeup source read_lock()/unlock() APIs as
> > > KF_ACQUIRE/KF_RELEASE kfuncs. Just need to ensure we pair each lock
> > > with unlock by passing the index, which can be an opaque value
> > > returned from the kfunc (e.g. dummy empty struct pointer encoding the
> > > index). Then you can use bpf_for() to open-code the list iteration.
> >
> > I had a longer discussion with Paul on this. When having multiple open
> > SRCU sections, mixing nested calls doesn't flatten the protection
> > scopes and leads to all kinds of issues, an example of which is in
> > [0]. That said, in this case, the verifier isn't tracking what the
> > underlying SRCU section protects or isn't even aware of it, all it
> > needs to ensure is that section is closed eventually, out-of-order
> > unlocks are ok, it would just exist to allow looking at the list in a
> > stable fashion using safe bpf_core_cast() accesses.
> >
> > So, I just wanted to clarify why I think this should be perfectly ok.
> >
> >   [0]: https://www.kernel.org/pub/linux/kernel/people/paulmck/Answers/RCU/SRCUhoha.html
> >
> > (if you are like me, please follow the link at the bottom of [0]
> > before feeling good about understanding the example).
>
> Appreciate the thoughtful feedback from you and Paul. The example was
> nuanced but well-explained by the post, however, a key difference I
> see is that the BPF iterator solution creates continuous locking
> segments during traversal, as opposed to hand-over-hand locking in the
> example provided. I could be missing something fundamental here- do
> you mind elaborating?

It was just an illustration to identify what guarantees the verifier
would need to enforce.
The most we need to do is just ensure SRCU indices are passed back
during unlock, in whatever order.
Critical section will be using bpf_core_cast() anyway to do safe
walking of the list.
With the KF_ACQUIRE/KF_RELEASE kfuncs you will still have a continuous
locking segment.
Just enclose your list walking loop between lock() and unlock().

>
> >
> > > The iterator would become unnecessary, and everything can be done from
> > > a syscall program's BPF_PROG_RUN invocation. I didn't
> > > check how we can get the pointer to the wakeup_sources LIST_HEAD, but
> > > you could try several options (__ksym,
> > > bpf_kallsyms_lookup_name+bpf_core_cast and then walk it, or return
> > > pointer from a new kfunc and walk it using bpf_for()+bpf_core_cast()).
> > >
>
> Getting the static variable is key to the open-coded approach, but a
> clean way forward isn't obvious to me. Passing around raw addresses
> seems brittle and can raise security concerns. The wrapper kfunc to

I don't understand the security concern. All of this happens inside
the BPF program.
If you're writing these addresses to user space and leaking them, it's your bug.
You can do that with iterators too. You have all the raw addresses
available to you.

> get the static address might be the best option, but it feels like it
> violates the power subsystem's encapsulation. The iterator approach
> abstracts all of this cleanly.
>
> > > Also, how much speed up does this really provide? It is hard to
> > > understand the motivation from the current cover letter. Is the
> > > bottleneck due to opening multiple files in sysfs? You will still have
> > > to read text and parse text from the iterator's output in your current
> > > solution. Wouldn't it be faster if you just used a syscall program,
> > > and used binary output (using a ring buffer) to stream data to user
> > > space?
>
> With the BPF iterator approach on a Pixel 6 in a quiescent state, I'm
> seeing a 10x speedup when traversing wakeup sources; the speedup would
> likely be even greater on a loaded system.
>

I'm sure you can replicate it with the approach described above.
It might even be faster than parsing text from the file.

> As alluded to earlier, the next step is to remove the
> /sys/class/wakeup node and debugfs nodes. My apologies for not
> explicitly listing this in the cover letter, but ultimately this would
> save memory by removing those nodes, eliminate work by not creating
> the nodes, and improve performance with a faster traversal, all while
> transparently replacing legacy interfaces.

And you can do that regardless of the type of solution chosen above.

>
> Thanks!
> Sam
>
> [...]

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH bpf-next v5 0/2] Add wakeup_source iterators
  2026-03-11  9:06         ` Kumar Kartikeya Dwivedi
@ 2026-03-12 17:13           ` Samuel Wu
  0 siblings, 0 replies; 11+ messages in thread
From: Samuel Wu @ 2026-03-12 17:13 UTC (permalink / raw)
  To: Kumar Kartikeya Dwivedi
  Cc: Paul E . McKenney, Alexei Starovoitov, Daniel Borkmann,
	Andrii Nakryiko, Martin KaFai Lau, Eduard Zingerman, Song Liu,
	Yonghong Song, John Fastabend, KP Singh, Stanislav Fomichev,
	Hao Luo, Jiri Olsa, Shuah Khan, kernel-team, linux-kernel, bpf,
	linux-kselftest

On Wed, Mar 11, 2026 at 2:06 AM Kumar Kartikeya Dwivedi
<memxor@gmail.com> wrote:
>
> On Wed, 11 Mar 2026 at 01:23, Samuel Wu <wusamuel@google.com> wrote:
> >
> > On Sat, Mar 7, 2026 at 12:17 PM Kumar Kartikeya Dwivedi
> > <memxor@gmail.com> wrote:
> > >
> > > On Sat, 7 Mar 2026 at 00:28, Kumar Kartikeya Dwivedi <memxor@gmail.com> wrote:
> >
> > [...]
> >
> > > > I think it's still unclear why this cannot be achieved by exploring
> > > > what Andrii proposed initially in [0], i.e. by using existing
> > > > primitives and adding extra kfuncs where necessary to solve
> > > > bottlenecks, and I didn't see any indication where it was explored
> > > > sufficiently.
> > > >
> > This is my bad, I misinterpreted/misunderstood Andrii's original
> > proposal to apply only to open-coded BPF iterators, hence the follow
> > up patches with them removed. However, I don't disagree this can be
> > achieved via open-coding. Both proposals are viable to traverse wakeup
> > sources via BPF, however I see two distinct advantages of the BPF
> > iterator:
> >
> > 1. Iterators provide a stable kernel interface that decouples the BPF
> > code from the kernel's internal data structures and refactors
>
> I think this is my concern; committing to a stable interface that
> we'll have to maintain forever.
> It's a narrow use case that can be better served by dedicated kfuncs
> and rolling your own solution.
> There will be no end to the number of iterators if we keep making one
> for every linked list in the kernel.
>
> It would be different if the synchronization scheme for the
> wakeup_sources list was really complicated and hard to express inline
> in the verifier, but that is not the case.
>

I completely agree it's a viable approach, it seems to come down to
where/who carries the maintenance burden. Regardless, let me explore
how to prototype this cleanly to handle the details like the static
address, and then we will have something concrete for comparisons. I
appreciate the alternative suggestion and the feedback here.

> > 2. If wakeup sources are accessible via BPF, the next step should be
> > to deprecate the existing interfaces for wakeup sources, such as the
> > sysfs and debugfs stats. In that case, a text-based BPF iterator
> > output will provide less friction during the transition.
> >
>
> I would agree if this was a one-to-one conversion, where you just
> replace the directory for look up.
> But you'll be reading from one file vs several, so it's already a
> non-trivial change in the consumer.
> At that point, the conversion burden doesn't have the same weight in
> deciding whether to stick with textual representation.

My understanding is that most (all?) current consumers ultimately want
the aggregate list of wakeup source stats, not a single node in
isolation; the reason there are multiple nodes is to adhere to the
precedent that sysfs nodes contain a singular stat. A single
text-based output node with the aggregated stats would be a
straightforward change that simplifies userspace code consuming these
stats.

> > > > I think all we need is the wakeup source read_lock()/unlock() APIs as
> > > > KF_ACQUIRE/KF_RELEASE kfuncs. Just need to ensure we pair each lock
> > > > with unlock by passing the index, which can be an opaque value
> > > > returned from the kfunc (e.g. dummy empty struct pointer encoding the
> > > > index). Then you can use bpf_for() to open-code the list iteration.
> > >
> > > I had a longer discussion with Paul on this. When having multiple open
> > > SRCU sections, mixing nested calls doesn't flatten the protection
> > > scopes and leads to all kinds of issues, an example of which is in
> > > [0]. That said, in this case, the verifier isn't tracking what the
> > > underlying SRCU section protects or isn't even aware of it, all it
> > > needs to ensure is that section is closed eventually, out-of-order
> > > unlocks are ok, it would just exist to allow looking at the list in a
> > > stable fashion using safe bpf_core_cast() accesses.
> > >
> > > So, I just wanted to clarify why I think this should be perfectly ok.
> > >
> > >   [0]: https://www.kernel.org/pub/linux/kernel/people/paulmck/Answers/RCU/SRCUhoha.html
> > >
> > > (if you are like me, please follow the link at the bottom of [0]
> > > before feeling good about understanding the example).
> >
> > Appreciate the thoughtful feedback from you and Paul. The example was
> > nuanced but well-explained by the post, however, a key difference I
> > see is that the BPF iterator solution creates continuous locking
> > segments during traversal, as opposed to hand-over-hand locking in the
> > example provided. I could be missing something fundamental here- do
> > you mind elaborating?
>
> It was just an illustration to identify what guarantees the verifier
> would need to enforce.
> The most we need to do is just ensure SRCU indices are passed back
> during unlock, in whatever order.
> Critical section will be using bpf_core_cast() anyway to do safe
> walking of the list.
> With the KF_ACQUIRE/KF_RELEASE kfuncs you will still have a continuous
> locking segment.
> Just enclose your list walking loop between lock() and unlock().
>
> >
> > >
> > > > The iterator would become unnecessary, and everything can be done from
> > > > a syscall program's BPF_PROG_RUN invocation. I didn't
> > > > check how we can get the pointer to the wakeup_sources LIST_HEAD, but
> > > > you could try several options (__ksym,
> > > > bpf_kallsyms_lookup_name+bpf_core_cast and then walk it, or return
> > > > pointer from a new kfunc and walk it using bpf_for()+bpf_core_cast()).
> > > >
> >
> > Getting the static variable is key to the open-coded approach, but a
> > clean way forward isn't obvious to me. Passing around raw addresses
> > seems brittle and can raise security concerns. The wrapper kfunc to
>
> I don't understand the security concern. All of this happens inside
> the BPF program.
> If you're writing these addresses to user space and leaking them, it's your bug.
> You can do that with iterators too. You have all the raw addresses
> available to you.
>
> > get the static address might be the best option, but it feels like it
> > violates the power subsystem's encapsulation. The iterator approach
> > abstracts all of this cleanly.
> >
> > > > Also, how much speed up does this really provide? It is hard to
> > > > understand the motivation from the current cover letter. Is the
> > > > bottleneck due to opening multiple files in sysfs? You will still have
> > > > to read text and parse text from the iterator's output in your current
> > > > solution. Wouldn't it be faster if you just used a syscall program,
> > > > and used binary output (using a ring buffer) to stream data to user
> > > > space?
> >
> > With the BPF iterator approach on a Pixel 6 in a quiescent state, I'm
> > seeing a 10x speedup when traversing wakeup sources; the speedup would
> > likely be even greater on a loaded system.
> >
>
> I'm sure you can replicate it with the approach described above.
> It might even be faster than parsing text from the file.
>
> > As alluded to earlier, the next step is to remove the
> > /sys/class/wakeup node and debugfs nodes. My apologies for not
> > explicitly listing this in the cover letter, but ultimately this would
> > save memory by removing those nodes, eliminate work by not creating
> > the nodes, and improve performance with a faster traversal, all while
> > transparently replacing legacy interfaces.
>
> And you can do that regardless of the type of solution chosen above.
>
> >
> > Thanks!
> > Sam
> >
> > [...]

^ permalink raw reply	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2026-03-12 17:13 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-02-25 21:08 [PATCH bpf-next v5 0/2] Add wakeup_source iterators Samuel Wu
2026-02-25 21:08 ` [PATCH bpf-next v5 1/2] bpf: Add wakeup_source iterator Samuel Wu
2026-02-25 22:03   ` bot+bpf-ci
2026-02-25 22:35     ` Samuel Wu
2026-02-25 21:08 ` [PATCH bpf-next v5 2/2] selftests/bpf: Add tests for wakeup_sources Samuel Wu
2026-03-05  1:31 ` [PATCH bpf-next v5 0/2] Add wakeup_source iterators Samuel Wu
2026-03-06 23:28   ` Kumar Kartikeya Dwivedi
2026-03-07 20:17     ` Kumar Kartikeya Dwivedi
2026-03-11  0:23       ` Samuel Wu
2026-03-11  9:06         ` Kumar Kartikeya Dwivedi
2026-03-12 17:13           ` Samuel Wu

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox