| Message ID | 20200930002011.521337-1-songliubraving@fb.com |
|---|---|
| State | New |
| Headers | show |
| Series | [v2,bpf-next] bpf: fix raw_tp test run in preempt kernel | expand |
On Tue, Sep 29, 2020 at 5:20 PM Song Liu <songliubraving@fb.com> wrote: > > In preempt kernel, BPF_PROG_TEST_RUN on raw_tp triggers: > > [ 35.874974] BUG: using smp_processor_id() in preemptible [00000000] > code: new_name/87 > [ 35.893983] caller is bpf_prog_test_run_raw_tp+0xd4/0x1b0 > [ 35.900124] CPU: 1 PID: 87 Comm: new_name Not tainted 5.9.0-rc6-g615bd02bf #1 > [ 35.907358] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), > BIOS 1.10.2-1ubuntu1 04/01/2014 > [ 35.916941] Call Trace: > [ 35.919660] dump_stack+0x77/0x9b > [ 35.923273] check_preemption_disabled+0xb4/0xc0 > [ 35.928376] bpf_prog_test_run_raw_tp+0xd4/0x1b0 > [ 35.933872] ? selinux_bpf+0xd/0x70 > [ 35.937532] __do_sys_bpf+0x6bb/0x21e0 > [ 35.941570] ? find_held_lock+0x2d/0x90 > [ 35.945687] ? vfs_write+0x150/0x220 > [ 35.949586] do_syscall_64+0x2d/0x40 > [ 35.953443] entry_SYSCALL_64_after_hwframe+0x44/0xa9 > > Fix this by calling migrate_disable() before smp_processor_id(). > > Fixes: 1b4d60ec162f ("bpf: Enable BPF_PROG_TEST_RUN for raw_tracepoint") > Reported-by: Alexei Starovoitov <ast@kernel.org> > Signed-off-by: Song Liu <songliubraving@fb.com> > > --- > Changes v1 => v2: > 1. Keep rcu_read_lock/unlock() in original places. (Alexei) > 2. Use get_cpu() instead of smp_processor_id(). (Alexei) Applying: bpf: fix raw_tp test run in preempt kernel Using index info to reconstruct a base tree... error: patch failed: net/bpf/test_run.c:293 error: net/bpf/test_run.c: patch does not apply error: Did you hand edit your patch?
> On Sep 29, 2020, at 8:23 PM, Alexei Starovoitov <alexei.starovoitov@gmail.com> wrote: > > On Tue, Sep 29, 2020 at 5:20 PM Song Liu <songliubraving@fb.com> wrote: >> >> In preempt kernel, BPF_PROG_TEST_RUN on raw_tp triggers: >> >> [ 35.874974] BUG: using smp_processor_id() in preemptible [00000000] >> code: new_name/87 >> [ 35.893983] caller is bpf_prog_test_run_raw_tp+0xd4/0x1b0 >> [ 35.900124] CPU: 1 PID: 87 Comm: new_name Not tainted 5.9.0-rc6-g615bd02bf #1 >> [ 35.907358] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), >> BIOS 1.10.2-1ubuntu1 04/01/2014 >> [ 35.916941] Call Trace: >> [ 35.919660] dump_stack+0x77/0x9b >> [ 35.923273] check_preemption_disabled+0xb4/0xc0 >> [ 35.928376] bpf_prog_test_run_raw_tp+0xd4/0x1b0 >> [ 35.933872] ? selinux_bpf+0xd/0x70 >> [ 35.937532] __do_sys_bpf+0x6bb/0x21e0 >> [ 35.941570] ? find_held_lock+0x2d/0x90 >> [ 35.945687] ? vfs_write+0x150/0x220 >> [ 35.949586] do_syscall_64+0x2d/0x40 >> [ 35.953443] entry_SYSCALL_64_after_hwframe+0x44/0xa9 >> >> Fix this by calling migrate_disable() before smp_processor_id(). >> >> Fixes: 1b4d60ec162f ("bpf: Enable BPF_PROG_TEST_RUN for raw_tracepoint") >> Reported-by: Alexei Starovoitov <ast@kernel.org> >> Signed-off-by: Song Liu <songliubraving@fb.com> >> >> --- >> Changes v1 => v2: >> 1. Keep rcu_read_lock/unlock() in original places. (Alexei) >> 2. Use get_cpu() instead of smp_processor_id(). (Alexei) > > Applying: bpf: fix raw_tp test run in preempt kernel > Using index info to reconstruct a base tree... > error: patch failed: net/bpf/test_run.c:293 > error: net/bpf/test_run.c: patch does not apply > error: Did you hand edit your patch? This is so weird. I cannot apply it myself. :( [localhost] g co -b bpf-next-temp Switched to a new branch 'bpf-next-temp' [localhost] g format-patch -b HEAD~1 --subject-prefix "PATCH v3 bpf-next" 0001-bpf-fix-raw_tp-test-run-in-preempt-kernel.patch [localhost] g reset --hard HEAD~1 HEAD is now at b0efc216f5779 libbpf: Compile in PIC mode only for shared library case [localhost] g am 0001-bpf-fix-raw_tp-test-run-in-preempt-kernel.patch Applying: bpf: fix raw_tp test run in preempt kernel error: patch failed: net/bpf/test_run.c:293 error: net/bpf/test_run.c: patch does not apply Patch failed at 0001 bpf: fix raw_tp test run in preempt kernel hint: Use 'git am --show-current-patch' to see the failed patch When you have resolved this problem, run "git am --continue". If you prefer to skip this patch, run "git am --skip" instead. To restore the original branch and stop patching, run "git am --abort". Any hint on how to fix this? Alternatively, could you please pull the change from https://git.kernel.org/pub/scm/linux/kernel/git/song/linux.git raw_tp_preempt_fix Thanks, Song
On 9/29/20 11:45 PM, Song Liu wrote: > > >> On Sep 29, 2020, at 8:23 PM, Alexei Starovoitov <alexei.starovoitov@gmail.com> wrote: >> >> On Tue, Sep 29, 2020 at 5:20 PM Song Liu <songliubraving@fb.com> wrote: >>> >>> In preempt kernel, BPF_PROG_TEST_RUN on raw_tp triggers: >>> >>> [ 35.874974] BUG: using smp_processor_id() in preemptible [00000000] >>> code: new_name/87 >>> [ 35.893983] caller is bpf_prog_test_run_raw_tp+0xd4/0x1b0 >>> [ 35.900124] CPU: 1 PID: 87 Comm: new_name Not tainted 5.9.0-rc6-g615bd02bf #1 >>> [ 35.907358] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), >>> BIOS 1.10.2-1ubuntu1 04/01/2014 >>> [ 35.916941] Call Trace: >>> [ 35.919660] dump_stack+0x77/0x9b >>> [ 35.923273] check_preemption_disabled+0xb4/0xc0 >>> [ 35.928376] bpf_prog_test_run_raw_tp+0xd4/0x1b0 >>> [ 35.933872] ? selinux_bpf+0xd/0x70 >>> [ 35.937532] __do_sys_bpf+0x6bb/0x21e0 >>> [ 35.941570] ? find_held_lock+0x2d/0x90 >>> [ 35.945687] ? vfs_write+0x150/0x220 >>> [ 35.949586] do_syscall_64+0x2d/0x40 >>> [ 35.953443] entry_SYSCALL_64_after_hwframe+0x44/0xa9 >>> >>> Fix this by calling migrate_disable() before smp_processor_id(). >>> >>> Fixes: 1b4d60ec162f ("bpf: Enable BPF_PROG_TEST_RUN for raw_tracepoint") >>> Reported-by: Alexei Starovoitov <ast@kernel.org> >>> Signed-off-by: Song Liu <songliubraving@fb.com> >>> >>> --- >>> Changes v1 => v2: >>> 1. Keep rcu_read_lock/unlock() in original places. (Alexei) >>> 2. Use get_cpu() instead of smp_processor_id(). (Alexei) >> >> Applying: bpf: fix raw_tp test run in preempt kernel >> Using index info to reconstruct a base tree... >> error: patch failed: net/bpf/test_run.c:293 >> error: net/bpf/test_run.c: patch does not apply >> error: Did you hand edit your patch? > > This is so weird. I cannot apply it myself. :( > > [localhost] g co -b bpf-next-temp > Switched to a new branch 'bpf-next-temp' > > [localhost] g format-patch -b HEAD~1 --subject-prefix "PATCH v3 bpf-next" > 0001-bpf-fix-raw_tp-test-run-in-preempt-kernel.patch could you try without -b ? > [localhost] g reset --hard HEAD~1 > HEAD is now at b0efc216f5779 libbpf: Compile in PIC mode only for shared library case > > [localhost] g am 0001-bpf-fix-raw_tp-test-run-in-preempt-kernel.patch > Applying: bpf: fix raw_tp test run in preempt kernel > error: patch failed: net/bpf/test_run.c:293 > error: net/bpf/test_run.c: patch does not apply > Patch failed at 0001 bpf: fix raw_tp test run in preempt kernel > hint: Use 'git am --show-current-patch' to see the failed patch > When you have resolved this problem, run "git am --continue". > If you prefer to skip this patch, run "git am --skip" instead. > To restore the original branch and stop patching, run "git am --abort". very odd indeed. > Any hint on how to fix this? Alternatively, could you please pull the > change from > > https://git.kernel.org/pub/scm/linux/kernel/git/song/linux.git raw_tp_preempt_fix pulled. thanks
> On Sep 30, 2020, at 8:35 AM, Alexei Starovoitov <ast@fb.com> wrote: > > On 9/29/20 11:45 PM, Song Liu wrote: >>> On Sep 29, 2020, at 8:23 PM, Alexei Starovoitov <alexei.starovoitov@gmail.com> wrote: >>> >>> On Tue, Sep 29, 2020 at 5:20 PM Song Liu <songliubraving@fb.com> wrote: >>>> >>>> In preempt kernel, BPF_PROG_TEST_RUN on raw_tp triggers: >>>> >>>> [ 35.874974] BUG: using smp_processor_id() in preemptible [00000000] >>>> code: new_name/87 >>>> [ 35.893983] caller is bpf_prog_test_run_raw_tp+0xd4/0x1b0 >>>> [ 35.900124] CPU: 1 PID: 87 Comm: new_name Not tainted 5.9.0-rc6-g615bd02bf #1 >>>> [ 35.907358] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), >>>> BIOS 1.10.2-1ubuntu1 04/01/2014 >>>> [ 35.916941] Call Trace: >>>> [ 35.919660] dump_stack+0x77/0x9b >>>> [ 35.923273] check_preemption_disabled+0xb4/0xc0 >>>> [ 35.928376] bpf_prog_test_run_raw_tp+0xd4/0x1b0 >>>> [ 35.933872] ? selinux_bpf+0xd/0x70 >>>> [ 35.937532] __do_sys_bpf+0x6bb/0x21e0 >>>> [ 35.941570] ? find_held_lock+0x2d/0x90 >>>> [ 35.945687] ? vfs_write+0x150/0x220 >>>> [ 35.949586] do_syscall_64+0x2d/0x40 >>>> [ 35.953443] entry_SYSCALL_64_after_hwframe+0x44/0xa9 >>>> >>>> Fix this by calling migrate_disable() before smp_processor_id(). >>>> >>>> Fixes: 1b4d60ec162f ("bpf: Enable BPF_PROG_TEST_RUN for raw_tracepoint") >>>> Reported-by: Alexei Starovoitov <ast@kernel.org> >>>> Signed-off-by: Song Liu <songliubraving@fb.com> >>>> >>>> --- >>>> Changes v1 => v2: >>>> 1. Keep rcu_read_lock/unlock() in original places. (Alexei) >>>> 2. Use get_cpu() instead of smp_processor_id(). (Alexei) >>> >>> Applying: bpf: fix raw_tp test run in preempt kernel >>> Using index info to reconstruct a base tree... >>> error: patch failed: net/bpf/test_run.c:293 >>> error: net/bpf/test_run.c: patch does not apply >>> error: Did you hand edit your patch? >> This is so weird. I cannot apply it myself. :( >> [localhost] g co -b bpf-next-temp >> Switched to a new branch 'bpf-next-temp' >> [localhost] g format-patch -b HEAD~1 --subject-prefix "PATCH v3 bpf-next" >> 0001-bpf-fix-raw_tp-test-run-in-preempt-kernel.patch > > could you try without -b ? Yes! -b is the problem here. I think the right way to format-patch is to use --ignore-space-at-eol instead of -b. Thanks, Song
diff --git a/net/bpf/test_run.c b/net/bpf/test_run.c index fde5db93507c4..c1c30a9f76f34 100644 --- a/net/bpf/test_run.c +++ b/net/bpf/test_run.c @@ -252,9 +252,7 @@ __bpf_prog_test_run_raw_tp(void *data) struct bpf_raw_tp_test_run_info *info = data; rcu_read_lock(); - migrate_disable(); info->retval = BPF_PROG_RUN(info->prog, info->ctx); - migrate_enable(); rcu_read_unlock(); } @@ -266,6 +264,7 @@ int bpf_prog_test_run_raw_tp(struct bpf_prog *prog, __u32 ctx_size_in = kattr->test.ctx_size_in; struct bpf_raw_tp_test_run_info info; int cpu = kattr->test.cpu, err = 0; + int current_cpu; /* doesn't support data_in/out, ctx_out, duration, or repeat */ if (kattr->test.data_in || kattr->test.data_out || @@ -293,27 +292,25 @@ int bpf_prog_test_run_raw_tp(struct bpf_prog *prog, info.prog = prog; + current_cpu = get_cpu(); if ((kattr->test.flags & BPF_F_TEST_RUN_ON_CPU) == 0 || - cpu == smp_processor_id()) { + cpu == current_cpu) { __bpf_prog_test_run_raw_tp(&info); - } else { + } else if (cpu >= nr_cpu_ids || !cpu_online(cpu)) { /* smp_call_function_single() also checks cpu_online() * after csd_lock(). However, since cpu is from user * space, let's do an extra quick check to filter out * invalid value before smp_call_function_single(). */ - if (cpu >= nr_cpu_ids || !cpu_online(cpu)) { err = -ENXIO; - goto out; - } - + } else { err = smp_call_function_single(cpu, __bpf_prog_test_run_raw_tp, &info, 1); - if (err) - goto out; } + put_cpu(); - if (copy_to_user(&uattr->test.retval, &info.retval, sizeof(u32))) + if (!err && + copy_to_user(&uattr->test.retval, &info.retval, sizeof(u32))) err = -EFAULT; out:
In preempt kernel, BPF_PROG_TEST_RUN on raw_tp triggers: [ 35.874974] BUG: using smp_processor_id() in preemptible [00000000] code: new_name/87 [ 35.893983] caller is bpf_prog_test_run_raw_tp+0xd4/0x1b0 [ 35.900124] CPU: 1 PID: 87 Comm: new_name Not tainted 5.9.0-rc6-g615bd02bf #1 [ 35.907358] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.10.2-1ubuntu1 04/01/2014 [ 35.916941] Call Trace: [ 35.919660] dump_stack+0x77/0x9b [ 35.923273] check_preemption_disabled+0xb4/0xc0 [ 35.928376] bpf_prog_test_run_raw_tp+0xd4/0x1b0 [ 35.933872] ? selinux_bpf+0xd/0x70 [ 35.937532] __do_sys_bpf+0x6bb/0x21e0 [ 35.941570] ? find_held_lock+0x2d/0x90 [ 35.945687] ? vfs_write+0x150/0x220 [ 35.949586] do_syscall_64+0x2d/0x40 [ 35.953443] entry_SYSCALL_64_after_hwframe+0x44/0xa9 Fix this by calling migrate_disable() before smp_processor_id(). Fixes: 1b4d60ec162f ("bpf: Enable BPF_PROG_TEST_RUN for raw_tracepoint") Reported-by: Alexei Starovoitov <ast@kernel.org> Signed-off-by: Song Liu <songliubraving@fb.com> --- Changes v1 => v2: 1. Keep rcu_read_lock/unlock() in original places. (Alexei) 2. Use get_cpu() instead of smp_processor_id(). (Alexei) --- net/bpf/test_run.c | 19 ++++++++----------- 1 file changed, 8 insertions(+), 11 deletions(-)