| Message ID | 20220429014240.3434866-2-pulehui@huawei.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Delegated to: | BPF |
| Headers | show |
| Series | Support riscv jit to provide | expand |
| Context | Check | Description |
|---|---|---|
| netdev/tree_selection | success | Clearly marked for bpf-next |
| netdev/fixes_present | success | Fixes tag not required for -next series |
| netdev/subject_prefix | success | Link |
| netdev/cover_letter | success | Series has a cover letter |
| netdev/patch_count | success | Link |
| netdev/header_inline | success | No static functions without inline keyword in header files |
| netdev/build_32bit | success | Errors and warnings before: 15 this patch: 15 |
| netdev/cc_maintainers | success | CCed 14 of 14 maintainers |
| netdev/build_clang | success | Errors and warnings before: 11 this patch: 11 |
| netdev/module_param | success | Was 0 now: 0 |
| netdev/verify_signedoff | success | Signed-off-by tag matches author and committer |
| netdev/verify_fixes | success | No Fixes tag |
| netdev/build_allmodconfig_warn | success | Errors and warnings before: 15 this patch: 15 |
| netdev/checkpatch | warning | CHECK: No space is necessary after a cast |
| 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-1 | fail | Logs for Kernel LATEST on ubuntu-latest + selftests |
| bpf/vmtest-bpf-next-PR | fail | PR summary |
| bpf/vmtest-bpf-next-VM_Test-2 | success | Logs for Kernel LATEST on z15 + selftests |
Pu Lehui wrote: > We found that 32-bit environment can not print bpf line info due > to data inconsistency between jited_ksyms[0] and jited_linfo[0]. > > For example: > jited_kyms[0] = 0xb800067c, jited_linfo[0] = 0xffffffffb800067c > > We know that both of them store bpf func address, but due to the > different data extension operations when extended to u64, they may > not be the same. We need to unify the data extension operations of > them. > > Signed-off-by: Pu Lehui <pulehui@huawei.com> > --- > kernel/bpf/syscall.c | 5 ++++- > 1 file changed, 4 insertions(+), 1 deletion(-) > > diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c > index e9e3e49c0eb7..18137ea5190d 100644 > --- a/kernel/bpf/syscall.c > +++ b/kernel/bpf/syscall.c > @@ -3871,13 +3871,16 @@ static int bpf_prog_get_info_by_fd(struct file *file, > info.nr_jited_line_info = 0; > if (info.nr_jited_line_info && ulen) { > if (bpf_dump_raw_ok(file->f_cred)) { > + unsigned long jited_linfo_addr; > __u64 __user *user_linfo; > u32 i; > > user_linfo = u64_to_user_ptr(info.jited_line_info); > ulen = min_t(u32, info.nr_jited_line_info, ulen); > for (i = 0; i < ulen; i++) { > - if (put_user((__u64)(long)prog->aux->jited_linfo[i], > + jited_linfo_addr = (unsigned long) > + prog->aux->jited_linfo[i]; > + if (put_user((__u64) jited_linfo_addr, > &user_linfo[i])) the logic is fine but i'm going to nitpick a bit this 4 lines is ugly just make it slightly longer than 80chars or use a shoarter name? For example, for (i = 0; i < ulen; i++) { unsigned long l; l = (unsigned long) prog->aux->jited_linfo[i]; if (put_user((__u64) l, &user_linfo[i])) is much nicer -- no reason to smash single assignment across multiple lines. My $.02. Thanks, John
On 2022/5/7 4:52, John Fastabend wrote: > Pu Lehui wrote: >> We found that 32-bit environment can not print bpf line info due >> to data inconsistency between jited_ksyms[0] and jited_linfo[0]. >> >> For example: >> jited_kyms[0] = 0xb800067c, jited_linfo[0] = 0xffffffffb800067c >> >> We know that both of them store bpf func address, but due to the >> different data extension operations when extended to u64, they may >> not be the same. We need to unify the data extension operations of >> them. >> >> Signed-off-by: Pu Lehui <pulehui@huawei.com> >> --- >> kernel/bpf/syscall.c | 5 ++++- >> 1 file changed, 4 insertions(+), 1 deletion(-) >> >> diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c >> index e9e3e49c0eb7..18137ea5190d 100644 >> --- a/kernel/bpf/syscall.c >> +++ b/kernel/bpf/syscall.c >> @@ -3871,13 +3871,16 @@ static int bpf_prog_get_info_by_fd(struct file *file, >> info.nr_jited_line_info = 0; >> if (info.nr_jited_line_info && ulen) { >> if (bpf_dump_raw_ok(file->f_cred)) { >> + unsigned long jited_linfo_addr; >> __u64 __user *user_linfo; >> u32 i; >> >> user_linfo = u64_to_user_ptr(info.jited_line_info); >> ulen = min_t(u32, info.nr_jited_line_info, ulen); >> for (i = 0; i < ulen; i++) { >> - if (put_user((__u64)(long)prog->aux->jited_linfo[i], >> + jited_linfo_addr = (unsigned long) >> + prog->aux->jited_linfo[i]; >> + if (put_user((__u64) jited_linfo_addr, >> &user_linfo[i])) > > the logic is fine but i'm going to nitpick a bit this 4 lines is ugly > just make it slightly longer than 80chars or use a shoarter name? For > example, > > for (i = 0; i < ulen; i++) { > unsigned long l; > > l = (unsigned long) prog->aux->jited_linfo[i]; > if (put_user((__u64) l, &user_linfo[i])) > > is much nicer -- no reason to smash single assignment across multiple > lines. My $.02. > Okay, It sounds good. I will make change in next version. Thanks. > Thanks, > John > . >
diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c index e9e3e49c0eb7..18137ea5190d 100644 --- a/kernel/bpf/syscall.c +++ b/kernel/bpf/syscall.c @@ -3871,13 +3871,16 @@ static int bpf_prog_get_info_by_fd(struct file *file, info.nr_jited_line_info = 0; if (info.nr_jited_line_info && ulen) { if (bpf_dump_raw_ok(file->f_cred)) { + unsigned long jited_linfo_addr; __u64 __user *user_linfo; u32 i; user_linfo = u64_to_user_ptr(info.jited_line_info); ulen = min_t(u32, info.nr_jited_line_info, ulen); for (i = 0; i < ulen; i++) { - if (put_user((__u64)(long)prog->aux->jited_linfo[i], + jited_linfo_addr = (unsigned long) + prog->aux->jited_linfo[i]; + if (put_user((__u64) jited_linfo_addr, &user_linfo[i])) return -EFAULT; }
We found that 32-bit environment can not print bpf line info due to data inconsistency between jited_ksyms[0] and jited_linfo[0]. For example: jited_kyms[0] = 0xb800067c, jited_linfo[0] = 0xffffffffb800067c We know that both of them store bpf func address, but due to the different data extension operations when extended to u64, they may not be the same. We need to unify the data extension operations of them. Signed-off-by: Pu Lehui <pulehui@huawei.com> --- kernel/bpf/syscall.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-)