Re: [PATCH tip/core/rcu 16/27] rcu: Add comment documenting how rcu_seq_snap works

Paul E. McKenney
2018-06-28T17:40:45Z

On Wed, Jun 27, 2018 at 10:10:28PM -0700, Joel Fernandes wrote:
> On Wed, Jun 27, 2018 at 11:27:26AM -0700, Joel Fernandes wrote:
> [..]
> > > > > s = __ALIGN_MASK(s, RCU_SEQ_STATE_MASK);
> > > > >
> > > >
> > > > I agree with Peter's suggestions for both the verbiage reduction in the
> > > > comments in the header, as the new code he is proposing is more
> > > > self-documenting. I believe I proposed a big comment just because the code
> > > > wasn't self-documenting or obvious previously so needed an explanation.
> > > >
> > > > How would you like to proceed? Let me know what you guys decide, I am really
> > > > Ok with anything. If you guys agree, should I write a follow-up patch with
> > > > Peter's suggestion that applies on top of this one? Or do we want to drop
> > > > this one in favor of Peter's suggestion?
> > >
> > > Shortening the comment would be good, so please do that.
>
> Paul,
>
> Do you want to fold the below patch into the original one? Or do you prefer I
> resent the original patch fixed up?
>
> Following is the patch ontop of current 'dev' branch in your tree, with the
> excessive comments removed.
>
> Thanks to Peter for suggesting!

I merged this into the current commit, with the result shown below.

Thank you both!

Thanx, Paul

-----------------------------------------------------------------------

commit c9a6ed70aad9f4571afba3e12e869f5fccc26a40
Author: Joel Fernandes (Google) <joel@joelfernandes.org>
Date: Tue May 22 23:38:13 2018 -0700

rcu: Add comment documenting how rcu_seq_snap works

rcu_seq_snap may be tricky to decipher. Lets document how it works with
an example to make it easier.

Signed-off-by: Joel Fernandes (Google) <joel@joelfernandes.org>
Signed-off-by: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
[ paulmck: Shrink comment as suggested by Peter Zijlstra. ]

diff --git a/kernel/rcu/rcu.h b/kernel/rcu/rcu.h
index aa215d6355f8..89f13fffac73 100644
--- a/kernel/rcu/rcu.h
+++ b/kernel/rcu/rcu.h
@@ -91,7 +91,17 @@ static inline void rcu_seq_end(unsigned long *sp)
WRITE_ONCE(*sp, rcu_seq_endval(sp));
}

-/* Take a snapshot of the update side's sequence number. */
+/*
+ * rcu_seq_snap - Take a snapshot of the update side's sequence number.
+ *
+ * This function returns the earliest value of the grace-period sequence number
+ * that will indicate that a full grace period has elapsed since the current
+ * time. Once the grace-period sequence number has reached this value, it will
+ * be safe to invoke all callbacks that have been registered prior to the
+ * current time. This value is the current grace-period number plus two to the
+ * power of the number of low-order bits reserved for state, then rounded up to
+ * the next value in which the state bits are all zero.
+ */
static inline unsigned long rcu_seq_snap(unsigned long *sp)
{
unsigned long s;