Re: [PATCH linux-next] lib: Kconfig: use bool instead of boolean

[Russell King - ARM Linux <linux@arm.linux.org.uk>]

On Mon, Feb 02, 2015 at 02:27:32PM -0800, Andrew Morton wrote:
> On Mon,  2 Feb 2015 09:59:16 -0500 Christoph Jaeger <cj@linux.com> wrote:
> 
> > Keyword 'boolean' for type definition attributes is considered
> > deprecated and, therefore, should not be used anymore.
> > 
> > See http://lkml.kernel.org/r/cover.1418003065.git.cj@linux.com
> > See http://lkml.kernel.org/r/1419108071-11607-1-git-send-email-cj@linux.com
> > 
> > ...
> >
> > --- 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.

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, and I
couldn't locate anyone who'd be useful to ack the generic parts of the
patch.

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.

-- 
FTTC broadband for 0.8mile line: currently at 10.5Mbps down 400kbps up
according to speedtest.net.