Message-ID: <aVLEIWmS8bLcuB4V@gallifrey>
Date: Mon, 29 Dec 2025 18:10:41 +0000
From: "Dr. David Alan Gilbert" <dave@...blig.org>
To: "Paul E. McKenney" <paulmck@...nel.org>
Cc: Steven Rostedt <rostedt@...dmis.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

* Paul E. McKenney (paulmck@...nel.org) wrote:
> On Mon, Dec 29, 2025 at 05:02:04PM +0000, Dr. David Alan Gilbert wrote:
> > * Paul E. McKenney (paulmck@...nel.org) wrote:
> > > On Mon, Dec 29, 2025 at 10:40:05AM -0500, Steven Rostedt wrote:
> > > > On Sun, 28 Dec 2025 10:36:39 +0100 (CET)
> > > > Julia Lawall <julia.lawall@...ia.fr> wrote:
> > > > 
> > > > > > > > > j = (j + 2) / 3;  
> > > > > > > >
> > > > > > > > "Divide by three rounding up."  
> > > > 
> > > > That's as useful as:
> > > > 
> > > > 	/* Add one to X */
> > > > 	x++;
> > > > 
> > > > > > >
> > > > > > > That's not *that* obvious, but ok, but then why 3?
> > > > > > >
> > > > 
> > > > Bingo! You win a cigar! :-)
> > > > 
> > > > I know that was a round up (and yes, as David pointed out, we have macros
> > > > for that too). The question is why are you dividing it by 3? I don't see
> > > > anything in that function which suggests the reason for needing to divide j
> > > > by 3.
> > > > 
> > > > If the comments you were adding in the past was things like "Divide by
> > > > three rounding up" then yeah, I can see why people would say you have too
> > > > many comments. The comments are not to be discussing what is being done,
> > > > but why is it being done.
> > > > 
> > > > 			WRITE_ONCE(rcu_state.jiffies_kick_kthreads,
> > > > 				   jiffies + (j ? 3 * j : 2));
> > > > 
> > > > 
> > > > Why the: (j ? 3 * j : 2) ?
> > > > 
> > > > Why is 3 so magical?
> > > > 
> > > > 			/*
> > > > 			 * j is the Father, Son and Holy Ghost.
> > > > 			 * But only one may be active at a time.
> > > > 			 * They each take a third. Father is first,
> > > > 			 * Son is second, and Holy Ghost is third.
> > > > 			 */
> > > > 			j = (j + 2) / 3;
> > > > 
> > > > 			/*
> > > > 			 * j may not be zero, as that would lead to
> > > > 			 * damnation. 
> > > > 			 */
> > > > 			if (j <= 0)
> > > > 				j = 1;
> > > 
> > > I would of course nack that comment, amusing though it might be to track
> > > others' reactions to it over time.  ;-)
> > > 
> > > So you are now unwilling to do a walkthrough?  That would be unfortunate.
> > > 
> > > If your view is that I should just answer the question so that
> > > everyone can get on with life, please keep in mind that there are some
> > > tens of thousands of other lines of code in Linux-kernel RCU.  It is
> > > therefore only reasonable that I insist upon a more organized approach.
> > 
> > I'm actually not interested in the answer to what the magical 3 is *;
> > I just think this piece of code is a nice example of code that has
> > poor accessibility - both for human and AI - although frankly the
> > humans might find it harder.
> > 
> > My point of my previous response was that I don't think this is an
> > example of something that needs clever extra stuff for some of the
> > accessibility issues; just the basics of not using magic constants
> > and making sure clever tricks are either commented or use
> > appropriate named functions.  That's just basic good style!
> 
> I am *not* arguing against adding a commment.  I am instead arguing for
> an organized approach, for example, guided by the following:
> 
> 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.

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

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

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

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

> > > In addition, as noted earlier [1], you guys are members of one of the
> > > smaller audiences that need my assistance.  Plus you were on CC for the
> > > patch that added this line.  ;-)
> > > 
> > > On the other hand, if your view is instead that because the three of
> > > you don't immediately grok this line of code, I should be willing to
> > > take hundreds of lines of LLM-generated comments for each and every
> > > non-trivial RCU function (for some TBD definition of "non-trivial"),
> > > sorry, but no, that does not follow.
> > 
> > That indeed would be terrible; but a few clear basic comments around
> > clever stuff would be great.
> 
> I appreciate the compliment, but this line is not particularly clever.
> It just gets its job done.

If the job it was trying to get done was clear then that might be OK;
it's not!

These things aren't individual lines either; think of it more cumulative;
you've for some reason landed in needing to look at the RCU code:
  a) It's the RCU code, so everyone knows it's magic
  b) You're looking at a magic 3 constant
  c) With some clever rounding trick
  d) On the descriptively named variable 'j'
  e) many other variables in the function have most vowels missing.

If it was only one of those then you can take a deep breath and poke
your way through it before having to ask; but the combination
makes the learning curve crazy.

> Glad you agree on the "terrible" part, but this is exactly one of the
> things that was being seriously proposed at LPC.  And not just for RCU,
> but for the entire kernel.
> 
> > Dave
> > (* It's obviously for Huey, Dewey, and Louie)
> 
> Ah, Pascal!  I remember it well, having worked on two projects that used
> Pascal code in production.  I was quite happy to transition to 1980s
> C code.  ;-)

The transition was Wirth it.

Dave

> 							Thanx, Paul
> 
> > > [1] https://lore.kernel.org/all/fe8c7e28-b5fe-4411-b27c-b2efd89a74c7@paulmck-laptop/
> > -- 
> >  -----Open up your eyes, open up your mind, open up your code -------   
> > / Dr. David Alan Gilbert    |       Running GNU/Linux       | Happy  \ 
> > \        dave @ treblig.org |                               | In Hex /
> >  \ _________________________|_____ http://www.treblig.org   |_______/
-- 
 -----Open up your eyes, open up your mind, open up your code -------   
/ Dr. David Alan Gilbert    |       Running GNU/Linux       | Happy  \ 
\        dave @ treblig.org |                               | In Hex /
 \ _________________________|_____ http://www.treblig.org   |_______/

Powered by blists - more mailing lists