Re: [PATCH RFC 3/9] RCU: Preemptible RCU

!MAILaRCHIVE_VOTE_RePLACE
Previous message: [thread] [date] [author]
Next message: [thread] [date] [author]
To: Paul E. McKenney <paulmck@...>
Cc: <linux-kernel@...>, <linux-rt-users@...>, <mingo@...>, <akpm@...>, <dipankar@...>, <josht@...>, <tytso@...>, <dvhltc@...>, <tglx@...>, <a.p.zijlstra@...>, <bunk@...>, <ego@...>, <oleg@...>
Date: Friday, September 21, 2007 - 10:40 am

On Mon, Sep 10, 2007 at 11:34:12AM -0700, Paul E. McKenney wrote:

A bit extreme on the comments here.



I take it that GP stand for "grace period". Might want to state that
here. /* Grace period stages */  When I was looking at this code at 1am,
I kept asking myself "what's this GP?" (General Protection??). But
that's what happens when looking at code like this after midnight ;-)


Can you have a pointer somewhere that explains these states. And not a
"it's in this paper or directory". Either have a short discription here,
or specify where exactly to find the information (perhaps a
Documentation/RCU/preemptible_states.txt?).

Trying to understand these states has caused me the most agony in
reviewing these patches.


If they are identical, then why the separation?


Nitpick, but other places in the kernel usually use "t" or "p" as a
variable to assign current to.  It's just that "me" thows me off a
little while reviewing this.  But this is just a nitpick, so do as you
will.


Nitpick, "flags" is usually used for saving irq state.


Isn't the GP detection done via a tasklet/softirq. So wouldn't a
local_bh_disable be sufficient here? You already cover NMIs, which would
also handle normal interrupts.


s/coutner/counter/


Oh, come now! It's not "nuts" to use this ;-)


Just trying to understand all this. Here at flip_idle, only a CPU with
no pending RCU calls will flip it. Then all the cpus flags will be set
to rcu_flipped, and the ctrl.completed counter is incremented.

When a CPU calls process_callbacks, it would then move all it's
callbacks to the next list (next -> wait[GP...] -> done), and set it's
unique completed counter to completed. So no more moving up the lists
will be done. It also will set it's state flag to rcu_flip_seen. Even if
the cpu doesn't have any RCU callbacks, the state would be in
rcu_try_flip_waitack_state so we go to the next switch case for a
rcu_try_flip call.

Also the filp counter has been flipped so that all new rcu_read_locks
will increment the cpus "other" index. We just need to wait for the
index of the previous counters to reach zero.


Now, we just wait to see if all the cpus have called process_callbacks
and now have the flag rcu_flip_seen set. So all the cpus have pushed
their callbacks up to the next queue.


Now this is where we wait for the sum of all the CPUs counters to reach
zero. The reason for the sum is that the task may have been preempted
and migrated to another CPU and decremented that counter. So the one CPU
counter would be 1 and the other would be -1. But the sum is still zero.
Is there a chance that overflow of a counter (although probably very
very unlikely) would cause any problems?

Also, all the CPUs have their "check_mb" set.


I have to admit that this seems a bit of an overkill, but I guess you
know what you are doing.  After going through three states, we still
need to do a memory barrier on each CPU?


Finally we are back to the original state and we start the process all
over.

Is this analysis correct?


Didn't we have a discussion that this is no longer true? Or was it taken
out of Dynamic ticks before. I don't see any users of it.


OK, that's all I have on this patch (will take a bit of a break before
reviewing your other patches).  But I will say that RCU has grown quite
a bit, and is looking very good.

Basically, what I'm saying is "Great work, Paul!".  This is looking
good. Seems that we just need a little bit better explanation for those
that are not up at the IQ level of you.  I can write something up after
this all gets finalized. Sort of a rcu-design.txt, that really tries to
explain it to the simpleton's like me ;-)

-- Steve

-
Previous message: [thread] [date] [author]
Next message: [thread] [date] [author]

Messages in current thread:
[PATCH RFC 0/9] RCU: Preemptible RCU, Paul E. McKenney, (Mon Sep 10, 2:30 pm)
Re: [PATCH RFC 0/9] RCU: Preemptible RCU, Ingo Molnar, (Mon Sep 10, 2:44 pm)
[PATCH RFC 6/9] RCU priority boosting for preemptible RCU, Paul E. McKenney, (Mon Sep 10, 2:39 pm)
Re: [PATCH RFC 6/9] RCU priority boosting for preemptible RCU, Paul E. McKenney, (Fri Oct 5, 10:07 am)
Re: [PATCH RFC 6/9] RCU priority boosting for preemptible RCU, Gautham R Shenoy, (Fri Sep 28, 6:56 pm)
Re: [PATCH RFC 6/9] RCU priority boosting for preemptible RCU, Paul E. McKenney, (Sat Sep 29, 11:11 pm)
[PATCH RFC 5/9] RCU: CPU hotplug support for preemptible RCU, Paul E. McKenney, (Mon Sep 10, 2:36 pm)
[PATCH RFC 3/9] RCU: Preemptible RCU, Paul E. McKenney, (Mon Sep 10, 2:34 pm)
Re: [PATCH RFC 3/9] RCU: Preemptible RCU, Oleg Nesterov, (Sun Sep 23, 1:38 pm)
Re: [PATCH RFC 3/9] RCU: Preemptible RCU, Paul E. McKenney, (Sun Sep 23, 8:15 pm)
Re: [PATCH RFC 3/9] RCU: Preemptible RCU, Oleg Nesterov, (Wed Sep 26, 11:13 am)
Re: [PATCH RFC 3/9] RCU: Preemptible RCU, Paul E. McKenney, (Thu Sep 27, 11:46 am)
Re: [PATCH RFC 3/9] RCU: Preemptible RCU, Oleg Nesterov, (Fri Sep 28, 10:47 am)
Re: [PATCH RFC 3/9] RCU: Preemptible RCU, Paul E. McKenney, (Fri Sep 28, 2:57 pm)
Re: [PATCH RFC 3/9] RCU: Preemptible RCU, Oleg Nesterov, (Sun Sep 30, 12:31 pm)
Re: [PATCH RFC 3/9] RCU: Preemptible RCU, Paul E. McKenney, (Sun Sep 30, 9:20 pm)
Re: [PATCH RFC 3/9] RCU: Preemptible RCU, Davide Libenzi, (Sun Sep 30, 7:02 pm)
Re: [PATCH RFC 3/9] RCU: Preemptible RCU, Paul E. McKenney, (Sun Sep 30, 9:37 pm)
Re: [PATCH RFC 3/9] RCU: Preemptible RCU, Oleg Nesterov, (Tue Oct 2, 2:02 pm)
Re: [PATCH RFC 3/9] RCU: Preemptible RCU, Davide Libenzi, (Mon Oct 1, 2:44 pm)
Re: [PATCH RFC 3/9] RCU: Preemptible RCU, Paul E. McKenney, (Mon Oct 1, 3:21 pm)
Re: [PATCH RFC 3/9] RCU: Preemptible RCU, Davide Libenzi, (Mon Oct 1, 6:09 pm)
Re: [PATCH RFC 3/9] RCU: Preemptible RCU, Paul E. McKenney, (Mon Oct 1, 6:24 pm)
Re: [PATCH RFC 3/9] RCU: Preemptible RCU, Steven Rostedt, (Fri Sep 21, 11:20 am)
Re: [PATCH RFC 3/9] RCU: Preemptible RCU, Paul E. McKenney, (Fri Sep 21, 7:03 pm)
Re: [PATCH RFC 3/9] RCU: Preemptible RCU, Paul E. McKenney, (Fri Sep 21, 8:32 pm)
Re: [PATCH RFC 3/9] RCU: Preemptible RCU, Steven Rostedt, (Fri Sep 21, 9:19 pm)
Re: [PATCH RFC 3/9] RCU: Preemptible RCU, Paul E. McKenney, (Fri Sep 21, 9:43 pm)
Re: [PATCH RFC 3/9] RCU: Preemptible RCU, Steven Rostedt, (Fri Sep 21, 10:56 pm)
Re: [PATCH RFC 3/9] RCU: Preemptible RCU, Paul E. McKenney, (Sat Sep 22, 12:10 am)
Re: [PATCH RFC 3/9] RCU: Preemptible RCU, Steven Rostedt, (Fri Sep 21, 10:40 am)
Re: [PATCH RFC 3/9] RCU: Preemptible RCU, Paul E. McKenney, (Fri Sep 21, 8:26 pm)
Re: [PATCH RFC 3/9] RCU: Preemptible RCU, Steven Rostedt, (Fri Sep 21, 9:15 pm)
Re: [PATCH RFC 3/9] RCU: Preemptible RCU, Paul E. McKenney, (Fri Sep 21, 9:53 pm)
Re: [PATCH RFC 3/9] RCU: Preemptible RCU, Steven Rostedt, (Fri Sep 21, 11:15 pm)
Re: [PATCH RFC 3/9] RCU: Preemptible RCU, Paul E. McKenney, (Sat Sep 22, 12:07 am)
Re: [PATCH RFC 3/9] RCU: Preemptible RCU, Peter Zijlstra, (Fri Sep 21, 11:46 am)
Re: [PATCH RFC 3/9] RCU: Preemptible RCU, Steven Rostedt, (Fri Sep 21, 6:31 pm)
Re: [PATCH RFC 3/9] RCU: Preemptible RCU, Paul E. McKenney, (Fri Sep 21, 6:44 pm)
Re: [PATCH RFC 3/9] RCU: Preemptible RCU, Steven Rostedt, (Fri Sep 21, 7:23 pm)
Re: [PATCH RFC 3/9] RCU: Preemptible RCU, Paul E. McKenney, (Fri Sep 21, 7:44 pm)
Re: [PATCH RFC 3/9] RCU: Preemptible RCU, Paul E. McKenney, (Fri Sep 21, 6:06 pm)
Re: [PATCH RFC 3/9] RCU: Preemptible RCU, Steven Rostedt, (Fri Sep 21, 12:17 am)
Re: [PATCH RFC 3/9] RCU: Preemptible RCU, Dipankar Sarma, (Fri Sep 21, 1:56 am)
Re: [PATCH RFC 3/9] RCU: Preemptible RCU, Paul E. McKenney, (Fri Sep 21, 1:50 am)
[PATCH RFC 2/9] RCU: Fix barriers, Paul E. McKenney, (Mon Sep 10, 2:33 pm)