Messages in this thread | | | From | David Laight <> | Subject | RE: [PATCH 10/54] net: ethernet: replace bitmap_weight with bitmap_empty for qlogic | Date | Tue, 25 Jan 2022 22:14:58 +0000 |
| |
From: Yury Norov > Sent: 25 January 2022 21:10 > On Mon, Jan 24, 2022 at 4:29 AM Andy Shevchenko > <andriy.shevchenko@linux.intel.com> 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)
| |