On Tue, Jun 23, 2020 at 12:29 PM Jakub Bogusz <qboosh@xxxxxxxxxxxxx> wrote:
>
> On Mon, Jun 22, 2020 at 10:44:56PM -0700, Andrii Nakryiko wrote:
> > On Sun, Jun 21, 2020 at 7:34 AM Jakub Bogusz <qboosh@xxxxxxxxxxxxx> wrote:
> > >
> > > Hello,
> > >
> > > I noticed that _bpftool crashes when building kernel tools (5.7.x) for
> > > 32-bit targets because in libbpf hashmap implementation hash_bits()
> > > function returning numbers exceeding hashmap buckets capacity.
> > >
> > > Attached patch fixes this problem.
> > >
> >
> > Thanks! But this was already fixed by Arnaldo Carvalho de Melo <acme@xxxxxxxxxx>
> > in 8ca8d4a84173 ("libbpf: Define __WORDSIZE if not available").
>
> No, it's not:
> This change worked around __WORDSIZE not always being available.
>
> But the issue on (I)LP32 platforms is that 64-bit value is shifted by
> (32-bits) instead of (64-bits).
>
> (__SIZEOF_LONG__ * 8) is 32 on such architectures (i686, arm).
> I used __SIZEOF_LONG_LONG__ to get proper bit shift both on (I)LP32 and
> LP64 architectures.
>
Ah, I see. I actually mentioned __SIZEOF_ constants on the original
fix patch. But I think in this case it has to use __SIZEOF_SIZE_T,
which on 32-bit should be 4, right?
> Should I provide an updated patch to apply on top of acme change?
Yes, that would be good. But I think there is no need to penalize
32-bit arches with use of 64-bit long longs, and instead it's better
to use #ifdef for 32-bit case vs 64-bit case. The multiplication
constant will change, of course, should be 2654435769. I'd appreciate
it if you can do the patch, thanks!
>
>
> Regards,
>
> --
> Jakub Bogusz http://qboosh.pl/
