Re: [PATCH bpf-next 5/5] selftests/bpf: Remove the casting about jited_ksyms and jited_linfo

On 2022/7/17 9:46, Yonghong Song wrote:

On 7/16/22 5:51 AM, Pu Lehui wrote:

quoted

We have unified data extension operation of jited_ksyms and jited_linfo
into zero extension, so there's no need to cast u64 memory address to
long data type.

For subject, we are not 'Remove the casting ...'. What the code did is
to change the casting.

Also, I don't understand the above commit message. What does this mean
about 'data extension operation of jited_ksyms and jited_linfo into zero 
extension'?

In prog_tests/btf.c, we have a few other places to cast 
jited_linfo[...]/jited_ksyms[...] to 'long' type. Maybe casting
to 'unsigned long' is a better choice. Casting to 'unsigned long long'
of course will work, but is it necessary? Or you are talking about
64bit kernel and 32bit user space?

Hi Yonghong,

Thanks for your review. We introduced riscv jited line info in series 
[0], and we found that 32-bit systems can not display bpf line info due 
to the inconsistent data extension between jited_ksyms and jited_linfo. 
And we finally unify them to zero extension. By the way, we cleanup the 
related code. jited_ksyms and jited_linfo both are u64 address, no need 
to casting to long, and we previously remove it. But u64 in some arch is 
%ld, so to avoid compiler warnings we just cast to unsigned long long.

And sorry for not updating the subject and comment. I will corret it.

[0] 
https://lore.kernel.org/bpf/CAEf4Bza4RT=KFhr9ev29967dyT0eF_+6ZRqK35beUvnA_NbcqQ@mail.gmail.com/ (local)

quoted

Signed-off-by: Pu Lehui <pulehui@huawei.com>
---
  tools/testing/selftests/bpf/prog_tests/btf.c | 16 +++++++++-------
  1 file changed, 9 insertions(+), 7 deletions(-)

diff --git a/tools/testing/selftests/bpf/prog_tests/btf.c 

b/tools/testing/selftests/bpf/prog_tests/btf.c
index e852a9df779d..db10fa1745d1 100644

--- a/tools/testing/selftests/bpf/prog_tests/btf.c
+++ b/tools/testing/selftests/bpf/prog_tests/btf.c

@@ -6613,8 +6613,9 @@ static int test_get_linfo(const struct 

prog_info_raw_test *test,
      }
      if (CHECK(jited_linfo[0] != jited_ksyms[0],
-          "jited_linfo[0]:%lx != jited_ksyms[0]:%lx",
-          (long)(jited_linfo[0]), (long)(jited_ksyms[0]))) {
+          "jited_linfo[0]:%llx != jited_ksyms[0]:%llx",
+          (unsigned long long)(jited_linfo[0]),
+          (unsigned long long)(jited_ksyms[0]))) {
          err = -1;
          goto done;
      }

@@ -6632,16 +6633,17 @@ static int test_get_linfo(const struct 

prog_info_raw_test *test,
          }
          if (CHECK(jited_linfo[i] <= jited_linfo[i - 1],
-              "jited_linfo[%u]:%lx <= jited_linfo[%u]:%lx",
-              i, (long)jited_linfo[i],
-              i - 1, (long)(jited_linfo[i - 1]))) {
+              "jited_linfo[%u]:%llx <= jited_linfo[%u]:%llx",
+              i, (unsigned long long)jited_linfo[i],
+              i - 1, (unsigned long long)(jited_linfo[i - 1]))) {
              err = -1;
              goto done;
          }
          if (CHECK(jited_linfo[i] - cur_func_ksyms > cur_func_len,
-              "jited_linfo[%u]:%lx - %lx > %u",
-              i, (long)jited_linfo[i], (long)cur_func_ksyms,
+              "jited_linfo[%u]:%llx - %llx > %u",
+              i, (unsigned long long)jited_linfo[i],
+              (unsigned long long)cur_func_ksyms,
                cur_func_len)) {
              err = -1;
              goto done;

.