On Fri, May 14, 2021 at 12:26 PM Dan Carpenter <dan.carpenter@xxxxxxxxxx> wrote: > On Thu, May 13, 2021 at 09:52:27AM +0100, Colin King wrote: ... > > const unsigned long offset = (bit % BITS_PER_LONG) & BIT(5); > > > > map[index] &= ~(0xFFFFFFFFul << offset); > > - map[index] |= v << offset; > > + map[index] |= (unsigned long)v << offset; > > Doing a shift by BIT(5) is super weird. Not the first place in the kernel with such a trick. > It looks like a double shift > bug and should probably trigger a static checker warning. It's like > when people do BIT(BIT(5)). > > It would be more readable to write it as: > > int shift = (bit % BITS_PER_LONG) ? 32 : 0; Usually this code is in a kinda fast path. Have you checked if the compiler generates the same or better code when you are using ternary? -- With Best Regards, Andy Shevchenko
