linux-kernel - Re: [PATCH v5 tip/core/rcu 14/16] rcu: Remove redundant preempt_disable() from rcu_note_voluntary_context

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: <20140813140751.GT4752@linux.vnet.ibm.com>
Date:	Wed, 13 Aug 2014 07:07:51 -0700
From:	"Paul E. McKenney" <paulmck@...ux.vnet.ibm.com>
To:	Peter Zijlstra <peterz@...radead.org>
Cc:	linux-kernel@...r.kernel.org, mingo@...nel.org,
	laijs@...fujitsu.com, dipankar@...ibm.com,
	akpm@...ux-foundation.org, mathieu.desnoyers@...icios.com,
	josh@...htriplett.org, tglx@...utronix.de, rostedt@...dmis.org,
	dhowells@...hat.com, edumazet@...gle.com, dvhart@...ux.intel.com,
	fweisbec@...il.com, oleg@...hat.com, bobby.prani@...il.com
Subject: Re: [PATCH v5 tip/core/rcu 14/16] rcu: Remove redundant
 preempt_disable() from rcu_note_voluntary_context_switch()

On Wed, Aug 13, 2014 at 12:56:18PM +0200, Peter Zijlstra wrote:
> On Mon, Aug 11, 2014 at 03:49:03PM -0700, Paul E. McKenney wrote:
> > From: "Paul E. McKenney" <paulmck@...ux.vnet.ibm.com>
> > 
> > In theory, synchronize_sched() requires a read-side critical section to
> > order against.  In practice, preemption can be thought of as being
> > disabled across every machine instruction.  So this commit removes
> > the redundant preempt_disable() from rcu_note_voluntary_context_switch().
> 
> >  #define rcu_note_voluntary_context_switch(t) \
> >  	do { \
> > -		preempt_disable(); /* Exclude synchronize_sched(); */ \
> >  		if (ACCESS_ONCE((t)->rcu_tasks_holdout)) \
> >  			ACCESS_ONCE((t)->rcu_tasks_holdout) = 0; \
> > -		preempt_enable(); \
> >  	} while (0)
> 
> But that's more than 1 instruction.

Yeah, the commit log could use some help.  The instruction in question
is the store.  The "if" is just an optimization.

So suppose that this sequence is preempted between the "if" and the store,
and that the synchronize_sched() (and quite a bit more besides!) takes
place during this preemption.  The task is still in a quiescent state
at the time of the store, so the store is still legitimate.

That said, it might be better to just leave preemption disabled, as that
certainly makes things simpler.  Thoughts?

							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/