Re: [PATCH 2/5] bits_per_long.h: introduce SMALL_CONST() macro

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Fri, Jan 29, 2021 at 1:11 PM Andy Shevchenko
<andy.shevchenko@xxxxxxxxx> wrote:

On Fri, Jan 29, 2021 at 10:49 PM Yury Norov <yury.norov@xxxxxxxxx> wrote:

Many algorithms become simplier if they are passed with relatively small

simpler

input values. One example is bitmap operations when the whole bitmap fits
into one word. To implement such simplifications, linux/bitmap.h declares
small_const_nbits() macro.

Other subsystems may also benefit from optimizations of this sort, like
find_bit API in the following patches. So it looks helpful to generalize
the macro and extend it's visibility.

It should probably go to linux/kernel.h, but doing that creates circular
dependencies. So put it in asm-generic/bitsperlong.h.

No, no, please leave kernel.h alone. It's already quite a mess.

And this shouldn't be in the commit message either.

...

-       if (small_const_nbits(nbits))
+       if (SMALL_CONST(nbits - 1))

Not sure if we need to rename it.

Lower case for generic macro kind of breaking the rules.

Behaviour has changed too. Before it was:
0 < VAL <= 32
Now it's
0 <= VAL < 32
Which is better for generic macro.

So changing the name looks reasonable to me.

...

--- a/include/linux/bits.h
+++ b/include/linux/bits.h
@@ -37,7 +37,7 @@
 #define GENMASK(h, l) \
        (GENMASK_INPUT_CHECK(h, l) + __GENMASK(h, l))

-#define BITS_FIRST(nr)         GENMASK(nr), 0)
+#define BITS_FIRST(nr)         GENMASK((nr), 0)

How come?!

git send-email wrong_dir/000*
Will resubmit today later.

...

diff --git a/tools/include/asm-generic/bitsperlong.h b/tools/include/asm-generic/bitsperlong.h
index 8f2283052333..432d272baf27 100644
--- a/tools/include/asm-generic/bitsperlong.h
+++ b/tools/include/asm-generic/bitsperlong.h

I think a tools update would be better to have in a separate patch.

--
With Best Regards,
Andy Shevchenko



[Index of Archives]     [Video for Linux]     [Yosemite News]     [Linux S/390]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux