Message-ID: <20251229153517.0001e755@gandalf.local.home>
Date: Mon, 29 Dec 2025 15:35:17 -0500
From: Steven Rostedt <rostedt@...dmis.org>
To: "Paul E. McKenney" <paulmck@...nel.org>
Cc: "Dr. David Alan Gilbert" <dave@...blig.org>, Julia Lawall
 <julia.lawall@...ia.fr>, Theodore Tso <tytso@....edu>, Sasha Levin
 <sashal@...nel.org>, Gabriele Paoloni <gpaoloni@...hat.com>, Kate Stewart
 <kstewart@...uxfoundation.org>, Chuck Wolber <chuckwolber@...il.com>,
 Dmitry Vyukov <dvyukov@...gle.com>, Mark Rutland <mark.rutland@....com>,
 Thomas Gleixner <tglx@...utronix.de>, Lorenzo Stoakes
 <lorenzo.stoakes@...cle.com>, Shuah Khan <skhan@...uxfoundation.org>, Chris
 Mason <clm@...a.com>, linux-kernel@...r.kernel.org
Subject: Re: Follow-up on Linux-kernel code accessibility

On Mon, 29 Dec 2025 10:59:12 -0800
"Paul E. McKenney" <paulmck@...nel.org> wrote:

> > > 1.	What lines should be commented?  
> > 
> > IMHO thinking in 'lines' is wrong; if this was a /3 in something where
> > the 3 was obvious (say in an x/y/z set or in a handle_triangle function)
> > it wouldn't need anything.
> > This loop is apparently doing something clever in moving ahead a few
> > jiffies, but no one knows why it's working in 3s or why that other ?: line
> > Steve mentioned chose to use the :2 in that set of 3.  
> 
> Well, you guys pointed to this line, and not to some other code construct,
> so it is on you guys to un-wrong your own thinking.  ;-)

Specifically, it was me that pointed it out.

> 
> The point here is that there are a lot of uncommented lines of code in
> Linux-kernel RCU.  There are no doubt also lines that are commented,
> but whose comments are not sufficiently clear to some target audience
> or another.  Which should receive additional comments and why?  Of those
> that should receive additional comments, what is the priority ordering
> for actually doing that work?

The point is that there should be no magic numbers without some comments to
why they are magical. I pointed this code out because it is a real life
example. I saw Joel's RFC about jiffies_till_first_fqs being off by one and
wanted to investigate. It brought me to this code. Once I saw that the
variable that Joel specified had some unknown division by 3, I gave up on
looking at it.

I don't expect to have to call for a meeting with the developers to do a
walkthrough of their code every time I see there's an issue with their
code. I hardly doubt that anyone would be up to that (besides you).

You have yet to even say why "3" is of value here. And it appears I'll only
find out about this magically 3 when we go through a walk through. Why is
it so secretive?

If it requires a walkthrough to understand why you used 3, then damn, it
should most definitely have a comment about it.

The target audience of comments should be anyone interested in how that
code functions. (AKA any kernel developer). Why does this have to be so
difficult?


> 
> > > 2.	What level of detail should be provided?  
> > 
> > Enough to point people at better documentation for hard stuff,
> > but commented enough that you don't have to go looking at the docs
> > all the time.  
> 
> Experience indicates that this line of code does not appear to qualify
> as the hard stuff in the context of the Linux-kernel RCU implementation.

What code is not the "hard stuff". Why the division of 3? Details matter,
especially when it comes to constants in code.

> 
> > > 3.	What prerequisites should be assumed for those diving into
> > > 	Linux-kernel RCU code?  
> > 
> > IMHO all code should be 'readable' - no magic constants is a standard
> > readability thing in any project.  
> 
> What is your basis for stating that this particular constant qualifies
> as magic?

  ALL CONSTANTS ARE MAGIC!

This is something I learned in CS-101. No constants without comments. In
fact, I try to replace all constants with enums or macros, so that they
have meaning. Right now, "3" has no meaning. I used the example of "Father,
Son and Holy Ghost" to show that it could mean anything!


> 
> > But, it might depend on why they're diving in - did it show up in
> > a profile when they were working on something else?  Did their compiler
> > break on it?   Do they have a backtrace from somewhere including it?  
> 
> Show up in a profile?  Unlikely.  Compiler break on it?  Who knows, but
> this line is just simple arithmetic.  Appear in a backtrace?  Not likely
> unless the compiler broke on it.

You're missing the point! It is not about the arithmetic. If you think the
arithmetic is the problem than we are having a major miscommunication here.

The problem is why did you choose "3". Why not "4" or "5" or "42"???

There's nothing there that says "3" is needed. Why divide at all? Why not
just add 1?

The arithmetic is not the issue, it's why you picked this particular
arithmetic to begin with.


> 
> > Or do they actually feel like improving RCU.
> > If they actually want to improve RCU then sure the prerequisities
> > involve reading every reasonable piece of RCU docs; but in the other
> > cases?  Don't make it too hard.  
> 
> I only get 24 hours in each day, my friend.  Experience has shown that
> there are way more people struggling with code using RCU than with code
> implementing RCU, so the former get priority.  I could hit you with a
> Spock quote from a certain movie, if you would like.  ;-)
> 
> And as noted earlier in this thread, the mm subtree just got a 1300-page
> textbook to explain its workings.  If the 150LoC per textbook page
> translation holds, that suggests somewhere around 100-200 pages of
> textbook for the RCU implementation.  So yes, significant learning curve.

That's for general design for a deep dive. Great for people who are going
to architect and make big changes if needed. But for those just trying to
understand why a function exists, it's way too much of a big hammer.

> 
> And yes, I have burned more time on this than would be consumed by just
> proposing a comment.  That would be because you guys are straining on
> a gnat while swallowing huge numbers of camels.  We fix the camels
> first, and that requires some way of identifying them.  Plus it is
> entirely possible that an accurate comment describing this line would
> make things *less* clear for our poor unprepared developer who is diving
> into this code.

I think we are talking way past each other and this is why it's not moving
forward.

> 
> > > If we instead do random drive-by comment additions at random times
> > > throughout kernel/rcu, all we will accomplish is making a bigger mess.  
> > 
> > Well, cleaning up bits people find hard to understand isn't that bad an idea
> > as long as they really do make it easier.  
> 
> Fair enough, but from what I can see, you guys are just piling onto
> a randomly selected (and rarely executed) piece of code without any
> regard to whether any added comment would actually help anyone.  I am
> willing to put up with this if (and only if) Steve actually does the
> walkthrough.  ;-)

I picked this random code because it was an example of something that I hit
very recently. And doing a walkthrough is fine, but what is the purpose of
this walkthrough? So I can understand the issue that Joel is dealing with?

> 
> But as long as you are here, what is your opinion on adding inline
> comments to local variables?

Not sure what you mean by that. Names of variables should be self
descriptive, but how they are used should be commented (if it's not totally
obvious).

-- Steve

Powered by blists - more mailing lists