On Fri, Sep 27, 2019 at 07:39:13AM -0500, Bjorn Helgaas wrote: > On Tue, Aug 27, 2019 at 06:18:22PM +0300, Andy Shevchenko wrote: > > This simplifies and standardizes slot manipulation code > > by using for_each_set_bit() library function. > > + unsigned long status = info->status & ~info->mask; > > + int i, max = -1; > > - for (i = 0; i < max; i++) > > - if (status & (1 << i)) > > - counter[i]++; > > + for_each_set_bit(i, &status, max) > > I applied this, Thank you! > but I confess to being a little ambivalent. It's > arguably a little easier to read, I have another opinion here. Instead of parsing body of for-loop, the name of the function tells you exactly what it's done. Besides the fact that reading and parsing two lines, with zero conditionals, is faster. > but it's not nearly as efficient > (not a great concern here) David, do you know why for_each_set_bit() has no optimization for the cases when nbits <= BITS_PER_LONG? (Actually find_*bit() family of functions) > and more importantly much harder to verify > that it's correct because you have to chase through > for_each_set_bit(), find_first_bit(), _ffs(), etc, etc. If for_each_set_bit() or any other fundamental bit operation helper is broken, PCI subsystem is a little concern here. > No doubt it's > great for bitmaps of arbitrary size, but for a simple 32-bit register > I'm a little hesitant. But I applied it anyway. > > > + counter[i]++; -- With Best Regards, Andy Shevchenko
