Wednesday, December 29, 2021

Re: power of 2 revisited

On Wed, Dec 29, 2021 at 09:27:34PM -0700, Ted Bullock wrote:
> This started out as a direct question to Ingo, but I have readdressed it
> to misc@
>
> This is around documenting peculiar behaviour around power of 2 math in
> the kernel.
>
> I noticed that in sys/dev/pci/drm/radeon/radeon_device.c:1137 there is a
> call to radeon_check_pot_argument, this function isn't correct in that
> it returns true for a value of zero (as you know, 0 is not a power of two).
>
> I sent a diff to Jonathan to correct the behaviour which he rejected
> because he doesn't want to add local changes to change behaviour which
> isn't hitting a bug here. Fair Enough.
>
> Then reading through old mail archives I came across this discussion
> from a few years back:
> https://marc.info/?t=144321527600004&r=1&w=2
>
> I'm wondering if it's worth documenting the peculiarities here, and
> possibly putting an actually mathematically correct check somewhere in
> the kernel or maybe userland? Perhaps hypothetical PEER REVIEWED
> functions with prototypes like isnpotu(uint64_t n) or isnpot(int64_t n).
>
> Is this at all worthwhile? Maybe this would help stop people from
> incorrectly reinventing the wheel?
>
> For instance in the kernel right now there is :
> radeon_check_pot_argument
> IS_POWER_OF_2
> is_power_of_2
> is_power_of_2_u64
> powerof2
> probably others too.

IS_POWER_OF_2 and is_power_of_2_u64 are inteldrm specific
is_power_of_2 is a linux interface.

radeon_check_pot_argument should be replaced in linux by
is_power_of_2.
https://lists.freedesktop.org/archives/amd-gfx/2021-December/073108.html

No code outside of drm should be using the linux interfaces and
I don't think we should be documenting them.

>
> And manual checks like
> sys/arch/amd64/amd64/identcpu.c:804
> powerof2 = ((x - 1) & x) == 0;
>
> --
> Ted Bullock <tbullock@comlore.com>
>
>

No comments:

Post a Comment