This looks more clear and simplifies the code. While at it, remove the
unnecessary initialization of pos/task at the start of bpf_iter_task_new().

Note that we can even kill kit->task, we can just use pos->group_leader,
but I don't understand the BUILD_BUG_ON() checks in bpf_iter_task_new().

Signed-off-by: Oleg Nesterov <oleg@redhat.com>
---
 kernel/bpf/task_iter.c | 14 +++++---------
 1 file changed, 5 insertions(+), 9 deletions(-)

diff --git a/kernel/bpf/task_iter.c b/kernel/bpf/task_iter.c
index d42e08d0d0b7..e5c3500443c6 100644
--- a/kernel/bpf/task_iter.c
+++ b/kernel/bpf/task_iter.c
@@ -978,7 +978,6 @@ __bpf_kfunc int bpf_iter_task_new(struct bpf_iter_task *it,
 	BUILD_BUG_ON(__alignof__(struct bpf_iter_task_kern) !=
 					__alignof__(struct bpf_iter_task));
 
-	kit->task = kit->pos = NULL;
 	switch (flags) {
 	case BPF_TASK_ITER_ALL_THREADS:
 	case BPF_TASK_ITER_ALL_PROCS:
@@ -1016,18 +1015,15 @@ __bpf_kfunc struct task_struct *bpf_iter_task_next(struct bpf_iter_task *it)
 		goto get_next_task;
 
 	kit->pos = __next_thread(kit->pos);
-	if (!kit->pos) {
-		if (flags == BPF_TASK_ITER_PROC_THREADS)
-			return pos;
-		kit->pos = kit->task;
-	} else
+	if (kit->pos || flags == BPF_TASK_ITER_PROC_THREADS)
 		return pos;
 
 get_next_task:
-	kit->pos = next_task(kit->pos);
-	kit->task = kit->pos;
-	if (kit->pos == &init_task)
+	kit->task = next_task(kit->task);
+	if (kit->task == &init_task)
 		kit->pos = NULL;
+	else
+		kit->pos = kit->task;
 
 	return pos;
 }
-- 
2.25.1.362.g51ebf55

On 11/14/23 11:32 AM, Oleg Nesterov wrote:
> This looks more clear and simplifies the code. While at it, remove the
> unnecessary initialization of pos/task at the start of bpf_iter_task_new().
>
> Note that we can even kill kit->task, we can just use pos->group_leader,
> but I don't understand the BUILD_BUG_ON() checks in bpf_iter_task_new().

Let us keep kit->task, which is used in later function
bpf_iter_task_next(). The patch looks good to me.

>
> Signed-off-by: Oleg Nesterov <oleg@redhat.com>

Acked-by: Yonghong Song <yonghong.song@linux.dev>

On 11/16, Yonghong Song wrote:
>
> On 11/14/23 11:32 AM, Oleg Nesterov wrote:
> >This looks more clear and simplifies the code. While at it, remove the
> >unnecessary initialization of pos/task at the start of bpf_iter_task_new().
> >
> >Note that we can even kill kit->task, we can just use pos->group_leader,
> >but I don't understand the BUILD_BUG_ON() checks in bpf_iter_task_new().
>
> Let us keep kit->task, which is used in later function
> bpf_iter_task_next(). The patch looks good to me.

Yes, but it can use pos->group_leader instead of kit->task.
But I agree, lets keep kit->task.

> Acked-by: Yonghong Song <yonghong.song@linux.dev>

Thanks!

Oleg.