* [PATCH] ftace: remove struct list_head in struct dyn_ftrace @ 2009-03-13 9:51 Lai Jiangshan 2009-03-13 10:39 ` [tip:tracing/ftrace] ftrace: remove struct list_head from " Lai Jiangshan 0 siblings, 1 reply; 6+ messages in thread From: Lai Jiangshan @ 2009-03-13 9:51 UTC (permalink / raw) To: Steven Rostedt, Ingo Molnar, LKML Impact: save memory The struct dyn_ftrace table is very large, this patch will save about 50% Signed-off-by: Lai Jiangshan <laijs@cn.fujitsu.com> --- diff --git a/include/linux/ftrace.h b/include/linux/ftrace.h index e1583f2..4dc32c8 100644 --- a/include/linux/ftrace.h +++ b/include/linux/ftrace.h @@ -145,7 +145,6 @@ enum { }; struct dyn_ftrace { - struct list_head list; unsigned long ip; /* address of mcount call-site */ unsigned long flags; struct dyn_arch_ftrace arch; diff --git a/kernel/trace/ftrace.c b/kernel/trace/ftrace.c index d33d306..40eaaae 100644 --- a/kernel/trace/ftrace.c +++ b/kernel/trace/ftrace.c @@ -272,7 +272,7 @@ enum { static int ftrace_filtered; -static LIST_HEAD(ftrace_new_addrs); +static struct dyn_ftrace *ftrace_new_addrs; static DEFINE_MUTEX(ftrace_regex_lock); @@ -408,8 +408,8 @@ ftrace_record_ip(unsigned long ip) return NULL; rec->ip = ip; - - list_add(&rec->list, &ftrace_new_addrs); + rec->flags = (unsigned long)ftrace_new_addrs; + ftrace_new_addrs = rec; return rec; } @@ -714,19 +714,21 @@ unsigned long ftrace_update_tot_cnt; static int ftrace_update_code(struct module *mod) { - struct dyn_ftrace *p, *t; + struct dyn_ftrace *p; cycle_t start, stop; start = ftrace_now(raw_smp_processor_id()); ftrace_update_cnt = 0; - list_for_each_entry_safe(p, t, &ftrace_new_addrs, list) { + while (ftrace_new_addrs) { /* If something went wrong, bail without enabling anything */ if (unlikely(ftrace_disabled)) return -1; - list_del_init(&p->list); + p = ftrace_new_addrs; + ftrace_new_addrs = (struct dyn_ftrace *)p->flags; + p->flags = 0L; /* convert record (i.e, patch mcount-call with NOP) */ if (ftrace_code_disable(mod, p)) { ^ permalink raw reply related [flat|nested] 6+ messages in thread
* [tip:tracing/ftrace] ftrace: remove struct list_head from struct dyn_ftrace 2009-03-13 9:51 [PATCH] ftace: remove struct list_head in struct dyn_ftrace Lai Jiangshan @ 2009-03-13 10:39 ` Lai Jiangshan 2009-03-13 13:16 ` Steven Rostedt 2009-03-16 12:36 ` Frederic Weisbecker 0 siblings, 2 replies; 6+ messages in thread From: Lai Jiangshan @ 2009-03-13 10:39 UTC (permalink / raw) To: linux-tip-commits; +Cc: linux-kernel, hpa, mingo, srostedt, tglx, laijs, mingo Commit-ID: e94142a67f8bad494c593f0a07c9fc2fbec98c0e Gitweb: http://git.kernel.org/tip/e94142a67f8bad494c593f0a07c9fc2fbec98c0e Author: Lai Jiangshan <laijs@cn.fujitsu.com> AuthorDate: Fri, 13 Mar 2009 17:51:27 +0800 Commit: Ingo Molnar <mingo@elte.hu> CommitDate: Fri, 13 Mar 2009 11:36:20 +0100 ftrace: remove struct list_head from struct dyn_ftrace Impact: save memory The struct dyn_ftrace table is very large, this patch will save about 50%. Signed-off-by: Lai Jiangshan <laijs@cn.fujitsu.com> Cc: Steven Rostedt <srostedt@redhat.com> LKML-Reference: <49BA2C9F.8020009@cn.fujitsu.com> Signed-off-by: Ingo Molnar <mingo@elte.hu> --- include/linux/ftrace.h | 1 - kernel/trace/ftrace.c | 14 ++++++++------ 2 files changed, 8 insertions(+), 7 deletions(-) diff --git a/include/linux/ftrace.h b/include/linux/ftrace.h index c146c10..9d598bb 100644 --- a/include/linux/ftrace.h +++ b/include/linux/ftrace.h @@ -145,7 +145,6 @@ enum { }; struct dyn_ftrace { - struct list_head list; unsigned long ip; /* address of mcount call-site */ unsigned long flags; struct dyn_arch_ftrace arch; diff --git a/kernel/trace/ftrace.c b/kernel/trace/ftrace.c index bf78a4c..90d5729 100644 --- a/kernel/trace/ftrace.c +++ b/kernel/trace/ftrace.c @@ -272,7 +272,7 @@ enum { static int ftrace_filtered; -static LIST_HEAD(ftrace_new_addrs); +static struct dyn_ftrace *ftrace_new_addrs; static DEFINE_MUTEX(ftrace_regex_lock); @@ -409,8 +409,8 @@ ftrace_record_ip(unsigned long ip) return NULL; rec->ip = ip; - - list_add(&rec->list, &ftrace_new_addrs); + rec->flags = (unsigned long)ftrace_new_addrs; + ftrace_new_addrs = rec; return rec; } @@ -716,19 +716,21 @@ unsigned long ftrace_update_tot_cnt; static int ftrace_update_code(struct module *mod) { - struct dyn_ftrace *p, *t; + struct dyn_ftrace *p; cycle_t start, stop; start = ftrace_now(raw_smp_processor_id()); ftrace_update_cnt = 0; - list_for_each_entry_safe(p, t, &ftrace_new_addrs, list) { + while (ftrace_new_addrs) { /* If something went wrong, bail without enabling anything */ if (unlikely(ftrace_disabled)) return -1; - list_del_init(&p->list); + p = ftrace_new_addrs; + ftrace_new_addrs = (struct dyn_ftrace *)p->flags; + p->flags = 0L; /* convert record (i.e, patch mcount-call with NOP) */ if (ftrace_code_disable(mod, p)) { ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [tip:tracing/ftrace] ftrace: remove struct list_head from struct dyn_ftrace 2009-03-13 10:39 ` [tip:tracing/ftrace] ftrace: remove struct list_head from " Lai Jiangshan @ 2009-03-13 13:16 ` Steven Rostedt 2009-03-16 12:36 ` Frederic Weisbecker 1 sibling, 0 replies; 6+ messages in thread From: Steven Rostedt @ 2009-03-13 13:16 UTC (permalink / raw) To: mingo, hpa, linux-kernel, tglx, laijs, mingo; +Cc: linux-tip-commits On Fri, 2009-03-13 at 10:39 +0000, Lai Jiangshan wrote: > Commit-ID: e94142a67f8bad494c593f0a07c9fc2fbec98c0e > Gitweb: http://git.kernel.org/tip/e94142a67f8bad494c593f0a07c9fc2fbec98c0e > Author: Lai Jiangshan <laijs@cn.fujitsu.com> > AuthorDate: Fri, 13 Mar 2009 17:51:27 +0800 > Commit: Ingo Molnar <mingo@elte.hu> > CommitDate: Fri, 13 Mar 2009 11:36:20 +0100 > > ftrace: remove struct list_head from struct dyn_ftrace > Impact: save memory > > The struct dyn_ftrace table is very large, this patch will save > about 50%. > > Signed-off-by: Lai Jiangshan <laijs@cn.fujitsu.com> > Cc: Steven Rostedt <srostedt@redhat.com> > LKML-Reference: <49BA2C9F.8020009@cn.fujitsu.com> > Signed-off-by: Ingo Molnar <mingo@elte.hu> > > > --- > include/linux/ftrace.h | 1 - > kernel/trace/ftrace.c | 14 ++++++++------ > 2 files changed, 8 insertions(+), 7 deletions(-) > > diff --git a/include/linux/ftrace.h b/include/linux/ftrace.h > index c146c10..9d598bb 100644 > --- a/include/linux/ftrace.h > +++ b/include/linux/ftrace.h > @@ -145,7 +145,6 @@ enum { > }; > > struct dyn_ftrace { > - struct list_head list; > unsigned long ip; /* address of mcount call-site */ > unsigned long flags; > struct dyn_arch_ftrace arch; > diff --git a/kernel/trace/ftrace.c b/kernel/trace/ftrace.c > index bf78a4c..90d5729 100644 > --- a/kernel/trace/ftrace.c > +++ b/kernel/trace/ftrace.c > @@ -272,7 +272,7 @@ enum { > > static int ftrace_filtered; > > -static LIST_HEAD(ftrace_new_addrs); > +static struct dyn_ftrace *ftrace_new_addrs; > > static DEFINE_MUTEX(ftrace_regex_lock); > > @@ -409,8 +409,8 @@ ftrace_record_ip(unsigned long ip) > return NULL; > > rec->ip = ip; > - > - list_add(&rec->list, &ftrace_new_addrs); > + rec->flags = (unsigned long)ftrace_new_addrs; Lets not play games with pointers. If we are going to do this, make it into a union. struct dyn_arch_ftrace { unsigned long ip; union { unsigned long flags; struct dyn_arch_ftrace *next; } struct dyn_arch_ftrace arch; }; This way it is much clearer to what is going on. > + ftrace_new_addrs = rec; > > return rec; > } > @@ -716,19 +716,21 @@ unsigned long ftrace_update_tot_cnt; > > static int ftrace_update_code(struct module *mod) > { > - struct dyn_ftrace *p, *t; > + struct dyn_ftrace *p; > cycle_t start, stop; > > start = ftrace_now(raw_smp_processor_id()); > ftrace_update_cnt = 0; > > - list_for_each_entry_safe(p, t, &ftrace_new_addrs, list) { > + while (ftrace_new_addrs) { > > /* If something went wrong, bail without enabling anything */ > if (unlikely(ftrace_disabled)) > return -1; > > - list_del_init(&p->list); > + p = ftrace_new_addrs; > + ftrace_new_addrs = (struct dyn_ftrace *)p->flags; This confused me (and scared me) for a bit. Having a union would not have. Make the change to a union, and then you have my: Acked-by: Steven Rostedt <srostedt@redhat.com> Thanks, -- Steve > + p->flags = 0L; > > /* convert record (i.e, patch mcount-call with NOP) */ > if (ftrace_code_disable(mod, p)) { ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [tip:tracing/ftrace] ftrace: remove struct list_head from struct dyn_ftrace 2009-03-13 10:39 ` [tip:tracing/ftrace] ftrace: remove struct list_head from " Lai Jiangshan 2009-03-13 13:16 ` Steven Rostedt @ 2009-03-16 12:36 ` Frederic Weisbecker 2009-03-16 12:42 ` Frederic Weisbecker 1 sibling, 1 reply; 6+ messages in thread From: Frederic Weisbecker @ 2009-03-16 12:36 UTC (permalink / raw) To: mingo, hpa, linux-kernel, srostedt, tglx, laijs, mingo; +Cc: linux-tip-commits On Fri, Mar 13, 2009 at 10:39:36AM +0000, Lai Jiangshan wrote: > Commit-ID: e94142a67f8bad494c593f0a07c9fc2fbec98c0e > Gitweb: http://git.kernel.org/tip/e94142a67f8bad494c593f0a07c9fc2fbec98c0e > Author: Lai Jiangshan <laijs@cn.fujitsu.com> > AuthorDate: Fri, 13 Mar 2009 17:51:27 +0800 > Commit: Ingo Molnar <mingo@elte.hu> > CommitDate: Fri, 13 Mar 2009 11:36:20 +0100 > > ftrace: remove struct list_head from struct dyn_ftrace > > Impact: save memory > > The struct dyn_ftrace table is very large, this patch will save > about 50%. > > Signed-off-by: Lai Jiangshan <laijs@cn.fujitsu.com> > Cc: Steven Rostedt <srostedt@redhat.com> > LKML-Reference: <49BA2C9F.8020009@cn.fujitsu.com> > Signed-off-by: Ingo Molnar <mingo@elte.hu> > > > --- > include/linux/ftrace.h | 1 - > kernel/trace/ftrace.c | 14 ++++++++------ > 2 files changed, 8 insertions(+), 7 deletions(-) > > diff --git a/include/linux/ftrace.h b/include/linux/ftrace.h > index c146c10..9d598bb 100644 > --- a/include/linux/ftrace.h > +++ b/include/linux/ftrace.h > @@ -145,7 +145,6 @@ enum { > }; > > struct dyn_ftrace { > - struct list_head list; > unsigned long ip; /* address of mcount call-site */ > unsigned long flags; > struct dyn_arch_ftrace arch; > diff --git a/kernel/trace/ftrace.c b/kernel/trace/ftrace.c > index bf78a4c..90d5729 100644 > --- a/kernel/trace/ftrace.c > +++ b/kernel/trace/ftrace.c > @@ -272,7 +272,7 @@ enum { > > static int ftrace_filtered; > > -static LIST_HEAD(ftrace_new_addrs); > +static struct dyn_ftrace *ftrace_new_addrs; > > static DEFINE_MUTEX(ftrace_regex_lock); > > @@ -409,8 +409,8 @@ ftrace_record_ip(unsigned long ip) > return NULL; > > rec->ip = ip; > - > - list_add(&rec->list, &ftrace_new_addrs); > + rec->flags = (unsigned long)ftrace_new_addrs; > + ftrace_new_addrs = rec; But... you are overwritting all the ftrace flags infrastructure by using it as a list pointer. This is breaking dynamic tracing. > > return rec; > } > @@ -716,19 +716,21 @@ unsigned long ftrace_update_tot_cnt; > > static int ftrace_update_code(struct module *mod) > { > - struct dyn_ftrace *p, *t; > + struct dyn_ftrace *p; > cycle_t start, stop; > > start = ftrace_now(raw_smp_processor_id()); > ftrace_update_cnt = 0; > > - list_for_each_entry_safe(p, t, &ftrace_new_addrs, list) { > + while (ftrace_new_addrs) { > > /* If something went wrong, bail without enabling anything */ > if (unlikely(ftrace_disabled)) > return -1; > > - list_del_init(&p->list); > + p = ftrace_new_addrs; > + ftrace_new_addrs = (struct dyn_ftrace *)p->flags; > + p->flags = 0L; > > /* convert record (i.e, patch mcount-call with NOP) */ > if (ftrace_code_disable(mod, p)) { > -- > To unsubscribe from this list: send the line "unsubscribe linux-tip-commits" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [tip:tracing/ftrace] ftrace: remove struct list_head from struct dyn_ftrace 2009-03-16 12:36 ` Frederic Weisbecker @ 2009-03-16 12:42 ` Frederic Weisbecker 2009-03-18 2:50 ` Lai Jiangshan 0 siblings, 1 reply; 6+ messages in thread From: Frederic Weisbecker @ 2009-03-16 12:42 UTC (permalink / raw) To: mingo, hpa, linux-kernel, srostedt, tglx, laijs, mingo; +Cc: linux-tip-commits On Mon, Mar 16, 2009 at 01:36:09PM +0100, Frederic Weisbecker wrote: > On Fri, Mar 13, 2009 at 10:39:36AM +0000, Lai Jiangshan wrote: > > Commit-ID: e94142a67f8bad494c593f0a07c9fc2fbec98c0e > > Gitweb: http://git.kernel.org/tip/e94142a67f8bad494c593f0a07c9fc2fbec98c0e > > Author: Lai Jiangshan <laijs@cn.fujitsu.com> > > AuthorDate: Fri, 13 Mar 2009 17:51:27 +0800 > > Commit: Ingo Molnar <mingo@elte.hu> > > CommitDate: Fri, 13 Mar 2009 11:36:20 +0100 > > > > ftrace: remove struct list_head from struct dyn_ftrace > > > > Impact: save memory > > > > The struct dyn_ftrace table is very large, this patch will save > > about 50%. > > > > Signed-off-by: Lai Jiangshan <laijs@cn.fujitsu.com> > > Cc: Steven Rostedt <srostedt@redhat.com> > > LKML-Reference: <49BA2C9F.8020009@cn.fujitsu.com> > > Signed-off-by: Ingo Molnar <mingo@elte.hu> > > > > > > --- > > include/linux/ftrace.h | 1 - > > kernel/trace/ftrace.c | 14 ++++++++------ > > 2 files changed, 8 insertions(+), 7 deletions(-) > > > > diff --git a/include/linux/ftrace.h b/include/linux/ftrace.h > > index c146c10..9d598bb 100644 > > --- a/include/linux/ftrace.h > > +++ b/include/linux/ftrace.h > > @@ -145,7 +145,6 @@ enum { > > }; > > > > struct dyn_ftrace { > > - struct list_head list; > > unsigned long ip; /* address of mcount call-site */ > > unsigned long flags; > > struct dyn_arch_ftrace arch; > > diff --git a/kernel/trace/ftrace.c b/kernel/trace/ftrace.c > > index bf78a4c..90d5729 100644 > > --- a/kernel/trace/ftrace.c > > +++ b/kernel/trace/ftrace.c > > @@ -272,7 +272,7 @@ enum { > > > > static int ftrace_filtered; > > > > -static LIST_HEAD(ftrace_new_addrs); > > +static struct dyn_ftrace *ftrace_new_addrs; > > > > static DEFINE_MUTEX(ftrace_regex_lock); > > > > @@ -409,8 +409,8 @@ ftrace_record_ip(unsigned long ip) > > return NULL; > > > > rec->ip = ip; > > - > > - list_add(&rec->list, &ftrace_new_addrs); > > + rec->flags = (unsigned long)ftrace_new_addrs; > > + ftrace_new_addrs = rec; > > > But... you are overwritting all the ftrace flags infrastructure by > using it as a list pointer. > > This is breaking dynamic tracing. May be I'm wrong. May be I misunderstood this patch. But I don't understand how flags can be used at the same time as a pointer and as a flag value. > > > > return rec; > > } > > @@ -716,19 +716,21 @@ unsigned long ftrace_update_tot_cnt; > > > > static int ftrace_update_code(struct module *mod) > > { > > - struct dyn_ftrace *p, *t; > > + struct dyn_ftrace *p; > > cycle_t start, stop; > > > > start = ftrace_now(raw_smp_processor_id()); > > ftrace_update_cnt = 0; > > > > - list_for_each_entry_safe(p, t, &ftrace_new_addrs, list) { > > + while (ftrace_new_addrs) { > > > > /* If something went wrong, bail without enabling anything */ > > if (unlikely(ftrace_disabled)) > > return -1; > > > > - list_del_init(&p->list); > > + p = ftrace_new_addrs; > > + ftrace_new_addrs = (struct dyn_ftrace *)p->flags; > > + p->flags = 0L; > > > > /* convert record (i.e, patch mcount-call with NOP) */ > > if (ftrace_code_disable(mod, p)) { > > -- > > To unsubscribe from this list: send the line "unsubscribe linux-tip-commits" in > > the body of a message to majordomo@vger.kernel.org > > More majordomo info at http://vger.kernel.org/majordomo-info.html > ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [tip:tracing/ftrace] ftrace: remove struct list_head from struct dyn_ftrace 2009-03-16 12:42 ` Frederic Weisbecker @ 2009-03-18 2:50 ` Lai Jiangshan 0 siblings, 0 replies; 6+ messages in thread From: Lai Jiangshan @ 2009-03-18 2:50 UTC (permalink / raw) To: Frederic Weisbecker Cc: mingo, hpa, linux-kernel, srostedt, tglx, mingo, linux-tip-commits Frederic Weisbecker wrote: > > > May be I'm wrong. May be I misunderstood this patch. > But I don't understand how flags can be used at the same time as a pointer > and as a flag value. > When a new dyn_ftrace is added to ftrace_new_addrs list, the flag is 0, it's always 0 until this dyn_ftrace is removed from the list, so we can reuse the flag field. As Steven's suggest, using an union is better. ftrace_convert_nops() ftrace_record_ip() // allocate dyn_ftrace, and add it // to ftrace_new_addrs list ftrace_update_code() // remove it from ftrace_new_addrs. Lai. ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2009-03-18 2:51 UTC | newest] Thread overview: 6+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2009-03-13 9:51 [PATCH] ftace: remove struct list_head in struct dyn_ftrace Lai Jiangshan 2009-03-13 10:39 ` [tip:tracing/ftrace] ftrace: remove struct list_head from " Lai Jiangshan 2009-03-13 13:16 ` Steven Rostedt 2009-03-16 12:36 ` Frederic Weisbecker 2009-03-16 12:42 ` Frederic Weisbecker 2009-03-18 2:50 ` Lai Jiangshan
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox