On Mon, 2 Feb 2015 23:05:48 +0000 Russell King - ARM Linux <[email protected]> wrote:
> On Mon, Feb 02, 2015 at 02:27:32PM -0800, Andrew Morton wrote: > > On Mon, 2 Feb 2015 09:59:16 -0500 Christoph Jaeger <[email protected]> wrote: > > > > > Keyword 'boolean' for type definition attributes is considered > > > deprecated and, therefore, should not be used anymore. > > > > > > See http://lkml.kernel.org/r/[email protected] > > > See > > > http://lkml.kernel.org/r/[email protected] > > > > > > ... > > > > > > --- a/lib/Kconfig > > > +++ b/lib/Kconfig > > > @@ -14,7 +14,7 @@ config BITREVERSE > > > tristate > > > > > > config HAVE_ARCH_BITREVERSE > > > - boolean > > > + bool > > > default n > > > depends on BITREVERSE > > > help > > > > Your patch patches 556d2f055bf6d ("ARM: 8187/1: add > > CONFIG_HAVE_ARCH_BITREVERSE to support rbit instruction") which appears > > in linux-next via the ARM tree. > > > > There are many uses of "boolean" in lib/Kconfig. Converting just one > > of them is inefficient and odd. > > > > 556d2f055bf6d is a bit of a surprise. It looks good to me from a > > non-ARM perspective - the __builtin_constant_p() optimisation is > > sensible, although bitrev on a constant probably isn't very common. > > > > I'm not sure about the ARM part though! __bitrev8() is pretty damn > > fast. Presumably an inlined rbit instruction is faster still, but not > > very much? > > > > The Kconfig help text in 556d2f055bf6d rather needs some caring for. > > The patches had already been round six iterations, and had been posted > on LKML for every iteration. People saw "ARM" and went Zzzzz ;) I'm a bit surprised that nobody helped out with the Kconfig text. I queued the below. Looks OK? --- a/lib/Kconfig~a +++ a/lib/Kconfig @@ -18,9 +18,8 @@ config HAVE_ARCH_BITREVERSE default n depends on BITREVERSE help - This option provides an config for the architecture which have instruction - can do bitreverse operation, we use the hardware instruction if the architecture - have this capability. + This option enables the use of hardware bit-reversal instructions on + architectures which support such operations. config RATIONAL bool _ > When people started pushing to have the patches merged, there were two > dependent patches which had been merged via other random trees, so I > held them off for a cycle. At that point, I even questioned whether I > should be merging them; that question was ignored by everyone. > > Eventually, (and after some testing) I ended up giving up and merging > them because they're believed to be a net benefit for ARM, I know the feeling ;) > and I > couldn't locate anyone who'd be useful to ack the generic parts of the > patch. I usually look after lib/ and hereby ack the patch! I really should do a MAINTAINERS patch but I'm shy. > As for whether __bitrev8 is fast or not, that depends whether the table > has been speculatively prefetched and is available without having to go > out to memory - and it's not just about the table itself, there's also > the loading from the individual function's literal pool to get the > address of the table too. So that's two memory loads per rbit at > minimum. > > The rbit instruction is probably at least half the average cycles of > two dependent loads. OK. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
