`On Sat, May 30, 2020 at 12:42 AM Andy Shevchenko<andy.shevchenko@gmail.com> wrote:> On Fri, May 29, 2020 at 11:07 PM Syed Nayyar Waris <syednwaris@gmail.com> wrote:> > On Sat, May 30, 2020 at 12:08 AM Andy Shevchenko> > <andriy.shevchenko@linux.intel.com> 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`