On Sat, May 30, 2020 at 12:42 AM Andy Shevchenko <andy.shevchenko@xxxxxxxxx> wrote: > On Fri, May 29, 2020 at 11:07 PM Syed Nayyar Waris <syednwaris@xxxxxxxxx> wrote: > > On Sat, May 30, 2020 at 12:08 AM Andy Shevchenko > > <andriy.shevchenko@xxxxxxxxxxxxxxx> wrote: ... > > Explanation: > > Actually when nbits (clump size) is 64 (BITS_PER_LONG is 64 on my computer), > > (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 happening. > > > > 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. > > > > Since this macro is being 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). > > I see, something like > https://elixir.bootlin.com/linux/latest/source/include/linux/dma-mapping.h#L139 > should be done. > But yes, let's try to fix GENMASK(). > > So, if we modify the following > > #define GENMASK_INPUT_CHECK(h, l) \ > (BUILD_BUG_ON_ZERO(__builtin_choose_expr( \ > __builtin_constant_p((l) > (h)), (l) > (h), 0))) > > to be > > #define GENMASK_INPUT_CHECK(h, l) \ > (BUILD_BUG_ON_ZERO(__builtin_choose_expr( \ > __builtin_constant_p((l) > (h)), (l) ? (l) > (h) : 0, 0))) > > would it work? Actually it needs an amendment for signed types... (l) ? (l) > (h) : !((h) > 0) ...but today is Friday night, so, mistakes are warranted :-) -- With Best Regards, Andy Shevchenko
