| Message ID | 20250320032258.116156-1-yangfeng59949@163.com (mailing list archive) |
|---|---|
| State | Handled Elsewhere |
| Headers | show |
| Series | bpf: Remove duplicate judgments | expand |
On Wed, Mar 19, 2025 at 8:23 PM Feng Yang <yangfeng59949@163.com> wrote: > > From: Feng Yang <yangfeng@kylinos.cn> > > Most of the judgments also exist in bpf_base_func_deto, remove them. "Most" of them also exist is not enough. Please make sure that this does not introduce any behavior change. For example, we should not remove return of bpf_perf_event_read_value_proto. For future patches, please read Documentation/bpf/bpf_devel_QA.rst and follow rules for email subject, etc. For example, this patch should have a subject like "[PATCH bpf-next] xxx". Thanks, Song > > Signed-off-by: Feng Yang <yangfeng@kylinos.cn> > --- > kernel/trace/bpf_trace.c | 72 ---------------------------------------- > 1 file changed, 72 deletions(-) > > diff --git a/kernel/trace/bpf_trace.c b/kernel/trace/bpf_trace.c > index 6b07fa7081d9..c89b25344422 100644 > --- a/kernel/trace/bpf_trace.c > +++ b/kernel/trace/bpf_trace.c > @@ -1443,56 +1443,14 @@ bpf_tracing_func_proto(enum bpf_func_id func_id, const struct bpf_prog *prog) > const struct bpf_func_proto *func_proto; > > switch (func_id) { > - case BPF_FUNC_map_lookup_elem: > - return &bpf_map_lookup_elem_proto; > - case BPF_FUNC_map_update_elem: > - return &bpf_map_update_elem_proto; > - case BPF_FUNC_map_delete_elem: > - return &bpf_map_delete_elem_proto; > - case BPF_FUNC_map_push_elem: > - return &bpf_map_push_elem_proto; > - case BPF_FUNC_map_pop_elem: > - return &bpf_map_pop_elem_proto; > - case BPF_FUNC_map_peek_elem: > - return &bpf_map_peek_elem_proto; > - case BPF_FUNC_map_lookup_percpu_elem: > - return &bpf_map_lookup_percpu_elem_proto; > - case BPF_FUNC_ktime_get_ns: > - return &bpf_ktime_get_ns_proto; > - case BPF_FUNC_ktime_get_boot_ns: > - return &bpf_ktime_get_boot_ns_proto; > - case BPF_FUNC_tail_call: > - return &bpf_tail_call_proto; > - case BPF_FUNC_get_current_task: > - return &bpf_get_current_task_proto; > - case BPF_FUNC_get_current_task_btf: > - return &bpf_get_current_task_btf_proto; > - case BPF_FUNC_task_pt_regs: > - return &bpf_task_pt_regs_proto; > case BPF_FUNC_get_current_uid_gid: > return &bpf_get_current_uid_gid_proto; > case BPF_FUNC_get_current_comm: > return &bpf_get_current_comm_proto; > - case BPF_FUNC_trace_printk: > - return bpf_get_trace_printk_proto(); > case BPF_FUNC_get_smp_processor_id: > return &bpf_get_smp_processor_id_proto; > - case BPF_FUNC_get_numa_node_id: > - return &bpf_get_numa_node_id_proto; > case BPF_FUNC_perf_event_read: > return &bpf_perf_event_read_proto; > - case BPF_FUNC_get_prandom_u32: > - return &bpf_get_prandom_u32_proto; > - case BPF_FUNC_probe_read_user: > - return &bpf_probe_read_user_proto; > - case BPF_FUNC_probe_read_kernel: > - return security_locked_down(LOCKDOWN_BPF_READ_KERNEL) < 0 ? > - NULL : &bpf_probe_read_kernel_proto; > - case BPF_FUNC_probe_read_user_str: > - return &bpf_probe_read_user_str_proto; > - case BPF_FUNC_probe_read_kernel_str: > - return security_locked_down(LOCKDOWN_BPF_READ_KERNEL) < 0 ? > - NULL : &bpf_probe_read_kernel_str_proto; > #ifdef CONFIG_ARCH_HAS_NON_OVERLAPPING_ADDRESS_SPACE > case BPF_FUNC_probe_read: > return security_locked_down(LOCKDOWN_BPF_READ_KERNEL) < 0 ? > @@ -1502,10 +1460,6 @@ bpf_tracing_func_proto(enum bpf_func_id func_id, const struct bpf_prog *prog) > NULL : &bpf_probe_read_compat_str_proto; > #endif > #ifdef CONFIG_CGROUPS > - case BPF_FUNC_cgrp_storage_get: > - return &bpf_cgrp_storage_get_proto; > - case BPF_FUNC_cgrp_storage_delete: > - return &bpf_cgrp_storage_delete_proto; > case BPF_FUNC_current_task_under_cgroup: > return &bpf_current_task_under_cgroup_proto; > #endif > @@ -1513,20 +1467,6 @@ bpf_tracing_func_proto(enum bpf_func_id func_id, const struct bpf_prog *prog) > return &bpf_send_signal_proto; > case BPF_FUNC_send_signal_thread: > return &bpf_send_signal_thread_proto; > - case BPF_FUNC_perf_event_read_value: > - return &bpf_perf_event_read_value_proto; > - case BPF_FUNC_ringbuf_output: > - return &bpf_ringbuf_output_proto; > - case BPF_FUNC_ringbuf_reserve: > - return &bpf_ringbuf_reserve_proto; > - case BPF_FUNC_ringbuf_submit: > - return &bpf_ringbuf_submit_proto; > - case BPF_FUNC_ringbuf_discard: > - return &bpf_ringbuf_discard_proto; > - case BPF_FUNC_ringbuf_query: > - return &bpf_ringbuf_query_proto; > - case BPF_FUNC_jiffies64: > - return &bpf_jiffies64_proto; > case BPF_FUNC_get_task_stack: > return prog->sleepable ? &bpf_get_task_stack_sleepable_proto > : &bpf_get_task_stack_proto; > @@ -1534,12 +1474,6 @@ bpf_tracing_func_proto(enum bpf_func_id func_id, const struct bpf_prog *prog) > return &bpf_copy_from_user_proto; > case BPF_FUNC_copy_from_user_task: > return &bpf_copy_from_user_task_proto; > - case BPF_FUNC_snprintf_btf: > - return &bpf_snprintf_btf_proto; > - case BPF_FUNC_per_cpu_ptr: > - return &bpf_per_cpu_ptr_proto; > - case BPF_FUNC_this_cpu_ptr: > - return &bpf_this_cpu_ptr_proto; > case BPF_FUNC_task_storage_get: > if (bpf_prog_check_recur(prog)) > return &bpf_task_storage_get_recur_proto; > @@ -1548,18 +1482,12 @@ bpf_tracing_func_proto(enum bpf_func_id func_id, const struct bpf_prog *prog) > if (bpf_prog_check_recur(prog)) > return &bpf_task_storage_delete_recur_proto; > return &bpf_task_storage_delete_proto; > - case BPF_FUNC_for_each_map_elem: > - return &bpf_for_each_map_elem_proto; > - case BPF_FUNC_snprintf: > - return &bpf_snprintf_proto; > case BPF_FUNC_get_func_ip: > return &bpf_get_func_ip_proto_tracing; > case BPF_FUNC_get_branch_snapshot: > return &bpf_get_branch_snapshot_proto; > case BPF_FUNC_find_vma: > return &bpf_find_vma_proto; > - case BPF_FUNC_trace_vprintk: > - return bpf_get_trace_vprintk_proto(); > default: > break; > } > -- > 2.43.0 >
On Thu, 20 Mar 2025 09:45:17 -0700 Song Liu <song@kernel.org> wrote: > > On Wed, Mar 19, 2025 at 8:23 PM Feng Yang <yangfeng59949@163.com> wrote: > > > > From: Feng Yang <yangfeng@kylinos.cn> > > > > Most of the judgments also exist in bpf_base_func_deto, remove them. > > "Most" of them also exist is not enough. Please make sure that this does > not introduce any behavior change. For example, we should not remove > return of bpf_perf_event_read_value_proto. in trace/bpf_trace.c: const struct bpf_func_proto *bpf_get_perf_event_read_value_proto(void) { return &bpf_perf_event_read_value_proto; } in bpf/core.c: const struct bpf_func_proto * __weak bpf_get_perf_event_read_value_proto(void) { return NULL; } And weak symbols will be covered nm vmlinux | grep bpf_get_perf_event_read_value_proto ffffffff814b90e0 T bpf_get_perf_event_read_value_proto ffffffff814b90d0 T __pfx_bpf_get_perf_event_read_value_proto So the return of bpf_perf_event_read_value_proto can be done through the bpf_base_func_proto function. bpf_base_func_proto ...... case BPF_FUNC_perf_event_read_value: return bpf_get_perf_event_read_value_proto(); I think this can be removed. > For future patches, please read Documentation/bpf/bpf_devel_QA.rst > and follow rules for email subject, etc. For example, this patch should > have a subject like "[PATCH bpf-next] xxx". Thank you very much for your suggestion. I will pay attention to it next time. > Thanks, > Song
On Thu, Mar 20, 2025 at 9:36 PM Feng Yang <yangfeng59949@163.com> wrote: > [...] > > So the return of bpf_perf_event_read_value_proto can be done through the bpf_base_func_proto function. > bpf_base_func_proto > ...... > case BPF_FUNC_perf_event_read_value: > return bpf_get_perf_event_read_value_proto(); > > I think this can be removed. I see. I misunderstood this part because I was reading code before ae0a457f5d33c336f3c4259a258f8b537531a04b. Now it looks good. Acked-by: Song Liu <song@kernel.org> > > For future patches, please read Documentation/bpf/bpf_devel_QA.rst > > and follow rules for email subject, etc. For example, this patch should > > have a subject like "[PATCH bpf-next] xxx". > > Thank you very much for your suggestion. I will pay attention to it next time. >
diff --git a/kernel/trace/bpf_trace.c b/kernel/trace/bpf_trace.c index 6b07fa7081d9..c89b25344422 100644 --- a/kernel/trace/bpf_trace.c +++ b/kernel/trace/bpf_trace.c @@ -1443,56 +1443,14 @@ bpf_tracing_func_proto(enum bpf_func_id func_id, const struct bpf_prog *prog) const struct bpf_func_proto *func_proto; switch (func_id) { - case BPF_FUNC_map_lookup_elem: - return &bpf_map_lookup_elem_proto; - case BPF_FUNC_map_update_elem: - return &bpf_map_update_elem_proto; - case BPF_FUNC_map_delete_elem: - return &bpf_map_delete_elem_proto; - case BPF_FUNC_map_push_elem: - return &bpf_map_push_elem_proto; - case BPF_FUNC_map_pop_elem: - return &bpf_map_pop_elem_proto; - case BPF_FUNC_map_peek_elem: - return &bpf_map_peek_elem_proto; - case BPF_FUNC_map_lookup_percpu_elem: - return &bpf_map_lookup_percpu_elem_proto; - case BPF_FUNC_ktime_get_ns: - return &bpf_ktime_get_ns_proto; - case BPF_FUNC_ktime_get_boot_ns: - return &bpf_ktime_get_boot_ns_proto; - case BPF_FUNC_tail_call: - return &bpf_tail_call_proto; - case BPF_FUNC_get_current_task: - return &bpf_get_current_task_proto; - case BPF_FUNC_get_current_task_btf: - return &bpf_get_current_task_btf_proto; - case BPF_FUNC_task_pt_regs: - return &bpf_task_pt_regs_proto; case BPF_FUNC_get_current_uid_gid: return &bpf_get_current_uid_gid_proto; case BPF_FUNC_get_current_comm: return &bpf_get_current_comm_proto; - case BPF_FUNC_trace_printk: - return bpf_get_trace_printk_proto(); case BPF_FUNC_get_smp_processor_id: return &bpf_get_smp_processor_id_proto; - case BPF_FUNC_get_numa_node_id: - return &bpf_get_numa_node_id_proto; case BPF_FUNC_perf_event_read: return &bpf_perf_event_read_proto; - case BPF_FUNC_get_prandom_u32: - return &bpf_get_prandom_u32_proto; - case BPF_FUNC_probe_read_user: - return &bpf_probe_read_user_proto; - case BPF_FUNC_probe_read_kernel: - return security_locked_down(LOCKDOWN_BPF_READ_KERNEL) < 0 ? - NULL : &bpf_probe_read_kernel_proto; - case BPF_FUNC_probe_read_user_str: - return &bpf_probe_read_user_str_proto; - case BPF_FUNC_probe_read_kernel_str: - return security_locked_down(LOCKDOWN_BPF_READ_KERNEL) < 0 ? - NULL : &bpf_probe_read_kernel_str_proto; #ifdef CONFIG_ARCH_HAS_NON_OVERLAPPING_ADDRESS_SPACE case BPF_FUNC_probe_read: return security_locked_down(LOCKDOWN_BPF_READ_KERNEL) < 0 ? @@ -1502,10 +1460,6 @@ bpf_tracing_func_proto(enum bpf_func_id func_id, const struct bpf_prog *prog) NULL : &bpf_probe_read_compat_str_proto; #endif #ifdef CONFIG_CGROUPS - case BPF_FUNC_cgrp_storage_get: - return &bpf_cgrp_storage_get_proto; - case BPF_FUNC_cgrp_storage_delete: - return &bpf_cgrp_storage_delete_proto; case BPF_FUNC_current_task_under_cgroup: return &bpf_current_task_under_cgroup_proto; #endif @@ -1513,20 +1467,6 @@ bpf_tracing_func_proto(enum bpf_func_id func_id, const struct bpf_prog *prog) return &bpf_send_signal_proto; case BPF_FUNC_send_signal_thread: return &bpf_send_signal_thread_proto; - case BPF_FUNC_perf_event_read_value: - return &bpf_perf_event_read_value_proto; - case BPF_FUNC_ringbuf_output: - return &bpf_ringbuf_output_proto; - case BPF_FUNC_ringbuf_reserve: - return &bpf_ringbuf_reserve_proto; - case BPF_FUNC_ringbuf_submit: - return &bpf_ringbuf_submit_proto; - case BPF_FUNC_ringbuf_discard: - return &bpf_ringbuf_discard_proto; - case BPF_FUNC_ringbuf_query: - return &bpf_ringbuf_query_proto; - case BPF_FUNC_jiffies64: - return &bpf_jiffies64_proto; case BPF_FUNC_get_task_stack: return prog->sleepable ? &bpf_get_task_stack_sleepable_proto : &bpf_get_task_stack_proto; @@ -1534,12 +1474,6 @@ bpf_tracing_func_proto(enum bpf_func_id func_id, const struct bpf_prog *prog) return &bpf_copy_from_user_proto; case BPF_FUNC_copy_from_user_task: return &bpf_copy_from_user_task_proto; - case BPF_FUNC_snprintf_btf: - return &bpf_snprintf_btf_proto; - case BPF_FUNC_per_cpu_ptr: - return &bpf_per_cpu_ptr_proto; - case BPF_FUNC_this_cpu_ptr: - return &bpf_this_cpu_ptr_proto; case BPF_FUNC_task_storage_get: if (bpf_prog_check_recur(prog)) return &bpf_task_storage_get_recur_proto; @@ -1548,18 +1482,12 @@ bpf_tracing_func_proto(enum bpf_func_id func_id, const struct bpf_prog *prog) if (bpf_prog_check_recur(prog)) return &bpf_task_storage_delete_recur_proto; return &bpf_task_storage_delete_proto; - case BPF_FUNC_for_each_map_elem: - return &bpf_for_each_map_elem_proto; - case BPF_FUNC_snprintf: - return &bpf_snprintf_proto; case BPF_FUNC_get_func_ip: return &bpf_get_func_ip_proto_tracing; case BPF_FUNC_get_branch_snapshot: return &bpf_get_branch_snapshot_proto; case BPF_FUNC_find_vma: return &bpf_find_vma_proto; - case BPF_FUNC_trace_vprintk: - return bpf_get_trace_vprintk_proto(); default: break; }