Re: [PATCH] rcu: Is it safe to enter an RCU read-side criticalsection?

[Paul E. McKenney]

On Fri, Sep 06, 2013 at 10:52:38AM -0700, Paul E. McKenney wrote:
> On Fri, Sep 06, 2013 at 01:16:31PM -0400, Steven Rostedt wrote:
> > On Fri, 6 Sep 2013 19:00:08 +0200
> > Frederic Weisbecker <fweisbec@xxxxxxxxx> wrote:
> > 
> > > On Fri, Sep 06, 2013 at 12:52:38PM -0400, Steven Rostedt wrote:
> > > > On Fri, 6 Sep 2013 18:40:18 +0200
> > > > Frederic Weisbecker <fweisbec@xxxxxxxxx> wrote:
> > > >  
> > > > > > I can't use plain preempt_disable() in function tracing.
> > > > > > 
> > > > > > Also, since it's a misnomer to say the cpu is idle in NO_HZ_FULL when
> > > > > > we are coming from userspace, can we rename that?
> > > > > > 
> > > > > > Perhaps we can also have a __rcu_is_cpu_tracking() (or whatever), with
> > > > > > the "__" appended that does not do the preempt disable.
> > > > > 
> > > > > rcu_is_cpu_eqs() is probably better. It refers to other related "eqs" naming
> > > > > in RCU APIs.
> > > > 
> > > > But that will just confuse the heck out of people. When I see "eqs" I
> > > > equate that with "equals". What does the rcu cpu equal?
> > > 
> > > It's "extended quiescent state". There is already rcu_eqs_enter() and rcu_eqs_exit().
> > > You're right, may be we can rename that to avoid confusion with "equals". I don't mind much.
> > > I'm happy as long as the reader rcu_is_cpu_foo() and the writers rcu_foo_enter() and
> > > rcu_foo_exit() have consistant naming.
> > > 
> > 
> > What exactly does "extended quiescent state" mean? (Note, that's a
> > rhetorical question)
> 
> In which case my rhetorical (and therefore useless) answer has to be
> "it is a quiescent state that is extended".  ;-)
> 
> Sorry, couldn't resist...
> 
> > I wonder if we should change "rcu_cpu_ignore()" for "rcu_eqs_enter()"
> > and "rcu_cpu_heed()" for "rcu_eqs_exit()", as IMHO that's much more
> > straight forward to understand than trying to wrap you head around what
> > a quiescent state is, and why we are entering it or exiting it.
> > 
> > It also flat out explains to people that rcu is not processing that
> > current CPU, and things like rcu_read_lock() should not be used.
> > 
> > Then we can say "rcu_cpu_is_ignored()" for things like
> > "rcu_is_cpu_eqs()".
> 
> Currently, none of RCU's _eqs functions are exported, so they have
> the potential to confuse only people working on the RCU implementation
> itself, who had better understand what "eqs" means.
> 
> But I do count your vote against "eqs" appearing in the name of any
> function exported by RCU.
> 
> How about if I made rcu_is_cpu_idle() be as follows?
> 
> int rcu_is_cpu_idle(void)
> {
> 	int ret;
> 
> 	ret = (atomic_read(&per_cpu(rcu_dynticks.dynticks,
> 				    raw_smp_processor_id())) & 0x1) == 0;
> 	return ret;
> }
> 
> This should allow existing uses to function properly and should allow
> you to use it as well.

Never mind, I remember why this doesn't work...  The problem is that the
offset is calculated on one CPU, the task is preempted, the original CPU
goes idle, the task starts running on some other CPU, and then lockdep
complains about the CPU entering an RCU read-side critical section on
an idle CPU.

Blech.

But I -can- make them the same function on rcutiny, courtesy of the
ever-present C preprocessor.  ;-)

							Thanx, Paul

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/