On Fri, Oct 16, 2020 at 2:46 PM Andy Shevchenko <[email protected]> wrote: > > On Fri, Oct 16, 2020 at 04:23:05AM +0530, Syed Nayyar Waris wrote: > > On Tue, Oct 6, 2020 at 4:56 PM Andy Shevchenko > > <[email protected]> wrote: > > > On Tue, Oct 06, 2020 at 02:52:16PM +0530, Syed Nayyar Waris wrote: > > ... > > > > > + return (map[index] >> offset) & GENMASK(nbits - 1, 0); > > > > > > Have you considered to use rather BIT{_ULL}(nbits) - 1? > > > It maybe better for code generation. > > > > Yes I have considered using BIT{_ULL} in earlier versions of patchset. > > It has a problem: > > > > This macro when used in both bitmap_get_value and > > bitmap_set_value functions, it will give unexpected results when nbits or > > clump > > size is BITS_PER_LONG (32 or 64 depending on arch). > > > > Actually when nbits (clump size) is 64 (BITS_PER_LONG is 64, for example), > > (BIT(nbits) - 1) > > gives a value of zero and when this zero is ANDed with any value, it > > makes it full zero. This is unexpected, and incorrect calculation occurs. > > > > What actually happens is in the macro expansion of BIT(64), that is 1 > > << 64, the '1' overflows from leftmost bit position (most significant > > bit) and re-enters at the rightmost bit position (least significant > > bit), therefore 1 << 64 becomes '0x1', and when another '1' is > > subtracted from this, the final result becomes 0. > > > > This is undefined behavior in the C standard (section 6.5.7 in the N1124) > > I see, indeed, for 64/32 it is like this. > > ... > > > Yes I have incorporated your suggestion to use the '<<' operator. Thank You. > > One side note, consider the use round_up() vs. roundup(). I don't remember > which one is optimized to divisor being power of 2.
Yes. changed 'roundup' to 'round_up'. 'round_up' is optimized for power-of-2. Thank you. Syed Nayyar Waris
