Re: [PATCHv8 bpf-next 3/9] uprobe: Add uretprobe syscall to speed up return probe

Nathan Chancellor
2024-06-14T17:48:26Z

Hi Jiri,

On Tue, Jun 11, 2024 at 01:21:52PM +0200, Jiri Olsa wrote:
> Adding uretprobe syscall instead of trap to speed up return probe.
...
> diff --git a/kernel/events/uprobes.c b/kernel/events/uprobes.c
> index 2c83ba776fc7..2816e65729ac 100644
> --- a/kernel/events/uprobes.c
> +++ b/kernel/events/uprobes.c
> @@ -1474,11 +1474,20 @@ static int xol_add_vma(struct mm_struct *mm, struct xol_area *area)
> return ret;
> }
>
> +void * __weak arch_uprobe_trampoline(unsigned long *psize)
> +{
> + static uprobe_opcode_t insn = UPROBE_SWBP_INSN;

This change as commit ff474a78cef5 ("uprobe: Add uretprobe syscall to
speed up return probe") in -next causes the following build error for
ARCH=loongarch allmodconfig:

In file included from include/linux/uprobes.h:49,
from include/linux/mm_types.h:16,
from include/linux/mmzone.h:22,
from include/linux/gfp.h:7,
from include/linux/xarray.h:16,
from include/linux/list_lru.h:14,
from include/linux/fs.h:13,
from include/linux/highmem.h:5,
from kernel/events/uprobes.c:13:
kernel/events/uprobes.c: In function 'arch_uprobe_trampoline':
arch/loongarch/include/asm/uprobes.h:12:33: error: initializer element is not constant
12 | #define UPROBE_SWBP_INSN larch_insn_gen_break(BRK_UPROBE_BP)
| ^~~~~~~~~~~~~~~~~~~~
kernel/events/uprobes.c:1479:39: note: in expansion of macro 'UPROBE_SWBP_INSN'
1479 | static uprobe_opcode_t insn = UPROBE_SWBP_INSN;
| ^~~~~~~~~~~~~~~~

> + *psize = UPROBE_SWBP_INSN_SIZE;
> + return &insn;
> +}
> +
> static struct xol_area *__create_xol_area(unsigned long vaddr)
> {
> struct mm_struct *mm = current->mm;
> - uprobe_opcode_t insn = UPROBE_SWBP_INSN;
> + unsigned long insns_size;
> struct xol_area *area;
> + void *insns;
>
> area = kmalloc(sizeof(*area), GFP_KERNEL);
> if (unlikely(!area))
> @@ -1502,7 +1511,8 @@ static struct xol_area *__create_xol_area(unsigned long vaddr)
> /* Reserve the 1st slot for get_trampoline_vaddr() */
> set_bit(0, area->bitmap);
> atomic_set(&area->slot_count, 1);
> - arch_uprobe_copy_ixol(area->pages[0], 0, &insn, UPROBE_SWBP_INSN_SIZE);
> + insns = arch_uprobe_trampoline(&insns_size);
> + arch_uprobe_copy_ixol(area->pages[0], 0, insns, insns_size);
>
> if (!xol_add_vma(mm, area))
> return area;
> @@ -1827,7 +1837,7 @@ void uprobe_copy_process(struct task_struct *t, unsigned long flags)
> *
> * Returns -1 in case the xol_area is not allocated.
> */
> -static unsigned long get_trampoline_vaddr(void)
> +unsigned long uprobe_get_trampoline_vaddr(void)
> {
> struct xol_area *area;
> unsigned long trampoline_vaddr = -1;
> @@ -1878,7 +1888,7 @@ static void prepare_uretprobe(struct uprobe *uprobe, struct pt_regs *regs)
> if (!ri)
> return;
>
> - trampoline_vaddr = get_trampoline_vaddr();
> + trampoline_vaddr = uprobe_get_trampoline_vaddr();
> orig_ret_vaddr = arch_uretprobe_hijack_return_addr(trampoline_vaddr, regs);
> if (orig_ret_vaddr == -1)
> goto fail;
> @@ -2123,7 +2133,7 @@ static struct return_instance *find_next_ret_chain(struct return_instance *ri)
> return ri;
> }
>
> -static void handle_trampoline(struct pt_regs *regs)
> +void uprobe_handle_trampoline(struct pt_regs *regs)
> {
> struct uprobe_task *utask;
> struct return_instance *ri, *next;
> @@ -2187,8 +2197,8 @@ static void handle_swbp(struct pt_regs *regs)
> int is_swbp;
>
> bp_vaddr = uprobe_get_swbp_addr(regs);
> - if (bp_vaddr == get_trampoline_vaddr())
> - return handle_trampoline(regs);
> + if (bp_vaddr == uprobe_get_trampoline_vaddr())
> + return uprobe_handle_trampoline(regs);
>
> uprobe = find_active_uprobe(bp_vaddr, &is_swbp);
> if (!uprobe) {
> --
> 2.45.1
>

Cheers,
Nathan