On Fri, Feb 06, 2026 at 09:03:29AM -0800, Andrii Nakryiko wrote: > On Fri, Feb 6, 2026 at 12:18 AM Jiri Olsa <[email protected]> wrote: > > > > On Thu, Feb 05, 2026 at 07:55:19AM -0800, Alexei Starovoitov wrote: > > > On Thu, Feb 5, 2026 at 12:55 AM Jiri Olsa <[email protected]> wrote: > > > > > > > > On Wed, Feb 04, 2026 at 08:06:50AM -0800, Alexei Starovoitov wrote: > > > > > On Wed, Feb 4, 2026 at 4:36 AM Jiri Olsa <[email protected]> wrote: > > > > > > > > > > > > On Tue, Feb 03, 2026 at 03:17:05PM -0800, Alexei Starovoitov wrote: > > > > > > > On Tue, Feb 3, 2026 at 1:38 AM Jiri Olsa <[email protected]> wrote: > > > > > > > > > > > > > > > > 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. > > > > > > > > > > > > > > > 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
