diff mbox series

[bpf-next] bpf: Add necessary migrate_{disable,enable} in bpf arena

Message ID 20241115035257.2181074-1-yonghong.song@linux.dev (mailing list archive)
State Changes Requested
Delegated to: BPF
Headers show
Series [bpf-next] bpf: Add necessary migrate_{disable,enable} in bpf arena | expand

Checks

Context Check Description
bpf/vmtest-bpf-next-PR success PR summary
bpf/vmtest-bpf-next-VM_Test-0 success Logs for Lint
bpf/vmtest-bpf-next-VM_Test-1 success Logs for ShellCheck
bpf/vmtest-bpf-next-VM_Test-3 success Logs for Validate matrix.py
bpf/vmtest-bpf-next-VM_Test-5 success Logs for aarch64-gcc / build-release
bpf/vmtest-bpf-next-VM_Test-2 success Logs for Unittests
bpf/vmtest-bpf-next-VM_Test-4 success Logs for aarch64-gcc / build / build for aarch64 with gcc
bpf/vmtest-bpf-next-VM_Test-6 success Logs for aarch64-gcc / test (test_maps, false, 360) / test_maps on aarch64 with gcc
bpf/vmtest-bpf-next-VM_Test-9 success Logs for aarch64-gcc / test (test_verifier, false, 360) / test_verifier on aarch64 with gcc
bpf/vmtest-bpf-next-VM_Test-10 success Logs for aarch64-gcc / veristat
bpf/vmtest-bpf-next-VM_Test-12 success Logs for s390x-gcc / build-release
netdev/series_format success Single patches do not need cover letters
netdev/tree_selection success Clearly marked for bpf-next
netdev/ynl success Generated files up to date; no warnings/errors; no diff in generated;
netdev/fixes_present success Fixes tag not required for -next series
netdev/header_inline success No static functions without inline keyword in header files
netdev/build_32bit success Errors and warnings before: 3 this patch: 3
netdev/build_tools success No tools touched, skip
netdev/cc_maintainers fail 1 blamed authors not CCed: memxor@gmail.com; 9 maintainers not CCed: kpsingh@kernel.org jolsa@kernel.org eddyz87@gmail.com song@kernel.org haoluo@google.com john.fastabend@gmail.com martin.lau@linux.dev memxor@gmail.com sdf@fomichev.me
netdev/build_clang success Errors and warnings before: 3 this patch: 3
netdev/verify_signedoff success Signed-off-by tag matches author and committer
netdev/deprecated_api success None detected
netdev/check_selftest success No net selftest shell script
netdev/verify_fixes success Fixes tag looks correct
netdev/build_allmodconfig_warn success Errors and warnings before: 6 this patch: 6
netdev/checkpatch success total: 0 errors, 0 warnings, 0 checks, 27 lines checked
netdev/build_clang_rust success No Rust files in patch. Skipping build
netdev/kdoc success Errors and warnings before: 0 this patch: 0
netdev/source_inline success Was 0 now: 0
bpf/vmtest-bpf-next-VM_Test-7 success Logs for aarch64-gcc / test (test_progs, false, 360) / test_progs on aarch64 with gcc
bpf/vmtest-bpf-next-VM_Test-8 success Logs for aarch64-gcc / test (test_progs_no_alu32, false, 360) / test_progs_no_alu32 on aarch64 with gcc
bpf/vmtest-bpf-next-VM_Test-11 success Logs for s390x-gcc / build / build for s390x with gcc
bpf/vmtest-bpf-next-VM_Test-16 success Logs for s390x-gcc / veristat
bpf/vmtest-bpf-next-VM_Test-15 success Logs for s390x-gcc / test (test_verifier, false, 360) / test_verifier on s390x with gcc
bpf/vmtest-bpf-next-VM_Test-17 success Logs for set-matrix
bpf/vmtest-bpf-next-VM_Test-18 success Logs for x86_64-gcc / build / build for x86_64 with gcc
bpf/vmtest-bpf-next-VM_Test-19 success Logs for x86_64-gcc / build-release
bpf/vmtest-bpf-next-VM_Test-20 success Logs for x86_64-gcc / test (test_maps, false, 360) / test_maps on x86_64 with gcc
bpf/vmtest-bpf-next-VM_Test-22 success Logs for x86_64-gcc / test (test_progs_no_alu32, false, 360) / test_progs_no_alu32 on x86_64 with gcc
bpf/vmtest-bpf-next-VM_Test-21 success Logs for x86_64-gcc / test (test_progs, false, 360) / test_progs on x86_64 with gcc
bpf/vmtest-bpf-next-VM_Test-23 success Logs for x86_64-gcc / test (test_progs_no_alu32_parallel, true, 30) / test_progs_no_alu32_parallel on x86_64 with gcc
bpf/vmtest-bpf-next-VM_Test-24 success Logs for x86_64-gcc / test (test_progs_parallel, true, 30) / test_progs_parallel on x86_64 with gcc
bpf/vmtest-bpf-next-VM_Test-25 success Logs for x86_64-gcc / test (test_verifier, false, 360) / test_verifier on x86_64 with gcc
bpf/vmtest-bpf-next-VM_Test-26 success Logs for x86_64-gcc / veristat / veristat on x86_64 with gcc
bpf/vmtest-bpf-next-VM_Test-27 success Logs for x86_64-llvm-17 / build / build for x86_64 with llvm-17
bpf/vmtest-bpf-next-VM_Test-30 success Logs for x86_64-llvm-17 / test (test_progs, false, 360) / test_progs on x86_64 with llvm-17
bpf/vmtest-bpf-next-VM_Test-29 success Logs for x86_64-llvm-17 / test (test_maps, false, 360) / test_maps on x86_64 with llvm-17
bpf/vmtest-bpf-next-VM_Test-32 success Logs for x86_64-llvm-17 / test (test_verifier, false, 360) / test_verifier on x86_64 with llvm-17
bpf/vmtest-bpf-next-VM_Test-28 success Logs for x86_64-llvm-17 / build-release / build for x86_64 with llvm-17-O2
bpf/vmtest-bpf-next-VM_Test-31 success Logs for x86_64-llvm-17 / test (test_progs_no_alu32, false, 360) / test_progs_no_alu32 on x86_64 with llvm-17
bpf/vmtest-bpf-next-VM_Test-34 success Logs for x86_64-llvm-18 / build / build for x86_64 with llvm-18
bpf/vmtest-bpf-next-VM_Test-33 success Logs for x86_64-llvm-17 / veristat
bpf/vmtest-bpf-next-VM_Test-36 success Logs for x86_64-llvm-18 / test (test_maps, false, 360) / test_maps on x86_64 with llvm-18
bpf/vmtest-bpf-next-VM_Test-37 success Logs for x86_64-llvm-18 / test (test_progs, false, 360) / test_progs on x86_64 with llvm-18
bpf/vmtest-bpf-next-VM_Test-35 success Logs for x86_64-llvm-18 / build-release / build for x86_64 with llvm-18-O2
bpf/vmtest-bpf-next-VM_Test-39 success Logs for x86_64-llvm-18 / test (test_progs_no_alu32, false, 360) / test_progs_no_alu32 on x86_64 with llvm-18
bpf/vmtest-bpf-next-VM_Test-38 success Logs for x86_64-llvm-18 / test (test_progs_cpuv4, false, 360) / test_progs_cpuv4 on x86_64 with llvm-18
bpf/vmtest-bpf-next-VM_Test-41 success Logs for x86_64-llvm-18 / veristat
bpf/vmtest-bpf-next-VM_Test-40 success Logs for x86_64-llvm-18 / test (test_verifier, false, 360) / test_verifier on x86_64 with llvm-18
bpf/vmtest-bpf-next-VM_Test-13 success Logs for s390x-gcc / test (test_progs, false, 360) / test_progs on s390x with gcc
bpf/vmtest-bpf-next-VM_Test-14 success Logs for s390x-gcc / test (test_progs_no_alu32, false, 360) / test_progs_no_alu32 on s390x with gcc

Commit Message

Yonghong Song Nov. 15, 2024, 3:52 a.m. UTC 
When running bpf selftest (./test_progs -j), the following warnings
showed up:

  $ ./test_progs -t arena_atomics
  ...
  BUG: using smp_processor_id() in preemptible [00000000] code: kworker/u19:0/12501
  caller is bpf_mem_free+0x128/0x330
  ...
  Call Trace:
   <TASK>
   dump_stack_lvl
   check_preemption_disabled
   bpf_mem_free
   range_tree_destroy
   arena_map_free
   bpf_map_free_deferred
   process_scheduled_works
   ...

For selftests arena_htab and arena_list, similar smp_process_id() BUGs are
dumped, and the following are two stack trace:

   <TASK>
   dump_stack_lvl
   check_preemption_disabled
   bpf_mem_alloc
   range_tree_set
   arena_map_alloc
   map_create
   ...

   <TASK>
   dump_stack_lvl
   check_preemption_disabled
   bpf_mem_alloc
   range_tree_clear
   arena_vm_fault
   do_pte_missing
   handle_mm_fault
   do_user_addr_fault
   ...

Adding migrate_{disable,enable}() around related arena_*() calls can fix the issue.

Fixes: b795379757eb ("bpf: Introduce range_tree data structure and use it in bpf arena")
Signed-off-by: Yonghong Song <yonghong.song@linux.dev>
---
 kernel/bpf/arena.c | 6 ++++++
 1 file changed, 6 insertions(+)

Comments

Alexei Starovoitov Nov. 15, 2024, 5:08 a.m. UTC | #1 
On Thu, Nov 14, 2024 at 7:53 PM Yonghong Song <yonghong.song@linux.dev> wrote:
>
> When running bpf selftest (./test_progs -j), the following warnings
> showed up:
>
>   $ ./test_progs -t arena_atomics
>   ...
>   BUG: using smp_processor_id() in preemptible [00000000] code: kworker/u19:0/12501
>   caller is bpf_mem_free+0x128/0x330
>   ...
>   Call Trace:
>    <TASK>
>    dump_stack_lvl
>    check_preemption_disabled
>    bpf_mem_free
>    range_tree_destroy
>    arena_map_free
>    bpf_map_free_deferred
>    process_scheduled_works
>    ...
>
> For selftests arena_htab and arena_list, similar smp_process_id() BUGs are
> dumped, and the following are two stack trace:
>
>    <TASK>
>    dump_stack_lvl
>    check_preemption_disabled
>    bpf_mem_alloc
>    range_tree_set
>    arena_map_alloc
>    map_create
>    ...
>
>    <TASK>
>    dump_stack_lvl
>    check_preemption_disabled
>    bpf_mem_alloc
>    range_tree_clear
>    arena_vm_fault
>    do_pte_missing
>    handle_mm_fault
>    do_user_addr_fault
>    ...
>
> Adding migrate_{disable,enable}() around related arena_*() calls can fix the issue.
>
> Fixes: b795379757eb ("bpf: Introduce range_tree data structure and use it in bpf arena")
> Signed-off-by: Yonghong Song <yonghong.song@linux.dev>
> ---
>  kernel/bpf/arena.c | 6 ++++++
>  1 file changed, 6 insertions(+)
>
> diff --git a/kernel/bpf/arena.c b/kernel/bpf/arena.c
> index 3e1dfe349ced..9a55d18032a4 100644
> --- a/kernel/bpf/arena.c
> +++ b/kernel/bpf/arena.c
> @@ -134,7 +134,9 @@ static struct bpf_map *arena_map_alloc(union bpf_attr *attr)
>         INIT_LIST_HEAD(&arena->vma_list);
>         bpf_map_init_from_attr(&arena->map, attr);
>         range_tree_init(&arena->rt);
> +       migrate_disable();
>         range_tree_set(&arena->rt, 0, attr->max_entries);
> +       migrate_enable();
>         mutex_init(&arena->lock);
>
>         return &arena->map;
> @@ -185,7 +187,9 @@ static void arena_map_free(struct bpf_map *map)
>         apply_to_existing_page_range(&init_mm, bpf_arena_get_kern_vm_start(arena),
>                                      KERN_VM_SZ - GUARD_SZ, existing_page_cb, NULL);
>         free_vm_area(arena->kern_vm);
> +       migrate_disable();
>         range_tree_destroy(&arena->rt);
> +       migrate_enable();
>         bpf_map_area_free(arena);
>  }
>
> @@ -276,7 +280,9 @@ static vm_fault_t arena_vm_fault(struct vm_fault *vmf)
>                 /* User space requested to segfault when page is not allocated by bpf prog */
>                 return VM_FAULT_SIGSEGV;
>
> +       migrate_disable();
>         ret = range_tree_clear(&arena->rt, vmf->pgoff, 1);
> +       migrate_enable();

Thanks for the fix.
I thought I had all debug configs enabled :(

Could you please add migrate_disable/enable into range_tree.c
around bpf_mem_alloc/free calls instead ?
range_tree user shouldn't need to worry about this internal details.

pw-bot: cr
Yonghong Song Nov. 15, 2024, 5:50 a.m. UTC | #2 
On 11/14/24 9:08 PM, Alexei Starovoitov wrote:
> On Thu, Nov 14, 2024 at 7:53 PM Yonghong Song <yonghong.song@linux.dev> wrote:
>> When running bpf selftest (./test_progs -j), the following warnings
>> showed up:
>>
>>    $ ./test_progs -t arena_atomics
>>    ...
>>    BUG: using smp_processor_id() in preemptible [00000000] code: kworker/u19:0/12501
>>    caller is bpf_mem_free+0x128/0x330
>>    ...
>>    Call Trace:
>>     <TASK>
>>     dump_stack_lvl
>>     check_preemption_disabled
>>     bpf_mem_free
>>     range_tree_destroy
>>     arena_map_free
>>     bpf_map_free_deferred
>>     process_scheduled_works
>>     ...
>>
>> For selftests arena_htab and arena_list, similar smp_process_id() BUGs are
>> dumped, and the following are two stack trace:
>>
>>     <TASK>
>>     dump_stack_lvl
>>     check_preemption_disabled
>>     bpf_mem_alloc
>>     range_tree_set
>>     arena_map_alloc
>>     map_create
>>     ...
>>
>>     <TASK>
>>     dump_stack_lvl
>>     check_preemption_disabled
>>     bpf_mem_alloc
>>     range_tree_clear
>>     arena_vm_fault
>>     do_pte_missing
>>     handle_mm_fault
>>     do_user_addr_fault
>>     ...
>>
>> Adding migrate_{disable,enable}() around related arena_*() calls can fix the issue.
>>
>> Fixes: b795379757eb ("bpf: Introduce range_tree data structure and use it in bpf arena")
>> Signed-off-by: Yonghong Song <yonghong.song@linux.dev>
>> ---
>>   kernel/bpf/arena.c | 6 ++++++
>>   1 file changed, 6 insertions(+)
>>
>> diff --git a/kernel/bpf/arena.c b/kernel/bpf/arena.c
>> index 3e1dfe349ced..9a55d18032a4 100644
>> --- a/kernel/bpf/arena.c
>> +++ b/kernel/bpf/arena.c
>> @@ -134,7 +134,9 @@ static struct bpf_map *arena_map_alloc(union bpf_attr *attr)
>>          INIT_LIST_HEAD(&arena->vma_list);
>>          bpf_map_init_from_attr(&arena->map, attr);
>>          range_tree_init(&arena->rt);
>> +       migrate_disable();
>>          range_tree_set(&arena->rt, 0, attr->max_entries);
>> +       migrate_enable();
>>          mutex_init(&arena->lock);
>>
>>          return &arena->map;
>> @@ -185,7 +187,9 @@ static void arena_map_free(struct bpf_map *map)
>>          apply_to_existing_page_range(&init_mm, bpf_arena_get_kern_vm_start(arena),
>>                                       KERN_VM_SZ - GUARD_SZ, existing_page_cb, NULL);
>>          free_vm_area(arena->kern_vm);
>> +       migrate_disable();
>>          range_tree_destroy(&arena->rt);
>> +       migrate_enable();
>>          bpf_map_area_free(arena);
>>   }
>>
>> @@ -276,7 +280,9 @@ static vm_fault_t arena_vm_fault(struct vm_fault *vmf)
>>                  /* User space requested to segfault when page is not allocated by bpf prog */
>>                  return VM_FAULT_SIGSEGV;
>>
>> +       migrate_disable();
>>          ret = range_tree_clear(&arena->rt, vmf->pgoff, 1);
>> +       migrate_enable();
> Thanks for the fix.
> I thought I had all debug configs enabled :(
>
> Could you please add migrate_disable/enable into range_tree.c
> around bpf_mem_alloc/free calls instead ?
> range_tree user shouldn't need to worry about this internal details.

Sure. Will send v2 shortly.

>
> pw-bot: cr
diff mbox series

Patch

diff --git a/kernel/bpf/arena.c b/kernel/bpf/arena.c
index 3e1dfe349ced..9a55d18032a4 100644
--- a/kernel/bpf/arena.c
+++ b/kernel/bpf/arena.c
@@ -134,7 +134,9 @@  static struct bpf_map *arena_map_alloc(union bpf_attr *attr)
 	INIT_LIST_HEAD(&arena->vma_list);
 	bpf_map_init_from_attr(&arena->map, attr);
 	range_tree_init(&arena->rt);
+	migrate_disable();
 	range_tree_set(&arena->rt, 0, attr->max_entries);
+	migrate_enable();
 	mutex_init(&arena->lock);
 
 	return &arena->map;
@@ -185,7 +187,9 @@  static void arena_map_free(struct bpf_map *map)
 	apply_to_existing_page_range(&init_mm, bpf_arena_get_kern_vm_start(arena),
 				     KERN_VM_SZ - GUARD_SZ, existing_page_cb, NULL);
 	free_vm_area(arena->kern_vm);
+	migrate_disable();
 	range_tree_destroy(&arena->rt);
+	migrate_enable();
 	bpf_map_area_free(arena);
 }
 
@@ -276,7 +280,9 @@  static vm_fault_t arena_vm_fault(struct vm_fault *vmf)
 		/* User space requested to segfault when page is not allocated by bpf prog */
 		return VM_FAULT_SIGSEGV;
 
+	migrate_disable();
 	ret = range_tree_clear(&arena->rt, vmf->pgoff, 1);
+	migrate_enable();
 	if (ret)
 		return VM_FAULT_SIGSEGV;