lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <81eda579-1909-4a10-ad81-6f5551bc7db6@paulmck-laptop>
Date: Thu, 8 Jan 2026 17:34:14 -0800
From: "Paul E. McKenney" <paulmck@...nel.org>
To: Steven Rostedt <rostedt@...dmis.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, Dec 29, 2025 at 03:35:17PM -0500, Steven Rostedt wrote:
> 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.

Fair enough, so I will let you continue that discussion with Dave.

> > 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).

The Linux-kernel RCU internal documentation is not (yet) intended to
support random entry points.  Plus the context of this email thread is
not a bug report, but rather the question of general accessibilty raised
at Plumbers.  Which I am still having difficulty believing is not really
about auto-creating certification documents from Linux-kernel comments.

Had you instead sent me a separate email asking about this constant,
I would likely have just answered.  I would imagine that Joel would have
done the same in response to your asking him.

> 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 point of the walkthrough is not to answer that specific question,
but instead to upgrade the documentation of the function containing that
line.  Please keep in mind that I never have had the experience of being
taught about RCU in general, let alone any particular implementation.
Listening to the questions that come up during a walkthrough helps me
figure out what needs to be commented.

And we did walk through that particular function some time back,
but it was apparently overshadowed by the complexity of the related
rcu_gp_init(), rcu_gp_cleanup(), rcu_watching_snap_save(), and
rcu_watching_snap_recheck() functions.  So we did not add much (or maybe
any) in the way of added comments.

Hence my suggestion of a walkthrough.  But if you don't want to do that,
not a problem.  We will get to it at some point, just as we will get to
other functions in need of better comments at some point.

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

Because it is inherently difficult!!!

Please recall that your initial ask was that we comment Linux-kernel RCU
to provide any needed guidance for any kernel developer (plus others
wanted to include non-developers) parachuting anywhere into the code.
Especially for something like Linux-kernel RCU, that is still in
grand-challenge territory.

Don't get me wrong, I am all for that as an aspiration, but it cannot
be a realistic goal at this time.

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

Our experience walking through this code was that it was in fact not
the hard stuff.

And this constant is but one of a huge number of details of various types.
Next time, someone will dive into some other part of the code for some
other reason and have some other difficulty.

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

Sure, we could create a long list of commenting requirements in the
style of Strunk's and White's "The Elements of Style", starting with "No
constants without comments".  We would end up with a mess.  There would be
disconnected comments providing details that made no sense taken together.
Similarly, I would recommend something like Joseph M. Willams's "Style:
Towards Clarity and Grace" over Strunk's and White's list of rules.

I am sorry to disappoint you, but the only way I know to get coherent
comments for Linux-kernel RCU is to have some people walk through the
code so that I can learn what is needed.

I can of course walk through the code and see what *I* need, but that
might or might not be all that helpful to others.

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

No, the problem with this function is *not* "why did Paul choose 3?".
The problem is instead making that function's purpose and operation
clear to people going through the code.  At present, the people going
through RCU code need considerable background or some serious time and
stubbornness.  This will get better over time, but I have no wand to wave,
whether for RCU or for any of the other obscure portions of the kernel.

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

On what evidence do you base this claim?

We are after all still working to get the internal documentation to the
point where it serves the people making big changes.  Need to walk
before running, after all.

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

No argument with the "talking way past each other"!  ;-)

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

OK, if this still matters (as opposed to your above statement about having
given up), please first ask Joel CCing rcu@...r.kernel.org.  If that
goes sideways, I will see it and will help on this specific question.

> > 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).

I did not fallow that, but I mean something this:

	int self_descriptive; // Additional self_descriptive details.

							Thanx, Paul



Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ