From: Oleg Nesterov <oleg@redhat.com>
To: Yonghong Song <yonghong.song@linux.dev>
Cc: Alexei Starovoitov <ast@kernel.org>,
Chuyi Zhou <zhouchuyi@bytedance.com>,
Daniel Borkmann <daniel@iogearbox.net>,
Kui-Feng Lee <kuifeng@fb.com>,
linux-kernel@vger.kernel.org, bpf@vger.kernel.org
Subject: Re: [PATCH 0/3] bpf: kernel/bpf/task_iter.c: don't abuse next_thread()
Date: Thu, 16 Nov 2023 10:54:39 +0100 [thread overview]
Message-ID: <20231116095439.GC18748@redhat.com> (raw)
In-Reply-To: <c768aae4-1c41-41ef-895d-33556b99dc15@linux.dev>
On 11/15, Yonghong Song wrote:
>
> On 11/14/23 11:32 AM, Oleg Nesterov wrote:
> >Compile tested.
> >
> >Every lockless usage of next_thread() was wrong, bpf/task_iter.c is
> >the last user and is no exception.
>
> It would be great if you can give more information in the commit message
> about why the usage of next_thread() is wrong in bpf/task_iter.c.
I tried to explain the problems in the changelogs:
1/3:
task_group_seq_get_next() can return the group leader twice if it races
with mt-thread exec which changes the group->leader's pid.
2/3:
bpf_iter_task_next() can loop forever, "kit->pos == kit->task" can never
happen if kit->pos execs.
> IIUC, some information is presented in :
> https://lore.kernel.org/all/20230824143112.GA31208@redhat.com/
Yes, Linus and Eric suggest to simply kill next_thread(). I am not
sure, this needs another discussion.
But as for bpf/task_iter.c... Even _if_ the usage was correct, this
code simply doesn't need the "circular" next_thread(), NULL at the
end simplifies the code.
> Also, please add 'bpf' in the subject tag ([PATCH bpf 0/3]) to
> make it clear the patch should be applied to bpf tree.
OK, will do next time. Or should I resend this series with 'bpf'
in the subject tag?
Thanks,
Oleg.
next prev parent reply other threads:[~2023-11-16 9:55 UTC|newest]
Thread overview: 14+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-11-14 16:32 [PATCH 0/3] bpf: kernel/bpf/task_iter.c: don't abuse next_thread() Oleg Nesterov
2023-11-14 16:32 ` [PATCH 1/3] bpf: task_group_seq_get_next: use __next_thread() rather than next_thread() Oleg Nesterov
2023-11-16 3:31 ` Yonghong Song
2023-11-16 9:34 ` Oleg Nesterov
2023-11-16 11:46 ` Yonghong Song
2023-11-14 16:32 ` [PATCH 2/3] bpf: bpf_iter_task_next: " Oleg Nesterov
2023-11-16 3:34 ` Yonghong Song
2023-11-14 16:32 ` [PATCH 3/3] bpf: bpf_iter_task_next: use next_task(kit->task) rather than next_task(kit->pos) Oleg Nesterov
2023-11-16 5:16 ` Yonghong Song
2023-11-16 9:38 ` Oleg Nesterov
2023-11-16 3:13 ` [PATCH 0/3] bpf: kernel/bpf/task_iter.c: don't abuse next_thread() Yonghong Song
2023-11-16 9:54 ` Oleg Nesterov [this message]
2023-11-16 11:52 ` Yonghong Song
2023-11-19 20:00 ` patchwork-bot+netdevbpf
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20231116095439.GC18748@redhat.com \
--to=oleg@redhat.com \
--cc=ast@kernel.org \
--cc=bpf@vger.kernel.org \
--cc=daniel@iogearbox.net \
--cc=kuifeng@fb.com \
--cc=linux-kernel@vger.kernel.org \
--cc=yonghong.song@linux.dev \
--cc=zhouchuyi@bytedance.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.