Messages in this thread | | | Date | Sat, 15 Jan 2022 21:10:38 -0700 | From | Nathan Chancellor <> | Subject | Re: [for-next][PATCH 10/31] scripts: ftrace - move the sort-processing in ftrace_init |
| |
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
| |