[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <f94edf94-55d8-4ff4-a9d2-69886637091b@paulmck-laptop>
Date: Mon, 29 Dec 2025 10:59:12 -0800
From: "Paul E. McKenney" <paulmck@...nel.org>
To: "Dr. David Alan Gilbert" <dave@...blig.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
On Mon, Dec 29, 2025 at 06:10:41PM +0000, Dr. David Alan Gilbert wrote:
> * 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.
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. ;-)
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?
> > 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.
> > 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?
> 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.
> 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.
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.
> > 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. ;-)
But as long as you are here, what is your opinion on adding inline
comments to local variables?
> > > > 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.
My points 1, 2, and 3 above still stand. ;-)
> > 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.
;-) ;-) ;-)
Thanx, Paul
> Dave
> >
> > > > [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