Thread (54 messages) 54 messages, 8 authors, 2026-02-17

Re: [RFC bpf-next 00/12] bpf: tracing_multi link

flat view

From: Jiri Olsa <hidden>
Date: 2026-02-08 20:54:43
Also in: bpf

On Fri, Feb 06, 2026 at 09:03:29AM -0800, Andrii Nakryiko wrote:
On Fri, Feb 6, 2026 at 12:18 AM Jiri Olsa [off-list ref] wrote:
quoted
On Thu, Feb 05, 2026 at 07:55:19AM -0800, Alexei Starovoitov wrote:
quoted
On Thu, Feb 5, 2026 at 12:55 AM Jiri Olsa [off-list ref] wrote:
quoted
On Wed, Feb 04, 2026 at 08:06:50AM -0800, Alexei Starovoitov wrote:
quoted
On Wed, Feb 4, 2026 at 4:36 AM Jiri Olsa [off-list ref] wrote:
quoted
On Tue, Feb 03, 2026 at 03:17:05PM -0800, Alexei Starovoitov wrote:
quoted
On Tue, Feb 3, 2026 at 1:38 AM Jiri Olsa [off-list ref] wrote:
quoted
hi,
as an option to Meglong's change [1] I'm sending proposal for tracing_multi
link that does not add static trampoline but attaches program to all needed
trampolines.

This approach keeps the same performance but has some drawbacks:

 - when attaching 20k functions we allocate and attach 20k trampolines
 - during attachment we hold each trampoline mutex, so for above
   20k functions we will hold 20k mutexes during the attachment,
   should be very prone to deadlock, but haven't hit it yet
If you check that it's sorted and always take them in the same order
then there will be no deadlock.
Or just grab one global mutex first and then grab trampolines mutexes
next in any order. The global one will serialize this attach operation.
quoted
It looks the trampoline allocations/generation might not be big a problem
and I'll try to find a solution for holding that many mutexes. If there's
no better solution I think having one read/write mutex for tracing multi
link attach/detach should work.
If you mean to have one global mutex as I proposed above then I don't see
a downside. It only serializes multiple libbpf calls.
we also need to serialize it with standard single trampoline attach,
because the direct ftrace update is now done under trampoline->mutex:

  bpf_trampoline_link_prog(tr)
  {
    mutex_lock(&tr->mutex);
    ...
    update_ftrace_direct_*
    ...
    mutex_unlock(&tr->mutex);
  }

for tracing_multi we would link the program first (with tr->mutex)
and do the bulk ftrace update later (without tr->mutex)

  {
    for each involved trampoline:
      bpf_trampoline_link_prog

    --> and here we could race with some other thread doing single
        trampoline attach

    update_ftrace_direct_*
  }

note the current version locks all tr->mutex instances all the way
through the update_ftrace_direct_* update

I think we could use global rwsem and take read lock on single
trampoline attach path and write lock on tracing_multi attach,

I thought we could take direct_mutex early, but that would mean
different order with trampoline mutex than we already have in
single attach path
I feel we're talking past each other.
I meant:

For multi:
1. take some global mutex
2. take N tramp mutexes in any order

For single:
1. take that 1 specific tramp mutex.
ah ok, I understand, it's to prevent the lockup but keep holding all
the trampolines locks.. the rwsem I mentioned was for the 'fix', where
we do not take all the trampolines locks
I don't understand how rwsem would help.
All the operations on trampoline are protected by mutex.
Switching to rw makes sense only if we can designate certain
operations as "read" and others as "write" and number of "reads"
dominate. This won't be the case with multi-fentry.
And we still need to take all of them as "write" to update trampoline.
this applies to scenario where we do not hold all the trampoline locks,
in such case we could have race between single and multi attachment,
while single/single attachment race stays safe

as a fix the single attach would take read lock and multi attach would
take write lock, so single/single race is allowed and single/multi is
not ... showed in the patch below

but it might be too much.. in a sense that there's already many locks
involved in trampoline attach/detach, and simple global lock in multi
or just sorting the ids would be enough
I'll just throw this idea here, but we don't have to do it right away.
What if instead of having a per-trampoline lock, we just have a common
relatively small pool of locks that all trampolines share based on
some hash (i.e., we deterministically map trampoline to one of the
locks). Then multi-attach can just go and grab all of them in
predefined order, while singular trampoline attaches will just get
their own one. We won't need to sort anything, we reduce the amount of
different locks. I don't think lock contention (due to lock sharing
for some trampolines) is a real issue to be worried about either.
nice idea, I'll check on that

thanks,
jirka
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help