| Message ID | 20250125130320.38232-2-wsa+renesas@sang-engineering.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | bitops: use safer link explaining the algorithm | expand |
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
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
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
> 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.
> 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.
> 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.
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
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.
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;
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(-)