Re: [PATCH v7 11/11] kallsyms: Add self-test facility

From: Leizhen (ThunderTown)
Date: Tue Oct 18 2022 - 05:11:34 EST

Next message: Emil Renner Berthing: "[PATCH v1] pwm: sifive: Always let the first pwm_apply_state succeed"
Previous message: Baoquan He: "Re: [PATCH V3 1/2] RISC-V: Add arch_crash_save_vmcoreinfo support"
In reply to: kernel test robot: "Re: [PATCH v7 11/11] kallsyms: Add self-test facility"
Next in thread: kernel test robot: "Re: [PATCH v7 11/11] kallsyms: Add self-test facility"
Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]

On 2022/10/18 16:21, kernel test robot wrote:
> Hi Zhen,
>
> Thank you for the patch! Perhaps something to improve:
>
> [auto build test WARNING on masahiroy-kbuild/for-next]
> [also build test WARNING on linus/master v6.1-rc1 next-20221018]
> [cannot apply to mcgrof/modules-next]
> [If your patch is applied to the wrong git tree, kindly drop us a note.
> And when submitting patch, we suggest to use '--base' as documented in
> https://git-scm.com/docs/git-format-patch#_base_tree_information]
>
> url: https://github.com/intel-lab-lkp/linux/commits/Zhen-Lei/kallsyms-Optimizes-the-performance-of-lookup-symbols/20221017-145455
> base: https://git.kernel.org/pub/scm/linux/kernel/git/masahiroy/linux-kbuild.git for-next
> patch link: https://lore.kernel.org/r/20221017064950.2038-12-thunder.leizhen%40huawei.com
> patch subject: [PATCH v7 11/11] kallsyms: Add self-test facility
> config: sh-allmodconfig
> compiler: sh4-linux-gcc (GCC) 12.1.0
> reproduce (this is a W=1 build):
> wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
> chmod +x ~/bin/make.cross
> # https://github.com/intel-lab-lkp/linux/commit/3f5fc7fa1f657df865ef14b2d24f837a7cc079c9
> git remote add linux-review https://github.com/intel-lab-lkp/linux
> git fetch --no-tags linux-review Zhen-Lei/kallsyms-Optimizes-the-performance-of-lookup-symbols/20221017-145455
> git checkout 3f5fc7fa1f657df865ef14b2d24f837a7cc079c9
> # save the config file
> mkdir build_dir && cp config build_dir/.config
> COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0 make.cross W=1 O=build_dir ARCH=sh SHELL=/bin/bash
>
> If you fix the issue, kindly add following tag where applicable
> | Reported-by: kernel test robot <lkp@xxxxxxxxx>
>
> All warnings (new ones prefixed by >>):
>
> kernel/kallsyms_selftest.c:67:5: warning: no previous prototype for 'test_func' [-Wmissing-prototypes]
> 67 | int test_func(void)
> | ^~~~~~~~~
> kernel/kallsyms_selftest.c:72:12: warning: no previous prototype for 'test_func_weak' [-Wmissing-prototypes]
> 72 | __weak int test_func_weak(void)
> | ^~~~~~~~~~~~~~
> kernel/kallsyms_selftest.c: In function 'test_kallsyms_basic_function':
>>> kernel/kallsyms_selftest.c:424:1: warning: the frame size of 1124 bytes is larger than 1024 bytes [-Wframe-larger-than=]
> 424 | }
> | ^

OK, thanks. These warnings are minor. I will fix them in the next version after collecting review comments.

>
>
> vim +424 kernel/kallsyms_selftest.c
>
> 275
> 276 static int test_kallsyms_basic_function(void)
> 277 {
> 278 int i, j, ret;
> 279 int next = 0, nr_failed = 0;
> 280 char *prefix;
> 281 unsigned short rand;
> 282 unsigned long addr, lookup_addr;
> 283 char namebuf[KSYM_NAME_LEN];
> 284 struct test_stat stat, stat2;
> 285
> 286 prefix = "kallsyms_lookup_name() for";
> 287 for (i = 0; i < ARRAY_SIZE(test_items); i++) {
> 288 addr = kallsyms_lookup_name(test_items[i].name);
> 289 if (addr != test_items[i].addr) {
> 290 nr_failed++;
> 291 pr_info("%s %s failed: addr=%lx, expect %lx\n",
> 292 prefix, test_items[i].name, addr, test_items[i].addr);
> 293 }
> 294 }
> 295
> 296 prefix = "kallsyms_on_each_symbol() for";
> 297 for (i = 0; i < ARRAY_SIZE(test_items); i++) {
> 298 memset(&stat, 0, sizeof(stat));
> 299 stat.max = INT_MAX;
> 300 stat.name = test_items[i].name;
> 301 kallsyms_on_each_symbol(find_symbol, &stat);
> 302 if (stat.addr != test_items[i].addr || stat.real_cnt != 1) {
> 303 nr_failed++;
> 304 pr_info("%s %s failed: count=%d, addr=%lx, expect %lx\n",
> 305 prefix, test_items[i].name,
> 306 stat.real_cnt, stat.addr, test_items[i].addr);
> 307 }
> 308 }
> 309
> 310 prefix = "kallsyms_on_each_match_symbol() for";
> 311 for (i = 0; i < ARRAY_SIZE(test_items); i++) {
> 312 memset(&stat, 0, sizeof(stat));
> 313 stat.max = INT_MAX;
> 314 stat.name = test_items[i].name;
> 315 kallsyms_on_each_match_symbol(match_symbol, test_items[i].name, &stat);
> 316 if (stat.addr != test_items[i].addr || stat.real_cnt != 1) {
> 317 nr_failed++;
> 318 pr_info("%s %s failed: count=%d, addr=%lx, expect %lx\n",
> 319 prefix, test_items[i].name,
> 320 stat.real_cnt, stat.addr, test_items[i].addr);
> 321 }
> 322 }
> 323
> 324 if (nr_failed)
> 325 return -ESRCH;
> 326
> 327 for (i = 0; i < kallsyms_num_syms; i++) {
> 328 addr = kallsyms_sym_address(i);
> 329 if (!is_ksym_addr(addr))
> 330 continue;
> 331
> 332 ret = lookup_symbol_name(addr, namebuf);
> 333 if (unlikely(ret)) {
> 334 namebuf[0] = 0;
> 335 goto failed;
> 336 }
> 337
> 338 /*
> 339 * The first '.' may be the initial letter, in which case the
> 340 * entire symbol name will be truncated to an empty string in
> 341 * cleanup_symbol_name(). Do not test these symbols.
> 342 *
> 343 * For example:
> 344 * cat /proc/kallsyms | awk '{print $3}' | grep -E "^\." | head
> 345 * .E_read_words
> 346 * .E_leading_bytes
> 347 * .E_trailing_bytes
> 348 * .E_write_words
> 349 * .E_copy
> 350 * .str.292.llvm.12122243386960820698
> 351 * .str.24.llvm.12122243386960820698
> 352 * .str.29.llvm.12122243386960820698
> 353 * .str.75.llvm.12122243386960820698
> 354 * .str.99.llvm.12122243386960820698
> 355 */
> 356 if (IS_ENABLED(CONFIG_LTO_CLANG) && !namebuf[0])
> 357 continue;
> 358
> 359 lookup_addr = kallsyms_lookup_name(namebuf);
> 360
> 361 memset(&stat, 0, sizeof(stat));
> 362 stat.max = INT_MAX;
> 363 kallsyms_on_each_match_symbol(match_symbol, namebuf, &stat);
> 364
> 365 /*
> 366 * kallsyms_on_each_symbol() is too slow, randomly select some
> 367 * symbols for test.
> 368 */
> 369 if (i >= next) {
> 370 memset(&stat2, 0, sizeof(stat2));
> 371 stat2.max = INT_MAX;
> 372 stat2.name = namebuf;
> 373 kallsyms_on_each_symbol(find_symbol, &stat2);
> 374
> 375 /*
> 376 * kallsyms_on_each_symbol() and kallsyms_on_each_match_symbol()
> 377 * need to get the same traversal result.
> 378 */
> 379 if (stat.addr != stat2.addr ||
> 380 stat.real_cnt != stat2.real_cnt ||
> 381 memcmp(stat.addrs, stat2.addrs,
> 382 stat.save_cnt * sizeof(stat.addrs[0])))
> 383 goto failed;
> 384
> 385 /*
> 386 * The average of random increments is 128, that is, one of
> 387 * them is tested every 128 symbols.
> 388 */
> 389 get_random_bytes(&rand, sizeof(rand));
> 390 next = i + (rand & 0xff) + 1;
> 391 }
> 392
> 393 /* Need to be found at least once */
> 394 if (!stat.real_cnt)
> 395 goto failed;
> 396
> 397 /*
> 398 * kallsyms_lookup_name() returns the address of the first
> 399 * symbol found and cannot be NULL.
> 400 */
> 401 if (!lookup_addr || lookup_addr != stat.addrs[0])
> 402 goto failed;
> 403
> 404 /*
> 405 * If the addresses of all matching symbols are recorded, the
> 406 * target address needs to be exist.
> 407 */
> 408 if (stat.real_cnt <= MAX_NUM_OF_RECORDS) {
> 409 for (j = 0; j < stat.save_cnt; j++) {
> 410 if (stat.addrs[j] == addr)
> 411 break;
> 412 }
> 413
> 414 if (j == stat.save_cnt)
> 415 goto failed;
> 416 }
> 417 }
> 418
> 419 return 0;
> 420
> 421 failed:
> 422 pr_info("Test for %dth symbol failed: (%s) addr=%lx", i, namebuf, addr);
> 423 return -ESRCH;
> > 424 }
> 425
>

--
Regards,
Zhen Lei

Next message: Emil Renner Berthing: "[PATCH v1] pwm: sifive: Always let the first pwm_apply_state succeed"
Previous message: Baoquan He: "Re: [PATCH V3 1/2] RISC-V: Add arch_crash_save_vmcoreinfo support"
In reply to: kernel test robot: "Re: [PATCH v7 11/11] kallsyms: Add self-test facility"
Next in thread: kernel test robot: "Re: [PATCH v7 11/11] kallsyms: Add self-test facility"
Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]