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]
Signed-off-by: Christoph Jaeger <[email protected]>
---
lib/Kconfig | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/lib/Kconfig b/lib/Kconfig
index 147c7d9..b039e70 100644
--- 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
--
2.1.0
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.
On Mon, 2015-02-02 at 14:27 -0800, Andrew Morton wrote:
> There are many uses of "boolean" in lib/Kconfig. Converting just one
> of them is inefficient and odd.
$ git grep -n "^\s*boolean\b" next-20150202 -- "*Kconfig*"
next-20150202:drivers/usb/gadget/Kconfig:427: boolean "USB Webcam function"
next-20150202:lib/Kconfig:17: boolean
What are Christoph and I missing here?
Paul Bolle
On Mon, 02 Feb 2015 23:37:54 +0100 Paul Bolle <[email protected]> wrote:
> On Mon, 2015-02-02 at 14:27 -0800, Andrew Morton wrote:
> > There are many uses of "boolean" in lib/Kconfig. Converting just one
> > of them is inefficient and odd.
>
> $ git grep -n "^\s*boolean\b" next-20150202 -- "*Kconfig*"
> next-20150202:drivers/usb/gadget/Kconfig:427: boolean "USB Webcam function"
> next-20150202:lib/Kconfig:17: boolean
>
> What are Christoph and I missing here?
akpm3:/usr/src/linux-3.19-rc7> grep boolean lib/Kconfig | wc -l
15
On Mon, 2015-02-02 at 14:46 -0800, Andrew Morton wrote:
> On Mon, 02 Feb 2015 23:37:54 +0100 Paul Bolle <[email protected]> wrote:
> > On Mon, 2015-02-02 at 14:27 -0800, Andrew Morton wrote:
> > > There are many uses of "boolean" in lib/Kconfig. Converting just one
> > > of them is inefficient and odd.
> >
> > $ git grep -n "^\s*boolean\b" next-20150202 -- "*Kconfig*"
> > next-20150202:drivers/usb/gadget/Kconfig:427: boolean "USB Webcam function"
> > next-20150202:lib/Kconfig:17: boolean
> >
> > What are Christoph and I missing here?
>
> akpm3:/usr/src/linux-3.19-rc7> grep boolean lib/Kconfig | wc -l
> 15
Correct. However, Cristoph put "linux-next" in the subject line of this
patch. So this patch was meant to be applied on top of current
linux-next.
Paul Bolle
On 02/02/15 14:46, Andrew Morton wrote:
> On Mon, 02 Feb 2015 23:37:54 +0100 Paul Bolle <[email protected]> wrote:
>
>> On Mon, 2015-02-02 at 14:27 -0800, Andrew Morton wrote:
>>> There are many uses of "boolean" in lib/Kconfig. Converting just one
>>> of them is inefficient and odd.
>>
>> $ git grep -n "^\s*boolean\b" next-20150202 -- "*Kconfig*"
>> next-20150202:drivers/usb/gadget/Kconfig:427: boolean "USB Webcam function"
>> next-20150202:lib/Kconfig:17: boolean
>>
>> What are Christoph and I missing here?
>
> akpm3:/usr/src/linux-3.19-rc7> grep boolean lib/Kconfig | wc -l
> 15
Those are mostly "corrected" in linux-next or mmotm.
Also, Documentation/kbuild/kconfig-language.txt only uses "bool".
--
~Randy
Hi Andrew,
On Mon, 2 Feb 2015 14:46:57 -0800 Andrew Morton <[email protected]> wrote:
>
> On Mon, 02 Feb 2015 23:37:54 +0100 Paul Bolle <[email protected]> wrote:
>
> > On Mon, 2015-02-02 at 14:27 -0800, Andrew Morton wrote:
> > > There are many uses of "boolean" in lib/Kconfig. Converting just one
> > > of them is inefficient and odd.
> >
> > $ git grep -n "^\s*boolean\b" next-20150202 -- "*Kconfig*"
> > next-20150202:drivers/usb/gadget/Kconfig:427: boolean "USB Webcam function"
> > next-20150202:lib/Kconfig:17: boolean
> >
> > What are Christoph and I missing here?
>
> akpm3:/usr/src/linux-3.19-rc7> grep boolean lib/Kconfig | wc -l
> 15
All the others are changed by commit 6341e62b212a ("kconfig: use bool
instead of boolean for type definition attributes") from the kbuild
tree. Been in linux-next since (at least) next-20150108.
--
Cheers,
Stephen Rothwell [email protected]
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.
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.
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.
Oh, sorry, yes; it should be applied by the ARM folks to their tree then, of course.
Cheers,
Christoph
> 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.
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.
On Mon, 2 Feb 2015 18:12:13 -0500 Christoph Jaeger <[email protected]> wrote:
> > 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.
>
> Oh, sorry, yes; it should be applied by the ARM folks to their tree then, of course.
Is OK, I grabbed it. I don't think Russell needs to yank and rebase
the current diff.
On Mon, Feb 02, 2015 at 03:18:24PM -0800, Andrew Morton wrote:
> 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.
Looks fine to me. How are we going to handle this change (and the bool
bit as well?) Is it something you want me to merge, or will you send
these changes towards the end of the merge window?
--
FTTC broadband for 0.8mile line: currently at 10.5Mbps down 400kbps up
according to speedtest.net.
On Mon, 2 Feb 2015 23:30:00 +0000 Russell King - ARM Linux <[email protected]> wrote:
> On Mon, Feb 02, 2015 at 03:18:24PM -0800, Andrew Morton wrote:
> > 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.
>
> Looks fine to me. How are we going to handle this change (and the bool
> bit as well?) Is it something you want me to merge, or will you send
> these changes towards the end of the merge window?
Yep, I can take care of them.