linux-kernel - Re: [PATCH] rcu: Is it safe to enter an RCU read-side critical section?

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:	Mon, 9 Sep 2013 09:58:36 -0700
From:	"Paul E. McKenney" <paulmck@...ux.vnet.ibm.com>
To:	Steven Rostedt <rostedt@...dmis.org>
Cc:	Frederic Weisbecker <fweisbec@...il.com>,
	Peter Zijlstra <peterz@...radead.org>,
	Eric Dumazet <eric.dumazet@...il.com>,
	linux-kernel@...r.kernel.org, mingo@...e.hu, laijs@...fujitsu.com,
	dipankar@...ibm.com, akpm@...ux-foundation.org,
	mathieu.desnoyers@...icios.com, josh@...htriplett.org,
	niv@...ibm.com, tglx@...utronix.de, dhowells@...hat.com,
	edumazet@...gle.com, darren@...art.com, sbw@....edu
Subject: Re: [PATCH] rcu: Is it safe to enter an RCU read-side critical
 section?

On Mon, Sep 09, 2013 at 12:34:22PM -0400, Steven Rostedt wrote:
> On Mon, 9 Sep 2013 09:17:08 -0700
> "Paul E. McKenney" <paulmck@...ux.vnet.ibm.com> wrote:
> 
> > On Mon, Sep 09, 2013 at 10:16:29AM -0400, Steven Rostedt wrote:
> > > On Mon, 9 Sep 2013 06:56:56 -0700
> > > "Paul E. McKenney" <paulmck@...ux.vnet.ibm.com> wrote:
> > > 
> > > 
> > > > Indeed, there is on ongoing naming debate as well.  About the only point
> > > > of agreement thus far is that the current names are inadequate. ;-)
> > > > 
> > > > My current feeling is that rcu_is_cpu_idle() should be called
> > > > rcu_watching_this_cpu() and what is called rcu_watching_this_cpu()
> > > > in my local tree should be called __rcu_watching_this_cpu().
> > > 
> > > I disagree. Then it would not make sense if we take a return value of
> > > "__rcu_watching_this_cpu()" and use it on another CPU to make other
> > > decisions for that other CPU.
> > 
> > Frederic and I both went through why this works.
> 
> My concern is people stumbling over why preemption can be enabled here?
> 
> If it must *always* be called with preemption disabled (no
> rcu_watching_this_cpu() version that disables preemption for you) then
> I would be OK with it.
> 
> The problem I'm having is, anything that uses "this_cpu()" can cause
> problems with understanding the code, because the first thing I think
> is "if we get the result for 'this_cpu', it may not be 'this_cpu' when
> I use it".

Yep, this is an exception to the usual rule about not passing per-CPU
variables out of preempt_disable() regions, and will need to be commented
appropriately.

> > > I still think we are confusing concepts with implementation. Yes, the
> > > RCU implementation tracks CPU state, but the concept is still based on
> > > the task.
> > 
> > You keep asserting this, but I am not seeing it.  Sure, you can argue
> > that grace periods are based on tasks as well as or instead of CPUs.
> > But I am not convinced that it helps at the dynticks interface.
> > 
> > > But you are right, with dynamic ticks, things get a little more
> > > complex, as dynamic ticks is a CPU state, not a task state, as it can
> > > be something other than the running task that changes the state
> > > (another task gets scheduled on that CPU).
> > > 
> > > But I think we are coupling RCU a bit too much with dynamic ticks here.
> > > Maybe we need to take a step back to visualize concepts again.
> > 
> > If we don't couple it pretty tightly, it won't work.  And whatever we
> > want to call this thing that determines what RCU is paying attention to
> > has to be at the implementation level.  For things like rcu_read_lock()
> > and synchronize_rcu(), yes, the task view is important -- and in recent
> > documentation is the POV I use.
> > 
> > > The state of being in dynamic tick mode is determined by what a task or
> > > tasks are doing on the CPU. One of those things is if the task needs to
> > > be tracked by RCU. And here, is where I think we are getting our
> > > confusion from. The dynamic tick state needs to check if the running
> > > task is requiring RCU or not, and thus we ask for "is rcu needed on
> > > this CPU?" when the real question is "is the task running on this CPU
> > > requiring RCU?"
> > > 
> > > Again, if we keep things in a conceptual mode, and not look too much at
> > > the implementation details, I think more people will understand what's
> > > going on. Especially those that don't know why something was
> > > implemented the way it was.
> > 
> > All this aside, do you have a name you are nominating?
> 
> Something that doesn't specify "this_cpu" or "cpu" if the result can be
> used on another cpu correctly.
> 
> "rcu_is_ignored()" or "rcu_is_not_active()", "rcu_is_watching_you()"

You know, I am strongly tempted by "rcu_is_watching_you()", but I have
this feeling that it is too cute for its own good.  ;-)

							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/