Re: [GIT PULL] BPF changes for 6.18

[Jiri Olsa <olsajiri@gmail.com>]

On Tue, Sep 30, 2025 at 07:09:43PM -0700, Linus Torvalds wrote:
> [ Jiri added to participants ]
> 
> On Sun, 28 Sept 2025 at 08:46, Alexei Starovoitov
> <alexei.starovoitov@gmail.com> wrote:
> >
> > Note, there is a trivial conflict between tip and bpf-next trees:
> > in kernel/events/uprobes.c between commit:
> >   4363264111e12 ("uprobe: Do not emulate/sstep original instruction when ip is changed")
> > from the bpf-next tree and commit:
> >   ba2bfc97b4629 ("uprobes/x86: Add support to optimize uprobes")
> > from the tip tree:
> > https://lore.kernel.org/all/aNVMR5rjA2geHNLn@sirena.org.uk/
> > since Jiri's two separate uprobe/bpf related patch series landed
> > in different trees. One was mostly uprobe. Another was mostly bpf.
> 
> So the conflict isn't complicated and I did it the way linux-next did
> it, but honestly, the placement of that arch_uprobe_optimize() thing
> isn't obvious.
> 
> My first reaction was to put it before the instruction_pointer()
> check, because it seems like whatever rewriting the arch wants to do
> might as well be done regardless.
> 
> It's very confusing how it's sometimes skipped, and sometimes not
> skipped. For example. if the uprobe is skipped because of
> single-stepping disabling it, the arch optimization still *will* be
> done, because the "skip_sstep()" test is done after - but other
> skipping tests are done before.
> 
> Jiri, it would be good to just add a note about when that optimization
> is done and when not done. Because as-is, it's very confusing.
> 
> The answer may well be "it doesn't matter, semantics are the same" (I
> suspect that _is_ the answer), but even so that current ordering is
> just confusing when it sometimes goes through that
> arch_uprobe_optimize() and sometimes skips it.

yes, either way will work fine, but perhaps the other way round to
first optimize and then skip uprobe if needed is less confusing

> 
> Side note: the conflict in the selftests was worse, and the magic to
> build it is not obvious. It errors out randomly with various kernel
> configs with useless error messages, and I eventually just gave up
> entirely with a
> 
>    attempt to use poisoned ‘gettid’
> 
> error.
> 
>              Linus

I ended up with changes below, should I send formal patches?

thanks,
jirka


---
diff --git a/kernel/events/uprobes.c b/kernel/events/uprobes.c
index 5dcf927310fd..c14ec27b976d 100644
--- a/kernel/events/uprobes.c
+++ b/kernel/events/uprobes.c
@@ -2765,6 +2765,9 @@ static void handle_swbp(struct pt_regs *regs)
 
 	handler_chain(uprobe, regs);
 
+	/* Try to optimize after first hit. */
+	arch_uprobe_optimize(&uprobe->arch, bp_vaddr);
+
 	/*
 	 * If user decided to take execution elsewhere, it makes little sense
 	 * to execute the original instruction, so let's skip it.
@@ -2772,9 +2775,6 @@ static void handle_swbp(struct pt_regs *regs)
 	if (instruction_pointer(regs) != bp_vaddr)
 		goto out;
 
-	/* Try to optimize after first hit. */
-	arch_uprobe_optimize(&uprobe->arch, bp_vaddr);
-
 	if (arch_uprobe_skip_sstep(&uprobe->arch, regs))
 		goto out;
 
diff --git a/tools/testing/selftests/bpf/prog_tests/uprobe_syscall.c b/tools/testing/selftests/bpf/prog_tests/uprobe_syscall.c
index 6d75ede16e7c..955a37751b52 100644
--- a/tools/testing/selftests/bpf/prog_tests/uprobe_syscall.c
+++ b/tools/testing/selftests/bpf/prog_tests/uprobe_syscall.c
@@ -661,7 +661,7 @@ static void *worker_trigger(void *arg)
 		rounds++;
 	}
 
-	printf("tid %d trigger rounds: %lu\n", gettid(), rounds);
+	printf("tid %ld trigger rounds: %lu\n", sys_gettid(), rounds);
 	return NULL;
 }
 
@@ -704,7 +704,7 @@ static void *worker_attach(void *arg)
 		rounds++;
 	}
 
-	printf("tid %d attach rounds: %lu hits: %d\n", gettid(), rounds, skel->bss->executed);
+	printf("tid %ld attach rounds: %lu hits: %d\n", sys_gettid(), rounds, skel->bss->executed);
 	uprobe_syscall_executed__destroy(skel);
 	free(ref);
 	return NULL;
diff --git a/tools/testing/selftests/bpf/prog_tests/usdt.c b/tools/testing/selftests/bpf/prog_tests/usdt.c
index 4f7f45e69315..f4be5269fa90 100644
--- a/tools/testing/selftests/bpf/prog_tests/usdt.c
+++ b/tools/testing/selftests/bpf/prog_tests/usdt.c
@@ -142,7 +142,7 @@ static void subtest_basic_usdt(bool optimized)
 		goto cleanup;
 #endif
 
-	alled = TRIGGER(1);
+	called = TRIGGER(1);
 
 	ASSERT_EQ(bss->usdt0_called, called, "usdt0_called");
 	ASSERT_EQ(bss->usdt3_called, called, "usdt3_called");