Re: [PATCH v2 next 03/15] kallsyms: Improve selftest test_perf_kallsyms_lookup_name() output

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] kallsyms: unused variable kallsyms_very_long_name when !CONFIG_KALLSYMS_ALL
--

commit 3e15f2fb96b74e4386511b5db1ead3ae1d1fdf8a
Author: David Laight <david.laight.linux@gmail.com>

kallsyms: Improve selftest test_perf_kallsyms_lookup_name() output

This commit improves the kallsyms_lookup_name() performance selftest by
measuring the faster of two lookups, verifying returned addresses, printing
an 8-line histogram of lookup times, and adding a very long symbol to test
two-byte length handling.

> diff --git a/kernel/kallsyms_selftest.c b/kernel/kallsyms_selftest.c
> index 0db79237fe548..9072bd91c318c 100644
> --- a/kernel/kallsyms_selftest.c
> +++ b/kernel/kallsyms_selftest.c
> @@ -29,35 +31,56 @@ struct test_stat {

[ ... ]

> +/* This is 509 characters, compresses to about 330 */
> +#define kallsyms_very_long_name \
> +The_time_has_come_the_Walrus_said_To_talk_of_many_things\
> +_Of_shoes_and_ships_and_sealing_wax_Of_cabbages_and_kings\
> +_And_why_the_sea_is_boiling_hot_And_whether_pigs_have_wings\
> +\
> +_O_Oysters_said_the_Carpenter_Youve_had_a_pleasant_run\
> +_Shall_we_be_trotting_home_again_But_answer_came_there_none\
> +_And_this_was_scarcely_odd_because_Theyd_eaten_every_one\
> +\
> +_The_moon_was_shining_sulkily_Because_she_thought_the_sun\
> +_Had_got_no_business_to_be_there_After_the_day_was_done\
> +_Its_very_rude_of_him_she_said_To_come_and_spoil_the_fun
> +
> +static int kallsyms_very_long_name = 1;
> +
>  static int kallsyms_test_func_static(void)
>  {
>  	kallsyms_test_var_bss_static++;
> @@ -79,16 +102,17 @@ __weak int kallsyms_test_func_weak(void)
>  }
>  
>  static struct test_item test_items[] = {
> -	ITEM_FUNC(kallsyms_test_func_static),
> -	ITEM_FUNC(kallsyms_test_func),
> -	ITEM_FUNC(kallsyms_test_func_weak),
> -	ITEM_FUNC(vmalloc_noprof),
> -	ITEM_FUNC(vfree),
> +	TEST_ITEM(kallsyms_test_func_static),
> +	TEST_ITEM(kallsyms_test_func),
> +	TEST_ITEM(kallsyms_test_func_weak),
> +	TEST_ITEM(vmalloc_noprof),
> +	TEST_ITEM(vfree),
>  #ifdef CONFIG_KALLSYMS_ALL
> -	ITEM_DATA(kallsyms_test_var_bss_static),
> -	ITEM_DATA(kallsyms_test_var_data_static),
> -	ITEM_DATA(kallsyms_test_var_bss),
> -	ITEM_DATA(kallsyms_test_var_data),
> +	TEST_ITEM(kallsyms_test_var_bss_static),
> +	TEST_ITEM(kallsyms_test_var_data_static),
> +	TEST_ITEM(kallsyms_very_long_name),
> +	TEST_ITEM(kallsyms_test_var_bss),
> +	TEST_ITEM(kallsyms_test_var_data),
>  #endif
>  };

[Severity: Medium]
When CONFIG_KALLSYMS_SELFTEST=y and CONFIG_KALLSYMS_ALL=n, will
kallsyms_very_long_name trigger an unused-variable compiler warning?

Unlike kallsyms_test_var_bss_static and kallsyms_test_var_data_static, which
are incremented inside kallsyms_test_func_static(), kallsyms_very_long_name
is defined unconditionally as a static variable in
kernel/kallsyms_selftest.c, while its only reference in test_items[] is
guarded by #ifdef CONFIG_KALLSYMS_ALL.

Should kallsyms_very_long_name also be referenced in
kallsyms_test_func_static() or guarded by #ifdef CONFIG_KALLSYMS_ALL?

[ ... ]

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261018155206.132411-1-david.laight.linux@gmail.com?part=3