Hi Yury,
On 03/12, Yury Norov wrote:
Based on 'sizeof(x) == 4' condition, in 32-bit case the function is wired
to ffs(), while in 64-bit case to __ffs(). The difference is substantial:
ffs(x) == __ffs(x) + 1. Also, ffs(0) == 0, while __ffs(0) is undefined.
The 32-bit behaviour is inconsistent with the function description, so it
needs to get fixed.
There are 9 individual users for the function in 6 different subsystems.
Some arches and drivers are 64-bit only:
- arch/loongarch/kvm/intc/eiointc.c;
- drivers/hv/mshv_vtl_main.c;
- kernel/liveupdate/kexec_handover.c;
The others are:
- ib_umem_find_best_pgsz(): as per comment, __ffs() should be correct;
- rzv2m_csi_reg_write_bit(): ARCH_RENESAS only, unclear;
- lz77_match_len(): CIFS_COMPRESSION only, unclear, experimental;
None of them explicitly tweak their code for a word length, or x == 0.
Context for lz77_match_len() case:
const u64 diff = lz77_read64(cur) ^ lz77_read64(wnd);
if (!diff) {
...
}
cur += count_trailing_zeros(diff) >> 3;
So x == 0 is checked, however it does assume that
sizeof(unsigned long) == sizeof(u64). I'll have to fix it for when
that's not the case (even with your patch in, as __ffs() casts x to
unsigned long down the line). Thanks for the heads up.
Cheers,
Enzo
Requesting comments from the corresponding maintainers on how to proceed
with this.
The attached patch gets rid of 32-bit explicit support, so that both
32- and 64-bit versions rely on __ffs().
CC: "K. Y. Srinivasan" <[email protected]> (hyperv)
CC: Haiyang Zhang <[email protected]> (hyperv)
CC: Jason Gunthorpe <[email protected]> (infiniband)
CC: Leon Romanovsky <[email protected]> (infiniband)
CC: Mark Brown <[email protected]> (spi)
CC: Steve French <[email protected]> (smb)
CC: Alexander Graf <[email protected]> (kexec)
CC: Mike Rapoport <[email protected]> (kexec)
CC: Pasha Tatashin <[email protected]> (kexec)
Signed-off-by: Yury Norov <[email protected]>
---
include/linux/count_zeros.h | 9 +++------
1 file changed, 3 insertions(+), 6 deletions(-)
diff --git a/include/linux/count_zeros.h b/include/linux/count_zeros.h
index 4e5680327ece..5034a30b5c7c 100644
--- a/include/linux/count_zeros.h
+++ b/include/linux/count_zeros.h
@@ -10,6 +10,8 @@
#include <asm/bitops.h>
+#define COUNT_TRAILING_ZEROS_0 (-1)
+
/**
* count_leading_zeros - Count the number of zeros from the MSB back
* @x: The value
@@ -40,12 +42,7 @@ static inline int count_leading_zeros(unsigned long x)
*/
static inline int count_trailing_zeros(unsigned long x)
{
-#define COUNT_TRAILING_ZEROS_0 (-1)
-
- if (sizeof(x) == 4)
- return ffs(x);
- else
- return (x != 0) ? __ffs(x) : COUNT_TRAILING_ZEROS_0;
+ return (x != 0) ? __ffs(x) : COUNT_TRAILING_ZEROS_0;
}
#endif /* _LINUX_BITOPS_COUNT_ZEROS_H_ */
--
2.43.0
