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: <7f10c583-4d95-432e-a536-898c79b1766d@paulmck-laptop>
Date: Mon, 29 Dec 2025 09:37:17 -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 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?

2.	What level of detail should be provided?

3.	What prerequisites should be assumed for those diving into
	Linux-kernel RCU 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.

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

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

							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   |_______/

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ