* [PATCH bpf-next] bpf: Add necessary migrate_{disable,enable} in bpf arena
@ 2024-11-15 3:52 Yonghong Song
2024-11-15 5:08 ` Alexei Starovoitov
0 siblings, 1 reply; 3+ messages in thread
From: Yonghong Song @ 2024-11-15 3:52 UTC (permalink / raw)
To: bpf
Cc: Alexei Starovoitov, Andrii Nakryiko, Daniel Borkmann, kernel-team,
Martin KaFai Lau
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();
if (ret)
return VM_FAULT_SIGSEGV;
--
2.43.5
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH bpf-next] bpf: Add necessary migrate_{disable,enable} in bpf arena
2024-11-15 3:52 [PATCH bpf-next] bpf: Add necessary migrate_{disable,enable} in bpf arena Yonghong Song
@ 2024-11-15 5:08 ` Alexei Starovoitov
2024-11-15 5:50 ` Yonghong Song
0 siblings, 1 reply; 3+ messages in thread
From: Alexei Starovoitov @ 2024-11-15 5:08 UTC (permalink / raw)
To: Yonghong Song
Cc: bpf, Alexei Starovoitov, Andrii Nakryiko, Daniel Borkmann,
Kernel Team, Martin KaFai Lau
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
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH bpf-next] bpf: Add necessary migrate_{disable,enable} in bpf arena
2024-11-15 5:08 ` Alexei Starovoitov
@ 2024-11-15 5:50 ` Yonghong Song
0 siblings, 0 replies; 3+ messages in thread
From: Yonghong Song @ 2024-11-15 5:50 UTC (permalink / raw)
To: Alexei Starovoitov
Cc: bpf, Alexei Starovoitov, Andrii Nakryiko, Daniel Borkmann,
Kernel Team, Martin KaFai Lau
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
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2024-11-15 5:50 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-11-15 3:52 [PATCH bpf-next] bpf: Add necessary migrate_{disable,enable} in bpf arena Yonghong Song
2024-11-15 5:08 ` Alexei Starovoitov
2024-11-15 5:50 ` Yonghong Song
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.