RE: [PATCH 10/54] net: ethernet: replace bitmap_weight with bitmap_empty for qlogic

[David Laight]

From: Yury Norov
> Sent: 25 January 2022 21:10
> On Mon, Jan 24, 2022 at 4:29 AM Andy Shevchenko
> <andriy.shevchenko@xxxxxxxxxxxxxxx> wrote:
> >
> > On Sun, Jan 23, 2022 at 10:38:41AM -0800, Yury Norov wrote:
> > > qlogic/qed code calls bitmap_weight() to check if any bit of a given
> > > bitmap is set. It's better to use bitmap_empty() in that case because
> > > bitmap_empty() stops traversing the bitmap as soon as it finds first
> > > set bit, while bitmap_weight() counts all bits unconditionally.
> >
> > > -             if (bitmap_weight((unsigned long *)&pmap[item], 64 * 8))
> > > +             if (!bitmap_empty((unsigned long *)&pmap[item], 64 * 8))
> >
> > > -         (bitmap_weight((unsigned long *)&pmap[item],
> > > +         (!bitmap_empty((unsigned long *)&pmap[item],
> >
> > Side note, these castings reminds me previous discussion and I'm wondering
> > if you have this kind of potentially problematic places in your TODO as
> > subject to fix.
> 
> In the discussion you mentioned above, the u32* was cast to u64*,
> which is wrong. The code
> here is safe because in the worst case, it casts u64* to u32*. This
> would be OK wrt
>  -Werror=array-bounds.
> 
> The function itself looks like doing this unsigned long <-> u64
> conversions just for printing
> purpose. I'm not a qlogic expert, so let's wait what people say?

It'll be wrong on BE systems.
You just can't cast the argument it has to be long[].

	David

-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
Registration No: 1397386 (Wales)