linux-kernel - Re: [PATCH] rudimentary tracing for Classic RCU

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20081009115018.GB6628@linux.vnet.ibm.com>
Date:	Thu, 9 Oct 2008 04:50:18 -0700
From:	"Paul E. McKenney" <paulmck@...ux.vnet.ibm.com>
To:	Lai Jiangshan <laijs@...fujitsu.com>
Cc:	linux-kernel@...r.kernel.org, mingo@...e.hu, rjw@...k.pl,
	dipankar@...ibm.com, tglx@...uxtronix.de, andi@...stfloor.org
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
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/