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

From: Steven Rostedt
Date: Mon Sep 09 2013 - 12:30:35 EST


On Mon, 9 Sep 2013 09:09:20 -0700
"Paul E. McKenney" <paulmck@xxxxxxxxxxxxxxxxxx> wrote:

> Oddly enough, the earliest implementations of RCU that I worked with
> back in the early 1990s had implementation but no concept. The concepts
> came later. Something about how dragging people through 1500 lines
> of highly optimized parallel C code not being a very effective way of
> teaching RCU. Surprisingly enough, there actually were some people who
> managed to learn it that way...

This is actually my point of my arguments :-)

Things that are shown outside of the rcu files should be an interface
that reflects the concept, not the implementation (if possible). Just
like there's nothing in "preempt_count()" that says the preempt count
is stored on a per task field.

We can't expect the rest of the community to understand the RCU
implementation, when there's still many that don't understand the
concept ;-)

You can implement RCU anyway you seem fit. My concern is where very
smart people like Peter Zijlstra stumble over code used by others that
show:

preempt_disable();
ret = yada_yada();
preempt_enable();

return ret;

Where the function name states that we want CPU state, but if that's
really true, then who ever wants CPU state must not preempt here.

The point being, here the concept actually makes more sense than the
implementation, because I think one thing we all can agree on, is that
the interface throughout the kernel should be something people can
understand quickly without too much effort. The better we all can
understand, the better the kernel will be.

I think we are starting to bike shed a bit too much. The real issue is,
what can "rcu_is_cpu_idle()" be called that makes sense with those that
need to look at this code?


"rcu_is_not_active()" or "rcu_is_ignored()" where a user can see that,
"Oh, RCU is ignored here, I shouldn't be using rcu_read_lock()" or if
their code is called within something that does "rcu_is_ignored()" and
they see that they used "rcu_read_lock()" they would understand what
the issue is.

If they see "rcu_is_cpu_idle()" they would get confused: "Hmm, the cpu
isn't idle here?"

Perhaps "rcu_watching_this_cpu()" may make sense, but again, if I see a
the above preempt_enable() in that code, I would think it's a bug,
because then the return value is not guaranteed to be on this cpu.

-- Steve

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