Re: [PATCH 1/2] linux/log2.h: Add round{up,down}_pow_of_two64()

[email protected]
Newsgroups org.freedesktop.lists.dri-devel
Message-ID <[email protected]>
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Medium] Calling `__roundup_pow_of_two64(1)` evaluates `ilog2(0)`, violating the `ilog2()` API contract and relying on undefined behavior for architecture fallbacks.
- [Low] The kernel-doc comment for `__rounddown_pow_of_two64()` incorrectly describes the `@n` parameter as 'value to round up'.
--

--- Patch [1]: [PATCH 1/2] linux/log2.h: Add round{up,down}_pow_of_two64() ---
commit 4cce4f9f15f91ab3c1df542bca356b3776146fef
Author: David Gow <[email protected]>

linux/log2.h: Add round{up,down}_pow_of_two64()

This commit adds 64-bit specific versions of round-up and round-down power
of two macros. It is intended to support 64-bit values on 32-bit systems,
particularly for the GPU buddy allocator.

> diff --git a/include/linux/log2.h b/include/linux/log2.h
> index e17ceb32e0c97..67446d5ce9a32 100644
> --- a/include/linux/log2.h
> +++ b/include/linux/log2.h
> @@ -195,6 +195,63 @@ unsigned long __rounddown_pow_of_two(unsigned long n)
>  	__rounddown_pow_of_two(n)		\
>   )
>  
> +/**
> + * __rounddown_pow_of_two64() - round a 64-bit value down to nearest power of two
> + * @n: value to round up

[Severity: Low]
This isn't a bug, but should this be "value to round down" for the
__rounddown_pow_of_two64() variant?

> + */
> +static inline __attribute_const__
> +u64 __rounddown_pow_of_two64(u64 n)
> +{

[ ... ]

> +/**
> + * __roundup_pow_of_two64() - round a 64-bit value up to nearest power of two
> + * @n: value to round up
> + */
> +static inline __attribute_const__
> +u64 __roundup_pow_of_two64(u64 n)
> +{
> +	return 1ULL << (ilog2(n - 1) + 1);

[Severity: Medium]
Will calling roundup_pow_of_two64(1) with a runtime variable evaluate to
ilog2(0) here?

The documentation in include/linux/log2.h states that the architecture is
not required to handle n == 0 for ilog2(). While current fallback
implementations evaluate fls64(0) - 1 to -1 and return 1, an architecture
providing an optimized hardware ilog2() implementation might trap or break
on 0.

The existing 32-bit __roundup_pow_of_two() avoids this by safely relying on
fls_long(n - 1).

> +}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=1
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.