Messages in this thread | | | Date | Thu, 9 Oct 2008 04:50:18 -0700 | From | "Paul E. McKenney" <> | Subject | Re: [PATCH] rudimentary tracing for Classic RCU |
| |
On Thu, Oct 09, 2008 at 02:20:22PM +0800, Lai Jiangshan wrote:
Thank you for looking this over!
> Paul E. McKenney wrote:
[ . . . ]
> > diff --git a/kernel/Kconfig.preempt b/kernel/Kconfig.preempt > > index 9fdba03..ba32338 100644 > > --- a/kernel/Kconfig.preempt > > +++ b/kernel/Kconfig.preempt > > @@ -68,7 +68,6 @@ config PREEMPT_RCU > > > > config RCU_TRACE > > bool "Enable tracing for RCU - currently stats in debugfs" > > - depends on PREEMPT_RCU > > select DEBUG_FS > > default y > > help > > I think that we can move it to lib/Kconfig.debug.
Makes sense!
[ . . . ]
> > +static int __init rcuclassic_trace_init(void) > > +{ > > + int ret; > > + > > + rcuclassic_trace_buf = kmalloc(RCUCLASSIC_TRACE_BUF_SIZE, GFP_KERNEL); > > I'm wondering about this. > rcuclassic_trace_buf is kmalloc-ed when init module. > > In this case, why not define it as: > static char buf[20*NR_CPUS + 100]; > > and use it like: > pos += snprintf(buf + cnt, sizeof(buf) - cnt, ...);
I will make Andi's suggested change, both here and in other previously submitted code.
[ . . . ]
> > + if (!rcuclassic_trace_buf) > > + return 1; > > + ret = rcuclassic_debugfs_init(); > > + if (ret) > > + kfree(rcuclassic_trace_buf); > > + return ret; > > +} > > + > > +static void __exit rcuclassic_trace_cleanup(void) > > +{ > > + debugfs_remove(cbdir); > > + debugfs_remove(rcudir); > > + kfree(rcuclassic_trace_buf); > > +} > > + > > + > > +module_init(rcuclassic_trace_init); > > +module_exit(rcuclassic_trace_cleanup); > > > I think this trace file shows too little vars for classic rcu. > > It is better that this trace file shows more data. > such as some data in struct rcu_data(quiescent state handling fields, batch, qlen). > > or add new files: rcu/rcu_data<cpu#>
Indeed! I will add the ability to dump the rcu_data structures, though I was planning to just dump all of them rather than CPU-by-CPU. (Same approach as in rcutree_trace.c patches.)
Thanx, Paul
| |