From: Yonghong Song <yonghong.song@linux.dev>
To: Oleg Nesterov <oleg@redhat.com>
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 06:52:16 -0500 [thread overview]
Message-ID: <41039b2a-6122-4392-96e2-4a833f41496d@linux.dev> (raw)
In-Reply-To: <20231116095439.GC18748@redhat.com>
On 11/16/23 4:54 AM, Oleg Nesterov wrote:
> 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?
There is no need then. We can wait for maintainers who may or
may not have additional requests.
>
> Thanks,
>
> Oleg.
>
next prev parent reply other threads:[~2023-11-16 11:52 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
2023-11-16 11:52 ` Yonghong Song [this message]
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=41039b2a-6122-4392-96e2-4a833f41496d@linux.dev \
--to=yonghong.song@linux.dev \
--cc=ast@kernel.org \
--cc=bpf@vger.kernel.org \
--cc=daniel@iogearbox.net \
--cc=kuifeng@fb.com \
--cc=linux-kernel@vger.kernel.org \
--cc=oleg@redhat.com \
--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.