linux-kernel - Re: [PATCH bpf-next v4 1/4] fprobe: use rhltable for fprobe_ip

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <CADxym3YB69_mZPWqupocOCvBju2ugNcO7hSddwfv+Xp4dAEgEQ@mail.gmail.com>
Date: Sun, 17 Aug 2025 09:43:33 +0800
From: Menglong Dong <menglong8.dong@...il.com>
To: Masami Hiramatsu <mhiramat@...nel.org>
Cc: olsajiri@...il.com, rostedt@...dmis.org, mathieu.desnoyers@...icios.com, 
	hca@...ux.ibm.com, revest@...omium.org, linux-kernel@...r.kernel.org, 
	linux-trace-kernel@...r.kernel.org, bpf@...r.kernel.org
Subject: Re: [PATCH bpf-next v4 1/4] fprobe: use rhltable for fprobe_ip_table

On Sat, Aug 16, 2025 at 10:50 PM Masami Hiramatsu <mhiramat@...nel.org> wrote:
>
> Hi Menglong,
>
> Sorry, one more thing.
>
> > @@ -260,14 +263,12 @@ static int fprobe_entry(struct ftrace_graph_ent *trace, struct fgraph_ops *gops,
> >       if (WARN_ON_ONCE(!fregs))
> >               return 0;
> >
> > -     first = node = find_first_fprobe_node(func);
> > -     if (unlikely(!first))
> > -             return 0;
> > -
> > +     rcu_read_lock();
>
> Actually, we don't need these rcu_read_lock() in this function, because
> the caller function_graph_enter_regs() uses ftrace_test_recursion_trylock()
> which disables preemption. Thus we don't need to do this again here.

Yeah, I understand it now. I wondered about this part
for a long time, as I didn't see any RCU lock in the
fprobe_entry(). Thanks for the explanation ;)

I'll send a V5 now.

>
> > +     head = rhltable_lookup(&fprobe_ip_table, &func, fprobe_rht_params);
> >       reserved_words = 0;
> > -     hlist_for_each_entry_from_rcu(node, hlist) {
> > +     rhl_for_each_entry_rcu(node, pos, head, hlist) {
> >               if (node->addr != func)
> > -                     break;
> > +                     continue;
> >               fp = READ_ONCE(node->fp);
> >               if (!fp || !fp->exit_handler)
> >                       continue;
> > @@ -278,17 +279,19 @@ static int fprobe_entry(struct ftrace_graph_ent *trace, struct fgraph_ops *gops,
> >               reserved_words +=
> >                       FPROBE_HEADER_SIZE_IN_LONG + SIZE_IN_LONG(fp->entry_data_size);
> >       }
> > -     node = first;
> > +     rcu_read_unlock();
> >       if (reserved_words) {
> >               fgraph_data = fgraph_reserve_data(gops->idx, reserved_words * sizeof(long));
> >               if (unlikely(!fgraph_data)) {
> > -                     hlist_for_each_entry_from_rcu(node, hlist) {
> > +                     rcu_read_lock();
>
> Ditto.
>
> > +                     rhl_for_each_entry_rcu(node, pos, head, hlist) {
> >                               if (node->addr != func)
> > -                                     break;
> > +                                     continue;
> >                               fp = READ_ONCE(node->fp);
> >                               if (fp && !fprobe_disabled(fp))
> >                                       fp->nmissed++;
> >                       }
> > +                     rcu_read_unlock();
> >                       return 0;
> >               }
> >       }
>
> Thank you,
>
>
> --
> Masami Hiramatsu (Google) <mhiramat@...nel.org>