linux-kernel - Re: [PATCH 1/3] tracepoints: Do not use call_rcu_sched() before early

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]

Date:	Sun, 14 Dec 2014 12:14:57 -0500
From:	Steven Rostedt <rostedt@...dmis.org>
To:	Mathieu Desnoyers <mathieu.desnoyers@...icios.com>
Cc:	linux-kernel@...r.kernel.org,
	Linus Torvalds <torvalds@...ux-foundation.org>,
	Ingo Molnar <mingo@...nel.org>,
	Andrew Morton <akpm@...ux-foundation.org>,
	Thomas Gleixner <tglx@...utronix.de>,
	"Paul E. McKenney" <paulmck@...ux.vnet.ibm.com>
Subject: Re: [PATCH 1/3] tracepoints: Do not use call_rcu_sched() before
 early_initcall()

On Sun, 14 Dec 2014 17:05:49 +0000 (UTC)
Mathieu Desnoyers <mathieu.desnoyers@...icios.com> wrote:


> >  #ifdef CONFIG_MODULES
> >  /*
> >   * Tracepoint module list mutex protects the local module list.
> > @@ -76,7 +85,16 @@ static inline void release_probes(struct tracepoint_func
> > *old)
> >  	if (old) {
> >  		struct tp_probes *tp_probes = container_of(old,
> >  			struct tp_probes, probes[0]);
> > -		call_rcu_sched(&tp_probes->rcu, rcu_free_old_probes);
> > +		/*
> > +		 * Tracepoints can be enabled before RCU is initialized
> > +		 * at boot up. If that is the case, do not bother using
> > +		 * call_rcu() (because that will fail), but instead just
> > +		 * free directly.
> > +		 */
> > +		if (likely(tracepoint_rcu_is_safe))
> > +			call_rcu_sched(&tp_probes->rcu, rcu_free_old_probes);
> > +		else
> > +			rcu_free_old_probes(&tp_probes->rcu);
> 
> Would it makes sense to have call_rcu() and call_rcu_sched()
> provide this "direct call" fallback at early boot instead
> of having this in the caller ?
> 

That was my original thought, and I even started down that path. But
then I realized that tracepoints are very unique, and this is really
special for tracepoints. Then I thought, why touch a complex system like
RCU for the one off case of tracepoints? I don't really see any other
user needing this feature (well, maybe lockdep, perf or ftrace, but
still they too are special). I decided the simplest and safest approach
was to just touch the tracepoint code. Other users shouldn't need this.

If IRQ setup, or other core pieces of code can use this, then I would
agree that rcu should be changed. But until then, I rather not add
another conditional to the core RCU code.

-- Steve
--
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/