Re: [PATCH] lib: count_zeros: fix 32/64-bit inconsistency in count_trailing_zeros()

Yury Norov
2026-03-13T16:32:03Z

On Thu, Mar 12, 2026 at 08:54:29PM -0300, Enzo Matsumiya wrote:
> 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.

Yes, in your case you need __ffs64() (which doesn't exist). Or simply
leverage bitmaps API:

DECLARE_BITMAP(bitmap, 64);

...

bitmap_from_u64(bitmap, diff);
cur += find_first_bit(bitmap, 64) >> 3;
>
> Cheers,
>
> Enzo

So, if you're not objecting to wire 32-bit count_trailing_zeros() to
__ffs(), can you please send your ack?

Thanks,
Yury