Re: [PATCH 4/5] Replace timeconst.bc with mktimeconst.c

[Thomas Gleixner]

Rob!

On Tue, Feb 21 2023 at 15:00, Rob Landley wrote:

See my previous comment about Documentation. This time you even failed
to CC the maintainer of this code.

> Update of my decade-old patch replacing timeconst.pl with mktimeconst.c,
> still removing the only user of "bc" from the build.

How is 'decade-old patch' relevant changelog information?

> All that's changed since the 2015 version at:
> https://github.com/landley/aboriginal/blob/master/sources/patches/linux-noperl-timeconst.patch

That's neither interesting.

> Is one extra iteration of the loop for nanoseconds and different
> makefile plumbing calling it. In theory this could calculate the values
> at runtime in a small _init function and eliminate the header or even
> allow HZ to change at runtime.

In theory we can also build a compiler into the decompressor which
compiles and bootstraps the kernel itself, right?

> See https://lkml.iu.edu/hypermail/linux/kernel/2211.0/02589.html

I haven't seen a changelog in a while, which provides so much useless
information and lacks any content which justifies and documents the
proposed change.

>  Kbuild                    |   7 ++-
>  kernel/time/mktimeconst.c | 111 ++++++++++++++++++++++++++++++++++++
>  kernel/time/timeconst.bc  | 117 --------------------------------------

Clearly you provided evidence that the output of both tools is identical
and because you decided to reorder the defines it's not even verifyable
with diff.

But I don't even need diff to figure out that the results are not
identical:

# grep -c '#define' orig.h
25

# grep -c '#define' patched.h
31

Which means this adds 6 more unused defines to the already 8 unused
defines of today.

You clearly spent a lot of time to make this palatable to the people who
are responsible for this code.

That said, I'm not completely opposed to get rid of the bc dependency,
but if you want to sell this, then follow the documented process and
present it in a form which is acceptable.

Thanks,

        tglx