2021-09-11 13:55:18

by Yinan Liu

Subject: [PATCH 0/2] ftrace: improve ftrace during compiling

Hi,
Some business scenarios require the kernel to start as quickly as possible,
so recently we are optimizing some processing during the kernel startup phase.
When the kernel is started, ftrace_init() takes 15-20ms to execute. Almost
all of the overhead belongs to the sorting and replacement of nop instructions,
and other processing only accounts for about 300us. Advance the processing of
these two parts to the compile time, and the time saved is very important
for certain scenarios, such as the quick start of containers.

Yinan Liu (2):
scripts: ftrace - move the sort-processing in ftrace_init to compile time
scripts: ftrace - move the nop-processing in ftrace_init to compile time

kernel/trace/ftrace.c | 9 +++-
scripts/link-vmlinux.sh | 6 +--
scripts/recordmcount.h | 14 +++++++
scripts/sorttable.c | 2 +
scripts/sorttable.h | 109 +++++++++++++++++++++++++++++++++++++++++++++++-
5 files changed, 133 insertions(+), 7 deletions(-)

--
2.14.4.44.g2045bb6
+++++++++++++++++++++++++++++++++++++++++++++++-

2021-09-11 13:55:18

by Yinan Liu

Subject: [PATCH 2/2] scripts: ftrace - move the nop-processing in ftrace_init to compile time

When ftrace is enabled, ftrace_init will consume a period of
time, usually around 15~20ms. Approximately 60% of the time is
consumed by nop-processing. Moving the nop-processing to the
compile time can speed up the kernel boot process.

performance test:
env: Intel(R) Xeon(R) CPU E5-2682 v4 @ 2.50GHz
method: before and after patching, compare the
total time of ftrace_init(), and verify
the functionality of ftrace.

avg_time of ftrace_init:
with patch: 7.114ms
without patch: 15.763ms

Signed-off-by: Yinan Liu <[email protected]>
---
kernel/trace/ftrace.c | 4 ++++
scripts/recordmcount.h | 14 ++++++++++++++
2 files changed, 18 insertions(+)

diff --git a/kernel/trace/ftrace.c b/kernel/trace/ftrace.c
index c236da868990..ae3fba331179 100644
--- a/kernel/trace/ftrace.c
+++ b/kernel/trace/ftrace.c
@@ -6261,6 +6261,10 @@ static int ftrace_process_locs(struct module *mod,
* until we are finished with it, and there's no
* reason to cause large interrupt latencies while we do it.
*/
+#if defined CONFIG_X86 || defined CONFIG_X86_64 || defined CONFIG_ARM || defined CONFIG_ARM64
+ ret = 0;
+ goto out;
+#endif
if (!mod)
local_irq_save(flags);
ftrace_update_code(mod, start_pg);
diff --git a/scripts/recordmcount.h b/scripts/recordmcount.h
index 1e9baa5c4fc6..152311639a0b 100644
--- a/scripts/recordmcount.h
+++ b/scripts/recordmcount.h
@@ -406,6 +406,8 @@ static uint_t *sift_rel_mcount(uint_t *mlocp,
uint_t const recval,
unsigned const reltype)
{
+ Elf_Shdr *const shdr0 = (Elf_Shdr *)(_w(ehdr->e_shoff) + (void *)ehdr);
+ Elf_Shdr const *const shdr = &shdr0[w(relhdr->sh_info)];
uint_t *const mloc0 = mlocp;
Elf_Rel *mrelp = *mrelpp;
Elf_Sym const *sym0;
@@ -419,6 +421,7 @@ static uint_t *sift_rel_mcount(uint_t *mlocp,
get_sym_str_and_relp(relhdr, ehdr, &sym0, &str0, &relp);

for (t = nrel; t; --t) {
+ int ret = -1;
if (!mcountsym)
mcountsym = get_mcountsym(sym0, relp, str0);

@@ -436,6 +439,17 @@ static uint_t *sift_rel_mcount(uint_t *mlocp,
*mlocp++ = addend;

mrelp = (Elf_Rel *)(rel_entsize + (void *)mrelp);
+ /* convert mcount into nop */
+ if (make_nop)
+ ret = make_nop((void *)ehdr,
+ _w(shdr->sh_offset) + _w(relp->r_offset));
+ if (!ret) {
+ Elf_Rel rel;
+ rel = *(Elf_Rel *)relp;
+ Elf_r_info(&rel, Elf_r_sym(relp), rel_type_nop);
+ ulseek(fd_map, (void *)relp - (void *)ehdr, SEEK_SET);
+ uwrite(fd_map, &rel, sizeof(rel));
+ }
}
relp = (Elf_Rel const *)(rel_entsize + (void *)relp);
}
--
2.14.4.44.g2045bb6

2021-09-11 13:55:18

by Yinan Liu

Subject: [PATCH 1/2] scripts: ftrace - move the sort-processing in ftrace_init to compile time

When ftrace is enabled, ftrace_init will consume a period of
time, usually around 15~20 ms. Approximately 40% of the time is
consumed by sort-processing. Moving the sort-processing to the
compile time can speed up the kernel boot process.

performance test:
env: Intel(R) Xeon(R) CPU E5-2682 v4 @ 2.50GHz
method: before and after patching, compare the
total time of ftrace_init(), and verify
the functionality of ftrace.

avg_time of ftrace_init:
with patch: 8.352 ms
without patch: 15.763 ms

Signed-off-by: Yinan Liu <[email protected]>
---
kernel/trace/ftrace.c | 5 ++-
scripts/link-vmlinux.sh | 6 +--
scripts/sorttable.c | 2 +
scripts/sorttable.h | 109 +++++++++++++++++++++++++++++++++++++++++++++++-
4 files changed, 115 insertions(+), 7 deletions(-)

diff --git a/kernel/trace/ftrace.c b/kernel/trace/ftrace.c
index 7efbc8aaf7f6..c236da868990 100644
--- a/kernel/trace/ftrace.c
+++ b/kernel/trace/ftrace.c
@@ -6189,8 +6189,9 @@ static int ftrace_process_locs(struct module *mod,
if (!count)
return 0;

- sort(start, count, sizeof(*start),
- ftrace_cmp_ips, NULL);
+ if (mod)
+ sort(start, count, sizeof(*start),
+ ftrace_cmp_ips, NULL);

start_pg = ftrace_allocate_pages(count);
if (!start_pg)
diff --git a/scripts/link-vmlinux.sh b/scripts/link-vmlinux.sh
index d74cee5c4326..57f53b0f4a8a 100755
--- a/scripts/link-vmlinux.sh
+++ b/scripts/link-vmlinux.sh
@@ -409,6 +409,9 @@ if [ -n "${CONFIG_DEBUG_INFO_BTF}" -a -n "${CONFIG_BPF}" ]; then
${RESOLVE_BTFIDS} vmlinux
fi

+info SYSMAP System.map
+mksysmap vmlinux System.map
+
if [ -n "${CONFIG_BUILDTIME_TABLE_SORT}" ]; then
info SORTTAB vmlinux
if ! sorttable vmlinux; then
@@ -417,9 +420,6 @@ if [ -n "${CONFIG_BUILDTIME_TABLE_SORT}" ]; then
fi
fi

-info SYSMAP System.map
-mksysmap vmlinux System.map
-
# step a (see comment above)
if [ -n "${CONFIG_KALLSYMS}" ]; then
mksysmap ${kallsyms_vmlinux} .tmp_System.map
diff --git a/scripts/sorttable.c b/scripts/sorttable.c
index 0ef3abfc4a51..11a595ca256b 100644
--- a/scripts/sorttable.c
+++ b/scripts/sorttable.c
@@ -30,6 +30,8 @@
#include <stdlib.h>
#include <string.h>
#include <unistd.h>
+#include <errno.h>
+#include <pthread.h>

#include <tools/be_byteshift.h>
#include <tools/le_byteshift.h>
diff --git a/scripts/sorttable.h b/scripts/sorttable.h
index a2baa2fefb13..db6c38c61986 100644
--- a/scripts/sorttable.h
+++ b/scripts/sorttable.h
@@ -19,6 +19,9 @@

#undef extable_ent_size
#undef compare_extable
+#undef get_mcount_loc
+#undef sort_mcount_loc
+#undef mcount_loc
#undef do_sort
#undef Elf_Addr
#undef Elf_Ehdr
@@ -41,6 +44,9 @@
#ifdef SORTTABLE_64
# define extable_ent_size 16
# define compare_extable compare_extable_64
+# define get_mcount_loc get_mcount_loc_64
+# define sort_mcount_loc sort_mcount_loc_64
+# define mcount_loc mcount_loc_64
# define do_sort do_sort_64
# define Elf_Addr Elf64_Addr
# define Elf_Ehdr Elf64_Ehdr
@@ -62,6 +68,9 @@
#else
# define extable_ent_size 8
# define compare_extable compare_extable_32
+# define get_mcount_loc get_mcount_loc_32
+# define sort_mcount_loc sort_mcount_loc_32
+# define mcount_loc mcount_loc_32
# define do_sort do_sort_32
# define Elf_Addr Elf32_Addr
# define Elf_Ehdr Elf32_Ehdr
@@ -84,8 +93,6 @@

#if defined(SORTTABLE_64) && defined(UNWINDER_ORC_ENABLED)
/* ORC unwinder only support X86_64 */
-#include <errno.h>
-#include <pthread.h>
#include <asm/orc_types.h>

#define ERRSTR_MAXSZ 256
@@ -191,6 +198,62 @@ static int compare_extable(const void *a, const void *b)
return 1;
return 0;
}
+struct mcount_loc {
+ Elf_Ehdr *ehdr;
+ Elf_Shdr *init_data_sec;
+ uint_t start_mcount_loc;
+ uint_t stop_mcount_loc;
+};
+
+/* Sort the addresses stored between __start_mcount to __stop_mcount */
+static void *sort_mcount_loc(void *arg)
+{
+ struct mcount_loc *mstruct = (struct mcount_loc *)arg;
+ uint_t offset = mstruct->start_mcount_loc - _r(&(mstruct->init_data_sec)->sh_addr)
+ + _r(&(mstruct->init_data_sec)->sh_offset);
+ uint_t count = mstruct->stop_mcount_loc - mstruct->start_mcount_loc;
+ unsigned char *start = (void *)mstruct->ehdr + offset;
+
+ qsort(start, count/sizeof(uint_t), sizeof(uint_t), compare_extable);
+ return NULL;
+}
+
+/* get the address of __start_mcount_loc and __stop_mcount_loc*/
+static void get_mcount_loc(uint_t *_start, uint_t *_stop)
+{
+ FILE *file_start, *file_stop;
+ char start_buff[20];
+ char end_buff[20];
+ int len = 0;
+
+ file_start = popen(" grep start_mcount System.map | awk '{print $1}' ", "r");
+ if (!file_start) {
+ fprintf(stderr, "get start_mcount_loc error!");
+ return;
+ }
+
+ file_stop = popen(" grep stop_mcount System.map | awk '{print $1}' ", "r");
+ if (!file_stop) {
+ fprintf(stderr, "get stop_mcount_loc error!");
+ pclose(file_start);
+ return;
+ }
+
+ while (fgets(start_buff, sizeof(start_buff), file_start) != NULL) {
+ len = strlen(start_buff);
+ start_buff[len - 1] = '\0';
+ }
+ *_start = strtoul(start_buff, NULL, 16);
+
+ while (fgets(end_buff, sizeof(end_buff), file_stop) != NULL) {
+ len = strlen(end_buff);
+ end_buff[len - 1] = '\0';
+ }
+ *_stop = strtoul(end_buff, NULL, 16);
+
+ pclose(file_start);
+ pclose(file_stop);
+}

static int do_sort(Elf_Ehdr *ehdr,
char const *const fname,
@@ -217,6 +280,10 @@ static int do_sort(Elf_Ehdr *ehdr,
int idx;
unsigned int shnum;
unsigned int shstrndx;
+ struct mcount_loc mstruct;
+ uint_t _start_mcount_loc = 0;
+ uint_t _stop_mcount_loc = 0;
+ pthread_t mcount_sort_thread;
#if defined(SORTTABLE_64) && defined(UNWINDER_ORC_ENABLED)
unsigned int orc_ip_size = 0;
unsigned int orc_size = 0;
@@ -253,6 +320,14 @@ static int do_sort(Elf_Ehdr *ehdr,
symtab_shndx = (Elf32_Word *)((const char *)ehdr +
_r(&s->sh_offset));

+ /* locate the mcount_loc */
+ if (!strcmp(secstrings + idx, ".init.data")) {
+ get_mcount_loc(&_start_mcount_loc, &_stop_mcount_loc);
+ mstruct.ehdr = ehdr;
+ mstruct.init_data_sec = s;
+ mstruct.start_mcount_loc = _start_mcount_loc;
+ mstruct.stop_mcount_loc = _stop_mcount_loc;
+ }
#if defined(SORTTABLE_64) && defined(UNWINDER_ORC_ENABLED)
/* locate the ORC unwind tables */
if (!strcmp(secstrings + idx, ".orc_unwind_ip")) {
@@ -294,6 +369,21 @@ static int do_sort(Elf_Ehdr *ehdr,
goto out;
}
#endif
+ if (!mstruct.init_data_sec || !_start_mcount_loc || !_stop_mcount_loc) {
+ fprintf(stderr,
+ "incomplete mcount_loc in file: %s\n",
+ fname);
+ goto out;
+ }
+
+ /* create thread to sort mcount_loc concurrently */
+ if (pthread_create(&mcount_sort_thread, NULL, &sort_mcount_loc, &mstruct)) {
+ fprintf(stderr,
+ "pthread_create mcount_sort_thread failed '%s': %s\n",
+ strerror(errno), fname);
+ goto out;
+ }
+
if (!extab_sec) {
fprintf(stderr, "no __ex_table in file: %s\n", fname);
goto out;
@@ -376,5 +466,20 @@ static int do_sort(Elf_Ehdr *ehdr,
}
}
#endif
+ if (mcount_sort_thread) {
+ void *retval = NULL;
+ /* wait for mcount sort done */
+ rc = pthread_join(mcount_sort_thread, &retval);
+ if (rc)
+ fprintf(stderr,
+ "pthread_join failed '%s': %s\n",
+ strerror(errno), fname);
+ else if (retval) {
+ rc = -1;
+ fprintf(stderr,
+ "failed to sort mcount '%s': %s\n",
+ (char *)retval, fname);
+ }
+ }
return rc;
}
--
2.14.4.44.g2045bb6

2021-09-11 14:03:52

by Steven Rostedt

Subject: Re: [PATCH 1/2] scripts: ftrace - move the sort-processing in ftrace_init to compile time

On Sat, 11 Sep 2021 21:50:42 +0800
Yinan Liu <[email protected]> wrote:

> When ftrace is enabled, ftrace_init will consume a period of
> time, usually around 15~20 ms. Approximately 40% of the time is
> consumed by sort-processing. Moving the sort-processing to the
> compile time can speed up the kernel boot process.
>

Nice. I like the idea of sorting at compile time.

> performance test:
> env: Intel(R) Xeon(R) CPU E5-2682 v4 @ 2.50GHz
> method: before and after patching, compare the
> total time of ftrace_init(), and verify
> the functionality of ftrace.
>
> avg_time of ftrace_init:
> with patch: 8.352 ms
> without patch: 15.763 ms
>
> Signed-off-by: Yinan Liu <[email protected]>
> ---
> kernel/trace/ftrace.c | 5 ++-
> scripts/link-vmlinux.sh | 6 +--
> scripts/sorttable.c | 2 +
> scripts/sorttable.h | 109 +++++++++++++++++++++++++++++++++++++++++++++++-
> 4 files changed, 115 insertions(+), 7 deletions(-)
>
> diff --git a/kernel/trace/ftrace.c b/kernel/trace/ftrace.c
> index 7efbc8aaf7f6..c236da868990 100644
> --- a/kernel/trace/ftrace.c
> +++ b/kernel/trace/ftrace.c
> @@ -6189,8 +6189,9 @@ static int ftrace_process_locs(struct module *mod,
> if (!count)
> return 0;
>
> - sort(start, count, sizeof(*start),
> - ftrace_cmp_ips, NULL);
> + if (mod)

Why can't we enforce modules to be sorted too?

> + sort(start, count, sizeof(*start),
> + ftrace_cmp_ips, NULL);


-- Steve

2021-09-11 14:14:05

by Steven Rostedt

Subject: Re: [PATCH 2/2] scripts: ftrace - move the nop-processing in ftrace_init to compile time

On Sat, 11 Sep 2021 21:50:43 +0800
Yinan Liu <[email protected]> wrote:

> When ftrace is enabled, ftrace_init will consume a period of
> time, usually around 15~20ms. Approximately 60% of the time is
> consumed by nop-processing. Moving the nop-processing to the
> compile time can speed up the kernel boot process.
>
> performance test:
> env: Intel(R) Xeon(R) CPU E5-2682 v4 @ 2.50GHz
> method: before and after patching, compare the
> total time of ftrace_init(), and verify
> the functionality of ftrace.
>
> avg_time of ftrace_init:
> with patch: 7.114ms
> without patch: 15.763ms

What compiler are you using? Because by default, gcc should already do
this for you. In fact, recordmcount isn't even called with the latest
gcc, as gcc creates mcount_loc and inserts nops.

This was implemented before, but because we use to have "ideal nops"
that was determined at run time, because the different CPUs had
different efficiency on what nop was used, we had to do it at run time.
But that is no longer the case today, so we can revisit this.

>
> Signed-off-by: Yinan Liu <[email protected]>
> ---
> kernel/trace/ftrace.c | 4 ++++
> scripts/recordmcount.h | 14 ++++++++++++++
> 2 files changed, 18 insertions(+)
>
> diff --git a/kernel/trace/ftrace.c b/kernel/trace/ftrace.c
> index c236da868990..ae3fba331179 100644
> --- a/kernel/trace/ftrace.c
> +++ b/kernel/trace/ftrace.c
> @@ -6261,6 +6261,10 @@ static int ftrace_process_locs(struct module *mod,
> * until we are finished with it, and there's no
> * reason to cause large interrupt latencies while we do it.
> */
> +#if defined CONFIG_X86 || defined CONFIG_X86_64 || defined CONFIG_ARM || defined CONFIG_ARM64

We don't list archs in generic files. The above needs to be something like:

#ifdef ARCH_HAS_MCOUNT_NOP

or some name like that, and then that macro gets defined by the arch
header (include/asm/ftrace.h)



> + ret = 0;
> + goto out;
> +#endif

space should be here.

> if (!mod)
> local_irq_save(flags);
> ftrace_update_code(mod, start_pg);

-- Steve

2021-09-11 14:35:02

by Peter Zijlstra

Subject: Re: [PATCH 2/2] scripts: ftrace - move the nop-processing in ftrace_init to compile time

On Sat, Sep 11, 2021 at 09:50:43PM +0800, Yinan Liu wrote:
> When ftrace is enabled, ftrace_init will consume a period of
> time, usually around 15~20ms. Approximately 60% of the time is
> consumed by nop-processing. Moving the nop-processing to the
> compile time can speed up the kernel boot process.
>
> performance test:
> env: Intel(R) Xeon(R) CPU E5-2682 v4 @ 2.50GHz

If you use objtool's -M argument instead of recordmcount, you already
get this and you can avoid running recordmcount entirely.

2021-09-11 15:30:18

by Yinan Liu

Subject: Re: [PATCH 2/2] scripts: ftrace - move the nop-processing in ftrace_init to compile time

This is my GCC version: GCC version 4.8.5 20150623 (red hat 4.8.5-44)
(GCC) .

In fact, I see the make_nop processing in recordmcount. I'm still
confused why this part can be directly replaced?


在 2021/9/11 下午10:12, Steven Rostedt 写道:
> On Sat, 11 Sep 2021 21:50:43 +0800
> Yinan Liu <[email protected]> wrote:
>
>> When ftrace is enabled, ftrace_init will consume a period of
>> time, usually around 15~20ms. Approximately 60% of the time is
>> consumed by nop-processing. Moving the nop-processing to the
>> compile time can speed up the kernel boot process.
>>
>> performance test:
>> env: Intel(R) Xeon(R) CPU E5-2682 v4 @ 2.50GHz
>> method: before and after patching, compare the
>> total time of ftrace_init(), and verify
>> the functionality of ftrace.
>>
>> avg_time of ftrace_init:
>> with patch: 7.114ms
>> without patch: 15.763ms
> What compiler are you using? Because by default, gcc should already do
> this for you. In fact, recordmcount isn't even called with the latest
> gcc, as gcc creates mcount_loc and inserts nops.
>
> This was implemented before, but because we use to have "ideal nops"
> that was determined at run time, because the different CPUs had
> different efficiency on what nop was used, we had to do it at run time.
> But that is no longer the case today, so we can revisit this.
>
>> Signed-off-by: Yinan Liu <[email protected]>
>> ---
>> kernel/trace/ftrace.c | 4 ++++
>> scripts/recordmcount.h | 14 ++++++++++++++
>> 2 files changed, 18 insertions(+)
>>
>> diff --git a/kernel/trace/ftrace.c b/kernel/trace/ftrace.c
>> index c236da868990..ae3fba331179 100644
>> --- a/kernel/trace/ftrace.c
>> +++ b/kernel/trace/ftrace.c
>> @@ -6261,6 +6261,10 @@ static int ftrace_process_locs(struct module *mod,
>> * until we are finished with it, and there's no
>> * reason to cause large interrupt latencies while we do it.
>> */
>> +#if defined CONFIG_X86 || defined CONFIG_X86_64 || defined CONFIG_ARM || defined CONFIG_ARM64
> We don't list archs in generic files. The above needs to be something like:
>
> #ifdef ARCH_HAS_MCOUNT_NOP
>
> or some name like that, and then that macro gets defined by the arch
> header (include/asm/ftrace.h)
>
>
>
>> + ret = 0;
>> + goto out;
>> +#endif
> space should be here.
>
>> if (!mod)
>> local_irq_save(flags);
>> ftrace_update_code(mod, start_pg);
> -- Steve

2021-09-11 16:08:32

by Peter Zijlstra

Subject: Re: [PATCH 2/2] scripts: ftrace - move the nop-processing in ftrace_init to compile time

On Sat, Sep 11, 2021 at 11:28:32PM +0800, Yinan Liu wrote:
> This is my GCC version: GCC version 4.8.5 20150623 (red hat 4.8.5-44) (GCC)

Please don't top post. Also please upgrade your GCC, that's too told to
even build the current kernel tree (IIRC we have minimum GCC-5.1, but I
can recommend using GCC-8 or later).

2021-09-11 17:17:10

by kernel test robot

Subject: Re: [PATCH 2/2] scripts: ftrace - move the nop-processing in ftrace_init to compile time

Hi Yinan,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on tip/perf/core]
[also build test ERROR on kbuild/for-next trace/for-next linus/master v5.14 next-20210910]
[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]

url: https://github.com/0day-ci/linux/commits/Yinan-Liu/ftrace-improve-ftrace-during-compiling/20210911-215230
base: https://git.kernel.org/pub/scm/linux/kernel/git/tip/tip.git 4034fb207e302cc0b1f304084d379640c1fb1436
config: sparc-allyesconfig (attached as .config)
compiler: sparc64-linux-gcc (GCC) 11.2.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/0day-ci/linux/commit/54d68f2f4f2f10bb9939f8f532615295bf52ce96
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review Yinan-Liu/ftrace-improve-ftrace-during-compiling/20210911-215230
git checkout 54d68f2f4f2f10bb9939f8f532615295bf52ce96
# save the attached .config to linux build tree
mkdir build_dir
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-11.2.0 make.cross O=build_dir ARCH=sparc olddefconfig

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <[email protected]>

All error/warnings (new ones prefixed by >>):

In file included from scripts/recordmcount.c:423:
scripts/recordmcount.h: In function 'sift32_rel_mcount':
>> scripts/recordmcount.h:450:5: error: too many arguments to function 'ulseek'
450 | ulseek(fd_map, (void *)relp - (void *)ehdr, SEEK_SET);
| ^~~~~~
scripts/recordmcount.c:81:14: note: declared here
81 | static off_t ulseek(off_t const offset, int const whence)
| ^~~~~~
In file included from scripts/recordmcount.c:423:
>> scripts/recordmcount.h:451:12: warning: passing argument 1 of 'uwrite' makes pointer from integer without a cast [-Wint-conversion]
451 | uwrite(fd_map, &rel, sizeof(rel));
| ^~~~~~
| |
| int
scripts/recordmcount.c:101:41: note: expected 'const void * const' but argument is of type 'int'
101 | static ssize_t uwrite(void const *const buf, size_t const count)
| ~~~~~~~~~~~~~~~~~~^~~
In file included from scripts/recordmcount.c:423:
>> scripts/recordmcount.h:451:20: warning: passing argument 2 of 'uwrite' makes integer from pointer without a cast [-Wint-conversion]
451 | uwrite(fd_map, &rel, sizeof(rel));
| ^~~~
| |
| Elf32_Rel *
scripts/recordmcount.c:101:59: note: expected 'size_t' {aka 'const long unsigned int'} but argument is of type 'Elf32_Rel *'
101 | static ssize_t uwrite(void const *const buf, size_t const count)
| ~~~~~~~~~~~~~^~~~~
In file included from scripts/recordmcount.c:423:
>> scripts/recordmcount.h:451:5: error: too many arguments to function 'uwrite'
451 | uwrite(fd_map, &rel, sizeof(rel));
| ^~~~~~
scripts/recordmcount.c:101:16: note: declared here
101 | static ssize_t uwrite(void const *const buf, size_t const count)
| ^~~~~~
In file included from scripts/recordmcount.c:425:
scripts/recordmcount.h: In function 'sift64_rel_mcount':
>> scripts/recordmcount.h:450:5: error: too many arguments to function 'ulseek'
450 | ulseek(fd_map, (void *)relp - (void *)ehdr, SEEK_SET);
| ^~~~~~
scripts/recordmcount.c:81:14: note: declared here
81 | static off_t ulseek(off_t const offset, int const whence)
| ^~~~~~
In file included from scripts/recordmcount.c:425:
>> scripts/recordmcount.h:451:12: warning: passing argument 1 of 'uwrite' makes pointer from integer without a cast [-Wint-conversion]
451 | uwrite(fd_map, &rel, sizeof(rel));
| ^~~~~~
| |
| int
scripts/recordmcount.c:101:41: note: expected 'const void * const' but argument is of type 'int'
101 | static ssize_t uwrite(void const *const buf, size_t const count)
| ~~~~~~~~~~~~~~~~~~^~~
In file included from scripts/recordmcount.c:425:
>> scripts/recordmcount.h:451:20: warning: passing argument 2 of 'uwrite' makes integer from pointer without a cast [-Wint-conversion]
451 | uwrite(fd_map, &rel, sizeof(rel));
| ^~~~
| |
| Elf64_Rel *
scripts/recordmcount.c:101:59: note: expected 'size_t' {aka 'const long unsigned int'} but argument is of type 'Elf64_Rel *'
101 | static ssize_t uwrite(void const *const buf, size_t const count)
| ~~~~~~~~~~~~~^~~~~
In file included from scripts/recordmcount.c:425:
>> scripts/recordmcount.h:451:5: error: too many arguments to function 'uwrite'
451 | uwrite(fd_map, &rel, sizeof(rel));
| ^~~~~~
scripts/recordmcount.c:101:16: note: declared here
101 | static ssize_t uwrite(void const *const buf, size_t const count)
| ^~~~~~
make[2]: *** [scripts/Makefile.host:95: scripts/recordmcount] Error 1
make[2]: Target '__build' not remade because of errors.
make[1]: *** [Makefile:1196: scripts] Error 2
make[1]: Target 'modules_prepare' not remade because of errors.
make: *** [Makefile:220: __sub-make] Error 2
make: Target 'modules_prepare' not remade because of errors.
--
In file included from scripts/recordmcount.c:423:
scripts/recordmcount.h: In function 'sift32_rel_mcount':
>> scripts/recordmcount.h:450:5: error: too many arguments to function 'ulseek'
450 | ulseek(fd_map, (void *)relp - (void *)ehdr, SEEK_SET);
| ^~~~~~
scripts/recordmcount.c:81:14: note: declared here
81 | static off_t ulseek(off_t const offset, int const whence)
| ^~~~~~
In file included from scripts/recordmcount.c:423:
>> scripts/recordmcount.h:451:12: warning: passing argument 1 of 'uwrite' makes pointer from integer without a cast [-Wint-conversion]
451 | uwrite(fd_map, &rel, sizeof(rel));
| ^~~~~~
| |
| int
scripts/recordmcount.c:101:41: note: expected 'const void * const' but argument is of type 'int'
101 | static ssize_t uwrite(void const *const buf, size_t const count)
| ~~~~~~~~~~~~~~~~~~^~~
In file included from scripts/recordmcount.c:423:
>> scripts/recordmcount.h:451:20: warning: passing argument 2 of 'uwrite' makes integer from pointer without a cast [-Wint-conversion]
451 | uwrite(fd_map, &rel, sizeof(rel));
| ^~~~
| |
| Elf32_Rel *
scripts/recordmcount.c:101:59: note: expected 'size_t' {aka 'const long unsigned int'} but argument is of type 'Elf32_Rel *'
101 | static ssize_t uwrite(void const *const buf, size_t const count)
| ~~~~~~~~~~~~~^~~~~
In file included from scripts/recordmcount.c:423:
>> scripts/recordmcount.h:451:5: error: too many arguments to function 'uwrite'
451 | uwrite(fd_map, &rel, sizeof(rel));
| ^~~~~~
scripts/recordmcount.c:101:16: note: declared here
101 | static ssize_t uwrite(void const *const buf, size_t const count)
| ^~~~~~
In file included from scripts/recordmcount.c:425:
scripts/recordmcount.h: In function 'sift64_rel_mcount':
>> scripts/recordmcount.h:450:5: error: too many arguments to function 'ulseek'
450 | ulseek(fd_map, (void *)relp - (void *)ehdr, SEEK_SET);
| ^~~~~~
scripts/recordmcount.c:81:14: note: declared here
81 | static off_t ulseek(off_t const offset, int const whence)
| ^~~~~~
In file included from scripts/recordmcount.c:425:
>> scripts/recordmcount.h:451:12: warning: passing argument 1 of 'uwrite' makes pointer from integer without a cast [-Wint-conversion]
451 | uwrite(fd_map, &rel, sizeof(rel));
| ^~~~~~
| |
| int
scripts/recordmcount.c:101:41: note: expected 'const void * const' but argument is of type 'int'
101 | static ssize_t uwrite(void const *const buf, size_t const count)
| ~~~~~~~~~~~~~~~~~~^~~
In file included from scripts/recordmcount.c:425:
>> scripts/recordmcount.h:451:20: warning: passing argument 2 of 'uwrite' makes integer from pointer without a cast [-Wint-conversion]
451 | uwrite(fd_map, &rel, sizeof(rel));
| ^~~~
| |
| Elf64_Rel *
scripts/recordmcount.c:101:59: note: expected 'size_t' {aka 'const long unsigned int'} but argument is of type 'Elf64_Rel *'
101 | static ssize_t uwrite(void const *const buf, size_t const count)
| ~~~~~~~~~~~~~^~~~~
In file included from scripts/recordmcount.c:425:
>> scripts/recordmcount.h:451:5: error: too many arguments to function 'uwrite'
451 | uwrite(fd_map, &rel, sizeof(rel));
| ^~~~~~
scripts/recordmcount.c:101:16: note: declared here
101 | static ssize_t uwrite(void const *const buf, size_t const count)
| ^~~~~~
make[2]: *** [scripts/Makefile.host:95: scripts/recordmcount] Error 1
make[2]: Target '__build' not remade because of errors.
make[1]: *** [Makefile:1196: scripts] Error 2
make[1]: Target 'prepare' not remade because of errors.
make: *** [Makefile:220: __sub-make] Error 2
make: Target 'prepare' not remade because of errors.


vim +/ulseek +450 scripts/recordmcount.h

394
395 /*
396 * Look at the relocations in order to find the calls to mcount.
397 * Accumulate the section offsets that are found, and their relocation info,
398 * onto the end of the existing arrays.
399 */
400 static uint_t *sift_rel_mcount(uint_t *mlocp,
401 unsigned const offbase,
402 Elf_Rel **const mrelpp,
403 Elf_Shdr const *const relhdr,
404 Elf_Ehdr const *const ehdr,
405 unsigned const recsym,
406 uint_t const recval,
407 unsigned const reltype)
408 {
409 Elf_Shdr *const shdr0 = (Elf_Shdr *)(_w(ehdr->e_shoff) + (void *)ehdr);
410 Elf_Shdr const *const shdr = &shdr0[w(relhdr->sh_info)];
411 uint_t *const mloc0 = mlocp;
412 Elf_Rel *mrelp = *mrelpp;
413 Elf_Sym const *sym0;
414 char const *str0;
415 Elf_Rel const *relp;
416 unsigned rel_entsize = _w(relhdr->sh_entsize);
417 unsigned const nrel = _w(relhdr->sh_size) / rel_entsize;
418 unsigned mcountsym = 0;
419 unsigned t;
420
421 get_sym_str_and_relp(relhdr, ehdr, &sym0, &str0, &relp);
422
423 for (t = nrel; t; --t) {
424 int ret = -1;
425 if (!mcountsym)
426 mcountsym = get_mcountsym(sym0, relp, str0);
427
428 if (mcountsym && mcountsym == Elf_r_sym(relp) &&
429 !is_fake_mcount(relp)) {
430 uint_t const addend =
431 _w(_w(relp->r_offset) - recval + mcount_adjust);
432 mrelp->r_offset = _w(offbase
433 + ((void *)mlocp - (void *)mloc0));
434 Elf_r_info(mrelp, recsym, reltype);
435 if (rel_entsize == sizeof(Elf_Rela)) {
436 ((Elf_Rela *)mrelp)->r_addend = addend;
437 *mlocp++ = 0;
438 } else
439 *mlocp++ = addend;
440
441 mrelp = (Elf_Rel *)(rel_entsize + (void *)mrelp);
442 /* convert mcount into nop */
443 if (make_nop)
444 ret = make_nop((void *)ehdr,
445 _w(shdr->sh_offset) + _w(relp->r_offset));
446 if (!ret) {
447 Elf_Rel rel;
448 rel = *(Elf_Rel *)relp;
449 Elf_r_info(&rel, Elf_r_sym(relp), rel_type_nop);
> 450 ulseek(fd_map, (void *)relp - (void *)ehdr, SEEK_SET);
> 451 uwrite(fd_map, &rel, sizeof(rel));
452 }
453 }
454 relp = (Elf_Rel const *)(rel_entsize + (void *)relp);
455 }
456 *mrelpp = mrelp;
457 return mlocp;
458 }
459

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/[email protected]

Attachments:

2021-09-11 18:07:10

by kernel test robot

Subject: Re: [PATCH 2/2] scripts: ftrace - move the nop-processing in ftrace_init to compile time

Hi Yinan,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on tip/perf/core]
[also build test ERROR on kbuild/for-next trace/for-next linus/master v5.14 next-20210910]
[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]

url: https://github.com/0day-ci/linux/commits/Yinan-Liu/ftrace-improve-ftrace-during-compiling/20210911-215230
base: https://git.kernel.org/pub/scm/linux/kernel/git/tip/tip.git 4034fb207e302cc0b1f304084d379640c1fb1436
config: powerpc-randconfig-r023-20210911 (attached as .config)
compiler: clang version 14.0.0 (https://github.com/llvm/llvm-project 261cbe98c38f8c1ee1a482fe76511110e790f58a)
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
# install powerpc cross compiling tool for clang build
# apt-get install binutils-powerpc-linux-gnu
# https://github.com/0day-ci/linux/commit/54d68f2f4f2f10bb9939f8f532615295bf52ce96
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review Yinan-Liu/ftrace-improve-ftrace-during-compiling/20210911-215230
git checkout 54d68f2f4f2f10bb9939f8f532615295bf52ce96
# save the attached .config to linux build tree
mkdir build_dir
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross O=build_dir ARCH=powerpc olddefconfig

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <[email protected]>

All errors (new ones prefixed by >>):

In file included from scripts/sorttable.c:191:
scripts/sorttable.h:372:6: warning: variable 'mcount_sort_thread' is used uninitialized whenever 'if' condition is true [-Wsometimes-uninitialized]
if (!mstruct.init_data_sec || !_start_mcount_loc || !_stop_mcount_loc) {
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
scripts/sorttable.h:469:6: note: uninitialized use occurs here
if (mcount_sort_thread) {
^~~~~~~~~~~~~~~~~~
scripts/sorttable.h:372:2: note: remove the 'if' if its condition is always false
if (!mstruct.init_data_sec || !_start_mcount_loc || !_stop_mcount_loc) {
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
scripts/sorttable.h:372:6: warning: variable 'mcount_sort_thread' is used uninitialized whenever '||' condition is true [-Wsometimes-uninitialized]
if (!mstruct.init_data_sec || !_start_mcount_loc || !_stop_mcount_loc) {
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
scripts/sorttable.h:469:6: note: uninitialized use occurs here
if (mcount_sort_thread) {
^~~~~~~~~~~~~~~~~~
scripts/sorttable.h:372:6: note: remove the '||' if its condition is always false
if (!mstruct.init_data_sec || !_start_mcount_loc || !_stop_mcount_loc) {
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
scripts/sorttable.h:372:6: warning: variable 'mcount_sort_thread' is used uninitialized whenever '||' condition is true [-Wsometimes-uninitialized]
if (!mstruct.init_data_sec || !_start_mcount_loc || !_stop_mcount_loc) {
^~~~~~~~~~~~~~~~~~~~~~
scripts/sorttable.h:469:6: note: uninitialized use occurs here
if (mcount_sort_thread) {
^~~~~~~~~~~~~~~~~~
scripts/sorttable.h:372:6: note: remove the '||' if its condition is always false
if (!mstruct.init_data_sec || !_start_mcount_loc || !_stop_mcount_loc) {
^~~~~~~~~~~~~~~~~~~~~~~~~
scripts/sorttable.h:286:30: note: initialize the variable 'mcount_sort_thread' to silence this warning
pthread_t mcount_sort_thread;
^
= 0
In file included from scripts/recordmcount.c:423:
>> scripts/recordmcount.h:450:49: error: too many arguments to function call, expected 2, have 3
ulseek(fd_map, (void *)relp - (void *)ehdr, SEEK_SET);
~~~~~~ ^~~~~~~~
/usr/include/stdio.h:109:18: note: expanded from macro 'SEEK_SET'
#define SEEK_SET 0 /* Seek from beginning of file. */
^
scripts/recordmcount.c:81:14: note: 'ulseek' declared here
static off_t ulseek(off_t const offset, int const whence)
^
In file included from scripts/sorttable.c:193:
scripts/sorttable.h:372:6: warning: variable 'mcount_sort_thread' is used uninitialized whenever 'if' condition is true [-Wsometimes-uninitialized]
if (!mstruct.init_data_sec || !_start_mcount_loc || !_stop_mcount_loc) {
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
scripts/sorttable.h:469:6: note: uninitialized use occurs here
if (mcount_sort_thread) {
^~~~~~~~~~~~~~~~~~
scripts/sorttable.h:372:2: note: remove the 'if' if its condition is always false
if (!mstruct.init_data_sec || !_start_mcount_loc || !_stop_mcount_loc) {
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
scripts/sorttable.h:372:6: warning: variable 'mcount_sort_thread' is used uninitialized whenever '||' condition is true [-Wsometimes-uninitialized]
if (!mstruct.init_data_sec || !_start_mcount_loc || !_stop_mcount_loc) {
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
scripts/sorttable.h:469:6: note: uninitialized use occurs here
if (mcount_sort_thread) {
^~~~~~~~~~~~~~~~~~
scripts/sorttable.h:372:6: note: remove the '||' if its condition is always false
if (!mstruct.init_data_sec || !_start_mcount_loc || !_stop_mcount_loc) {
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
scripts/sorttable.h:372:6: warning: variable 'mcount_sort_thread' is used uninitialized whenever '||' condition is true [-Wsometimes-uninitialized]
if (!mstruct.init_data_sec || !_start_mcount_loc || !_stop_mcount_loc) {
^~~~~~~~~~~~~~~~~~~~~~
scripts/sorttable.h:469:6: note: uninitialized use occurs here
if (mcount_sort_thread) {
^~~~~~~~~~~~~~~~~~
scripts/sorttable.h:372:6: note: remove the '||' if its condition is always false
if (!mstruct.init_data_sec || !_start_mcount_loc || !_stop_mcount_loc) {
^~~~~~~~~~~~~~~~~~~~~~~~~
scripts/sorttable.h:286:30: note: initialize the variable 'mcount_sort_thread' to silence this warning
pthread_t mcount_sort_thread;
^
= 0
In file included from scripts/recordmcount.c:423:
scripts/recordmcount.h:451:26: error: too many arguments to function call, expected 2, have 3
uwrite(fd_map, &rel, sizeof(rel));
~~~~~~ ^~~~~~~~~~~
scripts/recordmcount.c:101:16: note: 'uwrite' declared here
static ssize_t uwrite(void const *const buf, size_t const count)
^
In file included from scripts/recordmcount.c:425:
>> scripts/recordmcount.h:450:49: error: too many arguments to function call, expected 2, have 3
ulseek(fd_map, (void *)relp - (void *)ehdr, SEEK_SET);
~~~~~~ ^~~~~~~~
/usr/include/stdio.h:109:18: note: expanded from macro 'SEEK_SET'
#define SEEK_SET 0 /* Seek from beginning of file. */
^
scripts/recordmcount.c:81:14: note: 'ulseek' declared here
static off_t ulseek(off_t const offset, int const whence)
^
In file included from scripts/recordmcount.c:425:
scripts/recordmcount.h:451:26: error: too many arguments to function call, expected 2, have 3
uwrite(fd_map, &rel, sizeof(rel));
~~~~~~ ^~~~~~~~~~~
scripts/recordmcount.c:101:16: note: 'uwrite' declared here
static ssize_t uwrite(void const *const buf, size_t const count)
^
4 errors generated.
make[2]: *** [scripts/Makefile.host:95: scripts/recordmcount] Error 1
6 warnings generated.
/usr/bin/ld: /tmp/sorttable-29bd51.o: in function `main':
sorttable.c:(.text+0x716): undefined reference to `pthread_create'
/usr/bin/ld: sorttable.c:(.text+0xb2c): undefined reference to `pthread_create'
/usr/bin/ld: sorttable.c:(.text+0xe90): undefined reference to `pthread_join'
clang-14: error: linker command failed with exit code 1 (use -v to see invocation)
make[2]: *** [scripts/Makefile.host:95: scripts/sorttable] Error 1
make[2]: Target '__build' not remade because of errors.
make[1]: *** [Makefile:1196: scripts] Error 2
make[1]: Target 'modules_prepare' not remade because of errors.
make: *** [Makefile:220: __sub-make] Error 2
make: Target 'modules_prepare' not remade because of errors.
--
scripts/genksyms/parse.y: warning: 9 shift/reduce conflicts [-Wconflicts-sr]
scripts/genksyms/parse.y: warning: 5 reduce/reduce conflicts [-Wconflicts-rr]
In file included from scripts/sorttable.c:191:
scripts/sorttable.h:372:6: warning: variable 'mcount_sort_thread' is used uninitialized whenever 'if' condition is true [-Wsometimes-uninitialized]
if (!mstruct.init_data_sec || !_start_mcount_loc || !_stop_mcount_loc) {
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
scripts/sorttable.h:469:6: note: uninitialized use occurs here
if (mcount_sort_thread) {
^~~~~~~~~~~~~~~~~~
scripts/sorttable.h:372:2: note: remove the 'if' if its condition is always false
if (!mstruct.init_data_sec || !_start_mcount_loc || !_stop_mcount_loc) {
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
scripts/sorttable.h:372:6: warning: variable 'mcount_sort_thread' is used uninitialized whenever '||' condition is true [-Wsometimes-uninitialized]
if (!mstruct.init_data_sec || !_start_mcount_loc || !_stop_mcount_loc) {
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
scripts/sorttable.h:469:6: note: uninitialized use occurs here
if (mcount_sort_thread) {
^~~~~~~~~~~~~~~~~~
scripts/sorttable.h:372:6: note: remove the '||' if its condition is always false
if (!mstruct.init_data_sec || !_start_mcount_loc || !_stop_mcount_loc) {
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
scripts/sorttable.h:372:6: warning: variable 'mcount_sort_thread' is used uninitialized whenever '||' condition is true [-Wsometimes-uninitialized]
if (!mstruct.init_data_sec || !_start_mcount_loc || !_stop_mcount_loc) {
^~~~~~~~~~~~~~~~~~~~~~
scripts/sorttable.h:469:6: note: uninitialized use occurs here
if (mcount_sort_thread) {
^~~~~~~~~~~~~~~~~~
scripts/sorttable.h:372:6: note: remove the '||' if its condition is always false
In file included from scripts/recordmcount.c:423:
>> scripts/recordmcount.h:450:49: error: too many arguments to function call, expected 2, have 3
ulseek(fd_map, (void *)relp - (void *)ehdr, SEEK_SET);
~~~~~~ ^~~~~~~~
/usr/include/stdio.h:109:18: note: expanded from macro 'SEEK_SET'
#define SEEK_SET 0 /* Seek from beginning of file. */
^
scripts/recordmcount.c:81:14: note: 'ulseek' declared here
static off_t ulseek(off_t const offset, int const whence)
^
In file included from scripts/recordmcount.c:423:
scripts/recordmcount.h:451:26: error: too many arguments to function call, expected 2, have 3
uwrite(fd_map, &rel, sizeof(rel));
~~~~~~ ^~~~~~~~~~~
scripts/recordmcount.c:101:16: note: 'uwrite' declared here
static ssize_t uwrite(void const *const buf, size_t const count)
^
if (!mstruct.init_data_sec || !_start_mcount_loc || !_stop_mcount_loc) {
^~~~~~~~~~~~~~~~~~~~~~~~~
scripts/sorttable.h:286:30: note: initialize the variable 'mcount_sort_thread' to silence this warning
pthread_t mcount_sort_thread;
^
= 0
In file included from scripts/sorttable.c:193:
scripts/sorttable.h:372:6: warning: variable 'mcount_sort_thread' is used uninitialized whenever 'if' condition is true [-Wsometimes-uninitialized]
if (!mstruct.init_data_sec || !_start_mcount_loc || !_stop_mcount_loc) {
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
scripts/sorttable.h:469:6: note: uninitialized use occurs here
if (mcount_sort_thread) {
^~~~~~~~~~~~~~~~~~
scripts/sorttable.h:372:2: note: remove the 'if' if its condition is always false
if (!mstruct.init_data_sec || !_start_mcount_loc || !_stop_mcount_loc) {
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
scripts/sorttable.h:372:6: warning: variable 'mcount_sort_thread' is used uninitialized whenever '||' condition is true [-Wsometimes-uninitialized]
if (!mstruct.init_data_sec || !_start_mcount_loc || !_stop_mcount_loc) {
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
scripts/sorttable.h:469:6: note: uninitialized use occurs here
if (mcount_sort_thread) {
^~~~~~~~~~~~~~~~~~
scripts/sorttable.h:372:6: note: remove the '||' if its condition is always false
if (!mstruct.init_data_sec || !_start_mcount_loc || !_stop_mcount_loc) {
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
scripts/sorttable.h:372:6: warning: variable 'mcount_sort_thread' is used uninitialized whenever '||' condition is true [-Wsometimes-uninitialized]
if (!mstruct.init_data_sec || !_start_mcount_loc || !_stop_mcount_loc) {
^~~~~~~~~~~~~~~~~~~~~~
scripts/sorttable.h:469:6: note: uninitialized use occurs here
if (mcount_sort_thread) {
^~~~~~~~~~~~~~~~~~
scripts/sorttable.h:372:6: note: remove the '||' if its condition is always false
if (!mstruct.init_data_sec || !_start_mcount_loc || !_stop_mcount_loc) {
^~~~~~~~~~~~~~~~~~~~~~~~~
scripts/sorttable.h:286:30: note: initialize the variable 'mcount_sort_thread' to silence this warning
pthread_t mcount_sort_thread;
^
= 0
In file included from scripts/recordmcount.c:425:
>> scripts/recordmcount.h:450:49: error: too many arguments to function call, expected 2, have 3
ulseek(fd_map, (void *)relp - (void *)ehdr, SEEK_SET);
~~~~~~ ^~~~~~~~
/usr/include/stdio.h:109:18: note: expanded from macro 'SEEK_SET'
#define SEEK_SET 0 /* Seek from beginning of file. */
^
scripts/recordmcount.c:81:14: note: 'ulseek' declared here
static off_t ulseek(off_t const offset, int const whence)
^
In file included from scripts/recordmcount.c:425:
scripts/recordmcount.h:451:26: error: too many arguments to function call, expected 2, have 3
uwrite(fd_map, &rel, sizeof(rel));
~~~~~~ ^~~~~~~~~~~
scripts/recordmcount.c:101:16: note: 'uwrite' declared here
static ssize_t uwrite(void const *const buf, size_t const count)
^
4 errors generated.
make[2]: *** [scripts/Makefile.host:95: scripts/recordmcount] Error 1
6 warnings generated.
/usr/bin/ld: /tmp/sorttable-df0121.o: in function `main':
sorttable.c:(.text+0x716): undefined reference to `pthread_create'
/usr/bin/ld: sorttable.c:(.text+0xb2c): undefined reference to `pthread_create'
/usr/bin/ld: sorttable.c:(.text+0xe90): undefined reference to `pthread_join'
clang-14: error: linker command failed with exit code 1 (use -v to see invocation)
make[2]: *** [scripts/Makefile.host:95: scripts/sorttable] Error 1
make[2]: Target '__build' not remade because of errors.
make[1]: *** [Makefile:1196: scripts] Error 2
make[1]: Target 'prepare' not remade because of errors.
make: *** [Makefile:220: __sub-make] Error 2
make: Target 'prepare' not remade because of errors.


vim +450 scripts/recordmcount.h

394
395 /*
396 * Look at the relocations in order to find the calls to mcount.
397 * Accumulate the section offsets that are found, and their relocation info,
398 * onto the end of the existing arrays.
399 */
400 static uint_t *sift_rel_mcount(uint_t *mlocp,
401 unsigned const offbase,
402 Elf_Rel **const mrelpp,
403 Elf_Shdr const *const relhdr,
404 Elf_Ehdr const *const ehdr,
405 unsigned const recsym,
406 uint_t const recval,
407 unsigned const reltype)
408 {
409 Elf_Shdr *const shdr0 = (Elf_Shdr *)(_w(ehdr->e_shoff) + (void *)ehdr);
410 Elf_Shdr const *const shdr = &shdr0[w(relhdr->sh_info)];
411 uint_t *const mloc0 = mlocp;
412 Elf_Rel *mrelp = *mrelpp;
413 Elf_Sym const *sym0;
414 char const *str0;
415 Elf_Rel const *relp;
416 unsigned rel_entsize = _w(relhdr->sh_entsize);
417 unsigned const nrel = _w(relhdr->sh_size) / rel_entsize;
418 unsigned mcountsym = 0;
419 unsigned t;
420
421 get_sym_str_and_relp(relhdr, ehdr, &sym0, &str0, &relp);
422
423 for (t = nrel; t; --t) {
424 int ret = -1;
425 if (!mcountsym)
426 mcountsym = get_mcountsym(sym0, relp, str0);
427
428 if (mcountsym && mcountsym == Elf_r_sym(relp) &&
429 !is_fake_mcount(relp)) {
430 uint_t const addend =
431 _w(_w(relp->r_offset) - recval + mcount_adjust);
432 mrelp->r_offset = _w(offbase
433 + ((void *)mlocp - (void *)mloc0));
434 Elf_r_info(mrelp, recsym, reltype);
435 if (rel_entsize == sizeof(Elf_Rela)) {
436 ((Elf_Rela *)mrelp)->r_addend = addend;
437 *mlocp++ = 0;
438 } else
439 *mlocp++ = addend;
440
441 mrelp = (Elf_Rel *)(rel_entsize + (void *)mrelp);
442 /* convert mcount into nop */
443 if (make_nop)
444 ret = make_nop((void *)ehdr,
445 _w(shdr->sh_offset) + _w(relp->r_offset));
446 if (!ret) {
447 Elf_Rel rel;
448 rel = *(Elf_Rel *)relp;
449 Elf_r_info(&rel, Elf_r_sym(relp), rel_type_nop);
> 450 ulseek(fd_map, (void *)relp - (void *)ehdr, SEEK_SET);
451 uwrite(fd_map, &rel, sizeof(rel));
452 }
453 }
454 relp = (Elf_Rel const *)(rel_entsize + (void *)relp);
455 }
456 *mrelpp = mrelp;
457 return mlocp;
458 }
459

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/[email protected]

Attachments:

2021-10-03 13:44:51

by Yinan Liu

Subject: Re: [PATCH 1/2] scripts: ftrace - move the sort-processing in ftrace_init to compile time

在 2021/9/11 下午9:59, Steven Rostedt 写道:
> On Sat, 11 Sep 2021 21:50:42 +0800
> Yinan Liu <[email protected]> wrote:
>
>> When ftrace is enabled, ftrace_init will consume a period of
>> time, usually around 15~20 ms. Approximately 40% of the time is
>> consumed by sort-processing. Moving the sort-processing to the
>> compile time can speed up the kernel boot process.
>>
> Nice. I like the idea of sorting at compile time.
Thanks !
>> performance test:
>> env: Intel(R) Xeon(R) CPU E5-2682 v4 @ 2.50GHz
>> method: before and after patching, compare the
>> total time of ftrace_init(), and verify
>> the functionality of ftrace.
>>
>> avg_time of ftrace_init:
>> with patch: 8.352 ms
>> without patch: 15.763 ms
>>
>> Signed-off-by: Yinan Liu <[email protected]>
>> ---
>> kernel/trace/ftrace.c | 5 ++-
>> scripts/link-vmlinux.sh | 6 +--
>> scripts/sorttable.c | 2 +
>> scripts/sorttable.h | 109 +++++++++++++++++++++++++++++++++++++++++++++++-
>> 4 files changed, 115 insertions(+), 7 deletions(-)
>>
>> diff --git a/kernel/trace/ftrace.c b/kernel/trace/ftrace.c
>> index 7efbc8aaf7f6..c236da868990 100644
>> --- a/kernel/trace/ftrace.c
>> +++ b/kernel/trace/ftrace.c
>> @@ -6189,8 +6189,9 @@ static int ftrace_process_locs(struct module *mod,
>> if (!count)
>> return 0;
>>
>> - sort(start, count, sizeof(*start),
>> - ftrace_cmp_ips, NULL);
>> + if (mod)
> Why can't we enforce modules to be sorted too?

hi，

Sorry for my slow progress . I have encountered some problems with the
sorting
of the module's mcount in compile time. The .ko file will be relocated
after insmod
or modprobe, most of the mcount relocation is based on .text section,
but there are
also a small part of mcount relocation based on .init.text section such
as module_init().
The loading position of .init.text and .text does not seem to be in a
definite order.

For example, when I insmod ip_tables.ko twice, the front and back
positions of init.text
and .text are different, so we cannot sort the mcounts in the two
sections, which makes
the mcount sorting in the module meaningless.

What is your opinion on this?

>> + sort(start, count, sizeof(*start),
>> + ftrace_cmp_ips, NULL);

> Best regards!
> ---Yinan liu

2021-10-08 23:52:49

by Steven Rostedt

Subject: Re: [PATCH 1/2] scripts: ftrace - move the sort-processing in ftrace_init to compile time

On Sun, 3 Oct 2021 21:42:10 +0800
Yinan Liu <[email protected]> wrote:

> Sorry for my slow progress . I have encountered some problems with the
> sorting
> of the module's mcount in compile time. The .ko file will be relocated
> after insmod
> or modprobe, most of the mcount relocation is based on .text section,
> but there are
> also a small part of mcount relocation based on .init.text section such
> as module_init().
> The loading position of .init.text and .text does not seem to be in a
> definite order.

Right, there's no guarantee that the .text portion of a module is
placed before or after the .init.text portion.

>
> For example, when I insmod ip_tables.ko twice, the front and back
> positions of init.text
> and .text are different, so we cannot sort the mcounts in the two
> sections, which makes
> the mcount sorting in the module meaningless.
>
> What is your opinion on this?

Probably just keep the sorting algorithm in the kernel and take place
on module load.

If you still want to sort at compile time, then do the sort for .init
functions separate from the .text ones, and have a way to extract this
information (shouldn't be too hard) in the kernel at module load, and
then just swap the init and text functions if they were added in the
reverse order that was expect.

The functions in .init will either be before all the functions in .text
or after. They wont be intermingled. Thus, if they are both sorted,
then they are placed correctly or the two groups of functions need to
be switched. No other sorting should be necessary.

-- Steve

2021-10-09 02:58:28

by Yinan Liu

Subject: Re: [PATCH 1/2] scripts: ftrace - move the sort-processing in ftrace_init to compile time

On 2021/10/9 上午7:48, Steven Rostedt wrote:
> On Sun, 3 Oct 2021 21:42:10 +0800
> Yinan Liu <[email protected]> wrote:
>
>> Sorry for my slow progress . I have encountered some problems with the
>> sorting
>> of the module's mcount in compile time. The .ko file will be relocated
>> after insmod
>> or modprobe, most of the mcount relocation is based on .text section,
>> but there are
>> also a small part of mcount relocation based on .init.text section such
>> as module_init().
>> The loading position of .init.text and .text does not seem to be in a
>> definite order.
>
> Right, there's no guarantee that the .text portion of a module is
> placed before or after the .init.text portion.
yes.
>
>>
>> For example, when I insmod ip_tables.ko twice, the front and back
>> positions of init.text
>> and .text are different, so we cannot sort the mcounts in the two
>> sections, which makes
>> the mcount sorting in the module meaningless.
>>
>> What is your opinion on this?
>
> Probably just keep the sorting algorithm in the kernel and take place
> on module load.
>
> If you still want to sort at compile time, then do the sort for .init
> functions separate from the .text ones, and have a way to extract this
> information (shouldn't be too hard) in the kernel at module load, and
> then just swap the init and text functions if they were added in the
> reverse order that was expect.
>
> The functions in .init will either be before all the functions in .text
> or after. They wont be intermingled. Thus, if they are both sorted,
> then they are placed correctly or the two groups of functions need to
> be switched. No other sorting should be necessary.
Thanks so much! I see. And I will have a try.
>
> -- Steve
>
Best regards!
-- Yinan liu

2021-10-25 18:09:35

by Yinan Liu

Subject: Re: [PATCH 1/2] scripts: ftrace - move the sort-processing in ftrace_init to compile time

On 2021/10/9 上午10:56, Yinan Liu wrote:

>> Probably just keep the sorting algorithm in the kernel and take place
>> on module load.
>>
>> If you still want to sort at compile time, then do the sort for .init
>> functions separate from the .text ones, and have a way to extract this
>> information (shouldn't be too hard) in the kernel at module load, and
>> then just swap the init and text functions if they were added in the
>> reverse order that was expect.
>>
>> The functions in .init will either be before all the functions in .text
>> or after. They wont be intermingled. Thus, if they are both sorted,
>> then they are placed correctly or the two groups of functions need to
>> be switched. No other sorting should be necessary.
>>

Hi, Steven

I try to apply for memory during the relocation phase to separate and
adjust the mcount of .init.text and .text. After processing, I found
that the mcount redirection in .ko is based on ".text", ".init.text",
".ref.text", ".sched.text", ".spinlock.text", ".irqentry .text",
".softirqentry.text", ".kprobes.text", ".cpuidle.text",
".text.unlikely", which makes the sorting process at compile time
cumbersome and inefficient. And .ko is inserted after the kernel start，
to a certain extent, the optimization meaning of this part is relatively
small. Now I think it makes more sense to optimize mcount during the
startup period, because these mcounts based on different section
redirects are relatively orderly
just like 11, 13, 15, 17, 19, 1, 3, 5, 7, 9, 21, 23, 25, 27, 29
.

At present, it seems that the processing of compile-time sorting is only
suitable for vmlinux but not suitable for modules. Please review the
code of mcount sorting in vmlinux, thank you.

> Best regards!
> -- Yinan liu

2021-10-25 19:03:45

by Steven Rostedt

Subject: Re: [PATCH 1/2] scripts: ftrace - move the sort-processing in ftrace_init to compile time

On Mon, 25 Oct 2021 21:20:47 +0800
Yinan Liu <[email protected]> wrote:

> At present, it seems that the processing of compile-time sorting is only
> suitable for vmlinux but not suitable for modules. Please review the
> code of mcount sorting in vmlinux, thank you.

Agreed that only the vmlinux compile time sorting makes sense, and leave
the sorting of modules to the module load time.

I'll see if I can review your patches this week.

-- Steve

2021-11-16 02:51:45

by Yinan Liu

Subject: [PATCH v2 0/2] ftrace optimization at compile time

Hi,Steven

This is my V2 patch. If you haven't reviewed patch V1 yet,
just review this one. Thank you.

Yinan Liu (2):
scripts: ftrace - move the sort-processing in ftrace_init to compile
time
scripts: ftrace - move the nop-processing in ftrace_init to compile
time

kernel/trace/ftrace.c | 12 +++++-
scripts/link-vmlinux.sh | 6 +--
scripts/recordmcount.h | 14 +++++++
scripts/sorttable.c | 2 +
scripts/sorttable.h | 109 +++++++++++++++++++++++++++++++++++++++++++++++-
5 files changed, 136 insertions(+), 7 deletions(-)

--
2.14.4.44.g2045bb6

2021-11-16 02:51:56

by Yinan Liu

Subject: [PATCH v2 1/2] scripts: ftrace - move the sort-processing in ftrace_init to compile time

When the kernel starts, the initialization of fTrace takes
up a portion of the time (approximately 6~8ms) to sort mcount
addresses. We can save this time by moving mcount-sorting to
compile time.

After communication with Patch V1, it is concluded that it is
more meaningful to sort mcount addresses in vmlinux during
compilation, which is not suitable for kernel module at present.
Patch V2 has adjusted some variable names and comments.

Signed-off-by: Yinan Liu <[email protected]>
---
kernel/trace/ftrace.c | 5 ++-
scripts/link-vmlinux.sh | 6 +--
scripts/sorttable.c | 2 +
scripts/sorttable.h | 109 +++++++++++++++++++++++++++++++++++++++++++++++-
4 files changed, 115 insertions(+), 7 deletions(-)

diff --git a/kernel/trace/ftrace.c b/kernel/trace/ftrace.c
index 30bc880c3849..c776a2956237 100644
--- a/kernel/trace/ftrace.c
+++ b/kernel/trace/ftrace.c
@@ -6406,8 +6406,9 @@ static int ftrace_process_locs(struct module *mod,
if (!count)
return 0;

- sort(start, count, sizeof(*start),
- ftrace_cmp_ips, NULL);
+ if (mod)
+ sort(start, count, sizeof(*start),
+ ftrace_cmp_ips, NULL);

start_pg = ftrace_allocate_pages(count);
if (!start_pg)
diff --git a/scripts/link-vmlinux.sh b/scripts/link-vmlinux.sh
index 5cdd9bc5c385..dd9955f45774 100755
--- a/scripts/link-vmlinux.sh
+++ b/scripts/link-vmlinux.sh
@@ -400,6 +400,9 @@ if [ -n "${CONFIG_DEBUG_INFO_BTF}" -a -n "${CONFIG_BPF}" ]; then
${RESOLVE_BTFIDS} vmlinux
fi

+info SYSMAP System.map
+mksysmap vmlinux System.map
+
if [ -n "${CONFIG_BUILDTIME_TABLE_SORT}" ]; then
info SORTTAB vmlinux
if ! sorttable vmlinux; then
@@ -408,9 +411,6 @@ if [ -n "${CONFIG_BUILDTIME_TABLE_SORT}" ]; then
fi
fi

-info SYSMAP System.map
-mksysmap vmlinux System.map
-
# step a (see comment above)
if [ -n "${CONFIG_KALLSYMS}" ]; then
mksysmap ${kallsyms_vmlinux} .tmp_System.map
diff --git a/scripts/sorttable.c b/scripts/sorttable.c
index b7c2ad71f9cf..70bdc787ddfb 100644
--- a/scripts/sorttable.c
+++ b/scripts/sorttable.c
@@ -30,6 +30,8 @@
#include <stdlib.h>
#include <string.h>
#include <unistd.h>
+#include <errno.h>
+#include <pthread.h>

#include <tools/be_byteshift.h>
#include <tools/le_byteshift.h>
diff --git a/scripts/sorttable.h b/scripts/sorttable.h
index a2baa2fefb13..0743fb58812d 100644
--- a/scripts/sorttable.h
+++ b/scripts/sorttable.h
@@ -19,6 +19,9 @@

#undef extable_ent_size
#undef compare_extable
+#undef get_mcount_loc
+#undef sort_mcount_loc
+#undef elf_mcount_loc
#undef do_sort
#undef Elf_Addr
#undef Elf_Ehdr
@@ -41,6 +44,9 @@
#ifdef SORTTABLE_64
# define extable_ent_size 16
# define compare_extable compare_extable_64
+# define get_mcount_loc get_mcount_loc_64
+# define sort_mcount_loc sort_mcount_loc_64
+# define elf_mcount_loc elf_mcount_loc_64
# define do_sort do_sort_64
# define Elf_Addr Elf64_Addr
# define Elf_Ehdr Elf64_Ehdr
@@ -62,6 +68,9 @@
#else
# define extable_ent_size 8
# define compare_extable compare_extable_32
+# define get_mcount_loc get_mcount_loc_32
+# define sort_mcount_loc sort_mcount_loc_32
+# define elf_mcount_loc elf_mcount_loc_32
# define do_sort do_sort_32
# define Elf_Addr Elf32_Addr
# define Elf_Ehdr Elf32_Ehdr
@@ -84,8 +93,6 @@

#if defined(SORTTABLE_64) && defined(UNWINDER_ORC_ENABLED)
/* ORC unwinder only support X86_64 */
-#include <errno.h>
-#include <pthread.h>
#include <asm/orc_types.h>

#define ERRSTR_MAXSZ 256
@@ -191,6 +198,62 @@ static int compare_extable(const void *a, const void *b)
return 1;
return 0;
}
+struct elf_mcount_loc {
+ Elf_Ehdr *ehdr;
+ Elf_Shdr *init_data_sec;
+ uint_t start_mcount_loc;
+ uint_t stop_mcount_loc;
+};
+
+/* Sort the addresses stored between __start_mcount_loc to __stop_mcount_loc in vmlinux */
+static void *sort_mcount_loc(void *arg)
+{
+ struct elf_mcount_loc *emloc = (struct elf_mcount_loc *)arg;
+ uint_t offset = emloc->start_mcount_loc - _r(&(emloc->init_data_sec)->sh_addr)
+ + _r(&(emloc->init_data_sec)->sh_offset);
+ uint_t count = emloc->stop_mcount_loc - emloc->start_mcount_loc;
+ unsigned char *start_loc = (void *)emloc->ehdr + offset;
+
+ qsort(start_loc, count/sizeof(uint_t), sizeof(uint_t), compare_extable);
+ return NULL;
+}
+
+/* Get the address of __start_mcount_loc and __stop_mcount_loc in System.map */
+static void get_mcount_loc(uint_t *_start, uint_t *_stop)
+{
+ FILE *file_start, *file_stop;
+ char start_buff[20];
+ char stop_buff[20];
+ int len = 0;
+
+ file_start = popen(" grep start_mcount System.map | awk '{print $1}' ", "r");
+ if (!file_start) {
+ fprintf(stderr, "get start_mcount_loc error!");
+ return;
+ }
+
+ file_stop = popen(" grep stop_mcount System.map | awk '{print $1}' ", "r");
+ if (!file_stop) {
+ fprintf(stderr, "get stop_mcount_loc error!");
+ pclose(file_start);
+ return;
+ }
+
+ while (fgets(start_buff, sizeof(start_buff), file_start) != NULL) {
+ len = strlen(start_buff);
+ start_buff[len - 1] = '\0';
+ }
+ *_start = strtoul(start_buff, NULL, 16);
+
+ while (fgets(stop_buff, sizeof(stop_buff), file_stop) != NULL) {
+ len = strlen(stop_buff);
+ stop_buff[len - 1] = '\0';
+ }
+ *_stop = strtoul(stop_buff, NULL, 16);
+
+ pclose(file_start);
+ pclose(file_stop);
+}

static int do_sort(Elf_Ehdr *ehdr,
char const *const fname,
@@ -217,6 +280,10 @@ static int do_sort(Elf_Ehdr *ehdr,
int idx;
unsigned int shnum;
unsigned int shstrndx;
+ struct elf_mcount_loc mstruct;
+ uint_t _start_mcount_loc = 0;
+ uint_t _stop_mcount_loc = 0;
+ pthread_t mcount_sort_thread;
#if defined(SORTTABLE_64) && defined(UNWINDER_ORC_ENABLED)
unsigned int orc_ip_size = 0;
unsigned int orc_size = 0;
@@ -253,6 +320,14 @@ static int do_sort(Elf_Ehdr *ehdr,
symtab_shndx = (Elf32_Word *)((const char *)ehdr +
_r(&s->sh_offset));

+ /* locate the .init.data section in vmlinux */
+ if (!strcmp(secstrings + idx, ".init.data")) {
+ get_mcount_loc(&_start_mcount_loc, &_stop_mcount_loc);
+ mstruct.ehdr = ehdr;
+ mstruct.init_data_sec = s;
+ mstruct.start_mcount_loc = _start_mcount_loc;
+ mstruct.stop_mcount_loc = _stop_mcount_loc;
+ }
#if defined(SORTTABLE_64) && defined(UNWINDER_ORC_ENABLED)
/* locate the ORC unwind tables */
if (!strcmp(secstrings + idx, ".orc_unwind_ip")) {
@@ -294,6 +369,21 @@ static int do_sort(Elf_Ehdr *ehdr,
goto out;
}
#endif
+ if (!mstruct.init_data_sec || !_start_mcount_loc || !_stop_mcount_loc) {
+ fprintf(stderr,
+ "incomplete mcount's sort in file: %s\n",
+ fname);
+ goto out;
+ }
+
+ /* create thread to sort mcount_loc concurrently */
+ if (pthread_create(&mcount_sort_thread, NULL, &sort_mcount_loc, &mstruct)) {
+ fprintf(stderr,
+ "pthread_create mcount_sort_thread failed '%s': %s\n",
+ strerror(errno), fname);
+ goto out;
+ }
+
if (!extab_sec) {
fprintf(stderr, "no __ex_table in file: %s\n", fname);
goto out;
@@ -376,5 +466,20 @@ static int do_sort(Elf_Ehdr *ehdr,
}
}
#endif
+ if (mcount_sort_thread) {
+ void *retval = NULL;
+ /* wait for mcount sort done */
+ rc = pthread_join(mcount_sort_thread, &retval);
+ if (rc)
+ fprintf(stderr,
+ "pthread_join failed '%s': %s\n",
+ strerror(errno), fname);
+ else if (retval) {
+ rc = -1;
+ fprintf(stderr,
+ "failed to sort mcount '%s': %s\n",
+ (char *)retval, fname);
+ }
+ }
return rc;
}
--
2.14.4.44.g2045bb6

2021-11-16 02:52:19

by Yinan Liu

Subject: [PATCH v2 2/2] scripts: ftrace - move the nop-processing in ftrace_init to compile time

In some business scenarios, GCC versions are so old that
optimizations in ftrace cannot be completed, such as
-mrecord-mcount and -mnop-mcount. The recordmCount in the
kernel is actually used. In this case, ftrace_init will
consume a period of time, usually around 9~12ms. Do nop
substitution in recordmcount.c to speed up ftrace_init.

Signed-off-by: Yinan Liu <[email protected]>
---
kernel/trace/ftrace.c | 7 +++++++
scripts/recordmcount.h | 14 ++++++++++++++
2 files changed, 21 insertions(+)

diff --git a/kernel/trace/ftrace.c b/kernel/trace/ftrace.c
index c776a2956237..ccc690e81556 100644
--- a/kernel/trace/ftrace.c
+++ b/kernel/trace/ftrace.c
@@ -6478,6 +6478,13 @@ static int ftrace_process_locs(struct module *mod,
* until we are finished with it, and there's no
* reason to cause large interrupt latencies while we do it.
*/
+#ifdef CONFIG_FTRACE_MCOUNT_USE_RECORDMCOUNT
+#ifdef CONFIG_HAVE_C_RECORDMCOUNT
+ ret = 0;
+ goto out;
+#endif
+#endif
+
if (!mod)
local_irq_save(flags);
ftrace_update_code(mod, start_pg);
diff --git a/scripts/recordmcount.h b/scripts/recordmcount.h
index 1e9baa5c4fc6..152311639a0b 100644
--- a/scripts/recordmcount.h
+++ b/scripts/recordmcount.h
@@ -406,6 +406,8 @@ static uint_t *sift_rel_mcount(uint_t *mlocp,
uint_t const recval,
unsigned const reltype)
{
+ Elf_Shdr *const shdr0 = (Elf_Shdr *)(_w(ehdr->e_shoff) + (void *)ehdr);
+ Elf_Shdr const *const shdr = &shdr0[w(relhdr->sh_info)];
uint_t *const mloc0 = mlocp;
Elf_Rel *mrelp = *mrelpp;
Elf_Sym const *sym0;
@@ -419,6 +421,7 @@ static uint_t *sift_rel_mcount(uint_t *mlocp,
get_sym_str_and_relp(relhdr, ehdr, &sym0, &str0, &relp);

for (t = nrel; t; --t) {
+ int ret = -1;
if (!mcountsym)
mcountsym = get_mcountsym(sym0, relp, str0);

@@ -436,6 +439,17 @@ static uint_t *sift_rel_mcount(uint_t *mlocp,
*mlocp++ = addend;

mrelp = (Elf_Rel *)(rel_entsize + (void *)mrelp);
+ /* convert mcount into nop */
+ if (make_nop)
+ ret = make_nop((void *)ehdr,
+ _w(shdr->sh_offset) + _w(relp->r_offset));
+ if (!ret) {
+ Elf_Rel rel;
+ rel = *(Elf_Rel *)relp;
+ Elf_r_info(&rel, Elf_r_sym(relp), rel_type_nop);
+ ulseek(fd_map, (void *)relp - (void *)ehdr, SEEK_SET);
+ uwrite(fd_map, &rel, sizeof(rel));
+ }
}
relp = (Elf_Rel const *)(rel_entsize + (void *)relp);
}
--
2.14.4.44.g2045bb6

2021-11-16 08:07:59

by Peter Zijlstra

Subject: Re: [PATCH v2 1/2] scripts: ftrace - move the sort-processing in ftrace_init to compile time

On Tue, Nov 16, 2021 at 10:49:41AM +0800, Yinan Liu wrote:

> diff --git a/kernel/trace/ftrace.c b/kernel/trace/ftrace.c
> index 30bc880c3849..c776a2956237 100644
> --- a/kernel/trace/ftrace.c
> +++ b/kernel/trace/ftrace.c
> @@ -6406,8 +6406,9 @@ static int ftrace_process_locs(struct module *mod,
> if (!count)
> return 0;
>
> - sort(start, count, sizeof(*start),
> - ftrace_cmp_ips, NULL);
> + if (mod)
> + sort(start, count, sizeof(*start),
> + ftrace_cmp_ips, NULL);
>

/me hands a bucket of {} your way. Also, can't sorttable be ran on
modules ?

> @@ -376,5 +466,20 @@ static int do_sort(Elf_Ehdr *ehdr,
> }
> }
> #endif
> + if (mcount_sort_thread) {
> + void *retval = NULL;
> + /* wait for mcount sort done */
> + rc = pthread_join(mcount_sort_thread, &retval);
> + if (rc)
> + fprintf(stderr,
> + "pthread_join failed '%s': %s\n",
> + strerror(errno), fname);

Use some here too :-)

> + else if (retval) {
> + rc = -1;
> + fprintf(stderr,
> + "failed to sort mcount '%s': %s\n",
> + (char *)retval, fname);
> + }
> + }
> return rc;
> }

2021-11-16 08:10:35

by Peter Zijlstra

Subject: Re: [PATCH v2 2/2] scripts: ftrace - move the nop-processing in ftrace_init to compile time

On Tue, Nov 16, 2021 at 10:49:42AM +0800, Yinan Liu wrote:
> In some business scenarios, GCC versions are so old that
> optimizations in ftrace cannot be completed, such as
> -mrecord-mcount and -mnop-mcount. The recordmCount in the
> kernel is actually used. In this case, ftrace_init will
> consume a period of time, usually around 9~12ms. Do nop
> substitution in recordmcount.c to speed up ftrace_init.

I really don't buy this.. if you can build a fresh kernel, you can
install a fresh gcc too -- and if you care about performance that's a
very good idea anyway.

2021-11-16 12:42:35

by Yinan Liu

Subject: Re: [PATCH v2 1/2] scripts: ftrace - move the sort-processing in ftrace_init to compile time

在 2021/11/16 下午4:07, Peter Zijlstra 写道:

> /me hands a bucket of {} your way.

--- a/kernel/trace/ftrace.c
+++ b/kernel/trace/ftrace.c
@@ -6406,8 +6406,10 @@ static int ftrace_process_locs(struct module *mod,
if (!count)
return 0;

- sort(start, count, sizeof(*start),
- ftrace_cmp_ips, NULL);
+ if (mod) {
+ sort(start, count, sizeof(*start),
+ ftrace_cmp_ips, NULL);
+ }

hi，peter

you mean like this? I hope I'm not mistaken.


> Also, can't sorttable be ran on modules ?

The .ko file will be relocated after insmod or modprobe.
And the mcount redirection in .ko is based on ".text",
".init.text", ".ref.text", ".sched.text", ".spinlock.text",
".irqentry .text", ".softirqentry.text", ".kprobes.text",
".cpuidle.text", ".text.unlikely". These sections‘ loading
position are not in definite order.

So sorting this part at compile time doesn't make much sense.




Best regards!
--Yinan liu

2021-11-16 12:51:26

by Yinan Liu

Subject: Re: [PATCH v2 2/2] scripts: ftrace - move the nop-processing in ftrace_init to compile time

在 2021/11/16 下午4:10, Peter Zijlstra 写道:
> On Tue, Nov 16, 2021 at 10:49:42AM +0800, Yinan Liu wrote:
>> In some business scenarios, GCC versions are so old that
>> optimizations in ftrace cannot be completed, such as
>> -mrecord-mcount and -mnop-mcount. The recordmCount in the
>> kernel is actually used. In this case, ftrace_init will
>> consume a period of time, usually around 9~12ms. Do nop
>> substitution in recordmcount.c to speed up ftrace_init.
>
> I really don't buy this.. if you can build a fresh kernel, you can
> install a fresh gcc too -- and if you care about performance that's a
> very good idea anyway.
>
Thanks! You are right. We should get rid of the burden.

2021-11-16 13:06:15

by Peter Zijlstra

Subject: Re: [PATCH v2 1/2] scripts: ftrace - move the sort-processing in ftrace_init to compile time

On Tue, Nov 16, 2021 at 08:42:28PM +0800, Yinan Liu wrote:
>
>
> 在 2021/11/16 下午4:07, Peter Zijlstra 写道:
>
> > /me hands a bucket of {} your way.
>
> --- a/kernel/trace/ftrace.c
> +++ b/kernel/trace/ftrace.c
> @@ -6406,8 +6406,10 @@ static int ftrace_process_locs(struct module *mod,
> if (!count)
> return 0;
>
> - sort(start, count, sizeof(*start),
> - ftrace_cmp_ips, NULL);
> + if (mod) {
> + sort(start, count, sizeof(*start),
> + ftrace_cmp_ips, NULL);
> + }
>
> hi，peter
>
> you mean like this? I hope I'm not mistaken.

Exactly.

>
> > Also, can't sorttable be ran on modules ?
>
> The .ko file will be relocated after insmod or modprobe.
> And the mcount redirection in .ko is based on ".text",
> ".init.text", ".ref.text", ".sched.text", ".spinlock.text",
> ".irqentry .text", ".softirqentry.text", ".kprobes.text", ".cpuidle.text",
> ".text.unlikely". These sections‘ loading
> position are not in definite order.
>
> So sorting this part at compile time doesn't make much sense.

Bah.. I thought the sections would retain relative position at least,
but alas. if that isn't done you're quite right that sorting seems
pointless.

2021-11-16 13:08:02

by Steven Rostedt

Subject: Re: [PATCH v2 2/2] scripts: ftrace - move the nop-processing in ftrace_init to compile time

On Tue, 16 Nov 2021 09:10:20 +0100
Peter Zijlstra <[email protected]> wrote:

> On Tue, Nov 16, 2021 at 10:49:42AM +0800, Yinan Liu wrote:
> > In some business scenarios, GCC versions are so old that
> > optimizations in ftrace cannot be completed, such as
> > -mrecord-mcount and -mnop-mcount. The recordmCount in the
> > kernel is actually used. In this case, ftrace_init will
> > consume a period of time, usually around 9~12ms. Do nop
> > substitution in recordmcount.c to speed up ftrace_init.
>
> I really don't buy this.. if you can build a fresh kernel, you can
> install a fresh gcc too -- and if you care about performance that's a
> very good idea anyway.
>

I'm not sure this is true for all archs, is it? That is, is the nop
substitution available in all archs that support mcount updates. Some
(most) archs are special, because they have to deal with link registers and
such.

And because of that, I'm not sure the patch works for all those archs.

-- Steve

2021-11-16 14:46:29

by Yinan Liu

Subject: Re: [PATCH v2 1/2] scripts: ftrace - move the sort-processing in ftrace_init to compile time

>>> Also, can't sorttable be ran on modules ?
>>
>> The .ko file will be relocated after insmod or modprobe.
>> And the mcount redirection in .ko is based on ".text",
>> ".init.text", ".ref.text", ".sched.text", ".spinlock.text",
>> ".irqentry .text", ".softirqentry.text", ".kprobes.text", ".cpuidle.text",
>> ".text.unlikely". These sections‘ loading
>> position are not in definite order.
>>
>> So sorting this part at compile time doesn't make much sense.
>
> Bah.. I thought the sections would retain relative position at least,
> but alas. if that isn't done you're quite right that sorting seems
> pointless.
>

I found the problem when I was sorting mcount in .ko.
Initially I found that some mcount's relocation base on.text
and some base on .init.text.I tried insmod the same .ko several
times. The results show that there is no definite order between
the two sections，and the same situation occurs in several other
mcount base sections.





Best regards!
--Yinan liu

2021-11-16 15:04:15

by Yinan Liu

Subject: Re: [PATCH v2 2/2] scripts: ftrace - move the nop-processing in ftrace_init to compile time

在 2021/11/16 下午9:07, Steven Rostedt 写道:
> On Tue, 16 Nov 2021 09:10:20 +0100
> Peter Zijlstra <[email protected]> wrote:
>
>> On Tue, Nov 16, 2021 at 10:49:42AM +0800, Yinan Liu wrote:
>>> In some business scenarios, GCC versions are so old that
>>> optimizations in ftrace cannot be completed, such as
>>> -mrecord-mcount and -mnop-mcount. The recordmCount in the
>>> kernel is actually used. In this case, ftrace_init will
>>> consume a period of time, usually around 9~12ms. Do nop
>>> substitution in recordmcount.c to speed up ftrace_init.
>>
>> I really don't buy this.. if you can build a fresh kernel, you can
>> install a fresh gcc too -- and if you care about performance that's a
>> very good idea anyway.
>>
>
> I'm not sure this is true for all archs, is it? That is, is the nop
> substitution available in all archs that support mcount updates. Some
> (most) archs are special, because they have to deal with link registers and
> such.
>
> And because of that, I'm not sure the patch works for all those archs.
>
> -- Steve
>

At present, I have only verified it under x86. In other cases, I am
short of arch or scenarios. I am not sure whether the patch is
applicable to all the architectures, perhaps adding something to x86?
Do you think it makes sense?

In fact, many companies cannot upgrade GCC due to online business
reasons. This patch will help.




Best regards!
--Yinan liu

2021-11-16 16:06:29

by Steven Rostedt

Subject: Re: [PATCH v2 2/2] scripts: ftrace - move the nop-processing in ftrace_init to compile time

On Tue, 16 Nov 2021 10:49:42 +0800
Yinan Liu <[email protected]> wrote:

> In some business scenarios, GCC versions are so old that
> optimizations in ftrace cannot be completed, such as
> -mrecord-mcount and -mnop-mcount. The recordmCount in the
> kernel is actually used. In this case, ftrace_init will
> consume a period of time, usually around 9~12ms. Do nop
> substitution in recordmcount.c to speed up ftrace_init.
>
> Signed-off-by: Yinan Liu <[email protected]>
> ---
> kernel/trace/ftrace.c | 7 +++++++
> scripts/recordmcount.h | 14 ++++++++++++++
> 2 files changed, 21 insertions(+)
>
> diff --git a/kernel/trace/ftrace.c b/kernel/trace/ftrace.c
> index c776a2956237..ccc690e81556 100644
> --- a/kernel/trace/ftrace.c
> +++ b/kernel/trace/ftrace.c
> @@ -6478,6 +6478,13 @@ static int ftrace_process_locs(struct module *mod,
> * until we are finished with it, and there's no
> * reason to cause large interrupt latencies while we do it.
> */
> +#ifdef CONFIG_FTRACE_MCOUNT_USE_RECORDMCOUNT
> +#ifdef CONFIG_HAVE_C_RECORDMCOUNT

So the above alone does not guarantee that the callers are converted into
nops. So this is wrong.

There's already logic to convert to nops. But that's done in the
architecture code, not in the generic code.

> + ret = 0;
> + goto out;
> +#endif
> +#endif
> +
> if (!mod)
> local_irq_save(flags);
> ftrace_update_code(mod, start_pg);
> diff --git a/scripts/recordmcount.h b/scripts/recordmcount.h
> index 1e9baa5c4fc6..152311639a0b 100644
> --- a/scripts/recordmcount.h
> +++ b/scripts/recordmcount.h
> @@ -406,6 +406,8 @@ static uint_t *sift_rel_mcount(uint_t *mlocp,
> uint_t const recval,
> unsigned const reltype)
> {
> + Elf_Shdr *const shdr0 = (Elf_Shdr *)(_w(ehdr->e_shoff) + (void *)ehdr);
> + Elf_Shdr const *const shdr = &shdr0[w(relhdr->sh_info)];
> uint_t *const mloc0 = mlocp;
> Elf_Rel *mrelp = *mrelpp;
> Elf_Sym const *sym0;
> @@ -419,6 +421,7 @@ static uint_t *sift_rel_mcount(uint_t *mlocp,
> get_sym_str_and_relp(relhdr, ehdr, &sym0, &str0, &relp);
>
> for (t = nrel; t; --t) {
> + int ret = -1;
> if (!mcountsym)
> mcountsym = get_mcountsym(sym0, relp, str0);
>
> @@ -436,6 +439,17 @@ static uint_t *sift_rel_mcount(uint_t *mlocp,
> *mlocp++ = addend;
>
> mrelp = (Elf_Rel *)(rel_entsize + (void *)mrelp);
> + /* convert mcount into nop */
> + if (make_nop)

Here you even have an if statement that only converts to nops if it is
already done.

-- Steve


> + ret = make_nop((void *)ehdr,
> + _w(shdr->sh_offset) + _w(relp->r_offset));
> + if (!ret) {
> + Elf_Rel rel;
> + rel = *(Elf_Rel *)relp;
> + Elf_r_info(&rel, Elf_r_sym(relp), rel_type_nop);
> + ulseek(fd_map, (void *)relp - (void *)ehdr, SEEK_SET);
> + uwrite(fd_map, &rel, sizeof(rel));
> + }
> }
> relp = (Elf_Rel const *)(rel_entsize + (void *)relp);
> }

2021-11-17 13:35:13

by kernel test robot

Subject: Re: [PATCH v2 1/2] scripts: ftrace - move the sort-processing in ftrace_init to compile time

Hi Yinan,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on rostedt-trace/for-next]
[also build test ERROR on linux/master linus/master v5.16-rc1 next-20211117]
[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]

url: https://github.com/0day-ci/linux/commits/Yinan-Liu/scripts-ftrace-move-the-sort-processing-in-ftrace_init-to-compile-time/20211116-114954
base: https://git.kernel.org/pub/scm/linux/kernel/git/rostedt/linux-trace.git for-next
config: i386-debian-10.3 (attached as .config)
compiler: gcc-9 (Debian 9.3.0-22) 9.3.0
reproduce (this is a W=1 build):
# https://github.com/0day-ci/linux/commit/fe96b08ca13186c5821d3fabf66f4a209be20507
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review Yinan-Liu/scripts-ftrace-move-the-sort-processing-in-ftrace_init-to-compile-time/20211116-114954
git checkout fe96b08ca13186c5821d3fabf66f4a209be20507
# save the attached .config to linux build tree
mkdir build_dir
make W=1 O=build_dir ARCH=i386 prepare

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <[email protected]>

All errors (new ones prefixed by >>):

/usr/bin/ld: /tmp/ccTdfIIm.o: in function `main':
>> sorttable.c:(.text.startup+0xabb): undefined reference to `pthread_create'
>> /usr/bin/ld: sorttable.c:(.text.startup+0xc5a): undefined reference to `pthread_join'
>> /usr/bin/ld: sorttable.c:(.text.startup+0xe34): undefined reference to `pthread_create'
>> collect2: error: ld returned 1 exit status
make[2]: *** [scripts/Makefile.host:95: scripts/sorttable] Error 1
make[2]: Target '__build' not remade because of errors.
make[1]: *** [Makefile:1203: scripts] Error 2
make[1]: Target 'prepare' not remade because of errors.
make: *** [Makefile:219: __sub-make] Error 2
make: Target 'prepare' not remade because of errors.

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/[email protected]

Attachments:

2021-11-22 13:43:51

by Yinan Liu

Subject: [PATCH v3] scripts: ftrace - move the sort-processing in ftrace_init to compile time

Hi,

patch V3 has fixed the issue reported-by robot and adjusted
the formatting problem again.

Signed-off-by: Yinan Liu <[email protected]>
Reported-by: kernel test robot <[email protected]>
---
kernel/trace/ftrace.c | 6 ++-
scripts/Makefile | 2 +-
scripts/link-vmlinux.sh | 6 +--
scripts/sorttable.c | 2 +
scripts/sorttable.h | 113 ++++++++++++++++++++++++++++++++++++++++++++++--
5 files changed, 119 insertions(+), 10 deletions(-)

diff --git a/kernel/trace/ftrace.c b/kernel/trace/ftrace.c
index 7b180f6..f5af419 100644
--- a/kernel/trace/ftrace.c
+++ b/kernel/trace/ftrace.c
@@ -6189,8 +6189,10 @@ static int ftrace_process_locs(struct module *mod,
if (!count)
return 0;

- sort(start, count, sizeof(*start),
- ftrace_cmp_ips, NULL);
+ if (mod) {
+ sort(start, count, sizeof(*start),
+ ftrace_cmp_ips, NULL);
+ }

start_pg = ftrace_allocate_pages(count);
if (!start_pg)
diff --git a/scripts/Makefile b/scripts/Makefile
index 9adb6d2..b286e11 100644
--- a/scripts/Makefile
+++ b/scripts/Makefile
@@ -17,6 +17,7 @@ hostprogs-always-$(CONFIG_SYSTEM_EXTRA_CERTIFICATE) += insert-sys-cert
hostprogs-always-$(CONFIG_SYSTEM_REVOCATION_LIST) += extract-cert

HOSTCFLAGS_sorttable.o = -I$(srctree)/tools/include
+HOSTLDLIBS_sorttable = -lpthread
HOSTCFLAGS_asn1_compiler.o = -I$(srctree)/include
HOSTCFLAGS_sign-file.o = $(CRYPTO_CFLAGS)
HOSTLDLIBS_sign-file = $(CRYPTO_LIBS)
@@ -29,7 +30,6 @@ ARCH := x86
endif
HOSTCFLAGS_sorttable.o += -I$(srctree)/tools/arch/x86/include
HOSTCFLAGS_sorttable.o += -DUNWINDER_ORC_ENABLED
-HOSTLDLIBS_sorttable = -lpthread
endif

# The following programs are only built on demand
diff --git a/scripts/link-vmlinux.sh b/scripts/link-vmlinux.sh
index 36ef7b3..e2e1a8f 100755
--- a/scripts/link-vmlinux.sh
+++ b/scripts/link-vmlinux.sh
@@ -422,6 +422,9 @@ if [ -n "${CONFIG_DEBUG_INFO_BTF}" -a -n "${CONFIG_BPF}" ]; then
${RESOLVE_BTFIDS} vmlinux
fi

+info SYSMAP System.map
+mksysmap vmlinux System.map
+
if [ -n "${CONFIG_BUILDTIME_TABLE_SORT}" ]; then
info SORTTAB vmlinux
if ! sorttable vmlinux; then
@@ -430,9 +433,6 @@ if [ -n "${CONFIG_BUILDTIME_TABLE_SORT}" ]; then
fi
fi

-info SYSMAP System.map
-mksysmap vmlinux System.map
-
# step a (see comment above)
if [ -n "${CONFIG_KALLSYMS}" ]; then
mksysmap ${kallsyms_vmlinux} .tmp_System.map
diff --git a/scripts/sorttable.c b/scripts/sorttable.c
index 0ef3abf..11a595c 100644
--- a/scripts/sorttable.c
+++ b/scripts/sorttable.c
@@ -30,6 +30,8 @@
#include <stdlib.h>
#include <string.h>
#include <unistd.h>
+#include <errno.h>
+#include <pthread.h>

#include <tools/be_byteshift.h>
#include <tools/le_byteshift.h>
diff --git a/scripts/sorttable.h b/scripts/sorttable.h
index a2baa2f..1effc08 100644
--- a/scripts/sorttable.h
+++ b/scripts/sorttable.h
@@ -19,6 +19,9 @@

#undef extable_ent_size
#undef compare_extable
+#undef get_mcount_loc
+#undef sort_mcount_loc
+#undef elf_mcount_loc
#undef do_sort
#undef Elf_Addr
#undef Elf_Ehdr
@@ -41,6 +44,9 @@
#ifdef SORTTABLE_64
# define extable_ent_size 16
# define compare_extable compare_extable_64
+# define get_mcount_loc get_mcount_loc_64
+# define sort_mcount_loc sort_mcount_loc_64
+# define elf_mcount_loc elf_mcount_loc_64
# define do_sort do_sort_64
# define Elf_Addr Elf64_Addr
# define Elf_Ehdr Elf64_Ehdr
@@ -62,6 +68,9 @@
#else
# define extable_ent_size 8
# define compare_extable compare_extable_32
+# define get_mcount_loc get_mcount_loc_32
+# define sort_mcount_loc sort_mcount_loc_32
+# define elf_mcount_loc elf_mcount_loc_32
# define do_sort do_sort_32
# define Elf_Addr Elf32_Addr
# define Elf_Ehdr Elf32_Ehdr
@@ -84,8 +93,6 @@

#if defined(SORTTABLE_64) && defined(UNWINDER_ORC_ENABLED)
/* ORC unwinder only support X86_64 */
-#include <errno.h>
-#include <pthread.h>
#include <asm/orc_types.h>

#define ERRSTR_MAXSZ 256
@@ -191,6 +198,62 @@ static int compare_extable(const void *a, const void *b)
return 1;
return 0;
}
+struct elf_mcount_loc {
+ Elf_Ehdr *ehdr;
+ Elf_Shdr *init_data_sec;
+ uint_t start_mcount_loc;
+ uint_t stop_mcount_loc;
+};
+
+/* Sort the addresses stored between __start_mcount_loc to __stop_mcount_loc in vmlinux */
+static void *sort_mcount_loc(void *arg)
+{
+ struct elf_mcount_loc *emloc = (struct elf_mcount_loc *)arg;
+ uint_t offset = emloc->start_mcount_loc - _r(&(emloc->init_data_sec)->sh_addr)
+ + _r(&(emloc->init_data_sec)->sh_offset);
+ uint_t count = emloc->stop_mcount_loc - emloc->start_mcount_loc;
+ unsigned char *start_loc = (void *)emloc->ehdr + offset;
+
+ qsort(start_loc, count/sizeof(uint_t), sizeof(uint_t), compare_extable);
+ return NULL;
+}
+
+/* Get the address of __start_mcount_loc and __stop_mcount_loc in System.map */
+static void get_mcount_loc(uint_t *_start, uint_t *_stop)
+{
+ FILE *file_start, *file_stop;
+ char start_buff[20];
+ char stop_buff[20];
+ int len = 0;
+
+ file_start = popen(" grep start_mcount System.map | awk '{print $1}' ", "r");
+ if (!file_start) {
+ fprintf(stderr, "get start_mcount_loc error!");
+ return;
+ }
+
+ file_stop = popen(" grep stop_mcount System.map | awk '{print $1}' ", "r");
+ if (!file_stop) {
+ fprintf(stderr, "get stop_mcount_loc error!");
+ pclose(file_start);
+ return;
+ }
+
+ while (fgets(start_buff, sizeof(start_buff), file_start) != NULL) {
+ len = strlen(start_buff);
+ start_buff[len - 1] = '\0';
+ }
+ *_start = strtoul(start_buff, NULL, 16);
+
+ while (fgets(stop_buff, sizeof(stop_buff), file_stop) != NULL) {
+ len = strlen(stop_buff);
+ stop_buff[len - 1] = '\0';
+ }
+ *_stop = strtoul(stop_buff, NULL, 16);
+
+ pclose(file_start);
+ pclose(file_stop);
+}

static int do_sort(Elf_Ehdr *ehdr,
char const *const fname,
@@ -217,6 +280,10 @@ static int do_sort(Elf_Ehdr *ehdr,
int idx;
unsigned int shnum;
unsigned int shstrndx;
+ struct elf_mcount_loc mstruct;
+ uint_t _start_mcount_loc = 0;
+ uint_t _stop_mcount_loc = 0;
+ pthread_t mcount_sort_thread;
#if defined(SORTTABLE_64) && defined(UNWINDER_ORC_ENABLED)
unsigned int orc_ip_size = 0;
unsigned int orc_size = 0;
@@ -253,6 +320,14 @@ static int do_sort(Elf_Ehdr *ehdr,
symtab_shndx = (Elf32_Word *)((const char *)ehdr +
_r(&s->sh_offset));

+ /* locate the .init.data section in vmlinux */
+ if (!strcmp(secstrings + idx, ".init.data")) {
+ get_mcount_loc(&_start_mcount_loc, &_stop_mcount_loc);
+ mstruct.ehdr = ehdr;
+ mstruct.init_data_sec = s;
+ mstruct.start_mcount_loc = _start_mcount_loc;
+ mstruct.stop_mcount_loc = _stop_mcount_loc;
+ }
#if defined(SORTTABLE_64) && defined(UNWINDER_ORC_ENABLED)
/* locate the ORC unwind tables */
if (!strcmp(secstrings + idx, ".orc_unwind_ip")) {
@@ -294,6 +369,21 @@ static int do_sort(Elf_Ehdr *ehdr,
goto out;
}
#endif
+ if (!mstruct.init_data_sec || !_start_mcount_loc || !_stop_mcount_loc) {
+ fprintf(stderr,
+ "incomplete mcount's sort in file: %s\n",
+ fname);
+ goto out;
+ }
+
+ /* create thread to sort mcount_loc concurrently */
+ if (pthread_create(&mcount_sort_thread, NULL, &sort_mcount_loc, &mstruct)) {
+ fprintf(stderr,
+ "pthread_create mcount_sort_thread failed '%s': %s\n",
+ strerror(errno), fname);
+ goto out;
+ }
+
if (!extab_sec) {
fprintf(stderr, "no __ex_table in file: %s\n", fname);
goto out;
@@ -364,11 +454,11 @@ static int do_sort(Elf_Ehdr *ehdr,
void *retval = NULL;
/* wait for ORC tables sort done */
rc = pthread_join(orc_sort_thread, &retval);
- if (rc)
+ if (rc) {
fprintf(stderr,
"pthread_join failed '%s': %s\n",
strerror(errno), fname);
- else if (retval) {
+ } else if (retval) {
rc = -1;
fprintf(stderr,
"failed to sort ORC tables '%s': %s\n",
@@ -376,5 +466,20 @@ static int do_sort(Elf_Ehdr *ehdr,
}
}
#endif
+ if (mcount_sort_thread) {
+ void *retval = NULL;
+ /* wait for mcount sort done */
+ rc = pthread_join(mcount_sort_thread, &retval);
+ if (rc) {
+ fprintf(stderr,
+ "pthread_join failed '%s': %s\n",
+ strerror(errno), fname);
+ } else if (retval) {
+ rc = -1;
+ fprintf(stderr,
+ "failed to sort mcount '%s': %s\n",
+ (char *)retval, fname);
+ }
+ }
return rc;
}
--
1.8.3.1

2021-11-23 10:54:10

by Yinan Liu

Subject: [PATCH v4] ftrace sorting optimization changelog

Hi,

I found a code alignment problem in patch v3 and
modified it in patch v4. You can look directly at
patch v4, cover-letter introduces the changes
starting from patch v2.

v2 link:https://lore.kernel.org/all/[email protected]/

v2--->v3(alignment problem)--->v4

Modified the code style issue of if() {} raised by Peter.

Fix the compilation error by changing the position of
the relevant compilation options of pthread, which is
reported by the robot.

The two tabs show differences in vim and git diff.
In patch v3, the display in the patch is directly
adjusted, resulting in misalignment of the format
in the code.
In patch v4, the alignment in the code is maintained,
but the display in the patch looks very awkward, but
this is just the display problem.Both are two tabs.

Best regards!
-- Yinan Liu

Yinan Liu (1):
scripts: ftrace - move the sort-processing in ftrace_init to compile
time

kernel/trace/ftrace.c | 6 ++-
scripts/Makefile | 2 +-
scripts/link-vmlinux.sh | 6 +--
scripts/sorttable.c | 2 +
scripts/sorttable.h | 113 ++++++++++++++++++++++++++++++++++++++++++++++--
5 files changed, 119 insertions(+), 10 deletions(-)

--
1.8.3.1

2021-11-23 10:54:14

by Yinan Liu

Subject: [PATCH v4] scripts: ftrace - move the sort-processing in ftrace_init to compile time

When the kernel starts, the initialization of ftrace takes
up a portion of the time (approximately 6~8ms) to sort mcount
addresses. We can save this time by moving mcount-sorting to
compile time.

Signed-off-by: Yinan Liu <[email protected]>
Reported-by: kernel test robot <[email protected]>
---
kernel/trace/ftrace.c | 6 ++-
scripts/Makefile | 2 +-
scripts/link-vmlinux.sh | 6 +--
scripts/sorttable.c | 2 +
scripts/sorttable.h | 113 ++++++++++++++++++++++++++++++++++++++++++++++--
5 files changed, 119 insertions(+), 10 deletions(-)

diff --git a/kernel/trace/ftrace.c b/kernel/trace/ftrace.c
index 7b180f6..f5af419 100644
--- a/kernel/trace/ftrace.c
+++ b/kernel/trace/ftrace.c
@@ -6189,8 +6189,10 @@ static int ftrace_process_locs(struct module *mod,
if (!count)
return 0;

- sort(start, count, sizeof(*start),
- ftrace_cmp_ips, NULL);
+ if (mod) {
+ sort(start, count, sizeof(*start),
+ ftrace_cmp_ips, NULL);
+ }

start_pg = ftrace_allocate_pages(count);
if (!start_pg)
diff --git a/scripts/Makefile b/scripts/Makefile
index 9adb6d2..b286e11 100644
--- a/scripts/Makefile
+++ b/scripts/Makefile
@@ -17,6 +17,7 @@ hostprogs-always-$(CONFIG_SYSTEM_EXTRA_CERTIFICATE) += insert-sys-cert
hostprogs-always-$(CONFIG_SYSTEM_REVOCATION_LIST) += extract-cert

HOSTCFLAGS_sorttable.o = -I$(srctree)/tools/include
+HOSTLDLIBS_sorttable = -lpthread
HOSTCFLAGS_asn1_compiler.o = -I$(srctree)/include
HOSTCFLAGS_sign-file.o = $(CRYPTO_CFLAGS)
HOSTLDLIBS_sign-file = $(CRYPTO_LIBS)
@@ -29,7 +30,6 @@ ARCH := x86
endif
HOSTCFLAGS_sorttable.o += -I$(srctree)/tools/arch/x86/include
HOSTCFLAGS_sorttable.o += -DUNWINDER_ORC_ENABLED
-HOSTLDLIBS_sorttable = -lpthread
endif

# The following programs are only built on demand
diff --git a/scripts/link-vmlinux.sh b/scripts/link-vmlinux.sh
index 36ef7b3..e2e1a8f 100755
--- a/scripts/link-vmlinux.sh
+++ b/scripts/link-vmlinux.sh
@@ -422,6 +422,9 @@ if [ -n "${CONFIG_DEBUG_INFO_BTF}" -a -n "${CONFIG_BPF}" ]; then
${RESOLVE_BTFIDS} vmlinux
fi

+info SYSMAP System.map
+mksysmap vmlinux System.map
+
if [ -n "${CONFIG_BUILDTIME_TABLE_SORT}" ]; then
info SORTTAB vmlinux
if ! sorttable vmlinux; then
@@ -430,9 +433,6 @@ if [ -n "${CONFIG_BUILDTIME_TABLE_SORT}" ]; then
fi
fi

-info SYSMAP System.map
-mksysmap vmlinux System.map
-
# step a (see comment above)
if [ -n "${CONFIG_KALLSYMS}" ]; then
mksysmap ${kallsyms_vmlinux} .tmp_System.map
diff --git a/scripts/sorttable.c b/scripts/sorttable.c
index 0ef3abf..11a595c 100644
--- a/scripts/sorttable.c
+++ b/scripts/sorttable.c
@@ -30,6 +30,8 @@
#include <stdlib.h>
#include <string.h>
#include <unistd.h>
+#include <errno.h>
+#include <pthread.h>

#include <tools/be_byteshift.h>
#include <tools/le_byteshift.h>
diff --git a/scripts/sorttable.h b/scripts/sorttable.h
index a2baa2f..81dbf37 100644
--- a/scripts/sorttable.h
+++ b/scripts/sorttable.h
@@ -19,6 +19,9 @@

#undef extable_ent_size
#undef compare_extable
+#undef get_mcount_loc
+#undef sort_mcount_loc
+#undef elf_mcount_loc
#undef do_sort
#undef Elf_Addr
#undef Elf_Ehdr
@@ -41,6 +44,9 @@
#ifdef SORTTABLE_64
# define extable_ent_size 16
# define compare_extable compare_extable_64
+# define get_mcount_loc get_mcount_loc_64
+# define sort_mcount_loc sort_mcount_loc_64
+# define elf_mcount_loc elf_mcount_loc_64
# define do_sort do_sort_64
# define Elf_Addr Elf64_Addr
# define Elf_Ehdr Elf64_Ehdr
@@ -62,6 +68,9 @@
#else
# define extable_ent_size 8
# define compare_extable compare_extable_32
+# define get_mcount_loc get_mcount_loc_32
+# define sort_mcount_loc sort_mcount_loc_32
+# define elf_mcount_loc elf_mcount_loc_32
# define do_sort do_sort_32
# define Elf_Addr Elf32_Addr
# define Elf_Ehdr Elf32_Ehdr
@@ -84,8 +93,6 @@

#if defined(SORTTABLE_64) && defined(UNWINDER_ORC_ENABLED)
/* ORC unwinder only support X86_64 */
-#include <errno.h>
-#include <pthread.h>
#include <asm/orc_types.h>

#define ERRSTR_MAXSZ 256
@@ -191,6 +198,62 @@ static int compare_extable(const void *a, const void *b)
return 1;
return 0;
}
+struct elf_mcount_loc {
+ Elf_Ehdr *ehdr;
+ Elf_Shdr *init_data_sec;
+ uint_t start_mcount_loc;
+ uint_t stop_mcount_loc;
+};
+
+/* Sort the addresses stored between __start_mcount_loc to __stop_mcount_loc in vmlinux */
+static void *sort_mcount_loc(void *arg)
+{
+ struct elf_mcount_loc *emloc = (struct elf_mcount_loc *)arg;
+ uint_t offset = emloc->start_mcount_loc - _r(&(emloc->init_data_sec)->sh_addr)
+ + _r(&(emloc->init_data_sec)->sh_offset);
+ uint_t count = emloc->stop_mcount_loc - emloc->start_mcount_loc;
+ unsigned char *start_loc = (void *)emloc->ehdr + offset;
+
+ qsort(start_loc, count/sizeof(uint_t), sizeof(uint_t), compare_extable);
+ return NULL;
+}
+
+/* Get the address of __start_mcount_loc and __stop_mcount_loc in System.map */
+static void get_mcount_loc(uint_t *_start, uint_t *_stop)
+{
+ FILE *file_start, *file_stop;
+ char start_buff[20];
+ char stop_buff[20];
+ int len = 0;
+
+ file_start = popen(" grep start_mcount System.map | awk '{print $1}' ", "r");
+ if (!file_start) {
+ fprintf(stderr, "get start_mcount_loc error!");
+ return;
+ }
+
+ file_stop = popen(" grep stop_mcount System.map | awk '{print $1}' ", "r");
+ if (!file_stop) {
+ fprintf(stderr, "get stop_mcount_loc error!");
+ pclose(file_start);
+ return;
+ }
+
+ while (fgets(start_buff, sizeof(start_buff), file_start) != NULL) {
+ len = strlen(start_buff);
+ start_buff[len - 1] = '\0';
+ }
+ *_start = strtoul(start_buff, NULL, 16);
+
+ while (fgets(stop_buff, sizeof(stop_buff), file_stop) != NULL) {
+ len = strlen(stop_buff);
+ stop_buff[len - 1] = '\0';
+ }
+ *_stop = strtoul(stop_buff, NULL, 16);
+
+ pclose(file_start);
+ pclose(file_stop);
+}

static int do_sort(Elf_Ehdr *ehdr,
char const *const fname,
@@ -217,6 +280,10 @@ static int do_sort(Elf_Ehdr *ehdr,
int idx;
unsigned int shnum;
unsigned int shstrndx;
+ struct elf_mcount_loc mstruct;
+ uint_t _start_mcount_loc = 0;
+ uint_t _stop_mcount_loc = 0;
+ pthread_t mcount_sort_thread;
#if defined(SORTTABLE_64) && defined(UNWINDER_ORC_ENABLED)
unsigned int orc_ip_size = 0;
unsigned int orc_size = 0;
@@ -253,6 +320,14 @@ static int do_sort(Elf_Ehdr *ehdr,
symtab_shndx = (Elf32_Word *)((const char *)ehdr +
_r(&s->sh_offset));

+ /* locate the .init.data section in vmlinux */
+ if (!strcmp(secstrings + idx, ".init.data")) {
+ get_mcount_loc(&_start_mcount_loc, &_stop_mcount_loc);
+ mstruct.ehdr = ehdr;
+ mstruct.init_data_sec = s;
+ mstruct.start_mcount_loc = _start_mcount_loc;
+ mstruct.stop_mcount_loc = _stop_mcount_loc;
+ }
#if defined(SORTTABLE_64) && defined(UNWINDER_ORC_ENABLED)
/* locate the ORC unwind tables */
if (!strcmp(secstrings + idx, ".orc_unwind_ip")) {
@@ -294,6 +369,21 @@ static int do_sort(Elf_Ehdr *ehdr,
goto out;
}
#endif
+ if (!mstruct.init_data_sec || !_start_mcount_loc || !_stop_mcount_loc) {
+ fprintf(stderr,
+ "incomplete mcount's sort in file: %s\n",
+ fname);
+ goto out;
+ }
+
+ /* create thread to sort mcount_loc concurrently */
+ if (pthread_create(&mcount_sort_thread, NULL, &sort_mcount_loc, &mstruct)) {
+ fprintf(stderr,
+ "pthread_create mcount_sort_thread failed '%s': %s\n",
+ strerror(errno), fname);
+ goto out;
+ }
+
if (!extab_sec) {
fprintf(stderr, "no __ex_table in file: %s\n", fname);
goto out;
@@ -364,11 +454,11 @@ static int do_sort(Elf_Ehdr *ehdr,
void *retval = NULL;
/* wait for ORC tables sort done */
rc = pthread_join(orc_sort_thread, &retval);
- if (rc)
+ if (rc) {
fprintf(stderr,
"pthread_join failed '%s': %s\n",
strerror(errno), fname);
- else if (retval) {
+ } else if (retval) {
rc = -1;
fprintf(stderr,
"failed to sort ORC tables '%s': %s\n",
@@ -376,5 +466,20 @@ static int do_sort(Elf_Ehdr *ehdr,
}
}
#endif
+ if (mcount_sort_thread) {
+ void *retval = NULL;
+ /* wait for mcount sort done */
+ rc = pthread_join(mcount_sort_thread, &retval);
+ if (rc) {
+ fprintf(stderr,
+ "pthread_join failed '%s': %s\n",
+ strerror(errno), fname);
+ } else if (retval) {
+ rc = -1;
+ fprintf(stderr,
+ "failed to sort mcount '%s': %s\n",
+ (char *)retval, fname);
+ }
+ }
return rc;
}
--
1.8.3.1

2021-11-29 02:16:07

by Yinan Liu

Subject: Re: [PATCH v4] scripts: ftrace - move the sort-processing in ftrace_init to compile time

在 2021/11/23 下午6:54, Yinan Liu 写道:
> When the kernel starts, the initialization of ftrace takes
> up a portion of the time (approximately 6~8ms) to sort mcount
> addresses. We can save this time by moving mcount-sorting to
> compile time.
>
> Signed-off-by: Yinan Liu <[email protected]>
> Reported-by: kernel test robot <[email protected]>

Hi,

Is there anything wrong with this patch? Or are you too busy
to review? Let me make sure that it has not been forgotten.


Best regards!
-- Yinan Liu

2021-11-29 03:53:43

by Steven Rostedt

Subject: Re: [PATCH v4] scripts: ftrace - move the sort-processing in ftrace_init to compile time

On Mon, 29 Nov 2021 10:13:23 +0800
Yinan Liu <[email protected]> wrote:

> Is there anything wrong with this patch? Or are you too busy
> to review? Let me make sure that it has not been forgotten.

FYI, last week had a major US holiday, and things get a bit busy,
before hand.

I haven't had time to look at it, but will try to this week or next.

Cheers,

-- Steve

2021-11-29 06:54:30

by Yinan Liu

Subject: Re: [PATCH v4] scripts: ftrace - move the sort-processing in ftrace_init to compile time

在 2021/11/29 上午11:51, Steven Rostedt 写道:
> On Mon, 29 Nov 2021 10:13:23 +0800
> Yinan Liu <[email protected]> wrote:
>
>> Is there anything wrong with this patch? Or are you too busy
>> to review? Let me make sure that it has not been forgotten.
>
> FYI, last week had a major US holiday, and things get a bit busy,
> before hand.
>
> I haven't had time to look at it, but will try to this week or next.
>

Get it. THX!

cheers～

-- Yinan Liu

2021-11-30 17:08:16

by Steven Rostedt

Subject: Re: [PATCH v4] scripts: ftrace - move the sort-processing in ftrace_init to compile time

On Tue, 23 Nov 2021 18:54:04 +0800
Yinan Liu <[email protected]> wrote:

> When the kernel starts, the initialization of ftrace takes
> up a portion of the time (approximately 6~8ms) to sort mcount
> addresses. We can save this time by moving mcount-sorting to
> compile time.
>

Hmm, this depends on CONFIG_BUILDTIME_TABLE_SORT, and not every
architecture that defines CONFIG_HAVE_DYNAMIC_FTRACE also defines
CONFIG_BUILDTIME_TABLE_SORT. So this will break those architectures.

> Signed-off-by: Yinan Liu <[email protected]>
> Reported-by: kernel test robot <[email protected]>
> ---
> kernel/trace/ftrace.c | 6 ++-
> scripts/Makefile | 2 +-
> scripts/link-vmlinux.sh | 6 +--
> scripts/sorttable.c | 2 +
> scripts/sorttable.h | 113 ++++++++++++++++++++++++++++++++++++++++++++++--
> 5 files changed, 119 insertions(+), 10 deletions(-)
>
> diff --git a/kernel/trace/ftrace.c b/kernel/trace/ftrace.c
> index 7b180f6..f5af419 100644
> --- a/kernel/trace/ftrace.c
> +++ b/kernel/trace/ftrace.c
> @@ -6189,8 +6189,10 @@ static int ftrace_process_locs(struct module *mod,
> if (!count)
> return 0;
>
> - sort(start, count, sizeof(*start),
> - ftrace_cmp_ips, NULL);

Should have a comment:

/* Modules can not be sorted at build time */
> + if (mod) {

And perhaps even add:

if (!IS_ENABLED(CONFIG_BUILDTIME_TABLE_SORT) || mod) {

> + sort(start, count, sizeof(*start),
> + ftrace_cmp_ips, NULL);
> + }
>
> start_pg = ftrace_allocate_pages(count);
> if (!start_pg)
> diff --git a/scripts/Makefile b/scripts/Makefile
> index 9adb6d2..b286e11 100644
> --- a/scripts/Makefile
> +++ b/scripts/Makefile
> @@ -17,6 +17,7 @@ hostprogs-always-$(CONFIG_SYSTEM_EXTRA_CERTIFICATE) += insert-sys-cert
> hostprogs-always-$(CONFIG_SYSTEM_REVOCATION_LIST) += extract-cert
>
> HOSTCFLAGS_sorttable.o = -I$(srctree)/tools/include
> +HOSTLDLIBS_sorttable = -lpthread
> HOSTCFLAGS_asn1_compiler.o = -I$(srctree)/include
> HOSTCFLAGS_sign-file.o = $(CRYPTO_CFLAGS)
> HOSTLDLIBS_sign-file = $(CRYPTO_LIBS)
> @@ -29,7 +30,6 @@ ARCH := x86
> endif
> HOSTCFLAGS_sorttable.o += -I$(srctree)/tools/arch/x86/include
> HOSTCFLAGS_sorttable.o += -DUNWINDER_ORC_ENABLED
> -HOSTLDLIBS_sorttable = -lpthread
> endif
>
> # The following programs are only built on demand
> diff --git a/scripts/link-vmlinux.sh b/scripts/link-vmlinux.sh
> index 36ef7b3..e2e1a8f 100755
> --- a/scripts/link-vmlinux.sh
> +++ b/scripts/link-vmlinux.sh
> @@ -422,6 +422,9 @@ if [ -n "${CONFIG_DEBUG_INFO_BTF}" -a -n "${CONFIG_BPF}" ]; then
> ${RESOLVE_BTFIDS} vmlinux
> fi
>
> +info SYSMAP System.map
> +mksysmap vmlinux System.map
> +
> if [ -n "${CONFIG_BUILDTIME_TABLE_SORT}" ]; then
> info SORTTAB vmlinux
> if ! sorttable vmlinux; then
> @@ -430,9 +433,6 @@ if [ -n "${CONFIG_BUILDTIME_TABLE_SORT}" ]; then
> fi
> fi
>
> -info SYSMAP System.map
> -mksysmap vmlinux System.map
> -
> # step a (see comment above)
> if [ -n "${CONFIG_KALLSYMS}" ]; then
> mksysmap ${kallsyms_vmlinux} .tmp_System.map
> diff --git a/scripts/sorttable.c b/scripts/sorttable.c
> index 0ef3abf..11a595c 100644
> --- a/scripts/sorttable.c
> +++ b/scripts/sorttable.c
> @@ -30,6 +30,8 @@
> #include <stdlib.h>
> #include <string.h>
> #include <unistd.h>
> +#include <errno.h>
> +#include <pthread.h>
>
> #include <tools/be_byteshift.h>
> #include <tools/le_byteshift.h>
> diff --git a/scripts/sorttable.h b/scripts/sorttable.h
> index a2baa2f..81dbf37 100644
> --- a/scripts/sorttable.h
> +++ b/scripts/sorttable.h
> @@ -19,6 +19,9 @@
>
> #undef extable_ent_size
> #undef compare_extable
> +#undef get_mcount_loc
> +#undef sort_mcount_loc
> +#undef elf_mcount_loc
> #undef do_sort
> #undef Elf_Addr
> #undef Elf_Ehdr
> @@ -41,6 +44,9 @@
> #ifdef SORTTABLE_64
> # define extable_ent_size 16
> # define compare_extable compare_extable_64
> +# define get_mcount_loc get_mcount_loc_64
> +# define sort_mcount_loc sort_mcount_loc_64
> +# define elf_mcount_loc elf_mcount_loc_64
> # define do_sort do_sort_64
> # define Elf_Addr Elf64_Addr
> # define Elf_Ehdr Elf64_Ehdr
> @@ -62,6 +68,9 @@
> #else
> # define extable_ent_size 8
> # define compare_extable compare_extable_32
> +# define get_mcount_loc get_mcount_loc_32
> +# define sort_mcount_loc sort_mcount_loc_32
> +# define elf_mcount_loc elf_mcount_loc_32
> # define do_sort do_sort_32
> # define Elf_Addr Elf32_Addr
> # define Elf_Ehdr Elf32_Ehdr
> @@ -84,8 +93,6 @@
>
> #if defined(SORTTABLE_64) && defined(UNWINDER_ORC_ENABLED)
> /* ORC unwinder only support X86_64 */
> -#include <errno.h>
> -#include <pthread.h>
> #include <asm/orc_types.h>
>
> #define ERRSTR_MAXSZ 256
> @@ -191,6 +198,62 @@ static int compare_extable(const void *a, const void *b)
> return 1;
> return 0;
> }
> +struct elf_mcount_loc {
> + Elf_Ehdr *ehdr;
> + Elf_Shdr *init_data_sec;
> + uint_t start_mcount_loc;
> + uint_t stop_mcount_loc;
> +};
> +
> +/* Sort the addresses stored between __start_mcount_loc to __stop_mcount_loc in vmlinux */
> +static void *sort_mcount_loc(void *arg)
> +{
> + struct elf_mcount_loc *emloc = (struct elf_mcount_loc *)arg;
> + uint_t offset = emloc->start_mcount_loc - _r(&(emloc->init_data_sec)->sh_addr)
> + + _r(&(emloc->init_data_sec)->sh_offset);
> + uint_t count = emloc->stop_mcount_loc - emloc->start_mcount_loc;
> + unsigned char *start_loc = (void *)emloc->ehdr + offset;
> +
> + qsort(start_loc, count/sizeof(uint_t), sizeof(uint_t), compare_extable);
> + return NULL;
> +}
> +
> +/* Get the address of __start_mcount_loc and __stop_mcount_loc in System.map */
> +static void get_mcount_loc(uint_t *_start, uint_t *_stop)
> +{
> + FILE *file_start, *file_stop;
> + char start_buff[20];
> + char stop_buff[20];
> + int len = 0;
> +
> + file_start = popen(" grep start_mcount System.map | awk '{print $1}' ", "r");
> + if (!file_start) {
> + fprintf(stderr, "get start_mcount_loc error!");
> + return;
> + }
> +
> + file_stop = popen(" grep stop_mcount System.map | awk '{print $1}' ", "r");
> + if (!file_stop) {
> + fprintf(stderr, "get stop_mcount_loc error!");
> + pclose(file_start);
> + return;
> + }
> +
> + while (fgets(start_buff, sizeof(start_buff), file_start) != NULL) {
> + len = strlen(start_buff);
> + start_buff[len - 1] = '\0';
> + }
> + *_start = strtoul(start_buff, NULL, 16);
> +
> + while (fgets(stop_buff, sizeof(stop_buff), file_stop) != NULL) {
> + len = strlen(stop_buff);
> + stop_buff[len - 1] = '\0';
> + }
> + *_stop = strtoul(stop_buff, NULL, 16);
> +
> + pclose(file_start);
> + pclose(file_stop);
> +}
>
> static int do_sort(Elf_Ehdr *ehdr,
> char const *const fname,
> @@ -217,6 +280,10 @@ static int do_sort(Elf_Ehdr *ehdr,
> int idx;
> unsigned int shnum;
> unsigned int shstrndx;
> + struct elf_mcount_loc mstruct;
> + uint_t _start_mcount_loc = 0;
> + uint_t _stop_mcount_loc = 0;
> + pthread_t mcount_sort_thread;
> #if defined(SORTTABLE_64) && defined(UNWINDER_ORC_ENABLED)
> unsigned int orc_ip_size = 0;
> unsigned int orc_size = 0;
> @@ -253,6 +320,14 @@ static int do_sort(Elf_Ehdr *ehdr,
> symtab_shndx = (Elf32_Word *)((const char *)ehdr +
> _r(&s->sh_offset));
>
> + /* locate the .init.data section in vmlinux */
> + if (!strcmp(secstrings + idx, ".init.data")) {
> + get_mcount_loc(&_start_mcount_loc, &_stop_mcount_loc);
> + mstruct.ehdr = ehdr;
> + mstruct.init_data_sec = s;
> + mstruct.start_mcount_loc = _start_mcount_loc;
> + mstruct.stop_mcount_loc = _stop_mcount_loc;
> + }
> #if defined(SORTTABLE_64) && defined(UNWINDER_ORC_ENABLED)
> /* locate the ORC unwind tables */
> if (!strcmp(secstrings + idx, ".orc_unwind_ip")) {
> @@ -294,6 +369,21 @@ static int do_sort(Elf_Ehdr *ehdr,
> goto out;
> }
> #endif
> + if (!mstruct.init_data_sec || !_start_mcount_loc || !_stop_mcount_loc) {

This is triggered when CONFIG_FUNCTION_TRACER is not defined.

In other words, do not fail if mcount section does not exist.


> + fprintf(stderr,
> + "incomplete mcount's sort in file: %s\n",
> + fname);
> + goto out;
> + }
> +
> + /* create thread to sort mcount_loc concurrently */
> + if (pthread_create(&mcount_sort_thread, NULL, &sort_mcount_loc, &mstruct)) {
> + fprintf(stderr,
> + "pthread_create mcount_sort_thread failed '%s': %s\n",
> + strerror(errno), fname);
> + goto out;
> + }
> +
> if (!extab_sec) {
> fprintf(stderr, "no __ex_table in file: %s\n", fname);
> goto out;
> @@ -364,11 +454,11 @@ static int do_sort(Elf_Ehdr *ehdr,
> void *retval = NULL;
> /* wait for ORC tables sort done */
> rc = pthread_join(orc_sort_thread, &retval);
> - if (rc)
> + if (rc) {
> fprintf(stderr,
> "pthread_join failed '%s': %s\n",
> strerror(errno), fname);
> - else if (retval) {
> + } else if (retval) {
> rc = -1;
> fprintf(stderr,
> "failed to sort ORC tables '%s': %s\n",

Looks like you added parenthesis that was not yours. I would ignore this
for your patch, but feel free to send a separate clean up patch.

> @@ -376,5 +466,20 @@ static int do_sort(Elf_Ehdr *ehdr,
> }
> }
> #endif
> + if (mcount_sort_thread) {
> + void *retval = NULL;
> + /* wait for mcount sort done */
> + rc = pthread_join(mcount_sort_thread, &retval);
> + if (rc) {
> + fprintf(stderr,
> + "pthread_join failed '%s': %s\n",
> + strerror(errno), fname);
> + } else if (retval) {
> + rc = -1;
> + fprintf(stderr,
> + "failed to sort mcount '%s': %s\n",
> + (char *)retval, fname);
> + }
> + }
> return rc;
> }

-- Steve

2021-12-01 05:32:18

by Yinan Liu

Subject: [PATCH v5 0/2] ftrace sorting optimization changelog

Hi,steven

Patch1 added the configuration judgment in build time and boot time.
Patch2 fix code style issue in sorrtable.h.

Best regards!
-- Yinan Liu

Yinan Liu (2):
scripts: ftrace - move the sort-processing in ftrace_init to compile
time
script/sorttable: code style improvements

kernel/trace/ftrace.c | 11 +++-
scripts/Makefile | 2 +-
scripts/link-vmlinux.sh | 6 +-
scripts/sorttable.c | 2 +
scripts/sorttable.h | 124 ++++++++++++++++++++++++++++++++++++++--
5 files changed, 135 insertions(+), 10 deletions(-)

--
2.19.1.6.gb485710b

2021-12-01 05:32:28

by Yinan Liu

Subject: [PATCH v5 2/2] script/sorttable: code style improvements

Modified the code style issue of if() {},
keep the code style consistent.

Signed-off-by: Yinan Liu <[email protected]>
---
scripts/sorttable.h | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/scripts/sorttable.h b/scripts/sorttable.h
index da0911110c2a..1cf711248d60 100644
--- a/scripts/sorttable.h
+++ b/scripts/sorttable.h
@@ -462,11 +462,11 @@ static int do_sort(Elf_Ehdr *ehdr,
void *retval = NULL;
/* wait for ORC tables sort done */
rc = pthread_join(orc_sort_thread, &retval);
- if (rc)
+ if (rc) {
fprintf(stderr,
"pthread_join failed '%s': %s\n",
strerror(errno), fname);
- else if (retval) {
+ } else if (retval) {
rc = -1;
fprintf(stderr,
"failed to sort ORC tables '%s': %s\n",
--
2.19.1.6.gb485710b

2021-12-01 05:32:28

by Yinan Liu

Subject: [PATCH v5 1/2] scripts: ftrace - move the sort-processing in ftrace_init to compile time

When the kernel starts, the initialization of ftrace takes
up a portion of the time (approximately 6~8ms) to sort mcount
addresses. We can save this time by moving mcount-sorting to
compile time.

Signed-off-by: Yinan Liu <[email protected]>
Reported-by: kernel test robot <[email protected]>
---
kernel/trace/ftrace.c | 11 +++-
scripts/Makefile | 2 +-
scripts/link-vmlinux.sh | 6 +-
scripts/sorttable.c | 2 +
scripts/sorttable.h | 120 +++++++++++++++++++++++++++++++++++++++-
5 files changed, 133 insertions(+), 8 deletions(-)

diff --git a/kernel/trace/ftrace.c b/kernel/trace/ftrace.c
index 7b180f61e6d3..c0f95cae68b5 100644
--- a/kernel/trace/ftrace.c
+++ b/kernel/trace/ftrace.c
@@ -6189,8 +6189,15 @@ static int ftrace_process_locs(struct module *mod,
if (!count)
return 0;

- sort(start, count, sizeof(*start),
- ftrace_cmp_ips, NULL);
+ /*
+ * Sorting mcount in vmlinux at build time depend on
+ * CONFIG_BUILDTIME_TABLE_SORT, while mcount loc in
+ * modules can not be sorted at build time.
+ */
+ if (!IS_ENABLED(CONFIG_BUILDTIME_TABLE_SORT) || mod) {
+ sort(start, count, sizeof(*start),
+ ftrace_cmp_ips, NULL);
+ }

start_pg = ftrace_allocate_pages(count);
if (!start_pg)
diff --git a/scripts/Makefile b/scripts/Makefile
index 9adb6d247818..b286e112e3b0 100644
--- a/scripts/Makefile
+++ b/scripts/Makefile
@@ -17,6 +17,7 @@ hostprogs-always-$(CONFIG_SYSTEM_EXTRA_CERTIFICATE) += insert-sys-cert
hostprogs-always-$(CONFIG_SYSTEM_REVOCATION_LIST) += extract-cert

HOSTCFLAGS_sorttable.o = -I$(srctree)/tools/include
+HOSTLDLIBS_sorttable = -lpthread
HOSTCFLAGS_asn1_compiler.o = -I$(srctree)/include
HOSTCFLAGS_sign-file.o = $(CRYPTO_CFLAGS)
HOSTLDLIBS_sign-file = $(CRYPTO_LIBS)
@@ -29,7 +30,6 @@ ARCH := x86
endif
HOSTCFLAGS_sorttable.o += -I$(srctree)/tools/arch/x86/include
HOSTCFLAGS_sorttable.o += -DUNWINDER_ORC_ENABLED
-HOSTLDLIBS_sorttable = -lpthread
endif

# The following programs are only built on demand
diff --git a/scripts/link-vmlinux.sh b/scripts/link-vmlinux.sh
index 36ef7b37fc5d..e2e1a8f39f15 100755
--- a/scripts/link-vmlinux.sh
+++ b/scripts/link-vmlinux.sh
@@ -422,6 +422,9 @@ if [ -n "${CONFIG_DEBUG_INFO_BTF}" -a -n "${CONFIG_BPF}" ]; then
${RESOLVE_BTFIDS} vmlinux
fi

+info SYSMAP System.map
+mksysmap vmlinux System.map
+
if [ -n "${CONFIG_BUILDTIME_TABLE_SORT}" ]; then
info SORTTAB vmlinux
if ! sorttable vmlinux; then
@@ -430,9 +433,6 @@ if [ -n "${CONFIG_BUILDTIME_TABLE_SORT}" ]; then
fi
fi

-info SYSMAP System.map
-mksysmap vmlinux System.map
-
# step a (see comment above)
if [ -n "${CONFIG_KALLSYMS}" ]; then
mksysmap ${kallsyms_vmlinux} .tmp_System.map
diff --git a/scripts/sorttable.c b/scripts/sorttable.c
index 0ef3abfc4a51..11a595ca256b 100644
--- a/scripts/sorttable.c
+++ b/scripts/sorttable.c
@@ -30,6 +30,8 @@
#include <stdlib.h>
#include <string.h>
#include <unistd.h>
+#include <errno.h>
+#include <pthread.h>

#include <tools/be_byteshift.h>
#include <tools/le_byteshift.h>
diff --git a/scripts/sorttable.h b/scripts/sorttable.h
index a2baa2fefb13..da0911110c2a 100644
--- a/scripts/sorttable.h
+++ b/scripts/sorttable.h
@@ -19,6 +19,9 @@

#undef extable_ent_size
#undef compare_extable
+#undef get_mcount_loc
+#undef sort_mcount_loc
+#undef elf_mcount_loc
#undef do_sort
#undef Elf_Addr
#undef Elf_Ehdr
@@ -41,6 +44,9 @@
#ifdef SORTTABLE_64
# define extable_ent_size 16
# define compare_extable compare_extable_64
+# define get_mcount_loc get_mcount_loc_64
+# define sort_mcount_loc sort_mcount_loc_64
+# define elf_mcount_loc elf_mcount_loc_64
# define do_sort do_sort_64
# define Elf_Addr Elf64_Addr
# define Elf_Ehdr Elf64_Ehdr
@@ -62,6 +68,9 @@
#else
# define extable_ent_size 8
# define compare_extable compare_extable_32
+# define get_mcount_loc get_mcount_loc_32
+# define sort_mcount_loc sort_mcount_loc_32
+# define elf_mcount_loc elf_mcount_loc_32
# define do_sort do_sort_32
# define Elf_Addr Elf32_Addr
# define Elf_Ehdr Elf32_Ehdr
@@ -84,8 +93,6 @@

#if defined(SORTTABLE_64) && defined(UNWINDER_ORC_ENABLED)
/* ORC unwinder only support X86_64 */
-#include <errno.h>
-#include <pthread.h>
#include <asm/orc_types.h>

#define ERRSTR_MAXSZ 256
@@ -191,7 +198,64 @@ static int compare_extable(const void *a, const void *b)
return 1;
return 0;
}
+#if defined CONFIG_FUNCTION_TRACER
+struct elf_mcount_loc {
+ Elf_Ehdr *ehdr;
+ Elf_Shdr *init_data_sec;
+ uint_t start_mcount_loc;
+ uint_t stop_mcount_loc;
+};
+
+/* Sort the addresses stored between __start_mcount_loc to __stop_mcount_loc in vmlinux */
+static void *sort_mcount_loc(void *arg)
+{
+ struct elf_mcount_loc *emloc = (struct elf_mcount_loc *)arg;
+ uint_t offset = emloc->start_mcount_loc - _r(&(emloc->init_data_sec)->sh_addr)
+ + _r(&(emloc->init_data_sec)->sh_offset);
+ uint_t count = emloc->stop_mcount_loc - emloc->start_mcount_loc;
+ unsigned char *start_loc = (void *)emloc->ehdr + offset;
+
+ qsort(start_loc, count/sizeof(uint_t), sizeof(uint_t), compare_extable);
+ return NULL;
+}
+
+/* Get the address of __start_mcount_loc and __stop_mcount_loc in System.map */
+static void get_mcount_loc(uint_t *_start, uint_t *_stop)
+{
+ FILE *file_start, *file_stop;
+ char start_buff[20];
+ char stop_buff[20];
+ int len = 0;
+
+ file_start = popen(" grep start_mcount System.map | awk '{print $1}' ", "r");
+ if (!file_start) {
+ fprintf(stderr, "get start_mcount_loc error!");
+ return;
+ }
+
+ file_stop = popen(" grep stop_mcount System.map | awk '{print $1}' ", "r");
+ if (!file_stop) {
+ fprintf(stderr, "get stop_mcount_loc error!");
+ pclose(file_start);
+ return;
+ }
+
+ while (fgets(start_buff, sizeof(start_buff), file_start) != NULL) {
+ len = strlen(start_buff);
+ start_buff[len - 1] = '\0';
+ }
+ *_start = strtoul(start_buff, NULL, 16);
+
+ while (fgets(stop_buff, sizeof(stop_buff), file_stop) != NULL) {
+ len = strlen(stop_buff);
+ stop_buff[len - 1] = '\0';
+ }
+ *_stop = strtoul(stop_buff, NULL, 16);

+ pclose(file_start);
+ pclose(file_stop);
+}
+#endif
static int do_sort(Elf_Ehdr *ehdr,
char const *const fname,
table_sort_t custom_sort)
@@ -217,6 +281,12 @@ static int do_sort(Elf_Ehdr *ehdr,
int idx;
unsigned int shnum;
unsigned int shstrndx;
+#if defined CONFIG_FUNCTION_TRACER
+ struct elf_mcount_loc mstruct;
+ uint_t _start_mcount_loc = 0;
+ uint_t _stop_mcount_loc = 0;
+ pthread_t mcount_sort_thread;
+#endif
#if defined(SORTTABLE_64) && defined(UNWINDER_ORC_ENABLED)
unsigned int orc_ip_size = 0;
unsigned int orc_size = 0;
@@ -253,6 +323,17 @@ static int do_sort(Elf_Ehdr *ehdr,
symtab_shndx = (Elf32_Word *)((const char *)ehdr +
_r(&s->sh_offset));

+#if defined CONFIG_FUNCTION_TRACER
+ /* locate the .init.data section in vmlinux */
+ if (!strcmp(secstrings + idx, ".init.data")) {
+ get_mcount_loc(&_start_mcount_loc, &_stop_mcount_loc);
+ mstruct.ehdr = ehdr;
+ mstruct.init_data_sec = s;
+ mstruct.start_mcount_loc = _start_mcount_loc;
+ mstruct.stop_mcount_loc = _stop_mcount_loc;
+ }
+#endif
+
#if defined(SORTTABLE_64) && defined(UNWINDER_ORC_ENABLED)
/* locate the ORC unwind tables */
if (!strcmp(secstrings + idx, ".orc_unwind_ip")) {
@@ -294,6 +375,23 @@ static int do_sort(Elf_Ehdr *ehdr,
goto out;
}
#endif
+
+#if defined CONFIG_FUNCTION_TRACER
+ if (!mstruct.init_data_sec || !_start_mcount_loc || !_stop_mcount_loc) {
+ fprintf(stderr,
+ "incomplete mcount's sort in file: %s\n",
+ fname);
+ goto out;
+ }
+
+ /* create thread to sort mcount_loc concurrently */
+ if (pthread_create(&mcount_sort_thread, NULL, &sort_mcount_loc, &mstruct)) {
+ fprintf(stderr,
+ "pthread_create mcount_sort_thread failed '%s': %s\n",
+ strerror(errno), fname);
+ goto out;
+ }
+#endif
if (!extab_sec) {
fprintf(stderr, "no __ex_table in file: %s\n", fname);
goto out;
@@ -376,5 +474,23 @@ static int do_sort(Elf_Ehdr *ehdr,
}
}
#endif
+
+#if defined CONFIG_FUNCTION_TRACER
+ if (mcount_sort_thread) {
+ void *retval = NULL;
+ /* wait for mcount sort done */
+ rc = pthread_join(mcount_sort_thread, &retval);
+ if (rc) {
+ fprintf(stderr,
+ "pthread_join failed '%s': %s\n",
+ strerror(errno), fname);
+ } else if (retval) {
+ rc = -1;
+ fprintf(stderr,
+ "failed to sort mcount '%s': %s\n",
+ (char *)retval, fname);
+ }
+ }
+#endif
return rc;
}
--
2.19.1.6.gb485710b

2021-12-01 21:46:28

by Steven Rostedt

Subject: Re: [PATCH v5 1/2] scripts: ftrace - move the sort-processing in ftrace_init to compile time

On Wed, 1 Dec 2021 13:32:06 +0800
Yinan Liu <[email protected]> wrote:

>
> #if defined(SORTTABLE_64) && defined(UNWINDER_ORC_ENABLED)
> /* ORC unwinder only support X86_64 */
> -#include <errno.h>
> -#include <pthread.h>
> #include <asm/orc_types.h>
>
> #define ERRSTR_MAXSZ 256
> @@ -191,7 +198,64 @@ static int compare_extable(const void *a, const void *b)
> return 1;
> return 0;
> }
> +#if defined CONFIG_FUNCTION_TRACER

Note, "defined" is best used with parenthesis, but if it's for only a
single define, we usually just use:

#ifdef CONFIG_FUNCTION_TRACER

Same for all he below.

-- Steve


> +struct elf_mcount_loc {
> + Elf_Ehdr *ehdr;
> + Elf_Shdr *init_data_sec;
> + uint_t start_mcount_loc;
> + uint_t stop_mcount_loc;
> +};
> +
> +/* Sort the addresses stored between __start_mcount_loc to __stop_mcount_loc in vmlinux */
> +static void *sort_mcount_loc(void *arg)
> +{
> + struct elf_mcount_loc *emloc = (struct elf_mcount_loc *)arg;
> + uint_t offset = emloc->start_mcount_loc - _r(&(emloc->init_data_sec)->sh_addr)
> + + _r(&(emloc->init_data_sec)->sh_offset);
> + uint_t count = emloc->stop_mcount_loc - emloc->start_mcount_loc;
> + unsigned char *start_loc = (void *)emloc->ehdr + offset;
> +
> + qsort(start_loc, count/sizeof(uint_t), sizeof(uint_t), compare_extable);
> + return NULL;
> +}
> +
> +/* Get the address of __start_mcount_loc and __stop_mcount_loc in System.map */
> +static void get_mcount_loc(uint_t *_start, uint_t *_stop)
> +{
> + FILE *file_start, *file_stop;
> + char start_buff[20];
> + char stop_buff[20];
> + int len = 0;
> +
> + file_start = popen(" grep start_mcount System.map | awk '{print $1}' ", "r");
> + if (!file_start) {
> + fprintf(stderr, "get start_mcount_loc error!");
> + return;
> + }
> +
> + file_stop = popen(" grep stop_mcount System.map | awk '{print $1}' ", "r");
> + if (!file_stop) {
> + fprintf(stderr, "get stop_mcount_loc error!");
> + pclose(file_start);
> + return;
> + }
> +
> + while (fgets(start_buff, sizeof(start_buff), file_start) != NULL) {
> + len = strlen(start_buff);
> + start_buff[len - 1] = '\0';
> + }
> + *_start = strtoul(start_buff, NULL, 16);
> +
> + while (fgets(stop_buff, sizeof(stop_buff), file_stop) != NULL) {
> + len = strlen(stop_buff);
> + stop_buff[len - 1] = '\0';
> + }
> + *_stop = strtoul(stop_buff, NULL, 16);
>
> + pclose(file_start);
> + pclose(file_stop);
> +}
> +#endif
> static int do_sort(Elf_Ehdr *ehdr,
> char const *const fname,
> table_sort_t custom_sort)
> @@ -217,6 +281,12 @@ static int do_sort(Elf_Ehdr *ehdr,
> int idx;
> unsigned int shnum;
> unsigned int shstrndx;
> +#if defined CONFIG_FUNCTION_TRACER
> + struct elf_mcount_loc mstruct;
> + uint_t _start_mcount_loc = 0;
> + uint_t _stop_mcount_loc = 0;
> + pthread_t mcount_sort_thread;
> +#endif
> #if defined(SORTTABLE_64) && defined(UNWINDER_ORC_ENABLED)
> unsigned int orc_ip_size = 0;
> unsigned int orc_size = 0;
> @@ -253,6 +323,17 @@ static int do_sort(Elf_Ehdr *ehdr,
> symtab_shndx = (Elf32_Word *)((const char *)ehdr +
> _r(&s->sh_offset));
>
> +#if defined CONFIG_FUNCTION_TRACER
> + /* locate the .init.data section in vmlinux */
> + if (!strcmp(secstrings + idx, ".init.data")) {
> + get_mcount_loc(&_start_mcount_loc, &_stop_mcount_loc);
> + mstruct.ehdr = ehdr;
> + mstruct.init_data_sec = s;
> + mstruct.start_mcount_loc = _start_mcount_loc;
> + mstruct.stop_mcount_loc = _stop_mcount_loc;
> + }
> +#endif
> +
> #if defined(SORTTABLE_64) && defined(UNWINDER_ORC_ENABLED)
> /* locate the ORC unwind tables */
> if (!strcmp(secstrings + idx, ".orc_unwind_ip")) {
> @@ -294,6 +375,23 @@ static int do_sort(Elf_Ehdr *ehdr,
> goto out;
> }
> #endif
> +
> +#if defined CONFIG_FUNCTION_TRACER
> + if (!mstruct.init_data_sec || !_start_mcount_loc || !_stop_mcount_loc) {
> + fprintf(stderr,
> + "incomplete mcount's sort in file: %s\n",
> + fname);
> + goto out;
> + }
> +
> + /* create thread to sort mcount_loc concurrently */
> + if (pthread_create(&mcount_sort_thread, NULL, &sort_mcount_loc, &mstruct)) {
> + fprintf(stderr,
> + "pthread_create mcount_sort_thread failed '%s': %s\n",
> + strerror(errno), fname);
> + goto out;
> + }
> +#endif
> if (!extab_sec) {
> fprintf(stderr, "no __ex_table in file: %s\n", fname);
> goto out;
> @@ -376,5 +474,23 @@ static int do_sort(Elf_Ehdr *ehdr,
> }
> }
> #endif
> +
> +#if defined CONFIG_FUNCTION_TRACER
> + if (mcount_sort_thread) {
> + void *retval = NULL;
> + /* wait for mcount sort done */
> + rc = pthread_join(mcount_sort_thread, &retval);
> + if (rc) {
> + fprintf(stderr,
> + "pthread_join failed '%s': %s\n",
> + strerror(errno), fname);
> + } else if (retval) {
> + rc = -1;
> + fprintf(stderr,
> + "failed to sort mcount '%s': %s\n",
> + (char *)retval, fname);
> + }
> + }
> +#endif
> return rc;
> }

2021-12-02 02:16:13

by Yinan Liu

Subject: [PATCH v6 0/2] ftrace sorting optimization changelog

Modified the ifdef format problem of patch1.

Best regards!
-- Yinan Liu

Yinan Liu (2):
scripts: ftrace - move the sort-processing in ftrace_init
script/sorttable: code style improvements

kernel/trace/ftrace.c | 11 +++-
scripts/Makefile | 2 +-
scripts/link-vmlinux.sh | 6 +-
scripts/sorttable.c | 2 +
scripts/sorttable.h | 124 ++++++++++++++++++++++++++++++++++++++--
5 files changed, 135 insertions(+), 10 deletions(-)

--
2.19.1.6.gb485710b

2021-12-02 02:16:20

by Yinan Liu

Subject: [PATCH v6 1/2] scripts: ftrace - move the sort-processing in ftrace_init

When the kernel starts, the initialization of ftrace takes
up a portion of the time (approximately 6~8ms) to sort mcount
addresses. We can save this time by moving mcount-sorting to
compile time.

Signed-off-by: Yinan Liu <[email protected]>
Reported-by: kernel test robot <[email protected]>
---
kernel/trace/ftrace.c | 11 +++-
scripts/Makefile | 2 +-
scripts/link-vmlinux.sh | 6 +-
scripts/sorttable.c | 2 +
scripts/sorttable.h | 120 +++++++++++++++++++++++++++++++++++++++-
5 files changed, 133 insertions(+), 8 deletions(-)

diff --git a/kernel/trace/ftrace.c b/kernel/trace/ftrace.c
index 7b180f61e6d3..c0f95cae68b5 100644
--- a/kernel/trace/ftrace.c
+++ b/kernel/trace/ftrace.c
@@ -6189,8 +6189,15 @@ static int ftrace_process_locs(struct module *mod,
if (!count)
return 0;

- sort(start, count, sizeof(*start),
- ftrace_cmp_ips, NULL);
+ /*
+ * Sorting mcount in vmlinux at build time depend on
+ * CONFIG_BUILDTIME_TABLE_SORT, while mcount loc in
+ * modules can not be sorted at build time.
+ */
+ if (!IS_ENABLED(CONFIG_BUILDTIME_TABLE_SORT) || mod) {
+ sort(start, count, sizeof(*start),
+ ftrace_cmp_ips, NULL);
+ }

start_pg = ftrace_allocate_pages(count);
if (!start_pg)
diff --git a/scripts/Makefile b/scripts/Makefile
index 9adb6d247818..b286e112e3b0 100644
--- a/scripts/Makefile
+++ b/scripts/Makefile
@@ -17,6 +17,7 @@ hostprogs-always-$(CONFIG_SYSTEM_EXTRA_CERTIFICATE) += insert-sys-cert
hostprogs-always-$(CONFIG_SYSTEM_REVOCATION_LIST) += extract-cert

HOSTCFLAGS_sorttable.o = -I$(srctree)/tools/include
+HOSTLDLIBS_sorttable = -lpthread
HOSTCFLAGS_asn1_compiler.o = -I$(srctree)/include
HOSTCFLAGS_sign-file.o = $(CRYPTO_CFLAGS)
HOSTLDLIBS_sign-file = $(CRYPTO_LIBS)
@@ -29,7 +30,6 @@ ARCH := x86
endif
HOSTCFLAGS_sorttable.o += -I$(srctree)/tools/arch/x86/include
HOSTCFLAGS_sorttable.o += -DUNWINDER_ORC_ENABLED
-HOSTLDLIBS_sorttable = -lpthread
endif

# The following programs are only built on demand
diff --git a/scripts/link-vmlinux.sh b/scripts/link-vmlinux.sh
index 36ef7b37fc5d..e2e1a8f39f15 100755
--- a/scripts/link-vmlinux.sh
+++ b/scripts/link-vmlinux.sh
@@ -422,6 +422,9 @@ if [ -n "${CONFIG_DEBUG_INFO_BTF}" -a -n "${CONFIG_BPF}" ]; then
${RESOLVE_BTFIDS} vmlinux
fi

+info SYSMAP System.map
+mksysmap vmlinux System.map
+
if [ -n "${CONFIG_BUILDTIME_TABLE_SORT}" ]; then
info SORTTAB vmlinux
if ! sorttable vmlinux; then
@@ -430,9 +433,6 @@ if [ -n "${CONFIG_BUILDTIME_TABLE_SORT}" ]; then
fi
fi

-info SYSMAP System.map
-mksysmap vmlinux System.map
-
# step a (see comment above)
if [ -n "${CONFIG_KALLSYMS}" ]; then
mksysmap ${kallsyms_vmlinux} .tmp_System.map
diff --git a/scripts/sorttable.c b/scripts/sorttable.c
index 0ef3abfc4a51..11a595ca256b 100644
--- a/scripts/sorttable.c
+++ b/scripts/sorttable.c
@@ -30,6 +30,8 @@
#include <stdlib.h>
#include <string.h>
#include <unistd.h>
+#include <errno.h>
+#include <pthread.h>

#include <tools/be_byteshift.h>
#include <tools/le_byteshift.h>
diff --git a/scripts/sorttable.h b/scripts/sorttable.h
index a2baa2fefb13..a7a5b8078954 100644
--- a/scripts/sorttable.h
+++ b/scripts/sorttable.h
@@ -19,6 +19,9 @@

#undef extable_ent_size
#undef compare_extable
+#undef get_mcount_loc
+#undef sort_mcount_loc
+#undef elf_mcount_loc
#undef do_sort
#undef Elf_Addr
#undef Elf_Ehdr
@@ -41,6 +44,9 @@
#ifdef SORTTABLE_64
# define extable_ent_size 16
# define compare_extable compare_extable_64
+# define get_mcount_loc get_mcount_loc_64
+# define sort_mcount_loc sort_mcount_loc_64
+# define elf_mcount_loc elf_mcount_loc_64
# define do_sort do_sort_64
# define Elf_Addr Elf64_Addr
# define Elf_Ehdr Elf64_Ehdr
@@ -62,6 +68,9 @@
#else
# define extable_ent_size 8
# define compare_extable compare_extable_32
+# define get_mcount_loc get_mcount_loc_32
+# define sort_mcount_loc sort_mcount_loc_32
+# define elf_mcount_loc elf_mcount_loc_32
# define do_sort do_sort_32
# define Elf_Addr Elf32_Addr
# define Elf_Ehdr Elf32_Ehdr
@@ -84,8 +93,6 @@

#if defined(SORTTABLE_64) && defined(UNWINDER_ORC_ENABLED)
/* ORC unwinder only support X86_64 */
-#include <errno.h>
-#include <pthread.h>
#include <asm/orc_types.h>

#define ERRSTR_MAXSZ 256
@@ -191,7 +198,64 @@ static int compare_extable(const void *a, const void *b)
return 1;
return 0;
}
+#ifdef CONFIG_FUNCTION_TRACER
+struct elf_mcount_loc {
+ Elf_Ehdr *ehdr;
+ Elf_Shdr *init_data_sec;
+ uint_t start_mcount_loc;
+ uint_t stop_mcount_loc;
+};
+
+/* Sort the addresses stored between __start_mcount_loc to __stop_mcount_loc in vmlinux */
+static void *sort_mcount_loc(void *arg)
+{
+ struct elf_mcount_loc *emloc = (struct elf_mcount_loc *)arg;
+ uint_t offset = emloc->start_mcount_loc - _r(&(emloc->init_data_sec)->sh_addr)
+ + _r(&(emloc->init_data_sec)->sh_offset);
+ uint_t count = emloc->stop_mcount_loc - emloc->start_mcount_loc;
+ unsigned char *start_loc = (void *)emloc->ehdr + offset;
+
+ qsort(start_loc, count/sizeof(uint_t), sizeof(uint_t), compare_extable);
+ return NULL;
+}
+
+/* Get the address of __start_mcount_loc and __stop_mcount_loc in System.map */
+static void get_mcount_loc(uint_t *_start, uint_t *_stop)
+{
+ FILE *file_start, *file_stop;
+ char start_buff[20];
+ char stop_buff[20];
+ int len = 0;
+
+ file_start = popen(" grep start_mcount System.map | awk '{print $1}' ", "r");
+ if (!file_start) {
+ fprintf(stderr, "get start_mcount_loc error!");
+ return;
+ }
+
+ file_stop = popen(" grep stop_mcount System.map | awk '{print $1}' ", "r");
+ if (!file_stop) {
+ fprintf(stderr, "get stop_mcount_loc error!");
+ pclose(file_start);
+ return;
+ }
+
+ while (fgets(start_buff, sizeof(start_buff), file_start) != NULL) {
+ len = strlen(start_buff);
+ start_buff[len - 1] = '\0';
+ }
+ *_start = strtoul(start_buff, NULL, 16);
+
+ while (fgets(stop_buff, sizeof(stop_buff), file_stop) != NULL) {
+ len = strlen(stop_buff);
+ stop_buff[len - 1] = '\0';
+ }
+ *_stop = strtoul(stop_buff, NULL, 16);

+ pclose(file_start);
+ pclose(file_stop);
+}
+#endif
static int do_sort(Elf_Ehdr *ehdr,
char const *const fname,
table_sort_t custom_sort)
@@ -217,6 +281,12 @@ static int do_sort(Elf_Ehdr *ehdr,
int idx;
unsigned int shnum;
unsigned int shstrndx;
+#ifdef CONFIG_FUNCTION_TRACER
+ struct elf_mcount_loc mstruct;
+ uint_t _start_mcount_loc = 0;
+ uint_t _stop_mcount_loc = 0;
+ pthread_t mcount_sort_thread;
+#endif
#if defined(SORTTABLE_64) && defined(UNWINDER_ORC_ENABLED)
unsigned int orc_ip_size = 0;
unsigned int orc_size = 0;
@@ -253,6 +323,17 @@ static int do_sort(Elf_Ehdr *ehdr,
symtab_shndx = (Elf32_Word *)((const char *)ehdr +
_r(&s->sh_offset));

+#ifdef CONFIG_FUNCTION_TRACER
+ /* locate the .init.data section in vmlinux */
+ if (!strcmp(secstrings + idx, ".init.data")) {
+ get_mcount_loc(&_start_mcount_loc, &_stop_mcount_loc);
+ mstruct.ehdr = ehdr;
+ mstruct.init_data_sec = s;
+ mstruct.start_mcount_loc = _start_mcount_loc;
+ mstruct.stop_mcount_loc = _stop_mcount_loc;
+ }
+#endif
+
#if defined(SORTTABLE_64) && defined(UNWINDER_ORC_ENABLED)
/* locate the ORC unwind tables */
if (!strcmp(secstrings + idx, ".orc_unwind_ip")) {
@@ -294,6 +375,23 @@ static int do_sort(Elf_Ehdr *ehdr,
goto out;
}
#endif
+
+#ifdef CONFIG_FUNCTION_TRACER
+ if (!mstruct.init_data_sec || !_start_mcount_loc || !_stop_mcount_loc) {
+ fprintf(stderr,
+ "incomplete mcount's sort in file: %s\n",
+ fname);
+ goto out;
+ }
+
+ /* create thread to sort mcount_loc concurrently */
+ if (pthread_create(&mcount_sort_thread, NULL, &sort_mcount_loc, &mstruct)) {
+ fprintf(stderr,
+ "pthread_create mcount_sort_thread failed '%s': %s\n",
+ strerror(errno), fname);
+ goto out;
+ }
+#endif
if (!extab_sec) {
fprintf(stderr, "no __ex_table in file: %s\n", fname);
goto out;
@@ -376,5 +474,23 @@ static int do_sort(Elf_Ehdr *ehdr,
}
}
#endif
+
+#ifdef CONFIG_FUNCTION_TRACER
+ if (mcount_sort_thread) {
+ void *retval = NULL;
+ /* wait for mcount sort done */
+ rc = pthread_join(mcount_sort_thread, &retval);
+ if (rc) {
+ fprintf(stderr,
+ "pthread_join failed '%s': %s\n",
+ strerror(errno), fname);
+ } else if (retval) {
+ rc = -1;
+ fprintf(stderr,
+ "failed to sort mcount '%s': %s\n",
+ (char *)retval, fname);
+ }
+ }
+#endif
return rc;
}
--
2.19.1.6.gb485710b

2021-12-02 02:16:28

by Yinan Liu

Subject: [PATCH v6 2/2] script/sorttable: code style improvements

Modified the code style issue of if() {},
keep the code style consistent.

Signed-off-by: Yinan Liu <[email protected]>
---
scripts/sorttable.h | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/scripts/sorttable.h b/scripts/sorttable.h
index a7a5b8078954..7d51e53ba6aa 100644
--- a/scripts/sorttable.h
+++ b/scripts/sorttable.h
@@ -462,11 +462,11 @@ static int do_sort(Elf_Ehdr *ehdr,
void *retval = NULL;
/* wait for ORC tables sort done */
rc = pthread_join(orc_sort_thread, &retval);
- if (rc)
+ if (rc) {
fprintf(stderr,
"pthread_join failed '%s': %s\n",
strerror(errno), fname);
- else if (retval) {
+ } else if (retval) {
rc = -1;
fprintf(stderr,
"failed to sort ORC tables '%s': %s\n",
--
2.19.1.6.gb485710b

2021-12-02 17:31:08

by Peter Zijlstra

Subject: Re: [PATCH v6 0/2] ftrace sorting optimization changelog

On Thu, Dec 02, 2021 at 10:16:04AM +0800, Yinan Liu wrote:
> Modified the ifdef format problem of patch1.
>
> Best regards!
> -- Yinan Liu
>
> Yinan Liu (2):
> scripts: ftrace - move the sort-processing in ftrace_init
> script/sorttable: code style improvements

Acked-by: Peter Zijlstr (Intel) <[email protected]>

2021-12-02 17:58:10

by Steven Rostedt

Subject: Re: [PATCH v6 1/2] scripts: ftrace - move the sort-processing in ftrace_init

On Thu, 2 Dec 2021 10:16:05 +0800
Yinan Liu <[email protected]> wrote:

> When the kernel starts, the initialization of ftrace takes
> up a portion of the time (approximately 6~8ms) to sort mcount
> addresses. We can save this time by moving mcount-sorting to
> compile time.
>
> Signed-off-by: Yinan Liu <[email protected]>
> Reported-by: kernel test robot <[email protected]>

After applying this, I get a failure on the kprobe self tests at boot up:

Testing ftrace filter: OK
trace_kprobe: Testing kprobe tracing:
trace_kprobe: Could not probe notrace function kprobe_trace_selftest_target
------------[ cut here ]------------
WARNING: CPU: 2 PID: 1 at kernel/trace/trace_kprobe.c:1973 kprobe_trace_self_tests_init+0x5c/0x497
Modules linked in:
CPU: 2 PID: 1 Comm: swapper/0 Tainted: G W 5.16.0-rc3-test+ #40
Hardware name: Hewlett-Packard HP Compaq Pro 6300 SFF/339A, BIOS K01 v03.03 07/14/2016
RIP: 0010:kprobe_trace_self_tests_init+0x5c/0x497
Code: 5f d7 94 ff 00 0f 85 48 04 00 00 48 c7 c7 c0 ff 8e 88 e8 6b bb cd fd 48 c7 c7 20 00 8f 88 e8 a9 b6 db fc 41 89 c4 85 c0 74 16 <0f> 0b 48 c7 c7 a0 00 8f 88 41 bc 01 00 00 00 e8 44 bb cd fd eb 33
RSP: 0000:ffffc90000037d00 EFLAGS: 00010282
RAX: 00000000ffffffea RBX: 0000000000000000 RCX: dffffc0000000000
RDX: 0000000000000000 RSI: 0000000000000000 RDI: ffff8880c2b33eb4
RBP: 1ffff92000006fa9 R08: ffffffff872e1900 R09: 0000000080000171
R10: fffff52000006f15 R11: 0000000000000001 R12: 00000000ffffffea
R13: 0000000000000007 R14: ffffffff8a608098 R15: ffffffff8a6080a0
FS: 0000000000000000(0000) GS:ffff8880c2b00000(0000) knlGS:0000000000000000
CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
CR2: 0000000000000000 CR3: 000000009260e001 CR4: 00000000001706e0
Call Trace:
<TASK>
? init_kprobe_trace+0x1c4/0x1c4
do_one_initcall+0x89/0x2b0
? trace_event_raw_event_initcall_finish+0x150/0x150
? parameq+0x90/0x90
? _raw_spin_unlock_irqrestore+0x19/0x40
kernel_init_freeable+0x35a/0x3e7
? console_on_rootfs+0x52/0x52
? _raw_spin_unlock+0x30/0x30
? rest_init+0xf0/0xf0
? rest_init+0xf0/0xf0
kernel_init+0x19/0x140
ret_from_fork+0x22/0x30
</TASK>
---[ end trace 27c06839fac37d16 ]---


Config attached.

-- Steve




> ---
> kernel/trace/ftrace.c | 11 +++-
> scripts/Makefile | 2 +-
> scripts/link-vmlinux.sh | 6 +-
> scripts/sorttable.c | 2 +
> scripts/sorttable.h | 120 +++++++++++++++++++++++++++++++++++++++-
> 5 files changed, 133 insertions(+), 8 deletions(-)
>
> diff --git a/kernel/trace/ftrace.c b/kernel/trace/ftrace.c
> index 7b180f61e6d3..c0f95cae68b5 100644
> --- a/kernel/trace/ftrace.c
> +++ b/kernel/trace/ftrace.c
> @@ -6189,8 +6189,15 @@ static int ftrace_process_locs(struct module *mod,
> if (!count)
> return 0;
>
> - sort(start, count, sizeof(*start),
> - ftrace_cmp_ips, NULL);
> + /*
> + * Sorting mcount in vmlinux at build time depend on
> + * CONFIG_BUILDTIME_TABLE_SORT, while mcount loc in
> + * modules can not be sorted at build time.
> + */
> + if (!IS_ENABLED(CONFIG_BUILDTIME_TABLE_SORT) || mod) {
> + sort(start, count, sizeof(*start),
> + ftrace_cmp_ips, NULL);
> + }
>
> start_pg = ftrace_allocate_pages(count);
> if (!start_pg)
> diff --git a/scripts/Makefile b/scripts/Makefile
> index 9adb6d247818..b286e112e3b0 100644
> --- a/scripts/Makefile
> +++ b/scripts/Makefile
> @@ -17,6 +17,7 @@ hostprogs-always-$(CONFIG_SYSTEM_EXTRA_CERTIFICATE) += insert-sys-cert
> hostprogs-always-$(CONFIG_SYSTEM_REVOCATION_LIST) += extract-cert
>
> HOSTCFLAGS_sorttable.o = -I$(srctree)/tools/include
> +HOSTLDLIBS_sorttable = -lpthread
> HOSTCFLAGS_asn1_compiler.o = -I$(srctree)/include
> HOSTCFLAGS_sign-file.o = $(CRYPTO_CFLAGS)
> HOSTLDLIBS_sign-file = $(CRYPTO_LIBS)
> @@ -29,7 +30,6 @@ ARCH := x86
> endif
> HOSTCFLAGS_sorttable.o += -I$(srctree)/tools/arch/x86/include
> HOSTCFLAGS_sorttable.o += -DUNWINDER_ORC_ENABLED
> -HOSTLDLIBS_sorttable = -lpthread
> endif
>
> # The following programs are only built on demand
> diff --git a/scripts/link-vmlinux.sh b/scripts/link-vmlinux.sh
> index 36ef7b37fc5d..e2e1a8f39f15 100755
> --- a/scripts/link-vmlinux.sh
> +++ b/scripts/link-vmlinux.sh
> @@ -422,6 +422,9 @@ if [ -n "${CONFIG_DEBUG_INFO_BTF}" -a -n "${CONFIG_BPF}" ]; then
> ${RESOLVE_BTFIDS} vmlinux
> fi
>
> +info SYSMAP System.map
> +mksysmap vmlinux System.map
> +
> if [ -n "${CONFIG_BUILDTIME_TABLE_SORT}" ]; then
> info SORTTAB vmlinux
> if ! sorttable vmlinux; then
> @@ -430,9 +433,6 @@ if [ -n "${CONFIG_BUILDTIME_TABLE_SORT}" ]; then
> fi
> fi
>
> -info SYSMAP System.map
> -mksysmap vmlinux System.map
> -
> # step a (see comment above)
> if [ -n "${CONFIG_KALLSYMS}" ]; then
> mksysmap ${kallsyms_vmlinux} .tmp_System.map
> diff --git a/scripts/sorttable.c b/scripts/sorttable.c
> index 0ef3abfc4a51..11a595ca256b 100644
> --- a/scripts/sorttable.c
> +++ b/scripts/sorttable.c
> @@ -30,6 +30,8 @@
> #include <stdlib.h>
> #include <string.h>
> #include <unistd.h>
> +#include <errno.h>
> +#include <pthread.h>
>
> #include <tools/be_byteshift.h>
> #include <tools/le_byteshift.h>
> diff --git a/scripts/sorttable.h b/scripts/sorttable.h
> index a2baa2fefb13..a7a5b8078954 100644
> --- a/scripts/sorttable.h
> +++ b/scripts/sorttable.h
> @@ -19,6 +19,9 @@
>
> #undef extable_ent_size
> #undef compare_extable
> +#undef get_mcount_loc
> +#undef sort_mcount_loc
> +#undef elf_mcount_loc
> #undef do_sort
> #undef Elf_Addr
> #undef Elf_Ehdr
> @@ -41,6 +44,9 @@
> #ifdef SORTTABLE_64
> # define extable_ent_size 16
> # define compare_extable compare_extable_64
> +# define get_mcount_loc get_mcount_loc_64
> +# define sort_mcount_loc sort_mcount_loc_64
> +# define elf_mcount_loc elf_mcount_loc_64
> # define do_sort do_sort_64
> # define Elf_Addr Elf64_Addr
> # define Elf_Ehdr Elf64_Ehdr
> @@ -62,6 +68,9 @@
> #else
> # define extable_ent_size 8
> # define compare_extable compare_extable_32
> +# define get_mcount_loc get_mcount_loc_32
> +# define sort_mcount_loc sort_mcount_loc_32
> +# define elf_mcount_loc elf_mcount_loc_32
> # define do_sort do_sort_32
> # define Elf_Addr Elf32_Addr
> # define Elf_Ehdr Elf32_Ehdr
> @@ -84,8 +93,6 @@
>
> #if defined(SORTTABLE_64) && defined(UNWINDER_ORC_ENABLED)
> /* ORC unwinder only support X86_64 */
> -#include <errno.h>
> -#include <pthread.h>
> #include <asm/orc_types.h>
>
> #define ERRSTR_MAXSZ 256
> @@ -191,7 +198,64 @@ static int compare_extable(const void *a, const void *b)
> return 1;
> return 0;
> }
> +#ifdef CONFIG_FUNCTION_TRACER
> +struct elf_mcount_loc {
> + Elf_Ehdr *ehdr;
> + Elf_Shdr *init_data_sec;
> + uint_t start_mcount_loc;
> + uint_t stop_mcount_loc;
> +};
> +
> +/* Sort the addresses stored between __start_mcount_loc to __stop_mcount_loc in vmlinux */
> +static void *sort_mcount_loc(void *arg)
> +{
> + struct elf_mcount_loc *emloc = (struct elf_mcount_loc *)arg;
> + uint_t offset = emloc->start_mcount_loc - _r(&(emloc->init_data_sec)->sh_addr)
> + + _r(&(emloc->init_data_sec)->sh_offset);
> + uint_t count = emloc->stop_mcount_loc - emloc->start_mcount_loc;
> + unsigned char *start_loc = (void *)emloc->ehdr + offset;
> +
> + qsort(start_loc, count/sizeof(uint_t), sizeof(uint_t), compare_extable);
> + return NULL;
> +}
> +
> +/* Get the address of __start_mcount_loc and __stop_mcount_loc in System.map */
> +static void get_mcount_loc(uint_t *_start, uint_t *_stop)
> +{
> + FILE *file_start, *file_stop;
> + char start_buff[20];
> + char stop_buff[20];
> + int len = 0;
> +
> + file_start = popen(" grep start_mcount System.map | awk '{print $1}' ", "r");
> + if (!file_start) {
> + fprintf(stderr, "get start_mcount_loc error!");
> + return;
> + }
> +
> + file_stop = popen(" grep stop_mcount System.map | awk '{print $1}' ", "r");
> + if (!file_stop) {
> + fprintf(stderr, "get stop_mcount_loc error!");
> + pclose(file_start);
> + return;
> + }
> +
> + while (fgets(start_buff, sizeof(start_buff), file_start) != NULL) {
> + len = strlen(start_buff);
> + start_buff[len - 1] = '\0';
> + }
> + *_start = strtoul(start_buff, NULL, 16);
> +
> + while (fgets(stop_buff, sizeof(stop_buff), file_stop) != NULL) {
> + len = strlen(stop_buff);
> + stop_buff[len - 1] = '\0';
> + }
> + *_stop = strtoul(stop_buff, NULL, 16);
>
> + pclose(file_start);
> + pclose(file_stop);
> +}
> +#endif
> static int do_sort(Elf_Ehdr *ehdr,
> char const *const fname,
> table_sort_t custom_sort)
> @@ -217,6 +281,12 @@ static int do_sort(Elf_Ehdr *ehdr,
> int idx;
> unsigned int shnum;
> unsigned int shstrndx;
> +#ifdef CONFIG_FUNCTION_TRACER
> + struct elf_mcount_loc mstruct;
> + uint_t _start_mcount_loc = 0;
> + uint_t _stop_mcount_loc = 0;
> + pthread_t mcount_sort_thread;
> +#endif
> #if defined(SORTTABLE_64) && defined(UNWINDER_ORC_ENABLED)
> unsigned int orc_ip_size = 0;
> unsigned int orc_size = 0;
> @@ -253,6 +323,17 @@ static int do_sort(Elf_Ehdr *ehdr,
> symtab_shndx = (Elf32_Word *)((const char *)ehdr +
> _r(&s->sh_offset));
>
> +#ifdef CONFIG_FUNCTION_TRACER
> + /* locate the .init.data section in vmlinux */
> + if (!strcmp(secstrings + idx, ".init.data")) {
> + get_mcount_loc(&_start_mcount_loc, &_stop_mcount_loc);
> + mstruct.ehdr = ehdr;
> + mstruct.init_data_sec = s;
> + mstruct.start_mcount_loc = _start_mcount_loc;
> + mstruct.stop_mcount_loc = _stop_mcount_loc;
> + }
> +#endif
> +
> #if defined(SORTTABLE_64) && defined(UNWINDER_ORC_ENABLED)
> /* locate the ORC unwind tables */
> if (!strcmp(secstrings + idx, ".orc_unwind_ip")) {
> @@ -294,6 +375,23 @@ static int do_sort(Elf_Ehdr *ehdr,
> goto out;
> }
> #endif
> +
> +#ifdef CONFIG_FUNCTION_TRACER
> + if (!mstruct.init_data_sec || !_start_mcount_loc || !_stop_mcount_loc) {
> + fprintf(stderr,
> + "incomplete mcount's sort in file: %s\n",
> + fname);
> + goto out;
> + }
> +
> + /* create thread to sort mcount_loc concurrently */
> + if (pthread_create(&mcount_sort_thread, NULL, &sort_mcount_loc, &mstruct)) {
> + fprintf(stderr,
> + "pthread_create mcount_sort_thread failed '%s': %s\n",
> + strerror(errno), fname);
> + goto out;
> + }
> +#endif
> if (!extab_sec) {
> fprintf(stderr, "no __ex_table in file: %s\n", fname);
> goto out;
> @@ -376,5 +474,23 @@ static int do_sort(Elf_Ehdr *ehdr,
> }
> }
> #endif
> +
> +#ifdef CONFIG_FUNCTION_TRACER
> + if (mcount_sort_thread) {
> + void *retval = NULL;
> + /* wait for mcount sort done */
> + rc = pthread_join(mcount_sort_thread, &retval);
> + if (rc) {
> + fprintf(stderr,
> + "pthread_join failed '%s': %s\n",
> + strerror(errno), fname);
> + } else if (retval) {
> + rc = -1;
> + fprintf(stderr,
> + "failed to sort mcount '%s': %s\n",
> + (char *)retval, fname);
> + }
> + }
> +#endif
> return rc;
> }

Attachments:

2021-12-05 12:36:17

by kernel test robot

Subject: [scripts] 12955fb1c5: kernel-selftests.livepatch.test-ftrace.sh.fail

Greeting,

FYI, we noticed the following commit (built with gcc-9):

commit: 12955fb1c57635f54792fedfdb6cc094b0a706d4 ("[PATCH v5 1/2] scripts: ftrace - move the sort-processing in ftrace_init to compile time")
url: https://github.com/0day-ci/linux/commits/Yinan-Liu/scripts-ftrace-move-the-sort-processing-in-ftrace_init-to-compile-time/20211201-143303
base: https://git.kernel.org/cgit/linux/kernel/git/rostedt/linux-trace.git for-next
patch link: https://lore.kernel.org/lkml/[email protected]

in testcase: kernel-selftests
version: kernel-selftests-x86_64-a21458fc-1_20211201
with following parameters:

group: livepatch
ucode: 0xe2

test-description: The kernel contains a set of "self tests" under the tools/testing/selftests/ directory. These are intended to be small unit tests to exercise individual code paths in the kernel.
test-url: https://www.kernel.org/doc/Documentation/kselftest.txt


on test machine: 4 threads Intel(R) Core(TM) i5-6500 CPU @ 3.20GHz with 32G memory

caused below changes (please refer to attached dmesg/kmsg for entire log/backtrace):




If you fix the issue, kindly add following tag
Reported-by: kernel test robot <[email protected]>

TAP version 13
1..5
# selftests: livepatch: test-livepatch.sh
# TEST: basic function patching ... ERROR: modprobe: ERROR: could not insert 'test_klp_livepatch': Invalid argument
not ok 1 selftests: livepatch: test-livepatch.sh # exit=1
# selftests: livepatch: test-callbacks.sh
# TEST: target module before livepatch ... ok
# TEST: module_coming notifier ... ok
# TEST: module_going notifier ... ok
# TEST: module_coming and module_going notifiers ... ok
# TEST: target module not present ... ok
# TEST: pre-patch callback -ENODEV ... ok
# TEST: module_coming + pre-patch callback -ENODEV ... ok
# TEST: multiple target modules ... ok
# TEST: busy target module ... ok
# TEST: multiple livepatches ... ok
# TEST: atomic replace ... ok
ok 2 selftests: livepatch: test-callbacks.sh
# selftests: livepatch: test-shadow-vars.sh
# TEST: basic shadow variable API ... ok
ok 3 selftests: livepatch: test-shadow-vars.sh
# selftests: livepatch: test-state.sh
# TEST: system state modification ... ok
# TEST: taking over system state modification ... ok
# TEST: compatible cumulative livepatches ... ok
# TEST: incompatible cumulative livepatches ... ok
ok 4 selftests: livepatch: test-state.sh
# selftests: livepatch: test-ftrace.sh
# TEST: livepatch interaction with ftrace_enabled sysctl ... ERROR: modprobe: ERROR: could not insert 'test_klp_livepatch': Invalid argument
not ok 5 selftests: livepatch: test-ftrace.sh # exit=1
make: Leaving directory '/usr/src/perf_selftests-x86_64-rhel-8.3-kselftests-12955fb1c57635f54792fedfdb6cc094b0a706d4/tools/testing/selftests/livepatch'



To reproduce:

git clone https://github.com/intel/lkp-tests.git
cd lkp-tests
sudo bin/lkp install job.yaml # job file is attached in this email
bin/lkp split-job --compatible job.yaml # generate the yaml file for lkp run
sudo bin/lkp run generated-yaml-file

# if come across any failure that blocks the test,
# please remove ~/.lkp and /lkp dir to run from a clean state.



---
0DAY/LKP+ Test Infrastructure Open Source Technology Center
https://lists.01.org/hyperkitty/list/[email protected] Intel Corporation

Thanks,
Oliver Sang

Attachments:

2021-12-05 12:45:50

by Masami Hiramatsu

Subject: Re: [PATCH v6 1/2] scripts: ftrace - move the sort-processing in ftrace_init

Hi Steve,

On Thu, 2 Dec 2021 12:58:00 -0500
Steven Rostedt <[email protected]> wrote:

> On Thu, 2 Dec 2021 10:16:05 +0800
> Yinan Liu <[email protected]> wrote:
>
> > When the kernel starts, the initialization of ftrace takes
> > up a portion of the time (approximately 6~8ms) to sort mcount
> > addresses. We can save this time by moving mcount-sorting to
> > compile time.
> >
> > Signed-off-by: Yinan Liu <[email protected]>
> > Reported-by: kernel test robot <[email protected]>
>
> After applying this, I get a failure on the kprobe self tests at boot up:
>
> Testing ftrace filter: OK
> trace_kprobe: Testing kprobe tracing:
> trace_kprobe: Could not probe notrace function kprobe_trace_selftest_target
> ------------[ cut here ]------------
> WARNING: CPU: 2 PID: 1 at kernel/trace/trace_kprobe.c:1973 kprobe_trace_self_tests_init+0x5c/0x497
> Modules linked in:
> CPU: 2 PID: 1 Comm: swapper/0 Tainted: G W 5.16.0-rc3-test+ #40
> Hardware name: Hewlett-Packard HP Compaq Pro 6300 SFF/339A, BIOS K01 v03.03 07/14/2016
> RIP: 0010:kprobe_trace_self_tests_init+0x5c/0x497
> Code: 5f d7 94 ff 00 0f 85 48 04 00 00 48 c7 c7 c0 ff 8e 88 e8 6b bb cd fd 48 c7 c7 20 00 8f 88 e8 a9 b6 db fc 41 89 c4 85 c0 74 16 <0f> 0b 48 c7 c7 a0 00 8f 88 41 bc 01 00 00 00 e8 44 bb cd fd eb 33
> RSP: 0000:ffffc90000037d00 EFLAGS: 00010282
> RAX: 00000000ffffffea RBX: 0000000000000000 RCX: dffffc0000000000
> RDX: 0000000000000000 RSI: 0000000000000000 RDI: ffff8880c2b33eb4
> RBP: 1ffff92000006fa9 R08: ffffffff872e1900 R09: 0000000080000171
> R10: fffff52000006f15 R11: 0000000000000001 R12: 00000000ffffffea
> R13: 0000000000000007 R14: ffffffff8a608098 R15: ffffffff8a6080a0
> FS: 0000000000000000(0000) GS:ffff8880c2b00000(0000) knlGS:0000000000000000
> CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> CR2: 0000000000000000 CR3: 000000009260e001 CR4: 00000000001706e0
> Call Trace:
> <TASK>
> ? init_kprobe_trace+0x1c4/0x1c4
> do_one_initcall+0x89/0x2b0
> ? trace_event_raw_event_initcall_finish+0x150/0x150
> ? parameq+0x90/0x90
> ? _raw_spin_unlock_irqrestore+0x19/0x40
> kernel_init_freeable+0x35a/0x3e7
> ? console_on_rootfs+0x52/0x52
> ? _raw_spin_unlock+0x30/0x30
> ? rest_init+0xf0/0xf0
> ? rest_init+0xf0/0xf0
> kernel_init+0x19/0x140
> ret_from_fork+0x22/0x30
> </TASK>
> ---[ end trace 27c06839fac37d16 ]---

I confirmed this warning, but also I saw below warning too.
Maybe this also came from same commit?

[ 4.936720] Testing tracer function_graph: FAILED!
[ 6.186714] ------------[ cut here ]------------
[ 6.187301] WARNING: CPU: 0 PID: 1 at kernel/trace/trace.c:1953 run_tracer_selftest+0x13b/0x192
[ 6.187698] Modules linked in:
[ 6.188698] CPU: 0 PID: 1 Comm: swapper/0 Not tainted 5.16.0-rc3+ #174
[ 6.189381] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.13.0-1ubuntu1.1 04/01/4
[ 6.189698] RIP: 0010:run_tracer_selftest+0x13b/0x192
[ 6.190698] Code: 60 48 c7 c6 00 6b 35 82 48 89 df e8 d3 18 33 00 4c 89 2d a4 82 a8 00 41 89 c4 b
[ 6.191698] RSP: 0000:ffffc90000013e48 EFLAGS: 00010246
[ 6.192250] RAX: 0000000000000007 RBX: ffffffff823ff940 RCX: ffff88807dc00000
[ 6.192697] RDX: 0000000000000000 RSI: ffffffff810c7983 RDI: ffffffff810c7983
[ 6.193697] RBP: ffffc90000013e60 R08: 0000000000000000 R09: ffffc90000013c40
[ 6.194415] R10: 0000000000000001 R11: 0000000000000001 R12: 00000000ffffffed
[ 6.194697] R13: ffffffff8241c620 R14: ffffffff8266e290 R15: 0000000000000000
[ 6.195698] FS: 0000000000000000(0000) GS:ffff88807dc00000(0000) knlGS:0000000000000000
[ 6.196698] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[ 6.197317] CR2: 0000000000000000 CR3: 000000000220c000 CR4: 00000000000006b0
[ 6.197700] Call Trace:
[ 6.198698] <TASK>
[ 6.199017] register_tracer+0x123/0x1d4
[ 6.199698] ? init_graph_tracefs+0x2d/0x2d
[ 6.200159] init_graph_trace+0x62/0x64
[ 6.200583] do_one_initcall+0x48/0x200
[ 6.200699] kernel_init_freeable+0x1f0/0x23f
[ 6.201180] ? rest_init+0xd0/0xd0
[ 6.201699] kernel_init+0x1a/0x130
[ 6.202132] ret_from_fork+0x22/0x30
[ 6.202703] </TASK>
[ 6.203098] ---[ end trace e84e39016448db0f ]---
[ 6.203804] NET: Registered PF_NETLINK/PF_ROUTE protocol family

Thank you,


--
Masami Hiramatsu <[email protected]>

2021-12-06 20:19:16

by Steven Rostedt

Subject: Re: [PATCH v6 1/2] scripts: ftrace - move the sort-processing in ftrace_init

On Thu, 2 Dec 2021 12:58:00 -0500
Steven Rostedt <[email protected]> wrote:

> On Thu, 2 Dec 2021 10:16:05 +0800
> Yinan Liu <[email protected]> wrote:
>
> > When the kernel starts, the initialization of ftrace takes
> > up a portion of the time (approximately 6~8ms) to sort mcount
> > addresses. We can save this time by moving mcount-sorting to
> > compile time.
> >
> > Signed-off-by: Yinan Liu <[email protected]>
> > Reported-by: kernel test robot <[email protected]>
>
> After applying this, I get a failure on the kprobe self tests at boot up:
>
> Testing ftrace filter: OK
> trace_kprobe: Testing kprobe tracing:
> trace_kprobe: Could not probe notrace function kprobe_trace_selftest_target
> ------------[ cut here ]------------
> WARNING: CPU: 2 PID: 1 at kernel/trace/trace_kprobe.c:1973 kprobe_trace_self_tests_init+0x5c/0x497
> Modules linked in:

And I added the below patch, and it shows that the section is not properly
sorted. Something went wrong with your sorting.

I plan on keeping this check, at least as a compile option, to make sure
that the output is sorted, otherwise things will silently fail if they are
not.

This patch produced:

[ 1.315419] ftrace: allocating 43510 entries in 170 pages
[ 1.320638] ------------[ cut here ]------------
[ 1.325227] [3] x86_pnpbios_disabled+0x0/0x1c at 8a51c707 is not sorted with __traceiter_initcall_level+0x0/0x60 at 87000660

I'm dropping the patches until this is "sorted out" :-)

-- Steve

diff --git a/kernel/trace/ftrace.c b/kernel/trace/ftrace.c
index 9ca63df6553a..b112d00ba534 100644
--- a/kernel/trace/ftrace.c
+++ b/kernel/trace/ftrace.c
@@ -6388,6 +6388,19 @@ static int ftrace_cmp_ips(const void *a, const void
*b) return 0;
}

+static void test_is_sorted(unsigned long *start, unsigned long count)
+{
+ int i;
+
+ for (i = 1; i < count; i++) {
+ if (WARN(start[i - 1] > start[i],
+ "[%d] %pS at %x is not sorted with %pS at %x\n",
i,
+ (void *)start[i - 1], start[i - 1],
+ (void *)start[i], start[i]))
+ break;
+ }
+}
+
static int ftrace_process_locs(struct module *mod,
unsigned long *start,
unsigned long *end)
@@ -6414,6 +6427,8 @@ static int ftrace_process_locs(struct module *mod,
if (!IS_ENABLED(CONFIG_BUILDTIME_TABLE_SORT) || mod) {
sort(start, count, sizeof(*start),
ftrace_cmp_ips, NULL);
+ } else {
+ test_is_sorted(start, count);
}

start_pg = ftrace_allocate_pages(count);

2021-12-07 01:30:03

by Yinan Liu

Subject: Re: [PATCH v6 1/2] scripts: ftrace - move the sort-processing in ftrace_init

>> After applying this, I get a failure on the kprobe self tests at boot up:
>>
>> Testing ftrace filter: OK
>> trace_kprobe: Testing kprobe tracing:
>> trace_kprobe: Could not probe notrace function kprobe_trace_selftest_target
>> ------------[ cut here ]------------
>> WARNING: CPU: 2 PID: 1 at kernel/trace/trace_kprobe.c:1973 kprobe_trace_self_tests_init+0x5c/0x497
>> Modules linked in:
>
> And I added the below patch, and it shows that the section is not properly
> sorted. Something went wrong with your sorting.
>
> I plan on keeping this check, at least as a compile option, to make sure
> that the output is sorted, otherwise things will silently fail if they are
> not.
>
> This patch produced:
>
> [ 1.315419] ftrace: allocating 43510 entries in 170 pages
> [ 1.320638] ------------[ cut here ]------------
> [ 1.325227] [3] x86_pnpbios_disabled+0x0/0x1c at 8a51c707 is not sorted with __traceiter_initcall_level+0x0/0x60 at 87000660
>
> I'm dropping the patches until this is "sorted out" :-)
>
> -- Steve
>
Hi, steven

Sorry, I just saw the email yesterday. Recently my computer is not
around. I'm not sure what went wrong at the moment. I will fix it.



Best regards!
-- Yinan Liu

2021-12-07 15:13:54

by Yinan Liu

Subject: [PATCH v7 0/2] ftrace sorting optimization

Hi, Steven

In fact, the code enclosed by ifdef CONFIG_FUNCTION_TRACER
does not take effect. I saw similar usage under script and
judged directly after the compilation is passed, without
checking whether the sorting takes effect. Sorry for my
negligence.

Now, after the modification, the patch works normally and
passes the test case of livepatch's selftest which reported
by robot.

Yinan Liu (2):
scripts: ftrace - move the sort-processing in ftrace_init
script/sorttable: code style improvements

kernel/trace/ftrace.c | 11 +++-
scripts/Makefile | 6 +-
scripts/link-vmlinux.sh | 6 +-
scripts/sorttable.c | 2 +
scripts/sorttable.h | 124 ++++++++++++++++++++++++++++++++++++++--
5 files changed, 139 insertions(+), 10 deletions(-)

--
2.27.0

2021-12-07 15:13:56

by Yinan Liu

Subject: [PATCH v7 2/2] script/sorttable: code style improvements

Modified the code style issue of if() {},
keep the code style consistent.

Signed-off-by: Yinan Liu <[email protected]>
---
scripts/sorttable.h | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/scripts/sorttable.h b/scripts/sorttable.h
index d1fbee1f81ef..1e8b77928fa4 100644
--- a/scripts/sorttable.h
+++ b/scripts/sorttable.h
@@ -462,11 +462,11 @@ static int do_sort(Elf_Ehdr *ehdr,
void *retval = NULL;
/* wait for ORC tables sort done */
rc = pthread_join(orc_sort_thread, &retval);
- if (rc)
+ if (rc) {
fprintf(stderr,
"pthread_join failed '%s': %s\n",
strerror(errno), fname);
- else if (retval) {
+ } else if (retval) {
rc = -1;
fprintf(stderr,
"failed to sort ORC tables '%s': %s\n",
--
2.27.0

2021-12-07 15:14:06

by Yinan Liu

Subject: [PATCH v7 1/2] scripts: ftrace - move the sort-processing in ftrace_init

When the kernel starts, the initialization of ftrace takes
up a portion of the time (approximately 6~8ms) to sort mcount
addresses. We can save this time by moving mcount-sorting to
compile time.

Signed-off-by: Yinan Liu <[email protected]>
Reported-by: kernel test robot <[email protected]>
Reported-by: kernel test robot <[email protected]>
---
kernel/trace/ftrace.c | 11 +++-
scripts/Makefile | 6 +-
scripts/link-vmlinux.sh | 6 +-
scripts/sorttable.c | 2 +
scripts/sorttable.h | 120 +++++++++++++++++++++++++++++++++++++++-
5 files changed, 137 insertions(+), 8 deletions(-)

diff --git a/kernel/trace/ftrace.c b/kernel/trace/ftrace.c
index 30bc880c3849..9ca63df6553a 100644
--- a/kernel/trace/ftrace.c
+++ b/kernel/trace/ftrace.c
@@ -6406,8 +6406,15 @@ static int ftrace_process_locs(struct module *mod,
if (!count)
return 0;

- sort(start, count, sizeof(*start),
- ftrace_cmp_ips, NULL);
+ /*
+ * Sorting mcount in vmlinux at build time depend on
+ * CONFIG_BUILDTIME_TABLE_SORT, while mcount loc in
+ * modules can not be sorted at build time.
+ */
+ if (!IS_ENABLED(CONFIG_BUILDTIME_TABLE_SORT) || mod) {
+ sort(start, count, sizeof(*start),
+ ftrace_cmp_ips, NULL);
+ }

start_pg = ftrace_allocate_pages(count);
if (!start_pg)
diff --git a/scripts/Makefile b/scripts/Makefile
index 9adb6d247818..d27503469f4b 100644
--- a/scripts/Makefile
+++ b/scripts/Makefile
@@ -17,6 +17,7 @@ hostprogs-always-$(CONFIG_SYSTEM_EXTRA_CERTIFICATE) += insert-sys-cert
hostprogs-always-$(CONFIG_SYSTEM_REVOCATION_LIST) += extract-cert

HOSTCFLAGS_sorttable.o = -I$(srctree)/tools/include
+HOSTLDLIBS_sorttable = -lpthread
HOSTCFLAGS_asn1_compiler.o = -I$(srctree)/include
HOSTCFLAGS_sign-file.o = $(CRYPTO_CFLAGS)
HOSTLDLIBS_sign-file = $(CRYPTO_LIBS)
@@ -29,7 +30,10 @@ ARCH := x86
endif
HOSTCFLAGS_sorttable.o += -I$(srctree)/tools/arch/x86/include
HOSTCFLAGS_sorttable.o += -DUNWINDER_ORC_ENABLED
-HOSTLDLIBS_sorttable = -lpthread
+endif
+
+ifdef CONFIG_FUNCTION_TRACER
+HOSTCFLAGS_sorttable.o += -DMCOUNT_SORT_ENABLED
endif

# The following programs are only built on demand
diff --git a/scripts/link-vmlinux.sh b/scripts/link-vmlinux.sh
index 5cdd9bc5c385..dd9955f45774 100755
--- a/scripts/link-vmlinux.sh
+++ b/scripts/link-vmlinux.sh
@@ -400,6 +400,9 @@ if [ -n "${CONFIG_DEBUG_INFO_BTF}" -a -n "${CONFIG_BPF}" ]; then
${RESOLVE_BTFIDS} vmlinux
fi

+info SYSMAP System.map
+mksysmap vmlinux System.map
+
if [ -n "${CONFIG_BUILDTIME_TABLE_SORT}" ]; then
info SORTTAB vmlinux
if ! sorttable vmlinux; then
@@ -408,9 +411,6 @@ if [ -n "${CONFIG_BUILDTIME_TABLE_SORT}" ]; then
fi
fi

-info SYSMAP System.map
-mksysmap vmlinux System.map
-
# step a (see comment above)
if [ -n "${CONFIG_KALLSYMS}" ]; then
mksysmap ${kallsyms_vmlinux} .tmp_System.map
diff --git a/scripts/sorttable.c b/scripts/sorttable.c
index b7c2ad71f9cf..70bdc787ddfb 100644
--- a/scripts/sorttable.c
+++ b/scripts/sorttable.c
@@ -30,6 +30,8 @@
#include <stdlib.h>
#include <string.h>
#include <unistd.h>
+#include <errno.h>
+#include <pthread.h>

#include <tools/be_byteshift.h>
#include <tools/le_byteshift.h>
diff --git a/scripts/sorttable.h b/scripts/sorttable.h
index a2baa2fefb13..d1fbee1f81ef 100644
--- a/scripts/sorttable.h
+++ b/scripts/sorttable.h
@@ -19,6 +19,9 @@

#undef extable_ent_size
#undef compare_extable
+#undef get_mcount_loc
+#undef sort_mcount_loc
+#undef elf_mcount_loc
#undef do_sort
#undef Elf_Addr
#undef Elf_Ehdr
@@ -41,6 +44,9 @@
#ifdef SORTTABLE_64
# define extable_ent_size 16
# define compare_extable compare_extable_64
+# define get_mcount_loc get_mcount_loc_64
+# define sort_mcount_loc sort_mcount_loc_64
+# define elf_mcount_loc elf_mcount_loc_64
# define do_sort do_sort_64
# define Elf_Addr Elf64_Addr
# define Elf_Ehdr Elf64_Ehdr
@@ -62,6 +68,9 @@
#else
# define extable_ent_size 8
# define compare_extable compare_extable_32
+# define get_mcount_loc get_mcount_loc_32
+# define sort_mcount_loc sort_mcount_loc_32
+# define elf_mcount_loc elf_mcount_loc_32
# define do_sort do_sort_32
# define Elf_Addr Elf32_Addr
# define Elf_Ehdr Elf32_Ehdr
@@ -84,8 +93,6 @@

#if defined(SORTTABLE_64) && defined(UNWINDER_ORC_ENABLED)
/* ORC unwinder only support X86_64 */
-#include <errno.h>
-#include <pthread.h>
#include <asm/orc_types.h>

#define ERRSTR_MAXSZ 256
@@ -191,7 +198,64 @@ static int compare_extable(const void *a, const void *b)
return 1;
return 0;
}
+#ifdef MCOUNT_SORT_ENABLED
+struct elf_mcount_loc {
+ Elf_Ehdr *ehdr;
+ Elf_Shdr *init_data_sec;
+ uint_t start_mcount_loc;
+ uint_t stop_mcount_loc;
+};
+
+/* Sort the addresses stored between __start_mcount_loc to __stop_mcount_loc in vmlinux */
+static void *sort_mcount_loc(void *arg)
+{
+ struct elf_mcount_loc *emloc = (struct elf_mcount_loc *)arg;
+ uint_t offset = emloc->start_mcount_loc - _r(&(emloc->init_data_sec)->sh_addr)
+ + _r(&(emloc->init_data_sec)->sh_offset);
+ uint_t count = emloc->stop_mcount_loc - emloc->start_mcount_loc;
+ unsigned char *start_loc = (void *)emloc->ehdr + offset;
+
+ qsort(start_loc, count/sizeof(uint_t), sizeof(uint_t), compare_extable);
+ return NULL;
+}
+
+/* Get the address of __start_mcount_loc and __stop_mcount_loc in System.map */
+static void get_mcount_loc(uint_t *_start, uint_t *_stop)
+{
+ FILE *file_start, *file_stop;
+ char start_buff[20];
+ char stop_buff[20];
+ int len = 0;
+
+ file_start = popen(" grep start_mcount System.map | awk '{print $1}' ", "r");
+ if (!file_start) {
+ fprintf(stderr, "get start_mcount_loc error!");
+ return;
+ }
+
+ file_stop = popen(" grep stop_mcount System.map | awk '{print $1}' ", "r");
+ if (!file_stop) {
+ fprintf(stderr, "get stop_mcount_loc error!");
+ pclose(file_start);
+ return;
+ }
+
+ while (fgets(start_buff, sizeof(start_buff), file_start) != NULL) {
+ len = strlen(start_buff);
+ start_buff[len - 1] = '\0';
+ }
+ *_start = strtoul(start_buff, NULL, 16);
+
+ while (fgets(stop_buff, sizeof(stop_buff), file_stop) != NULL) {
+ len = strlen(stop_buff);
+ stop_buff[len - 1] = '\0';
+ }
+ *_stop = strtoul(stop_buff, NULL, 16);

+ pclose(file_start);
+ pclose(file_stop);
+}
+#endif
static int do_sort(Elf_Ehdr *ehdr,
char const *const fname,
table_sort_t custom_sort)
@@ -217,6 +281,12 @@ static int do_sort(Elf_Ehdr *ehdr,
int idx;
unsigned int shnum;
unsigned int shstrndx;
+#ifdef MCOUNT_SORT_ENABLED
+ struct elf_mcount_loc mstruct;
+ uint_t _start_mcount_loc = 0;
+ uint_t _stop_mcount_loc = 0;
+ pthread_t mcount_sort_thread;
+#endif
#if defined(SORTTABLE_64) && defined(UNWINDER_ORC_ENABLED)
unsigned int orc_ip_size = 0;
unsigned int orc_size = 0;
@@ -253,6 +323,17 @@ static int do_sort(Elf_Ehdr *ehdr,
symtab_shndx = (Elf32_Word *)((const char *)ehdr +
_r(&s->sh_offset));

+#ifdef MCOUNT_SORT_ENABLED
+ /* locate the .init.data section in vmlinux */
+ if (!strcmp(secstrings + idx, ".init.data")) {
+ get_mcount_loc(&_start_mcount_loc, &_stop_mcount_loc);
+ mstruct.ehdr = ehdr;
+ mstruct.init_data_sec = s;
+ mstruct.start_mcount_loc = _start_mcount_loc;
+ mstruct.stop_mcount_loc = _stop_mcount_loc;
+ }
+#endif
+
#if defined(SORTTABLE_64) && defined(UNWINDER_ORC_ENABLED)
/* locate the ORC unwind tables */
if (!strcmp(secstrings + idx, ".orc_unwind_ip")) {
@@ -294,6 +375,23 @@ static int do_sort(Elf_Ehdr *ehdr,
goto out;
}
#endif
+
+#ifdef MCOUNT_SORT_ENABLED
+ if (!mstruct.init_data_sec || !_start_mcount_loc || !_stop_mcount_loc) {
+ fprintf(stderr,
+ "incomplete mcount's sort in file: %s\n",
+ fname);
+ goto out;
+ }
+
+ /* create thread to sort mcount_loc concurrently */
+ if (pthread_create(&mcount_sort_thread, NULL, &sort_mcount_loc, &mstruct)) {
+ fprintf(stderr,
+ "pthread_create mcount_sort_thread failed '%s': %s\n",
+ strerror(errno), fname);
+ goto out;
+ }
+#endif
if (!extab_sec) {
fprintf(stderr, "no __ex_table in file: %s\n", fname);
goto out;
@@ -376,5 +474,23 @@ static int do_sort(Elf_Ehdr *ehdr,
}
}
#endif
+
+#ifdef MCOUNT_SORT_ENABLED
+ if (mcount_sort_thread) {
+ void *retval = NULL;
+ /* wait for mcount sort done */
+ rc = pthread_join(mcount_sort_thread, &retval);
+ if (rc) {
+ fprintf(stderr,
+ "pthread_join failed '%s': %s\n",
+ strerror(errno), fname);
+ } else if (retval) {
+ rc = -1;
+ fprintf(stderr,
+ "failed to sort mcount '%s': %s\n",
+ (char *)retval, fname);
+ }
+ }
+#endif
return rc;
}
--
2.27.0

2021-12-11 14:50:45

by Steven Rostedt

Subject: Re: [PATCH v7 1/2] scripts: ftrace - move the sort-processing in ftrace_init

On Tue, 7 Dec 2021 23:13:46 +0800
Yinan Liu <[email protected]> wrote:

> When the kernel starts, the initialization of ftrace takes
> up a portion of the time (approximately 6~8ms) to sort mcount
> addresses. We can save this time by moving mcount-sorting to
> compile time.
>
> Signed-off-by: Yinan Liu <[email protected]>
> Reported-by: kernel test robot <[email protected]>
> Reported-by: kernel test robot <[email protected]>
> ---
> kernel/trace/ftrace.c | 11 +++-
> scripts/Makefile | 6 +-
> scripts/link-vmlinux.sh | 6 +-
> scripts/sorttable.c | 2 +
> scripts/sorttable.h | 120 +++++++++++++++++++++++++++++++++++++++-
> 5 files changed, 137 insertions(+), 8 deletions(-)
>
> diff --git a/kernel/trace/ftrace.c b/kernel/trace/ftrace.c
> index 30bc880c3849..9ca63df6553a 100644
> --- a/kernel/trace/ftrace.c
> +++ b/kernel/trace/ftrace.c
> @@ -6406,8 +6406,15 @@ static int ftrace_process_locs(struct module *mod,
> if (!count)
> return 0;
>
> - sort(start, count, sizeof(*start),
> - ftrace_cmp_ips, NULL);
> + /*
> + * Sorting mcount in vmlinux at build time depend on
> + * CONFIG_BUILDTIME_TABLE_SORT, while mcount loc in
> + * modules can not be sorted at build time.
> + */
> + if (!IS_ENABLED(CONFIG_BUILDTIME_TABLE_SORT) || mod) {
> + sort(start, count, sizeof(*start),
> + ftrace_cmp_ips, NULL);
> + }
>
> start_pg = ftrace_allocate_pages(count);
> if (!start_pg)
> diff --git a/scripts/Makefile b/scripts/Makefile
> index 9adb6d247818..d27503469f4b 100644
> --- a/scripts/Makefile
> +++ b/scripts/Makefile
> @@ -17,6 +17,7 @@ hostprogs-always-$(CONFIG_SYSTEM_EXTRA_CERTIFICATE) += insert-sys-cert
> hostprogs-always-$(CONFIG_SYSTEM_REVOCATION_LIST) += extract-cert
>
> HOSTCFLAGS_sorttable.o = -I$(srctree)/tools/include
> +HOSTLDLIBS_sorttable = -lpthread
> HOSTCFLAGS_asn1_compiler.o = -I$(srctree)/include
> HOSTCFLAGS_sign-file.o = $(CRYPTO_CFLAGS)
> HOSTLDLIBS_sign-file = $(CRYPTO_LIBS)
> @@ -29,7 +30,10 @@ ARCH := x86
> endif
> HOSTCFLAGS_sorttable.o += -I$(srctree)/tools/arch/x86/include
> HOSTCFLAGS_sorttable.o += -DUNWINDER_ORC_ENABLED
> -HOSTLDLIBS_sorttable = -lpthread
> +endif
> +
> +ifdef CONFIG_FUNCTION_TRACER
> +HOSTCFLAGS_sorttable.o += -DMCOUNT_SORT_ENABLED
> endif
>

I dropped this patch again (note I pulled in patch 2, so you do not
need to resend it). The attached config fails to build. The reason is
that you need to test for CONFIG_DYNAMIC_FTRACE and not
CONFIG_FUNCTION_TRACER. The function tracer can be statically enabled,
which means all the functions just call "mcount" (or whatever), and
they are never patched. This means that the mcount location is not
created.

Just resend a v8 of this patch. Thanks!

-- Steve

Attachments:

2021-12-12 11:34:14

by Yinan Liu

Subject: [PATCH v8 0/1] change log

Hi, steven

The config check of mcount_sort_enabled have been change
from CONFIG_FUNCTION_TRACER to CONFIG_DYNAMIC_FTRACE.
The patch now can applies to situation that the function
tracer be statically enabled.


Best regards!
-- Yinan Liu

Yinan Liu (1):
scripts: ftrace - move the sort-processing in ftrace_init

kernel/trace/ftrace.c | 11 +++-
scripts/Makefile | 6 +-
scripts/link-vmlinux.sh | 6 +-
scripts/sorttable.c | 2 +
scripts/sorttable.h | 120 +++++++++++++++++++++++++++++++++++++++-
5 files changed, 137 insertions(+), 8 deletions(-)

--
2.27.0