[PATCH] bitops: use safer link explaining the algorithm

[PATCH] bitops: use safer link explaining the algorithm

[Wolfram Sang]

During review, a concern was raised that the link explaining the
algorithm might get stale. Meanwhile, the site has been archived in the
WayBack machine. So, use their link which is hopefully more stable.

Fixes: c320592f3f2a ("bitops: add generic parity calculation for u8")
Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
---

The original patch went upstream via I3C. Yury, do you want to take this
fixup or shall it also go via I3C?

 include/linux/bitops.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/include/linux/bitops.h b/include/linux/bitops.h
index c1cb53cf2f0f..dde4ad0034ae 100644
--- a/include/linux/bitops.h
+++ b/include/linux/bitops.h
@@ -254,7 +254,7 @@ static inline int parity8(u8 val)
 {
 	/*
 	 * One explanation of this algorithm:
-	 * https://funloop.org/codex/problem/parity/README.html
+	 * http://web.archive.org/web/20250105093316/https://funloop.org/codex/problem/parity/README.html
 	 */
 	val ^= val >> 4;
 	return (0x6996 >> (val & 0xf)) & 1;
-- 
2.45.2


-- 
linux-i3c mailing list
linux-i3c@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-i3c

Re: [PATCH] bitops: use safer link explaining the algorithm

[Geert Uytterhoeven]

Hi Wolfram,

CC Linus (the funloop one ;-)

On Sat, Jan 25, 2025 at 2:03 PM Wolfram Sang
<wsa+renesas@sang-engineering.com> wrote:
> During review, a concern was raised that the link explaining the
> algorithm might get stale. Meanwhile, the site has been archived in the
> WayBack machine. So, use their link which is hopefully more stable.
>
> Fixes: c320592f3f2a ("bitops: add generic parity calculation for u8")
> Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>

Thanks for your patch!

> --- a/include/linux/bitops.h
> +++ b/include/linux/bitops.h
> @@ -254,7 +254,7 @@ static inline int parity8(u8 val)
>  {
>         /*
>          * One explanation of this algorithm:
> -        * https://funloop.org/codex/problem/parity/README.html
> +        * http://web.archive.org/web/20250105093316/https://funloop.org/codex/problem/parity/README.html

Is the plan to replace all weblinks by webarchive links as a precaution?
Even websites backed by big companies may disappear[1]...
Putting the webarchive link here also impacts the funloop.org server
statistics, downplaying its relevance, and possibly even causing an
earlier shutdown.
The URL can always be updated when the original site disappears.

>          */
>         val ^= val >> 4;
>         return (0x6996 >> (val & 0xf)) & 1;

[1] The "LessWatts.org" T-shirt I got from Intel survived the
    corresponding website by many years ;-)

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

-- 
linux-i3c mailing list
linux-i3c@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-i3c

Re: [PATCH] bitops: use safer link explaining the algorithm

[Yury Norov]

On Sat, Jan 25, 2025 at 04:10:36PM +0100, Geert Uytterhoeven wrote:
> Hi Wolfram,
> 
> CC Linus (the funloop one ;-)
> 
> On Sat, Jan 25, 2025 at 2:03 PM Wolfram Sang
> <wsa+renesas@sang-engineering.com> wrote:
> > During review, a concern was raised that the link explaining the
> > algorithm might get stale. Meanwhile, the site has been archived in the
> > WayBack machine. So, use their link which is hopefully more stable.
> >
> > Fixes: c320592f3f2a ("bitops: add generic parity calculation for u8")
> > Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
> 
> Thanks for your patch!
> 
> > --- a/include/linux/bitops.h
> > +++ b/include/linux/bitops.h
> > @@ -254,7 +254,7 @@ static inline int parity8(u8 val)
> >  {
> >         /*
> >          * One explanation of this algorithm:
> > -        * https://funloop.org/codex/problem/parity/README.html
> > +        * http://web.archive.org/web/20250105093316/https://funloop.org/codex/problem/parity/README.html
> 
> Is the plan to replace all weblinks by webarchive links as a precaution?
> Even websites backed by big companies may disappear[1]...
> Putting the webarchive link here also impacts the funloop.org server
> statistics, downplaying its relevance, and possibly even causing an
> earlier shutdown.
> The URL can always be updated when the original site disappears.
> 
> >          */
> >         val ^= val >> 4;
> >         return (0x6996 >> (val & 0xf)) & 1;
> 
> [1] The "LessWatts.org" T-shirt I got from Intel survived the
>     corresponding website by many years ;-)

That was actually my concern. I used to think that kernel repository
should better be a self-contained thing. And the reason is that kernel
sources should be understandable even on an Atlantic flight.

But this days airliners have pretty good internet access. So I don't
want to look like a guy pushing 80-chars line limit, while we forgot
even the meaning of VGA acronym maybe 15 years ago.

Quick grepping says that the include/linux directory already has 235
'http' links. Even after excluding compiler linkage, we have 149 of them.

So...

If you guys feel that you can explain the algorithm you're employing
in a half-VGA-screen comment, it would be the best choice, and that's
enough.

If it's impossible or you think that external reference is really needed,
I trust you. But please refer the original source. In this case, it's:

        Warren, H. S. (2013). Hacker’s Delight (2nd ed), page 97.

If you think that pointing to a web-page with nice summary to the topic 
is helpful - please do. And if you would like to cache the link - I've
got nothing against it - as soon as you commit to maintain those links
up-to-date in the kernel sources.

Thanks,
Yury

-- 
linux-i3c mailing list
linux-i3c@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-i3c

Re: [PATCH] bitops: use safer link explaining the algorithm

[Rasmus Villemoes]

On Sun, 26 Jan 2025 at 18:46, Yury Norov <yury.norov@gmail.com> wrote:
>
> >
> > >          */
> > >         val ^= val >> 4;
> > >         return (0x6996 >> (val & 0xf)) & 1;
> >
>
> If you guys feel that you can explain the algorithm you're employing
> in a half-VGA-screen comment, it would be the best choice, and that's
> enough.

So when I saw this, I even wondered why it needed a comment or
reference at all. I thought table-in-a-constant was a standard trick
everybody knows.

Isn't "The constant 0x6996 has bits set in exactly the positions
corresponding to four-bit numbers with odd parity." enough?

Rasmus

-- 
linux-i3c mailing list
linux-i3c@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-i3c

Re: [PATCH] bitops: use safer link explaining the algorithm

[Wolfram Sang]

[-- Attachment #1.1: Type: text/plain, Size: 993 bytes --]


> Is the plan to replace all weblinks by webarchive links as a precaution?

Dunno if there is a plan but 'git grep' showed at least a trend.

> Even websites backed by big companies may disappear[1]...

I think you are comparing apples with oranges here. The main site
(intel.com) still exists last time I checked.

> Putting the webarchive link here also impacts the funloop.org server
> statistics, downplaying its relevance, and possibly even causing an
> earlier shutdown.

Could be. But could also be the opposite: not using an alternative will
eat all the traffic and increase maintenance duty, so it will be shut
down because of too much work. Who has the better crystal ball?

> The URL can always be updated when the original site disappears.

Still, the good thing is that the site is *now* backed by the Wayback
Machine. I needed to request the backup because I wanted the link.

That all being said, I don't care much which link we use. I just wanted
to address review comments.


[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

[-- Attachment #2: Type: text/plain, Size: 111 bytes --]

-- 
linux-i3c mailing list
linux-i3c@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-i3c

Re: [PATCH] bitops: use safer link explaining the algorithm

[Wolfram Sang]

[-- Attachment #1.1: Type: text/plain, Size: 1367 bytes --]


> If you guys feel that you can explain the algorithm you're employing
> in a half-VGA-screen comment, it would be the best choice, and that's
> enough.

I agree that in-kernel would be super-great to have. On the other hand,
I especially liked the *detailed* explanation of different approaches on
this website. Which is helpful for users wondering if they can use the
new generic helper even though the algorithm they want to replace looks
different. Yet, this is not half a VGA-screen.

> If it's impossible or you think that external reference is really needed,
> I trust you. But please refer the original source. In this case, it's:
> 
>         Warren, H. S. (2013). Hacker’s Delight (2nd ed), page 97.

I decided against this reference. It is a great book, but why pay bucks
when there is freely available information for just this one topic.
Besides, the book is a collection of algorithms and references other
sources for this implementation as well. Maybe as a second reference?

> If you think that pointing to a web-page with nice summary to the topic 
> is helpful - please do. And if you would like to cache the link - I've
> got nothing against it - as soon as you commit to maintain those links
> up-to-date in the kernel sources.

Sure, yet then I would prefer the wayback-machine link and have this
patch accepted.


[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

[-- Attachment #2: Type: text/plain, Size: 111 bytes --]

-- 
linux-i3c mailing list
linux-i3c@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-i3c

Re: [PATCH] bitops: use safer link explaining the algorithm

[Wolfram Sang]

[-- Attachment #1.1: Type: text/plain, Size: 476 bytes --]


> So when I saw this, I even wondered why it needed a comment or
> reference at all. I thought table-in-a-constant was a standard trick
> everybody knows.

I wouldn't go that far.

> Isn't "The constant 0x6996 has bits set in exactly the positions
> corresponding to four-bit numbers with odd parity." enough?

As I wrote in the mail before, I want to aid users who wonder if they
can replace an existing implementation with this generic helper although
it looks different.


[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

[-- Attachment #2: Type: text/plain, Size: 111 bytes --]

-- 
linux-i3c mailing list
linux-i3c@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-i3c

Re: [PATCH] bitops: use safer link explaining the algorithm

[Geert Uytterhoeven]

Hi Wolfram,

On Mon, 27 Jan 2025 at 12:09, Wolfram Sang
<wsa+renesas@sang-engineering.com> wrote:
> > Is the plan to replace all weblinks by webarchive links as a precaution?
>
> Dunno if there is a plan but 'git grep' showed at least a trend.

AFAIK most/all of these were added after the original links stopped
working...

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

-- 
linux-i3c mailing list
linux-i3c@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-i3c

Re: [PATCH] bitops: use safer link explaining the algorithm

[Linus Arver]

Hello Wolfram, funloop.org owner/author here (also, thanks Geert for
CC'ing me).

Wolfram Sang <wsa+renesas@sang-engineering.com> writes:

>> If you guys feel that you can explain the algorithm you're employing
>> in a half-VGA-screen comment, it would be the best choice, and that's
>> enough.
>
> I agree that in-kernel would be super-great to have. On the other hand,
> I especially liked the *detailed* explanation of different approaches on
> this website. Which is helpful for users wondering if they can use the
> new generic helper even though the algorithm they want to replace looks
> different. Yet, this is not half a VGA-screen.

I'm glad you found it useful!

>> If it's impossible or you think that external reference is really needed,
>> I trust you. But please refer the original source. In this case, it's:
>>
>>         Warren, H. S. (2013). Hacker’s Delight (2nd ed), page 97.
>
> I decided against this reference. It is a great book, but why pay bucks
> when there is freely available information for just this one topic.
> Besides, the book is a collection of algorithms and references other
> sources for this implementation as well. Maybe as a second reference?

Adding as a second reference sounds like a good idea to me. You could
also add the ISBN number of the book, although maybe that's overkill.

-- 
linux-i3c mailing list
linux-i3c@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-i3c