* Re: [for-next][PATCH 10/31] scripts: ftrace - move the sort-processing in ftrace_init [not found] ` <20220111173115.079437896@goodmis.org> @ 2022-01-15 20:36 ` Nathan Chancellor 2022-01-16 3:59 ` Steven Rostedt 0 siblings, 1 reply; 3+ messages in thread From: Nathan Chancellor @ 2022-01-15 20:36 UTC (permalink / raw) To: Steven Rostedt, Yinan Liu Cc: linux-kernel, Ingo Molnar, Andrew Morton, kernel test robot, kernel test robot, llvm Hi Steven and Yinan, On Tue, Jan 11, 2022 at 12:30:41PM -0500, Steven Rostedt wrote: > From: Yinan Liu <yinan@linux.alibaba.com> > > 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. > > Link: https://lkml.kernel.org/r/20211212113358.34208-2-yinan@linux.alibaba.com > > Signed-off-by: Yinan Liu <yinan@linux.alibaba.com> > Reported-by: kernel test robot <lkp@intel.com> > Reported-by: kernel test robot <oliver.sang@intel.com> > Signed-off-by: Steven Rostedt <rostedt@goodmis.org> This change as commit 72b3942a173c ("scripts: ftrace - move the sort-processing in ftrace_init") in -next causes a bunch of warnings at the beginning of the build when using clang as the host compiler: $ make -skj"$(nproc)" LLVM=1 distclean allmodconfig init/main.o In file included from scripts/sorttable.c:195: scripts/sorttable.h:380: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:479:6: note: uninitialized use occurs here if (mcount_sort_thread) { ^~~~~~~~~~~~~~~~~~ scripts/sorttable.h:380: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:380: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:479:6: note: uninitialized use occurs here if (mcount_sort_thread) { ^~~~~~~~~~~~~~~~~~ scripts/sorttable.h:380:6: note: remove the '||' if its condition is always false if (!mstruct.init_data_sec || !_start_mcount_loc || !_stop_mcount_loc) { ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ scripts/sorttable.h:380: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:479:6: note: uninitialized use occurs here if (mcount_sort_thread) { ^~~~~~~~~~~~~~~~~~ scripts/sorttable.h:380:6: note: remove the '||' if its condition is always false if (!mstruct.init_data_sec || !_start_mcount_loc || !_stop_mcount_loc) { ^~~~~~~~~~~~~~~~~~~~~~~~~ scripts/sorttable.h:288: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:197: scripts/sorttable.h:380: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:479:6: note: uninitialized use occurs here if (mcount_sort_thread) { ^~~~~~~~~~~~~~~~~~ scripts/sorttable.h:380: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:380: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:479:6: note: uninitialized use occurs here if (mcount_sort_thread) { ^~~~~~~~~~~~~~~~~~ scripts/sorttable.h:380:6: note: remove the '||' if its condition is always false if (!mstruct.init_data_sec || !_start_mcount_loc || !_stop_mcount_loc) { ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ scripts/sorttable.h:380: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:479:6: note: uninitialized use occurs here if (mcount_sort_thread) { ^~~~~~~~~~~~~~~~~~ scripts/sorttable.h:380:6: note: remove the '||' if its condition is always false if (!mstruct.init_data_sec || !_start_mcount_loc || !_stop_mcount_loc) { ^~~~~~~~~~~~~~~~~~~~~~~~~ scripts/sorttable.h:370:6: warning: variable 'mcount_sort_thread' is used uninitialized whenever 'if' condition is true [-Wsometimes-uninitialized] if (pthread_create(&orc_sort_thread, NULL, ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ scripts/sorttable.h:479:6: note: uninitialized use occurs here if (mcount_sort_thread) { ^~~~~~~~~~~~~~~~~~ scripts/sorttable.h:370:2: note: remove the 'if' if its condition is always false if (pthread_create(&orc_sort_thread, NULL, ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ scripts/sorttable.h:360:6: warning: variable 'mcount_sort_thread' is used uninitialized whenever 'if' condition is true [-Wsometimes-uninitialized] if (orc_ip_size % sizeof(int) != 0 || ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ scripts/sorttable.h:479:6: note: uninitialized use occurs here if (mcount_sort_thread) { ^~~~~~~~~~~~~~~~~~ scripts/sorttable.h:360:2: note: remove the 'if' if its condition is always false if (orc_ip_size % sizeof(int) != 0 || ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ scripts/sorttable.h:360:6: warning: variable 'mcount_sort_thread' is used uninitialized whenever '||' condition is true [-Wsometimes-uninitialized] if (orc_ip_size % sizeof(int) != 0 || ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ scripts/sorttable.h:479:6: note: uninitialized use occurs here if (mcount_sort_thread) { ^~~~~~~~~~~~~~~~~~ scripts/sorttable.h:360:6: note: remove the '||' if its condition is always false if (orc_ip_size % sizeof(int) != 0 || ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ scripts/sorttable.h:360:6: warning: variable 'mcount_sort_thread' is used uninitialized whenever '||' condition is true [-Wsometimes-uninitialized] if (orc_ip_size % sizeof(int) != 0 || ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ scripts/sorttable.h:479:6: note: uninitialized use occurs here if (mcount_sort_thread) { ^~~~~~~~~~~~~~~~~~ scripts/sorttable.h:360:6: note: remove the '||' if its condition is always false if (orc_ip_size % sizeof(int) != 0 || ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ scripts/sorttable.h:353:6: warning: variable 'mcount_sort_thread' is used uninitialized whenever 'if' condition is true [-Wsometimes-uninitialized] if (!g_orc_ip_table || !g_orc_table) { ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ scripts/sorttable.h:479:6: note: uninitialized use occurs here if (mcount_sort_thread) { ^~~~~~~~~~~~~~~~~~ scripts/sorttable.h:353:2: note: remove the 'if' if its condition is always false if (!g_orc_ip_table || !g_orc_table) { ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ scripts/sorttable.h:353:6: warning: variable 'mcount_sort_thread' is used uninitialized whenever '||' condition is true [-Wsometimes-uninitialized] if (!g_orc_ip_table || !g_orc_table) { ^~~~~~~~~~~~~~~ scripts/sorttable.h:479:6: note: uninitialized use occurs here if (mcount_sort_thread) { ^~~~~~~~~~~~~~~~~~ scripts/sorttable.h:353:6: note: remove the '||' if its condition is always false if (!g_orc_ip_table || !g_orc_table) { ^~~~~~~~~~~~~~~~~~ scripts/sorttable.h:288:30: note: initialize the variable 'mcount_sort_thread' to silence this warning pthread_t mcount_sort_thread; ^ = 0 12 warnings generated. Should mcount_sort_thread be zero initialized or is there something else going on here? I am currently hunting down a bunch of other regressions so apologies for just the report rather than a patch to fix it. Cheers, Nathan ^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [for-next][PATCH 10/31] scripts: ftrace - move the sort-processing in ftrace_init 2022-01-15 20:36 ` [for-next][PATCH 10/31] scripts: ftrace - move the sort-processing in ftrace_init Nathan Chancellor @ 2022-01-16 3:59 ` Steven Rostedt 2022-01-16 4:10 ` Nathan Chancellor 0 siblings, 1 reply; 3+ messages in thread From: Steven Rostedt @ 2022-01-16 3:59 UTC (permalink / raw) To: Nathan Chancellor Cc: Yinan Liu, linux-kernel, Ingo Molnar, Andrew Morton, kernel test robot, kernel test robot, llvm On Sat, 15 Jan 2022 13:36:04 -0700 Nathan Chancellor <nathan@kernel.org> wrote: > Hi Steven and Yinan, > > On Tue, Jan 11, 2022 at 12:30:41PM -0500, Steven Rostedt wrote: > > From: Yinan Liu <yinan@linux.alibaba.com> > > > > 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. > > > > Link: https://lkml.kernel.org/r/20211212113358.34208-2-yinan@linux.alibaba.com > > > > Signed-off-by: Yinan Liu <yinan@linux.alibaba.com> > > Reported-by: kernel test robot <lkp@intel.com> > > Reported-by: kernel test robot <oliver.sang@intel.com> > > Signed-off-by: Steven Rostedt <rostedt@goodmis.org> > > This change as commit 72b3942a173c ("scripts: ftrace - move the > sort-processing in ftrace_init") in -next causes a bunch of warnings at > the beginning of the build when using clang as the host compiler: > > > Should mcount_sort_thread be zero initialized or is there something else > going on here? I am currently hunting down a bunch of other regressions > so apologies for just the report rather than a patch to fix it. Can this really happen? We have: 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 (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); } } Shouldn't the pthread_create() initialize it? And I'm not even sure if we need that if statement? Or is there a path to get there without pthread_create() initializing it? -- Steve ^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [for-next][PATCH 10/31] scripts: ftrace - move the sort-processing in ftrace_init 2022-01-16 3:59 ` Steven Rostedt @ 2022-01-16 4:10 ` Nathan Chancellor 0 siblings, 0 replies; 3+ messages in thread From: Nathan Chancellor @ 2022-01-16 4:10 UTC (permalink / raw) To: Steven Rostedt Cc: Yinan Liu, linux-kernel, Ingo Molnar, Andrew Morton, kernel test robot, kernel test robot, llvm On Sat, Jan 15, 2022 at 10:59:20PM -0500, Steven Rostedt wrote: > On Sat, 15 Jan 2022 13:36:04 -0700 > Nathan Chancellor <nathan@kernel.org> wrote: > > > Hi Steven and Yinan, > > > > On Tue, Jan 11, 2022 at 12:30:41PM -0500, Steven Rostedt wrote: > > > From: Yinan Liu <yinan@linux.alibaba.com> > > > > > > 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. > > > > > > Link: https://lkml.kernel.org/r/20211212113358.34208-2-yinan@linux.alibaba.com > > > > > > Signed-off-by: Yinan Liu <yinan@linux.alibaba.com> > > > Reported-by: kernel test robot <lkp@intel.com> > > > Reported-by: kernel test robot <oliver.sang@intel.com> > > > Signed-off-by: Steven Rostedt <rostedt@goodmis.org> > > > > This change as commit 72b3942a173c ("scripts: ftrace - move the > > sort-processing in ftrace_init") in -next causes a bunch of warnings at > > the beginning of the build when using clang as the host compiler: > > > > > > > > Should mcount_sort_thread be zero initialized or is there something else > > going on here? I am currently hunting down a bunch of other regressions > > so apologies for just the report rather than a patch to fix it. > > Can this really happen? We have: The way the code is written now, yes. > 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 (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); > } > } > > Shouldn't the pthread_create() initialize it? And I'm not even sure if we > need that if statement? > > Or is there a path to get there without pthread_create() initializing it? Yes. If the if statment right above the pthread_create() call triggers, we jump to the out label, which hits the if (mcount_sort_thread), and mcount_sort_thread won't be initialized. if (!mstruct.init_data_sec || !_start_mcount_loc || !_stop_mcount_loc) { fprintf(stderr, "incomplete mcount's sort in file: %s\n", fname); goto out; } if (pthread_create(&mcount_sort_thread, ...)) { ... out: ... if (mcount_sort_thread) { If I am misunderstanding something, please let me know. Cheers, Nathan ^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2022-01-16 4:10 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <20220111173030.999527342@goodmis.org>
[not found] ` <20220111173115.079437896@goodmis.org>
2022-01-15 20:36 ` [for-next][PATCH 10/31] scripts: ftrace - move the sort-processing in ftrace_init Nathan Chancellor
2022-01-16 3:59 ` Steven Rostedt
2022-01-16 4:10 ` Nathan Chancellor
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox