Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751549AbbBLRf1 (ORCPT ); Thu, 12 Feb 2015 12:35:27 -0500 Received: from smtprelay0068.hostedemail.com ([216.40.44.68]:57244 "EHLO smtprelay.hostedemail.com" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S1751065AbbBLRf0 (ORCPT ); Thu, 12 Feb 2015 12:35:26 -0500 X-Session-Marker: 6E657665747340676F6F646D69732E6F7267 X-Spam-Summary: X-HE-Tag: blood37_4bf5d8bb0b54f X-Filterd-Recvd-Size: 5458 Date: Thu, 12 Feb 2015 12:35:20 -0500 From: Steven Rostedt To: Wang Nan Cc: , , , , , , , , , , , , , , , , , , , , , , , , , , , , , , , , Subject: Re: [RFC PATCH v2 06/26] ftrace: sort ftrace entries earlier. Message-ID: <20150212123520.2ca2999c@gandalf.local.home> In-Reply-To: <1423743581-12267-1-git-send-email-wangnan0@huawei.com> References: <1423743476-11927-1-git-send-email-wangnan0@huawei.com> <1423743581-12267-1-git-send-email-wangnan0@huawei.com> X-Mailer: Claws Mail 3.11.1 (GTK+ 2.24.25; x86_64-pc-linux-gnu) MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 3917 Lines: 129 On Thu, 12 Feb 2015 20:19:41 +0800 Wang Nan wrote: The header is not enough for a change log. You need to tell us why this patch is needed. BTW, the previous two patches look fine, and I'm willing to pull them into my 3.21 queue as clean ups. As for this one... > Signed-off-by: Wang Nan > --- > include/linux/ftrace.h | 2 ++ > init/main.c | 1 + > kernel/trace/ftrace.c | 29 +++++++++++++++++++++++++++-- > 3 files changed, 30 insertions(+), 2 deletions(-) > > diff --git a/include/linux/ftrace.h b/include/linux/ftrace.h > index 1da6029..8db315a 100644 > --- a/include/linux/ftrace.h > +++ b/include/linux/ftrace.h > @@ -701,8 +701,10 @@ static inline void __ftrace_enabled_restore(int enabled) > > #ifdef CONFIG_FTRACE_MCOUNT_RECORD > extern void ftrace_init(void); > +extern void ftrace_init_early(void); > #else > static inline void ftrace_init(void) { } > +static inline void ftrace_init_early(void) { } > #endif > > /* > diff --git a/init/main.c b/init/main.c > index 6f0f1c5f..eaafc3e 100644 > --- a/init/main.c > +++ b/init/main.c > @@ -517,6 +517,7 @@ asmlinkage __visible void __init start_kernel(void) > boot_cpu_init(); > page_address_init(); > pr_notice("%s", linux_banner); > + ftrace_init_early(); > setup_arch(&command_line); > mm_init_cpumask(&init_mm); > setup_command_line(command_line); > diff --git a/kernel/trace/ftrace.c b/kernel/trace/ftrace.c > index 6c6cbb1..a6a6b09 100644 > --- a/kernel/trace/ftrace.c > +++ b/kernel/trace/ftrace.c > @@ -1169,6 +1169,7 @@ struct ftrace_page { > > static struct ftrace_page *ftrace_pages_start; > static struct ftrace_page *ftrace_pages; > +static bool mcount_sorted = false; > > static bool __always_inline ftrace_hash_empty(struct ftrace_hash *hash) > { > @@ -4743,6 +4744,26 @@ static void ftrace_swap_ips(void *a, void *b, int size) > *ipb = t; > } > > +static void ftrace_sort_mcount_area(void) > +{ > + extern unsigned long __start_mcount_loc[]; > + extern unsigned long __stop_mcount_loc[]; > + > + unsigned long *start = __start_mcount_loc; > + unsigned long *end = __stop_mcount_loc; > + unsigned long count; > + > + count = end - start; > + if (!count) > + return; > + > + if (!mcount_sorted) { > + sort(start, count, sizeof(*start), > + ftrace_cmp_ips, ftrace_swap_ips); > + mcount_sorted = true; > + } > +} > + > static int ftrace_process_locs(struct module *mod, > unsigned long *start, > unsigned long *end) > @@ -4761,8 +4782,7 @@ static int ftrace_process_locs(struct module *mod, > if (!count) > return 0; > > - sort(start, count, sizeof(*start), > - ftrace_cmp_ips, ftrace_swap_ips); > + ftrace_sort_mcount_area(); Notice a problem with the above? You just lost start and count. They are not always the same. You can not just hard code __start_mcount_loc. In fact, I'm surprised this didn't crash, because the section that holds __start_mcount_loc is freed after boot. Modules use this code to pass in where they hold the mcount locations. Your change ignores that and uses the stale __start_mcount_loc that no longer exists at the point modules are loaded. The sort routine needs to have start and end passed to it, then it could calculate count from end - start. -- Steve > > start_pg = ftrace_allocate_pages(count); > if (!start_pg) > @@ -4965,6 +4985,11 @@ void __init ftrace_init(void) > ftrace_disabled = 1; > } > > +void __init ftrace_init_early(void) > +{ > + ftrace_sort_mcount_area(); > +} > + > /* Do nothing if arch does not support this */ > void __weak arch_ftrace_update_trampoline(struct ftrace_ops *ops) > { -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/