linux-kernel - Re: [PATCH v13 06/11] iio: afe: rescale: make use of units.h

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]

Date:   Mon, 31 Jan 2022 17:23:47 +0200
From:   Andy Shevchenko <andy.shevchenko@...il.com>
To:     Peter Rosin <peda@...ntia.se>
Cc:     Liam Beguin <liambeguin@...il.com>,
        Jonathan Cameron <jic23@...nel.org>,
        Lars-Peter Clausen <lars@...afoo.de>,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
        linux-iio <linux-iio@...r.kernel.org>,
        devicetree <devicetree@...r.kernel.org>,
        Rob Herring <robh+dt@...nel.org>
Subject: Re: [PATCH v13 06/11] iio: afe: rescale: make use of units.h

On Mon, Jan 31, 2022 at 4:50 PM Peter Rosin <peda@...ntia.se> wrote:
> On 2022-01-30 17:10, Liam Beguin wrote:

...

> > -             tmp = div_s64_rem(tmp, 1000000000LL, &rem);
> > +             tmp = div_s64_rem(tmp, GIGA, &rem);
>
> It is NOT easy for me to say which of GIGA/NANO is most fitting.

What do you mean? The idea behind is the use of the macro depending on
the actual power of 10 in use (taking into account the sign of the
exponent part).

> There are a couple of considerations:
>
> A) 1000000000 is just a big value (GIGA fits). Something big is
>    needed to not lose too much precision.

Does it have a physical meaning?

> B) 1000000000 is what the IIO core uses to print fractional-log
>    values with nano precision (NANO fits). This is not really
>    relevant in this context.

Same question.

> C) 1000000000 makes the int-plus-nano and fractional-log cases
>    align (NANO fits). This last consideration is introduced with
>    patch 4/11.
>
> There is simply no correct define to use.

Huh?!
I believe the answer to the A and B -- yes, which means there are the
correct and incorrect variants to use.

> And whichever define is
> chosen makes the other interpretation less obvious. Which is not
> desirable, obscures things and make both GIGA and NANO bad
> options.

The (main) idea of the macros is to avoid mistyping 0s in the numbers
and miscalculations. Besides that it also provides the same type for
the constants.

> So, I stepped back to the description provided by Andy in the
> comments of v11:
>
> On 2021-12-22 19:59, Andy Shevchenko wrote:
> | You should get the proper power after the operation.
> | Write a formula (mathematically speaking) and check each of them for this.
> |
> | 10^-5/10^-9 == 1*10^4 (Used NANO)
> | 10^-5/10^9 == 1/10^-14 (Used GIGA)
> |
> | See the difference?
>
> No, I don't really see the difference, that just makes me totally
> confused.

Sounds like we have a chat here between physicists and computer
science folks :-)

Let's try again, does the value in the tmp variable above have a
_physical_ meaning? (I believe so, because all IIO subsystem is about
physical values)

> Dividing by 10^-9 or multiplying by 10^9 is as we all
> know exactly the same, and the kernel cannot deal directly with
> 10^-9 so both will look the same in code (multiplying by 10^9).

Yes, and using proper macro makes much cleaner the mathematical and
physical point of view to the values.

> So,
> you must be referring to the "real formula" behind the code. But
> in that case, if the "real formula" behind the (then equivalent)
> code had instead been
>
>         10^-5*10^9 == 1*10^4 (Used GIGA)
>         10^-5*10^-9 == 1/10^-14 (Used NANO)
>
> the outcome is the opposite. NANO turns GIGA and vice versa.

Again, one needs to think carefully about the meaning.
That's the point. If we do not understand the proper values and their
scales, perhaps that is the issue we should solve first?

> Since you can express the same thing differently in math too, it
> all crumbles for me. Because of this duality, it will be a matter
> of taste if GIGA or NANO fits best in any given instance. Sometimes
> (perhaps commonly) it will be decidedly easy to pick one of them,
> but in other cases (see above) we will end up with a conflict.
>
> What to do then? Or, what am I missing?

Physical meaning of the values, certainly.

> My taste says NANO in this case, since A) is just some big number
> and not really about units and B) is as stated not really relevant.
> Which makes C) win the argument for me.

...

> >               *val2 = rem / (int)tmp;
> >               if (rem2)
> > -                     *val2 += div_s64((s64)rem2 * 1000000000LL, tmp);
> > +                     *val2 += div_s64((s64)rem2 * GIGA, tmp);
>
> Here, 1000000000 matches the above use. If we go with NANO above,
> we should go with NANO here as well.

Why? (Maybe you are right, maybe not)

...

> SI-defines that are a bit confusing to me.

Yeah, people hate mathematics at scholls, at university, in life...

-- 
With Best Regards,
Andy Shevchenko