Re: [PATCH 1/2] linux/log2.h: Add round{up,down}_pow_of_two64()
| 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