* sys_enter tracepoint ctx structure
@ 2023-04-20 13:50 Yauheni Kaliuta
[not found] ` <CAADnVQ+JdPGV95Y30PskgdOomU2K0UXsoCydgqaJfJ5j4S8BtQ@mail.gmail.com>
0 siblings, 1 reply; 10+ messages in thread
From: Yauheni Kaliuta @ 2023-04-20 13:50 UTC (permalink / raw)
To: Yonghong Song; +Cc: bpf, asavkov, vmalik
Hi!
Should perf_call_bpf_enter/exit (kernel/trace/trace_syscalls.c)
use struct trace_event_raw_sys_enter/exit instead of locally
crafted struct syscall_tp_t nowadays? test_progs's vmlinux test
expects it as the context.
Or at least use struct trace_entry instead of struct pt_regs?
I have a problem with one RT patch with extends trace_entry.
--
WBR,
Yauheni Kaliuta
^ permalink raw reply [flat|nested] 10+ messages in thread[parent not found: <CAADnVQ+JdPGV95Y30PskgdOomU2K0UXsoCydgqaJfJ5j4S8BtQ@mail.gmail.com>]
* Fwd: sys_enter tracepoint ctx structure [not found] ` <CAADnVQ+JdPGV95Y30PskgdOomU2K0UXsoCydgqaJfJ5j4S8BtQ@mail.gmail.com> @ 2023-04-20 16:02 ` Alexei Starovoitov 2023-04-20 20:37 ` Yauheni Kaliuta 1 sibling, 0 replies; 10+ messages in thread From: Alexei Starovoitov @ 2023-04-20 16:02 UTC (permalink / raw) To: Yauheni Kaliuta, Yonghong Song, bpf, Artem Savkov, Viktor Malik On Thu, Apr 20, 2023 at 6:57 AM Yauheni Kaliuta <ykaliuta@redhat.com> wrote: > > Hi! > > Should perf_call_bpf_enter/exit (kernel/trace/trace_syscalls.c) > use struct trace_event_raw_sys_enter/exit instead of locally > crafted struct syscall_tp_t nowadays? No. It needs syscall_tp_t. > test_progs's vmlinux test > expects it as the context. what do you mean? Pls share a code pointer? > > > Or at least use struct trace_entry instead of struct pt_regs? no. It needs a pointer to pt_regs. See all of the pe_* flavor of helpers. > > > I have a problem with one RT patch with extends trace_entry. Just extend it. It shouldn't matter. I'm likely missing something. ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: sys_enter tracepoint ctx structure [not found] ` <CAADnVQ+JdPGV95Y30PskgdOomU2K0UXsoCydgqaJfJ5j4S8BtQ@mail.gmail.com> 2023-04-20 16:02 ` Fwd: " Alexei Starovoitov @ 2023-04-20 20:37 ` Yauheni Kaliuta 2023-04-20 20:54 ` Alexei Starovoitov 1 sibling, 1 reply; 10+ messages in thread From: Yauheni Kaliuta @ 2023-04-20 20:37 UTC (permalink / raw) To: Alexei Starovoitov Cc: Yonghong Song, bpf, Artem Savkov, Viktor Malik, jmarchan Hi, Alexei! >>>>> On Thu, 20 Apr 2023 08:59:09 -0700, Alexei Starovoitov wrote: > On Thu, Apr 20, 2023 at 6:57 AM Yauheni Kaliuta <ykaliuta@redhat.com> wrote: >> Hi! >> >> Should perf_call_bpf_enter/exit (kernel/trace/trace_syscalls.c) >> use struct trace_event_raw_sys_enter/exit instead of locally >> crafted struct syscall_tp_t nowadays? > No. It needs syscall_tp_t. > test_progs's vmlinux test >> expects it as the context. >> > what do you mean? Pls share a code pointer? https://github.com/torvalds/linux/blob/master/tools/testing/selftests/bpf/progs/test_vmlinux.c#L19 SEC("tp/syscalls/sys_enter_nanosleep") int handle__tp(struct trace_event_raw_sys_enter *args) So, should it use different structure then? syscall_tp_t to be declared publicly? >> >> Or at least use struct trace_entry instead of struct pt_regs? >> > no. It needs a pointer to pt_regs. > See all of the pe_* flavor of helpers. >> >> I have a problem with one RT patch with extends trace_entry. >> > Just extend it. It shouldn't matter. > I'm likely missing something. Or me. Let's figure out :) -- WBR, Yauheni Kaliuta ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: sys_enter tracepoint ctx structure 2023-04-20 20:37 ` Yauheni Kaliuta @ 2023-04-20 20:54 ` Alexei Starovoitov 2023-04-20 21:40 ` Yauheni Kaliuta 0 siblings, 1 reply; 10+ messages in thread From: Alexei Starovoitov @ 2023-04-20 20:54 UTC (permalink / raw) To: Yauheni Kaliuta Cc: Yonghong Song, bpf, Artem Savkov, Viktor Malik, Jerome Marchand On Thu, Apr 20, 2023 at 1:37 PM Yauheni Kaliuta <ykaliuta@redhat.com> wrote: > > Hi, Alexei! > > >>>>> On Thu, 20 Apr 2023 08:59:09 -0700, Alexei Starovoitov wrote: > > > On Thu, Apr 20, 2023 at 6:57 AM Yauheni Kaliuta <ykaliuta@redhat.com> wrote: > >> Hi! > >> > >> Should perf_call_bpf_enter/exit (kernel/trace/trace_syscalls.c) > >> use struct trace_event_raw_sys_enter/exit instead of locally > >> crafted struct syscall_tp_t nowadays? > > > > No. It needs syscall_tp_t. > > > test_progs's vmlinux test > >> expects it as the context. > >> > > > what do you mean? Pls share a code pointer? > > https://github.com/torvalds/linux/blob/master/tools/testing/selftests/bpf/progs/test_vmlinux.c#L19 > > SEC("tp/syscalls/sys_enter_nanosleep") > int handle__tp(struct trace_event_raw_sys_enter *args) I see. That bit is correct and that's what bpftrace is doing when attaching to syscalls. What do you see in your patched RT kernel when you do: cat /sys/kernel/debug/tracing/events/syscalls/sys_enter_nanosleep/format ? Depending on the answer we might need to fix the kernel side that has to use struct trace_entry in syscall_tp_t instead of plain long long. ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: sys_enter tracepoint ctx structure 2023-04-20 20:54 ` Alexei Starovoitov @ 2023-04-20 21:40 ` Yauheni Kaliuta 2023-04-20 23:12 ` Alexei Starovoitov 0 siblings, 1 reply; 10+ messages in thread From: Yauheni Kaliuta @ 2023-04-20 21:40 UTC (permalink / raw) To: Alexei Starovoitov Cc: Yonghong Song, bpf, Artem Savkov, Viktor Malik, Jerome Marchand Hi, Alexei! >>>>> On Thu, 20 Apr 2023 13:54:26 -0700, Alexei Starovoitov wrote: > On Thu, Apr 20, 2023 at 1:37 PM Yauheni Kaliuta <ykaliuta@redhat.com> wrote: >> >>>>> On Thu, 20 Apr 2023 08:59:09 -0700, Alexei Starovoitov wrote: >> >> >> >> Should perf_call_bpf_enter/exit (kernel/trace/trace_syscalls.c) >> >> use struct trace_event_raw_sys_enter/exit instead of locally >> >> crafted struct syscall_tp_t nowadays? >> >> >> > No. It needs syscall_tp_t. >> >> > test_progs's vmlinux test >> >> expects it as the context. >> >> >> >> > what do you mean? Pls share a code pointer? >> >> https://github.com/torvalds/linux/blob/master/tools/testing/selftests/bpf/progs/test_vmlinux.c#L19 >> >> SEC("tp/syscalls/sys_enter_nanosleep") >> int handle__tp(struct trace_event_raw_sys_enter *args) > I see. That bit is correct and that's what bpftrace is doing > when attaching to syscalls. > What do you see in your patched RT kernel when you do: > cat /sys/kernel/debug/tracing/events/syscalls/sys_enter_nanosleep/format > ? > Depending on the answer we might need to fix > the kernel side that has to use struct trace_entry > in syscall_tp_t instead of plain long long. # cat /sys/kernel/debug/tracing/events/syscalls/sys_enter_nanosleep/format name: sys_enter_nanosleep ID: 374 format: field:unsigned short common_type; offset:0; size:2; signed:0; field:unsigned char common_flags; offset:2; size:1; signed:0; field:unsigned char common_preempt_count; offset:3; size:1; signed:0; field:int common_pid; offset:4; size:4; signed:1; field:unsigned char common_preempt_lazy_count; offset:8; size:1; signed:0; field:int __syscall_nr; offset:12; size:4; signed:1; field:struct __kernel_timespec * rqtp; offset:16; size:8; signed:0; field:struct __kernel_timespec * rmtp; offset:24; size:8; signed:0; print fmt: "rqtp: 0x%08lx, rmtp: 0x%08lx", ((unsigned long)(REC->rqtp)), ((unsigned long)(REC->rmtp)) -- WBR, Yauheni Kaliuta ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: sys_enter tracepoint ctx structure 2023-04-20 21:40 ` Yauheni Kaliuta @ 2023-04-20 23:12 ` Alexei Starovoitov 2023-04-21 11:17 ` Yauheni Kaliuta 0 siblings, 1 reply; 10+ messages in thread From: Alexei Starovoitov @ 2023-04-20 23:12 UTC (permalink / raw) To: Yauheni Kaliuta Cc: Yonghong Song, bpf, Artem Savkov, Viktor Malik, Jerome Marchand On Thu, Apr 20, 2023 at 2:40 PM Yauheni Kaliuta <ykaliuta@redhat.com> wrote: > > Hi, Alexei! > > >>>>> On Thu, 20 Apr 2023 13:54:26 -0700, Alexei Starovoitov wrote: > > On Thu, Apr 20, 2023 at 1:37 PM Yauheni Kaliuta <ykaliuta@redhat.com> wrote: > >> >>>>> On Thu, 20 Apr 2023 08:59:09 -0700, Alexei Starovoitov wrote: > >> >> > >> >> Should perf_call_bpf_enter/exit (kernel/trace/trace_syscalls.c) > >> >> use struct trace_event_raw_sys_enter/exit instead of locally > >> >> crafted struct syscall_tp_t nowadays? > >> > >> > >> > No. It needs syscall_tp_t. > >> > >> > test_progs's vmlinux test > >> >> expects it as the context. > >> >> > >> > >> > what do you mean? Pls share a code pointer? > >> > >> https://github.com/torvalds/linux/blob/master/tools/testing/selftests/bpf/progs/test_vmlinux.c#L19 > >> > >> SEC("tp/syscalls/sys_enter_nanosleep") > >> int handle__tp(struct trace_event_raw_sys_enter *args) > > > I see. That bit is correct and that's what bpftrace is doing > > when attaching to syscalls. > > What do you see in your patched RT kernel when you do: > > cat /sys/kernel/debug/tracing/events/syscalls/sys_enter_nanosleep/format > > ? > > Depending on the answer we might need to fix > > the kernel side that has to use struct trace_entry > > in syscall_tp_t instead of plain long long. > > # cat /sys/kernel/debug/tracing/events/syscalls/sys_enter_nanosleep/format > name: sys_enter_nanosleep > ID: 374 > format: > field:unsigned short common_type; offset:0; size:2; signed:0; > field:unsigned char common_flags; offset:2; size:1; signed:0; > field:unsigned char common_preempt_count; offset:3; size:1; signed:0; > field:int common_pid; offset:4; size:4; signed:1; > field:unsigned char common_preempt_lazy_count; offset:8; size:1; signed:0; > > field:int __syscall_nr; offset:12; size:4; signed:1; > field:struct __kernel_timespec * rqtp; offset:16; size:8; signed:0; > field:struct __kernel_timespec * rmtp; offset:24; size:8; signed:0; > > print fmt: "rqtp: 0x%08lx, rmtp: 0x%08lx", ((unsigned long)(REC->rqtp)), ((unsigned long)(REC->rmtp)) Lol. Jiri even fixed the issue with this format in bpftrace 3 years ago: https://github.com/iovisor/bpftrace/commit/a2e3d5dbc03ceb49b776cf5602d31896158844a7 Let's fix the kernel side too. Something like this should do it: diff --git a/kernel/trace/trace_syscalls.c b/kernel/trace/trace_syscalls.c index 942ddbdace4a..7aa1f4299486 100644 --- a/kernel/trace/trace_syscalls.c +++ b/kernel/trace/trace_syscalls.c @@ -555,7 +555,7 @@ static int perf_call_bpf_enter(struct trace_event_call *call, struct pt_regs *re struct syscall_trace_enter *rec) { struct syscall_tp_t { - unsigned long long regs; + struct trace_entry ent; unsigned long syscall_nr; unsigned long args[SYSCALL_DEFINE_MAXARGS]; } param; @@ -657,7 +657,7 @@ static int perf_call_bpf_exit(struct trace_event_call *call, struct pt_regs *reg struct syscall_trace_exit *rec) { struct syscall_tp_t { - unsigned long long regs; + struct trace_entry ent; pls add build_bug_on that sizeof(ent) >= sizeof(void*). ^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: sys_enter tracepoint ctx structure 2023-04-20 23:12 ` Alexei Starovoitov @ 2023-04-21 11:17 ` Yauheni Kaliuta 2023-04-21 16:02 ` Alexei Starovoitov 0 siblings, 1 reply; 10+ messages in thread From: Yauheni Kaliuta @ 2023-04-21 11:17 UTC (permalink / raw) To: Alexei Starovoitov Cc: Yonghong Song, bpf, Artem Savkov, Viktor Malik, Jerome Marchand Hi, Alexei! >>>>> On Thu, 20 Apr 2023 16:12:49 -0700, Alexei Starovoitov wrote: > On Thu, Apr 20, 2023 at 2:40 PM Yauheni Kaliuta <ykaliuta@redhat.com> wrote: >> >>>>> On Thu, 20 Apr 2023 13:54:26 -0700, Alexei Starovoitov wrote: >> > On Thu, Apr 20, 2023 at 1:37 PM Yauheni Kaliuta <ykaliuta@redhat.com> wrote: >> >> >>>>> On Thu, 20 Apr 2023 08:59:09 -0700, Alexei Starovoitov wrote: >> >> >> >> >> >> Should perf_call_bpf_enter/exit (kernel/trace/trace_syscalls.c) >> >> >> use struct trace_event_raw_sys_enter/exit instead of locally >> >> >> crafted struct syscall_tp_t nowadays? >> >> >> >> >> >> > No. It needs syscall_tp_t. >> >> >> >> > test_progs's vmlinux test >> >> >> expects it as the context. >> >> >> >> >> >> >> > what do you mean? Pls share a code pointer? >> >> >> >> https://github.com/torvalds/linux/blob/master/tools/testing/selftests/bpf/progs/test_vmlinux.c#L19 >> >> >> >> SEC("tp/syscalls/sys_enter_nanosleep") >> >> int handle__tp(struct trace_event_raw_sys_enter *args) >> >> > I see. That bit is correct and that's what bpftrace is doing >> > when attaching to syscalls. >> > What do you see in your patched RT kernel when you do: >> > cat /sys/kernel/debug/tracing/events/syscalls/sys_enter_nanosleep/format >> > ? >> > Depending on the answer we might need to fix >> > the kernel side that has to use struct trace_entry >> > in syscall_tp_t instead of plain long long. >> >> # cat /sys/kernel/debug/tracing/events/syscalls/sys_enter_nanosleep/format >> name: sys_enter_nanosleep >> ID: 374 >> format: >> field:unsigned short common_type; offset:0; size:2; signed:0; >> field:unsigned char common_flags; offset:2; size:1; signed:0; >> field:unsigned char common_preempt_count; offset:3; size:1; signed:0; >> field:int common_pid; offset:4; size:4; signed:1; >> field:unsigned char common_preempt_lazy_count; offset:8; size:1; signed:0; >> >> field:int __syscall_nr; offset:12; size:4; signed:1; >> field:struct __kernel_timespec * rqtp; offset:16; size:8; signed:0; >> field:struct __kernel_timespec * rmtp; offset:24; size:8; signed:0; >> >> print fmt: "rqtp: 0x%08lx, rmtp: 0x%08lx", ((unsigned long)(REC->rqtp)), ((unsigned long)(REC->rmtp)) > Lol. > Jiri even fixed the issue with this format in bpftrace 3 years ago: > https://github.com/iovisor/bpftrace/commit/a2e3d5dbc03ceb49b776cf5602d31896158844a7 Hehe :) > Let's fix the kernel side too. Something like this should do it: > diff --git a/kernel/trace/trace_syscalls.c b/kernel/trace/trace_syscalls.c > index 942ddbdace4a..7aa1f4299486 100644 > --- a/kernel/trace/trace_syscalls.c > +++ b/kernel/trace/trace_syscalls.c > @@ -555,7 +555,7 @@ static int perf_call_bpf_enter(struct > trace_event_call *call, struct pt_regs *re > struct syscall_trace_enter *rec) > { > struct syscall_tp_t { > - unsigned long long regs; > + struct trace_entry ent; > unsigned long syscall_nr; > unsigned long args[SYSCALL_DEFINE_MAXARGS]; > } param; > @@ -657,7 +657,7 @@ static int perf_call_bpf_exit(struct > trace_event_call *call, struct pt_regs *reg > struct syscall_trace_exit *rec) > { > struct syscall_tp_t { > - unsigned long long regs; > + struct trace_entry ent; > pls add build_bug_on that sizeof(ent) >= sizeof(void*). Ok. Should the line *(struct pt_regs **)¶m = regs; be commented somehow? -- WBR, Yauheni Kaliuta ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: sys_enter tracepoint ctx structure 2023-04-21 11:17 ` Yauheni Kaliuta @ 2023-04-21 16:02 ` Alexei Starovoitov 2023-04-23 9:04 ` Yauheni Kaliuta 0 siblings, 1 reply; 10+ messages in thread From: Alexei Starovoitov @ 2023-04-21 16:02 UTC (permalink / raw) To: Yauheni Kaliuta Cc: Yonghong Song, bpf, Artem Savkov, Viktor Malik, Jerome Marchand On Fri, Apr 21, 2023 at 4:17 AM Yauheni Kaliuta <ykaliuta@redhat.com> wrote: > > Hi, Alexei! > > >>>>> On Thu, 20 Apr 2023 16:12:49 -0700, Alexei Starovoitov wrote: > > On Thu, Apr 20, 2023 at 2:40 PM Yauheni Kaliuta <ykaliuta@redhat.com> wrote: > >> >>>>> On Thu, 20 Apr 2023 13:54:26 -0700, Alexei Starovoitov wrote: > >> > On Thu, Apr 20, 2023 at 1:37 PM Yauheni Kaliuta <ykaliuta@redhat.com> wrote: > >> >> >>>>> On Thu, 20 Apr 2023 08:59:09 -0700, Alexei Starovoitov wrote: > >> >> >> > >> >> >> Should perf_call_bpf_enter/exit (kernel/trace/trace_syscalls.c) > >> >> >> use struct trace_event_raw_sys_enter/exit instead of locally > >> >> >> crafted struct syscall_tp_t nowadays? > >> >> > >> >> > >> >> > No. It needs syscall_tp_t. > >> >> > >> >> > test_progs's vmlinux test > >> >> >> expects it as the context. > >> >> >> > >> >> > >> >> > what do you mean? Pls share a code pointer? > >> >> > >> >> https://github.com/torvalds/linux/blob/master/tools/testing/selftests/bpf/progs/test_vmlinux.c#L19 > >> >> > >> >> SEC("tp/syscalls/sys_enter_nanosleep") > >> >> int handle__tp(struct trace_event_raw_sys_enter *args) > >> > >> > I see. That bit is correct and that's what bpftrace is doing > >> > when attaching to syscalls. > >> > What do you see in your patched RT kernel when you do: > >> > cat /sys/kernel/debug/tracing/events/syscalls/sys_enter_nanosleep/format > >> > ? > >> > Depending on the answer we might need to fix > >> > the kernel side that has to use struct trace_entry > >> > in syscall_tp_t instead of plain long long. > >> > >> # cat /sys/kernel/debug/tracing/events/syscalls/sys_enter_nanosleep/format > >> name: sys_enter_nanosleep > >> ID: 374 > >> format: > >> field:unsigned short common_type; offset:0; size:2; signed:0; > >> field:unsigned char common_flags; offset:2; size:1; signed:0; > >> field:unsigned char common_preempt_count; offset:3; size:1; signed:0; > >> field:int common_pid; offset:4; size:4; signed:1; > >> field:unsigned char common_preempt_lazy_count; offset:8; size:1; signed:0; > >> > >> field:int __syscall_nr; offset:12; size:4; signed:1; > >> field:struct __kernel_timespec * rqtp; offset:16; size:8; signed:0; > >> field:struct __kernel_timespec * rmtp; offset:24; size:8; signed:0; > >> > >> print fmt: "rqtp: 0x%08lx, rmtp: 0x%08lx", ((unsigned long)(REC->rqtp)), ((unsigned long)(REC->rmtp)) > > > > Lol. > > Jiri even fixed the issue with this format in bpftrace 3 years ago: > > https://github.com/iovisor/bpftrace/commit/a2e3d5dbc03ceb49b776cf5602d31896158844a7 > > Hehe :) > > > Let's fix the kernel side too. Something like this should do it: > > > diff --git a/kernel/trace/trace_syscalls.c b/kernel/trace/trace_syscalls.c > > index 942ddbdace4a..7aa1f4299486 100644 > > --- a/kernel/trace/trace_syscalls.c > > +++ b/kernel/trace/trace_syscalls.c > > @@ -555,7 +555,7 @@ static int perf_call_bpf_enter(struct > > trace_event_call *call, struct pt_regs *re > > struct syscall_trace_enter *rec) > > { > > struct syscall_tp_t { > > - unsigned long long regs; > > + struct trace_entry ent; > > unsigned long syscall_nr; > > unsigned long args[SYSCALL_DEFINE_MAXARGS]; > > } param; > > @@ -657,7 +657,7 @@ static int perf_call_bpf_exit(struct > > trace_event_call *call, struct pt_regs *reg > > struct syscall_trace_exit *rec) > > { > > struct syscall_tp_t { > > - unsigned long long regs; > > + struct trace_entry ent; > > > > pls add build_bug_on that sizeof(ent) >= sizeof(void*). > > Ok. Should the line *(struct pt_regs **)¶m = regs; be commented somehow? commented out? No. It's mandatory. And the reason for build_bug_on existence... to make sure that there is enough space there. ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: sys_enter tracepoint ctx structure 2023-04-21 16:02 ` Alexei Starovoitov @ 2023-04-23 9:04 ` Yauheni Kaliuta 2023-04-23 16:15 ` Alexei Starovoitov 0 siblings, 1 reply; 10+ messages in thread From: Yauheni Kaliuta @ 2023-04-23 9:04 UTC (permalink / raw) To: Alexei Starovoitov Cc: Yonghong Song, bpf, Artem Savkov, Viktor Malik, Jerome Marchand Hi, Alexei! >>>>> On Fri, 21 Apr 2023 09:02:34 -0700, Alexei Starovoitov wrote: > On Fri, Apr 21, 2023 at 4:17 AM Yauheni Kaliuta <ykaliuta@redhat.com> wrote: >> >>>>> On Thu, 20 Apr 2023 16:12:49 -0700, Alexei Starovoitov wrote: >> > On Thu, Apr 20, 2023 at 2:40 PM Yauheni Kaliuta <ykaliuta@redhat.com> wrote: >> >> >>>>> On Thu, 20 Apr 2023 13:54:26 -0700, Alexei Starovoitov wrote: >> >> > On Thu, Apr 20, 2023 at 1:37 PM Yauheni Kaliuta <ykaliuta@redhat.com> wrote: >> >> >> >>>>> On Thu, 20 Apr 2023 08:59:09 -0700, Alexei Starovoitov wrote: [...] >> > Let's fix the kernel side too. Something like this should do it: >> >> > diff --git a/kernel/trace/trace_syscalls.c b/kernel/trace/trace_syscalls.c >> > index 942ddbdace4a..7aa1f4299486 100644 >> > --- a/kernel/trace/trace_syscalls.c >> > +++ b/kernel/trace/trace_syscalls.c >> > @@ -555,7 +555,7 @@ static int perf_call_bpf_enter(struct >> > trace_event_call *call, struct pt_regs *re >> > struct syscall_trace_enter *rec) >> > { >> > struct syscall_tp_t { >> > - unsigned long long regs; >> > + struct trace_entry ent; >> > unsigned long syscall_nr; >> > unsigned long args[SYSCALL_DEFINE_MAXARGS]; >> > } param; >> > @@ -657,7 +657,7 @@ static int perf_call_bpf_exit(struct >> > trace_event_call *call, struct pt_regs *reg >> > struct syscall_trace_exit *rec) >> > { >> > struct syscall_tp_t { >> > - unsigned long long regs; >> > + struct trace_entry ent; >> >> >> > pls add build_bug_on that sizeof(ent) >= sizeof(void*). >> >> Ok. Should the line *(struct pt_regs **)¶m = regs; be commented somehow? > commented out? No, no :) > No. It's mandatory. > And the reason for build_bug_on existence... to make sure that there > is enough space there. Yes, it's clear for sure. It can be not obvious why basically 'ent' is inited with 'regs'. Before it was called 'regs' at least. -- WBR, Yauheni Kaliuta ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: sys_enter tracepoint ctx structure 2023-04-23 9:04 ` Yauheni Kaliuta @ 2023-04-23 16:15 ` Alexei Starovoitov 0 siblings, 0 replies; 10+ messages in thread From: Alexei Starovoitov @ 2023-04-23 16:15 UTC (permalink / raw) To: Yauheni Kaliuta Cc: Yonghong Song, bpf, Artem Savkov, Viktor Malik, Jerome Marchand On Sun, Apr 23, 2023 at 2:04 AM Yauheni Kaliuta <ykaliuta@redhat.com> wrote: > > Hi, Alexei! > > >>>>> On Fri, 21 Apr 2023 09:02:34 -0700, Alexei Starovoitov wrote: > > On Fri, Apr 21, 2023 at 4:17 AM Yauheni Kaliuta <ykaliuta@redhat.com> wrote: > >> >>>>> On Thu, 20 Apr 2023 16:12:49 -0700, Alexei Starovoitov wrote: > >> > On Thu, Apr 20, 2023 at 2:40 PM Yauheni Kaliuta <ykaliuta@redhat.com> wrote: > >> >> >>>>> On Thu, 20 Apr 2023 13:54:26 -0700, Alexei Starovoitov wrote: > >> >> > On Thu, Apr 20, 2023 at 1:37 PM Yauheni Kaliuta <ykaliuta@redhat.com> wrote: > >> >> >> >>>>> On Thu, 20 Apr 2023 08:59:09 -0700, Alexei Starovoitov wrote: > > [...] > > >> > Let's fix the kernel side too. Something like this should do it: > >> > >> > diff --git a/kernel/trace/trace_syscalls.c b/kernel/trace/trace_syscalls.c > >> > index 942ddbdace4a..7aa1f4299486 100644 > >> > --- a/kernel/trace/trace_syscalls.c > >> > +++ b/kernel/trace/trace_syscalls.c > >> > @@ -555,7 +555,7 @@ static int perf_call_bpf_enter(struct > >> > trace_event_call *call, struct pt_regs *re > >> > struct syscall_trace_enter *rec) > >> > { > >> > struct syscall_tp_t { > >> > - unsigned long long regs; > >> > + struct trace_entry ent; > >> > unsigned long syscall_nr; > >> > unsigned long args[SYSCALL_DEFINE_MAXARGS]; > >> > } param; > >> > @@ -657,7 +657,7 @@ static int perf_call_bpf_exit(struct > >> > trace_event_call *call, struct pt_regs *reg > >> > struct syscall_trace_exit *rec) > >> > { > >> > struct syscall_tp_t { > >> > - unsigned long long regs; > >> > + struct trace_entry ent; > >> > >> > >> > pls add build_bug_on that sizeof(ent) >= sizeof(void*). > >> > >> Ok. Should the line *(struct pt_regs **)¶m = regs; be commented somehow? > > > commented out? > > No, no :) > > > No. It's mandatory. > > And the reason for build_bug_on existence... to make sure that there > > is enough space there. > > Yes, it's clear for sure. > > It can be not obvious why basically 'ent' is inited with > 'regs'. Before it was called 'regs' at least. Got it :) Yeah. A comment describing the intent would be nice. ^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2023-04-23 16:15 UTC | newest]
Thread overview: 10+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-04-20 13:50 sys_enter tracepoint ctx structure Yauheni Kaliuta
[not found] ` <CAADnVQ+JdPGV95Y30PskgdOomU2K0UXsoCydgqaJfJ5j4S8BtQ@mail.gmail.com>
2023-04-20 16:02 ` Fwd: " Alexei Starovoitov
2023-04-20 20:37 ` Yauheni Kaliuta
2023-04-20 20:54 ` Alexei Starovoitov
2023-04-20 21:40 ` Yauheni Kaliuta
2023-04-20 23:12 ` Alexei Starovoitov
2023-04-21 11:17 ` Yauheni Kaliuta
2023-04-21 16:02 ` Alexei Starovoitov
2023-04-23 9:04 ` Yauheni Kaliuta
2023-04-23 16:15 ` Alexei Starovoitov
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.