[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CAADnVQKK3vmfPmRxLuh6ad94FeioN2JV=v+L-93ZvwdYqR_Kcg@mail.gmail.com>
Date: Tue, 10 Dec 2024 10:51:17 -0800
From: Alexei Starovoitov <alexei.starovoitov@...il.com>
To: Juntong Deng <juntong.deng@...look.com>
Cc: Christian Brauner <brauner@...nel.org>, Alexei Starovoitov <ast@...nel.org>,
Daniel Borkmann <daniel@...earbox.net>, John Fastabend <john.fastabend@...il.com>,
Andrii Nakryiko <andrii@...nel.org>, Martin KaFai Lau <martin.lau@...ux.dev>, Eddy Z <eddyz87@...il.com>,
Song Liu <song@...nel.org>, Yonghong Song <yonghong.song@...ux.dev>, KP Singh <kpsingh@...nel.org>,
Stanislav Fomichev <sdf@...ichev.me>, Hao Luo <haoluo@...gle.com>, Jiri Olsa <jolsa@...nel.org>,
Kumar Kartikeya Dwivedi <memxor@...il.com>, snorcht@...il.com, bpf <bpf@...r.kernel.org>,
LKML <linux-kernel@...r.kernel.org>,
Linux-Fsdevel <linux-fsdevel@...r.kernel.org>
Subject: Re: [PATCH bpf-next v5 2/5] selftests/bpf: Add tests for open-coded
style process file iterator
On Tue, Dec 10, 2024 at 8:23 AM Juntong Deng <juntong.deng@...look.com> wrote:
>
> >> +SEC("fentry/" SYS_PREFIX "sys_nanosleep")
> >> +int test_bpf_iter_task_file(void *ctx)
> >> +{
> >> + struct bpf_iter_task_file task_file_it;
> >> + struct bpf_iter_task_file_item *item;
> >> + struct task_struct *task;
> >> +
> >> + task = bpf_get_current_task_btf();
> >> + if (task->parent->pid != parent_pid)
> >> + return 0;
> >> +
> >> + count++;
> >> +
> >> + bpf_rcu_read_lock();
> >
> > What does the RCU read lock do here exactly?
> >
>
> Thanks for your reply.
>
> This is used to solve the problem previously discussed in v3 [0].
>
> Task ref may be released during iteration.
>
> [0]:
> https://lore.kernel.org/bpf/CAADnVQ+0LUXxmfm1YgyGDz=cciy3+dGGM-Zysq84fpAdaB74Qw@mail.gmail.com/
I think you misunderstood my comment.
"If this object _was_ RCU protected ..."
Adding rcu_read_lock doesn't make 'task' pointer RCU protected.
That's not how RCU works.
So patch 1 doing:
item->task = task;
is not correct.
See bpf_iter_task_vma_new(). It's doing:
kit->data->task = get_task_struct(task);
to make sure task stays valid while iterating.
pw-bot: cr
Powered by blists - more mailing lists