+ bpf
On Thu, Jul 11, 2024 at 4:07 AM Peter Zijlstra <peterz@xxxxxxxxxxxxx> wrote:
>
> With handle_swbp() hitting concurrently on (all) CPUs, potentially on
> the same uprobe, the uprobe->refcount can get *very* hot. Move the
> struct uprobe lifetime into uprobes_srcu such that it covers both the
> uprobe and the uprobe->consumers list.
>
> With this, handle_swbp() can use a single large SRCU critical section
> to avoid taking a refcount on the uprobe for it's duration.
>
> Notably, the single-step and uretprobe paths need a reference that
> leaves handle_swbp() and will, for now, still use ->refcount.
>
> Signed-off-by: Peter Zijlstra (Intel) <peterz@xxxxxxxxxxxxx>
> ---
> kernel/events/uprobes.c | 68 ++++++++++++++++++++++++++++--------------------
> 1 file changed, 41 insertions(+), 27 deletions(-)
>
> --- a/kernel/events/uprobes.c
> +++ b/kernel/events/uprobes.c
> @@ -51,7 +51,7 @@ static struct mutex uprobes_mmap_mutex[U
> DEFINE_STATIC_PERCPU_RWSEM(dup_mmap_sem);
>
[...]
> @@ -1982,22 +1990,31 @@ pre_ssout(struct uprobe *uprobe, struct
> if (!utask)
> return -ENOMEM;
>
> + utask->active_uprobe = try_get_uprobe(uprobe);
> + if (!utask->active_uprobe)
> + return -ESRCH;
> +
> xol_vaddr = xol_get_insn_slot(uprobe);
> - if (!xol_vaddr)
> - return -ENOMEM;
> + if (!xol_vaddr) {
> + err = -ENOMEM;
> + goto err_uprobe;
> + }
>
> utask->xol_vaddr = xol_vaddr;
> utask->vaddr = bp_vaddr;
>
> err = arch_uprobe_pre_xol(&uprobe->arch, regs);
> - if (unlikely(err)) {
> - xol_free_insn_slot(current);
let's keep this here, because you later remove err_uprobe part and
err_xol is only jumped to from here; it's better to just drop err_xol
and err_uprobe altogether and keep the original xol_free_insn_slot()
here.
> - return err;
> - }
> + if (unlikely(err))
> + goto err_xol;
>
> - utask->active_uprobe = uprobe;
> utask->state = UTASK_SSTEP;
> return 0;
> +
> +err_xol:
> + xol_free_insn_slot(current);
> +err_uprobe:
> + put_uprobe(utask->active_uprobe);
utask->active_uprobe = NULL;
let's not leave garbage in utask (even if you remove this later anyways)
> + return err;
> }
>
> /*
[...]
