public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
* [PATCH] init/Kconfig: Fix break in middle of EXPERT menu
@ 2015-05-11 18:13 Josh Triplett
  2015-05-11 18:23 ` Randy Dunlap
                   ` (2 more replies)
  0 siblings, 3 replies; 17+ messages in thread
From: Josh Triplett @ 2015-05-11 18:13 UTC (permalink / raw)
  To: Ingo Molnar, Andrew Morton, Paul E. McKenney, Michal Hocko,
	Josh Triplett, Vladimir Davydov, Johannes Weiner,
	Geert Uytterhoeven, Andy Lutomirski, Bertrand Jacquin,
	Luis R. Rodriguez, Iulia Manda, Pranith Kumar, Clark Williams,
	Mel Gorman, linux-kernel

Commit e1abf2cc8d5 ("bpf: Fix the build on BPF_SYSCALL=y &&
!CONFIG_TRACING kernels, make it more configurable") made BPF_SYSCALL no
longer hidden with !EXPERT, but left it in the middle of the EXPERT
menu.  menuconfig stops putting config items under a submenu once it
encounters an item that doesn't depend on the menu's config item, so
this caused the remainder of the EXPERT menu to spill out into the
containing menu around it.  Fix by moving BPF_SYSCALL before the EXPERT
menu, next to BPF.

Fixes: e1abf2cc8d5 ("bpf: Fix the build on BPF_SYSCALL=y && !CONFIG_TRACING kernels, make it more configurable")
Signed-off-by: Josh Triplett <josh@joshtriplett.org>
---

Ingo, do you want to take this through -tip?  Or should this go through some
other tree?

I'm also thinking about splitting the entire EXPERT menu into a separate
Kconfig.expert and including it from init/Kconfig, to make it clear that
everything in that menu should only be visible if EXPERT.  Right now, the long
EXPERT menu blends into the longer init/Kconfig, and issues like this happen
every few kernel releases.

 init/Kconfig | 20 ++++++++++----------
 1 file changed, 10 insertions(+), 10 deletions(-)

diff --git a/init/Kconfig b/init/Kconfig
index dc24dec..e2f16f1 100644
--- a/init/Kconfig
+++ b/init/Kconfig
@@ -1341,6 +1341,16 @@ config HAVE_PCSPKR_PLATFORM
 config BPF
 	bool
 
+# syscall, maps, verifier
+config BPF_SYSCALL
+	bool "Enable bpf() system call"
+	select ANON_INODES
+	select BPF
+	default n
+	help
+	  Enable the bpf() system call that allows to manipulate eBPF
+	  programs and maps via file descriptors.
+
 menuconfig EXPERT
 	bool "Configure standard kernel features (expert users)"
 	# Unhide debug options, to make the on-by-default options visible
@@ -1535,16 +1545,6 @@ config EVENTFD
 
 	  If unsure, say Y.
 
-# syscall, maps, verifier
-config BPF_SYSCALL
-	bool "Enable bpf() system call"
-	select ANON_INODES
-	select BPF
-	default n
-	help
-	  Enable the bpf() system call that allows to manipulate eBPF
-	  programs and maps via file descriptors.
-
 config SHMEM
 	bool "Use full shmem filesystem" if EXPERT
 	default y
-- 
2.1.4


^ permalink raw reply related	[flat|nested] 17+ messages in thread

* Re: [PATCH] init/Kconfig: Fix break in middle of EXPERT menu
  2015-05-11 18:13 [PATCH] init/Kconfig: Fix break in middle of EXPERT menu Josh Triplett
@ 2015-05-11 18:23 ` Randy Dunlap
  2015-05-11 20:23 ` [PATCH 2/1] init/Kconfig: Split expert menu into a separate file, init/Kconfig.expert Josh Triplett
  2015-05-12  7:04 ` [PATCH] init/Kconfig: Fix break in middle of EXPERT menu Ingo Molnar
  2 siblings, 0 replies; 17+ messages in thread
From: Randy Dunlap @ 2015-05-11 18:23 UTC (permalink / raw)
  To: Josh Triplett, Ingo Molnar, Andrew Morton, Paul E. McKenney,
	Michal Hocko, Vladimir Davydov, Johannes Weiner,
	Geert Uytterhoeven, Andy Lutomirski, Bertrand Jacquin,
	Luis R. Rodriguez, Iulia Manda, Pranith Kumar, Clark Williams,
	Mel Gorman, linux-kernel

On 05/11/15 11:13, Josh Triplett wrote:
> Commit e1abf2cc8d5 ("bpf: Fix the build on BPF_SYSCALL=y &&
> !CONFIG_TRACING kernels, make it more configurable") made BPF_SYSCALL no
> longer hidden with !EXPERT, but left it in the middle of the EXPERT
> menu.  menuconfig stops putting config items under a submenu once it
> encounters an item that doesn't depend on the menu's config item, so
> this caused the remainder of the EXPERT menu to spill out into the
> containing menu around it.  Fix by moving BPF_SYSCALL before the EXPERT
> menu, next to BPF.
> 
> Fixes: e1abf2cc8d5 ("bpf: Fix the build on BPF_SYSCALL=y && !CONFIG_TRACING kernels, make it more configurable")
> Signed-off-by: Josh Triplett <josh@joshtriplett.org>
> ---
> 
> Ingo, do you want to take this through -tip?  Or should this go through some
> other tree?
> 
> I'm also thinking about splitting the entire EXPERT menu into a separate
> Kconfig.expert and including it from init/Kconfig, to make it clear that
> everything in that menu should only be visible if EXPERT.  Right now, the long
> EXPERT menu blends into the longer init/Kconfig, and issues like this happen
> every few kernel releases.

Please do make it more difficult to break this.  I have also fixed this
a few times IIRC.


-- 
~Randy

^ permalink raw reply	[flat|nested] 17+ messages in thread

* [PATCH 2/1] init/Kconfig: Split expert menu into a separate file, init/Kconfig.expert
  2015-05-11 18:13 [PATCH] init/Kconfig: Fix break in middle of EXPERT menu Josh Triplett
  2015-05-11 18:23 ` Randy Dunlap
@ 2015-05-11 20:23 ` Josh Triplett
  2015-05-11 21:01   ` Paul Bolle
                     ` (2 more replies)
  2015-05-12  7:04 ` [PATCH] init/Kconfig: Fix break in middle of EXPERT menu Ingo Molnar
  2 siblings, 3 replies; 17+ messages in thread
From: Josh Triplett @ 2015-05-11 20:23 UTC (permalink / raw)
  To: Ingo Molnar, Andrew Morton, Paul E. McKenney, Michal Hocko,
	Josh Triplett, Vladimir Davydov, Johannes Weiner,
	Geert Uytterhoeven, Andy Lutomirski, Bertrand Jacquin,
	Luis R. Rodriguez, Iulia Manda, Pranith Kumar, Clark Williams,
	Mel Gorman, Randy Dunlap, linux-kernel

The expert menu frequently gets broken by a config item in the middle
that leaves off the "if EXPERT" from its prompt.  This results in the
remainder of the menu spilling out into the parent "General setup" menu.
Move the entire expert menu into a separate Kconfig file,
init/Kconfig.expert, to make this harder to do accidentally, and to
break up the exceedingly long init/Kconfig a bit.

Signed-off-by: Josh Triplett <josh@joshtriplett.org>
---

This applies on top of "init/Kconfig: Fix break in middle of EXPERT
menu".  Please apply both.

I'd also like to factor the "if EXPERT" off of all the prompts and into a
single scoped item wrapped around all of them, but kconfig doesn't have any way
to do that.  "menuconfig" is just a hint, with no matching "endmenu" and no
implicit visibility; "menu" is scoped and has "visible if", but that would
create a separate option containing a menu, rather than a menu under EXPERT's
"Configure standard kernel features (expert users)".  And "if EXPERT ... endif"
produces a dependency, not a prompt-visibility condition.  So I think this
would require changes to the Kconfig language, to introduce either a scoped
"visible if EXPERT ... endvisible" or similar, or a scoped version of
menuconfig with a matching "endmenu" and implicit visibility (effectively a
"menu" statement with attached "config" rather than a "config" with a hint
"this might be a menu").  I'm leaning towards the latter.

So I'll send a followup patch enhancing kconfig to improve this case,
but I think splitting this into a separate file is still worth it even
without that.

 init/Kconfig        | 232 +---------------------------------------------------
 init/Kconfig.expert | 231 +++++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 232 insertions(+), 231 deletions(-)
 create mode 100644 init/Kconfig.expert

diff --git a/init/Kconfig b/init/Kconfig
index e2f16f1..a2de3f5 100644
--- a/init/Kconfig
+++ b/init/Kconfig
@@ -1351,237 +1351,7 @@ config BPF_SYSCALL
 	  Enable the bpf() system call that allows to manipulate eBPF
 	  programs and maps via file descriptors.
 
-menuconfig EXPERT
-	bool "Configure standard kernel features (expert users)"
-	# Unhide debug options, to make the on-by-default options visible
-	select DEBUG_KERNEL
-	help
-	  This option allows certain base kernel options and settings
-          to be disabled or tweaked. This is for specialized
-          environments which can tolerate a "non-standard" kernel.
-          Only use this if you really know what you are doing.
-
-config UID16
-	bool "Enable 16-bit UID system calls" if EXPERT
-	depends on HAVE_UID16 && MULTIUSER
-	default y
-	help
-	  This enables the legacy 16-bit UID syscall wrappers.
-
-config MULTIUSER
-	bool "Multiple users, groups and capabilities support" if EXPERT
-	default y
-	help
-	  This option enables support for non-root users, groups and
-	  capabilities.
-
-	  If you say N here, all processes will run with UID 0, GID 0, and all
-	  possible capabilities.  Saying N here also compiles out support for
-	  system calls related to UIDs, GIDs, and capabilities, such as setuid,
-	  setgid, and capset.
-
-	  If unsure, say Y here.
-
-config SGETMASK_SYSCALL
-	bool "sgetmask/ssetmask syscalls support" if EXPERT
-	def_bool PARISC || MN10300 || BLACKFIN || M68K || PPC || MIPS || X86 || SPARC || CRIS || MICROBLAZE || SUPERH
-	---help---
-	  sys_sgetmask and sys_ssetmask are obsolete system calls
-	  no longer supported in libc but still enabled by default in some
-	  architectures.
-
-	  If unsure, leave the default option here.
-
-config SYSFS_SYSCALL
-	bool "Sysfs syscall support" if EXPERT
-	default y
-	---help---
-	  sys_sysfs is an obsolete system call no longer supported in libc.
-	  Note that disabling this option is more secure but might break
-	  compatibility with some systems.
-
-	  If unsure say Y here.
-
-config SYSCTL_SYSCALL
-	bool "Sysctl syscall support" if EXPERT
-	depends on PROC_SYSCTL
-	default n
-	select SYSCTL
-	---help---
-	  sys_sysctl uses binary paths that have been found challenging
-	  to properly maintain and use.  The interface in /proc/sys
-	  using paths with ascii names is now the primary path to this
-	  information.
-
-	  Almost nothing using the binary sysctl interface so if you are
-	  trying to save some space it is probably safe to disable this,
-	  making your kernel marginally smaller.
-
-	  If unsure say N here.
-
-config KALLSYMS
-	 bool "Load all symbols for debugging/ksymoops" if EXPERT
-	 default y
-	 help
-	   Say Y here to let the kernel print out symbolic crash information and
-	   symbolic stack backtraces. This increases the size of the kernel
-	   somewhat, as all symbols have to be loaded into the kernel image.
-
-config KALLSYMS_ALL
-	bool "Include all symbols in kallsyms"
-	depends on DEBUG_KERNEL && KALLSYMS
-	help
-	   Normally kallsyms only contains the symbols of functions for nicer
-	   OOPS messages and backtraces (i.e., symbols from the text and inittext
-	   sections). This is sufficient for most cases. And only in very rare
-	   cases (e.g., when a debugger is used) all symbols are required (e.g.,
-	   names of variables from the data sections, etc).
-
-	   This option makes sure that all symbols are loaded into the kernel
-	   image (i.e., symbols from all sections) in cost of increased kernel
-	   size (depending on the kernel configuration, it may be 300KiB or
-	   something like this).
-
-	   Say N unless you really need all symbols.
-
-config PRINTK
-	default y
-	bool "Enable support for printk" if EXPERT
-	select IRQ_WORK
-	help
-	  This option enables normal printk support. Removing it
-	  eliminates most of the message strings from the kernel image
-	  and makes the kernel more or less silent. As this makes it
-	  very difficult to diagnose system problems, saying N here is
-	  strongly discouraged.
-
-config BUG
-	bool "BUG() support" if EXPERT
-	default y
-	help
-          Disabling this option eliminates support for BUG and WARN, reducing
-          the size of your kernel image and potentially quietly ignoring
-          numerous fatal conditions. You should only consider disabling this
-          option for embedded systems with no facilities for reporting errors.
-          Just say Y.
-
-config ELF_CORE
-	depends on COREDUMP
-	default y
-	bool "Enable ELF core dumps" if EXPERT
-	help
-	  Enable support for generating core dumps. Disabling saves about 4k.
-
-
-config PCSPKR_PLATFORM
-	bool "Enable PC-Speaker support" if EXPERT
-	depends on HAVE_PCSPKR_PLATFORM
-	select I8253_LOCK
-	default y
-	help
-          This option allows to disable the internal PC-Speaker
-          support, saving some memory.
-
-config BASE_FULL
-	default y
-	bool "Enable full-sized data structures for core" if EXPERT
-	help
-	  Disabling this option reduces the size of miscellaneous core
-	  kernel data structures. This saves memory on small machines,
-	  but may reduce performance.
-
-config FUTEX
-	bool "Enable futex support" if EXPERT
-	default y
-	select RT_MUTEXES
-	help
-	  Disabling this option will cause the kernel to be built without
-	  support for "fast userspace mutexes".  The resulting kernel may not
-	  run glibc-based applications correctly.
-
-config HAVE_FUTEX_CMPXCHG
-	bool
-	depends on FUTEX
-	help
-	  Architectures should select this if futex_atomic_cmpxchg_inatomic()
-	  is implemented and always working. This removes a couple of runtime
-	  checks.
-
-config EPOLL
-	bool "Enable eventpoll support" if EXPERT
-	default y
-	select ANON_INODES
-	help
-	  Disabling this option will cause the kernel to be built without
-	  support for epoll family of system calls.
-
-config SIGNALFD
-	bool "Enable signalfd() system call" if EXPERT
-	select ANON_INODES
-	default y
-	help
-	  Enable the signalfd() system call that allows to receive signals
-	  on a file descriptor.
-
-	  If unsure, say Y.
-
-config TIMERFD
-	bool "Enable timerfd() system call" if EXPERT
-	select ANON_INODES
-	default y
-	help
-	  Enable the timerfd() system call that allows to receive timer
-	  events on a file descriptor.
-
-	  If unsure, say Y.
-
-config EVENTFD
-	bool "Enable eventfd() system call" if EXPERT
-	select ANON_INODES
-	default y
-	help
-	  Enable the eventfd() system call that allows to receive both
-	  kernel notification (ie. KAIO) or userspace notifications.
-
-	  If unsure, say Y.
-
-config SHMEM
-	bool "Use full shmem filesystem" if EXPERT
-	default y
-	depends on MMU
-	help
-	  The shmem is an internal filesystem used to manage shared memory.
-	  It is backed by swap and manages resource limits. It is also exported
-	  to userspace as tmpfs if TMPFS is enabled. Disabling this
-	  option replaces shmem and tmpfs with the much simpler ramfs code,
-	  which may be appropriate on small systems without swap.
-
-config AIO
-	bool "Enable AIO support" if EXPERT
-	default y
-	help
-	  This option enables POSIX asynchronous I/O which may by used
-	  by some high performance threaded applications. Disabling
-	  this option saves about 7k.
-
-config ADVISE_SYSCALLS
-	bool "Enable madvise/fadvise syscalls" if EXPERT
-	default y
-	help
-	  This option enables the madvise and fadvise syscalls, used by
-	  applications to advise the kernel about their future memory or file
-	  usage, improving performance. If building an embedded system where no
-	  applications use these syscalls, you can disable this option to save
-	  space.
-
-config PCI_QUIRKS
-	default y
-	bool "Enable PCI quirk workarounds" if EXPERT
-	depends on PCI
-	help
-	  This enables workarounds for various PCI chipset
-	  bugs/quirks. Disable this only if your target machine is
-	  unaffected by PCI quirks.
+source init/Kconfig.expert
 
 config EMBEDDED
 	bool "Embedded system"
diff --git a/init/Kconfig.expert b/init/Kconfig.expert
new file mode 100644
index 0000000..c84a372
--- /dev/null
+++ b/init/Kconfig.expert
@@ -0,0 +1,231 @@
+menuconfig EXPERT
+	bool "Configure standard kernel features (expert users)"
+	# Unhide debug options, to make the on-by-default options visible
+	select DEBUG_KERNEL
+	help
+	  This option allows certain base kernel options and settings
+          to be disabled or tweaked. This is for specialized
+          environments which can tolerate a "non-standard" kernel.
+          Only use this if you really know what you are doing.
+
+config UID16
+	bool "Enable 16-bit UID system calls" if EXPERT
+	depends on HAVE_UID16 && MULTIUSER
+	default y
+	help
+	  This enables the legacy 16-bit UID syscall wrappers.
+
+config MULTIUSER
+	bool "Multiple users, groups and capabilities support" if EXPERT
+	default y
+	help
+	  This option enables support for non-root users, groups and
+	  capabilities.
+
+	  If you say N here, all processes will run with UID 0, GID 0, and all
+	  possible capabilities.  Saying N here also compiles out support for
+	  system calls related to UIDs, GIDs, and capabilities, such as setuid,
+	  setgid, and capset.
+
+	  If unsure, say Y here.
+
+config SGETMASK_SYSCALL
+	bool "sgetmask/ssetmask syscalls support" if EXPERT
+	def_bool PARISC || MN10300 || BLACKFIN || M68K || PPC || MIPS || X86 || SPARC || CRIS || MICROBLAZE || SUPERH
+	---help---
+	  sys_sgetmask and sys_ssetmask are obsolete system calls
+	  no longer supported in libc but still enabled by default in some
+	  architectures.
+
+	  If unsure, leave the default option here.
+
+config SYSFS_SYSCALL
+	bool "Sysfs syscall support" if EXPERT
+	default y
+	---help---
+	  sys_sysfs is an obsolete system call no longer supported in libc.
+	  Note that disabling this option is more secure but might break
+	  compatibility with some systems.
+
+	  If unsure say Y here.
+
+config SYSCTL_SYSCALL
+	bool "Sysctl syscall support" if EXPERT
+	depends on PROC_SYSCTL
+	default n
+	select SYSCTL
+	---help---
+	  sys_sysctl uses binary paths that have been found challenging
+	  to properly maintain and use.  The interface in /proc/sys
+	  using paths with ascii names is now the primary path to this
+	  information.
+
+	  Almost nothing using the binary sysctl interface so if you are
+	  trying to save some space it is probably safe to disable this,
+	  making your kernel marginally smaller.
+
+	  If unsure say N here.
+
+config KALLSYMS
+	 bool "Load all symbols for debugging/ksymoops" if EXPERT
+	 default y
+	 help
+	   Say Y here to let the kernel print out symbolic crash information and
+	   symbolic stack backtraces. This increases the size of the kernel
+	   somewhat, as all symbols have to be loaded into the kernel image.
+
+config KALLSYMS_ALL
+	bool "Include all symbols in kallsyms"
+	depends on DEBUG_KERNEL && KALLSYMS
+	help
+	   Normally kallsyms only contains the symbols of functions for nicer
+	   OOPS messages and backtraces (i.e., symbols from the text and inittext
+	   sections). This is sufficient for most cases. And only in very rare
+	   cases (e.g., when a debugger is used) all symbols are required (e.g.,
+	   names of variables from the data sections, etc).
+
+	   This option makes sure that all symbols are loaded into the kernel
+	   image (i.e., symbols from all sections) in cost of increased kernel
+	   size (depending on the kernel configuration, it may be 300KiB or
+	   something like this).
+
+	   Say N unless you really need all symbols.
+
+config PRINTK
+	default y
+	bool "Enable support for printk" if EXPERT
+	select IRQ_WORK
+	help
+	  This option enables normal printk support. Removing it
+	  eliminates most of the message strings from the kernel image
+	  and makes the kernel more or less silent. As this makes it
+	  very difficult to diagnose system problems, saying N here is
+	  strongly discouraged.
+
+config BUG
+	bool "BUG() support" if EXPERT
+	default y
+	help
+          Disabling this option eliminates support for BUG and WARN, reducing
+          the size of your kernel image and potentially quietly ignoring
+          numerous fatal conditions. You should only consider disabling this
+          option for embedded systems with no facilities for reporting errors.
+          Just say Y.
+
+config ELF_CORE
+	depends on COREDUMP
+	default y
+	bool "Enable ELF core dumps" if EXPERT
+	help
+	  Enable support for generating core dumps. Disabling saves about 4k.
+
+
+config PCSPKR_PLATFORM
+	bool "Enable PC-Speaker support" if EXPERT
+	depends on HAVE_PCSPKR_PLATFORM
+	select I8253_LOCK
+	default y
+	help
+          This option allows to disable the internal PC-Speaker
+          support, saving some memory.
+
+config BASE_FULL
+	default y
+	bool "Enable full-sized data structures for core" if EXPERT
+	help
+	  Disabling this option reduces the size of miscellaneous core
+	  kernel data structures. This saves memory on small machines,
+	  but may reduce performance.
+
+config FUTEX
+	bool "Enable futex support" if EXPERT
+	default y
+	select RT_MUTEXES
+	help
+	  Disabling this option will cause the kernel to be built without
+	  support for "fast userspace mutexes".  The resulting kernel may not
+	  run glibc-based applications correctly.
+
+config HAVE_FUTEX_CMPXCHG
+	bool
+	depends on FUTEX
+	help
+	  Architectures should select this if futex_atomic_cmpxchg_inatomic()
+	  is implemented and always working. This removes a couple of runtime
+	  checks.
+
+config EPOLL
+	bool "Enable eventpoll support" if EXPERT
+	default y
+	select ANON_INODES
+	help
+	  Disabling this option will cause the kernel to be built without
+	  support for epoll family of system calls.
+
+config SIGNALFD
+	bool "Enable signalfd() system call" if EXPERT
+	select ANON_INODES
+	default y
+	help
+	  Enable the signalfd() system call that allows to receive signals
+	  on a file descriptor.
+
+	  If unsure, say Y.
+
+config TIMERFD
+	bool "Enable timerfd() system call" if EXPERT
+	select ANON_INODES
+	default y
+	help
+	  Enable the timerfd() system call that allows to receive timer
+	  events on a file descriptor.
+
+	  If unsure, say Y.
+
+config EVENTFD
+	bool "Enable eventfd() system call" if EXPERT
+	select ANON_INODES
+	default y
+	help
+	  Enable the eventfd() system call that allows to receive both
+	  kernel notification (ie. KAIO) or userspace notifications.
+
+	  If unsure, say Y.
+
+config SHMEM
+	bool "Use full shmem filesystem" if EXPERT
+	default y
+	depends on MMU
+	help
+	  The shmem is an internal filesystem used to manage shared memory.
+	  It is backed by swap and manages resource limits. It is also exported
+	  to userspace as tmpfs if TMPFS is enabled. Disabling this
+	  option replaces shmem and tmpfs with the much simpler ramfs code,
+	  which may be appropriate on small systems without swap.
+
+config AIO
+	bool "Enable AIO support" if EXPERT
+	default y
+	help
+	  This option enables POSIX asynchronous I/O which may by used
+	  by some high performance threaded applications. Disabling
+	  this option saves about 7k.
+
+config ADVISE_SYSCALLS
+	bool "Enable madvise/fadvise syscalls" if EXPERT
+	default y
+	help
+	  This option enables the madvise and fadvise syscalls, used by
+	  applications to advise the kernel about their future memory or file
+	  usage, improving performance. If building an embedded system where no
+	  applications use these syscalls, you can disable this option to save
+	  space.
+
+config PCI_QUIRKS
+	default y
+	bool "Enable PCI quirk workarounds" if EXPERT
+	depends on PCI
+	help
+	  This enables workarounds for various PCI chipset
+	  bugs/quirks. Disable this only if your target machine is
+	  unaffected by PCI quirks.
-- 
2.1.4


^ permalink raw reply related	[flat|nested] 17+ messages in thread

* Re: [PATCH 2/1] init/Kconfig: Split expert menu into a separate file, init/Kconfig.expert
  2015-05-11 20:23 ` [PATCH 2/1] init/Kconfig: Split expert menu into a separate file, init/Kconfig.expert Josh Triplett
@ 2015-05-11 21:01   ` Paul Bolle
  2015-05-11 21:18     ` Josh Triplett
  2015-05-11 21:50   ` Paul Bolle
  2015-05-11 22:36   ` Luis R. Rodriguez
  2 siblings, 1 reply; 17+ messages in thread
From: Paul Bolle @ 2015-05-11 21:01 UTC (permalink / raw)
  To: Josh Triplett
  Cc: Ingo Molnar, Andrew Morton, Paul E. McKenney, Michal Hocko,
	Vladimir Davydov, Johannes Weiner, Geert Uytterhoeven,
	Andy Lutomirski, Bertrand Jacquin, Luis R. Rodriguez, Iulia Manda,
	Pranith Kumar, Clark Williams, Mel Gorman, Randy Dunlap,
	linux-kernel

On Mon, 2015-05-11 at 13:23 -0700, Josh Triplett wrote:
> I'd also like to factor the "if EXPERT" off of all the prompts and into a
> single scoped item wrapped around all of them, but kconfig doesn't have any way
> to do that.  "menuconfig" is just a hint, with no matching "endmenu" and no
> implicit visibility; "menu" is scoped and has "visible if", but that would
> create a separate option containing a menu, rather than a menu under EXPERT's
> "Configure standard kernel features (expert users)".  And "if EXPERT ... endif"
> produces a dependency, not a prompt-visibility condition.  So I think this
> would require changes to the Kconfig language, to introduce either a scoped
> "visible if EXPERT ... endvisible" or similar, or a scoped version of
> menuconfig with a matching "endmenu" and implicit visibility (effectively a
> "menu" statement with attached "config" rather than a "config" with a hint
> "this might be a menu").  I'm leaning towards the latter.

The behavior of menuconfig in this case is rather subtle. I must admit I
never noticed it.

The "visible" option to menus is little used, and I'm not really
familiar with it. So, for what it's worth: would adding a new menu with
	visible if EXPERT

attached to it, and putting all current
	prompt "Foo" if EXPERT

entries in that menu roughly do what you want?

> So I'll send a followup patch enhancing kconfig to improve this case,
> but I think splitting this into a separate file is still worth it even
> without that.

Thanks,


Paul Bolle


^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH 2/1] init/Kconfig: Split expert menu into a separate file, init/Kconfig.expert
  2015-05-11 21:01   ` Paul Bolle
@ 2015-05-11 21:18     ` Josh Triplett
  2015-05-11 21:32       ` Paul Bolle
  0 siblings, 1 reply; 17+ messages in thread
From: Josh Triplett @ 2015-05-11 21:18 UTC (permalink / raw)
  To: Paul Bolle
  Cc: Ingo Molnar, Andrew Morton, Paul E. McKenney, Michal Hocko,
	Vladimir Davydov, Johannes Weiner, Geert Uytterhoeven,
	Andy Lutomirski, Bertrand Jacquin, Luis R. Rodriguez, Iulia Manda,
	Pranith Kumar, Clark Williams, Mel Gorman, Randy Dunlap,
	linux-kernel

On Mon, May 11, 2015 at 11:01:22PM +0200, Paul Bolle wrote:
> On Mon, 2015-05-11 at 13:23 -0700, Josh Triplett wrote:
> > I'd also like to factor the "if EXPERT" off of all the prompts and into a
> > single scoped item wrapped around all of them, but kconfig doesn't have any way
> > to do that.  "menuconfig" is just a hint, with no matching "endmenu" and no
> > implicit visibility; "menu" is scoped and has "visible if", but that would
> > create a separate option containing a menu, rather than a menu under EXPERT's
> > "Configure standard kernel features (expert users)".  And "if EXPERT ... endif"
> > produces a dependency, not a prompt-visibility condition.  So I think this
> > would require changes to the Kconfig language, to introduce either a scoped
> > "visible if EXPERT ... endvisible" or similar, or a scoped version of
> > menuconfig with a matching "endmenu" and implicit visibility (effectively a
> > "menu" statement with attached "config" rather than a "config" with a hint
> > "this might be a menu").  I'm leaning towards the latter.
> 
> The behavior of menuconfig in this case is rather subtle. I must admit I
> never noticed it.
> 
> The "visible" option to menus is little used, and I'm not really
> familiar with it. So, for what it's worth: would adding a new menu with
> 	visible if EXPERT
> 
> attached to it, and putting all current
> 	prompt "Foo" if EXPERT
> 
> entries in that menu roughly do what you want?

Not quite.  menu has a prompt but no associated symbol, so with current
kconfig that would look like:

config EXPERT
    bool "Configure standard kernel features (expert users)"
    ...

menu "Some separate menu prompt here"
    visible if EXPERT

... various options that currently have if EXPERT on their prompts ...

endmenu


However, that would produce *two* entries under the "General setup"
menu: a yes/no entry "Configure standard kernel features (expert users)"
with no submenu, and a "Some separate menu prompt here" entry with a
submenu but no '[ ]' for a yes/no option.  Integrating the two (without
using menuconfig's implicit "add stuff to submenu until an option's
prompt doesn't depend on this symbol" magic) requires new a kconfig
mechanism.

- Josh Triplett

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH 2/1] init/Kconfig: Split expert menu into a separate file, init/Kconfig.expert
  2015-05-11 21:18     ` Josh Triplett
@ 2015-05-11 21:32       ` Paul Bolle
  2015-05-11 21:47         ` Josh Triplett
  0 siblings, 1 reply; 17+ messages in thread
From: Paul Bolle @ 2015-05-11 21:32 UTC (permalink / raw)
  To: Josh Triplett
  Cc: Ingo Molnar, Andrew Morton, Paul E. McKenney, Michal Hocko,
	Vladimir Davydov, Johannes Weiner, Geert Uytterhoeven,
	Andy Lutomirski, Bertrand Jacquin, Luis R. Rodriguez, Iulia Manda,
	Pranith Kumar, Clark Williams, Mel Gorman, Randy Dunlap,
	linux-kernel

On Mon, 2015-05-11 at 14:18 -0700, Josh Triplett wrote:
> However, that would produce *two* entries under the "General setup"
> menu: a yes/no entry "Configure standard kernel features (expert users)"
> with no submenu, and a "Some separate menu prompt here" entry with a
> submenu but no '[ ]' for a yes/no option.  Integrating the two (without
> using menuconfig's implicit "add stuff to submenu until an option's
> prompt doesn't depend on this symbol" magic) requires new a kconfig
> mechanism.

The diff pasted at the end of this message, which I quickly cobbled
together an applies on top of this 2/1, generates these two lines in
menuconfig (for EXPERT = 'y')
    [*] Configure standard kernel features (expert users)
        Standard kernel features  --->                   

Is squashing those two lines worth a new kconfig mechanism?

Thanks,


Paul Bolle

diff --git a/init/Kconfig b/init/Kconfig
index 06c585007964..dfb2a7405a83 100644
--- a/init/Kconfig
+++ b/init/Kconfig
@@ -1421,6 +1421,16 @@ config BPF_SYSCALL
 	  Enable the bpf() system call that allows to manipulate eBPF
 	  programs and maps via file descriptors.
 
+config EXPERT
+	bool "Configure standard kernel features (expert users)"
+	# Unhide debug options, to make the on-by-default options visible
+	select DEBUG_KERNEL
+	help
+	  This option allows certain base kernel options and settings
+          to be disabled or tweaked. This is for specialized
+          environments which can tolerate a "non-standard" kernel.
+          Only use this if you really know what you are doing.
+
 source init/Kconfig.expert
 
 config EMBEDDED
diff --git a/init/Kconfig.expert b/init/Kconfig.expert
index c84a3720164b..c5718f1dc978 100644
--- a/init/Kconfig.expert
+++ b/init/Kconfig.expert
@@ -1,22 +1,15 @@
-menuconfig EXPERT
-	bool "Configure standard kernel features (expert users)"
-	# Unhide debug options, to make the on-by-default options visible
-	select DEBUG_KERNEL
-	help
-	  This option allows certain base kernel options and settings
-          to be disabled or tweaked. This is for specialized
-          environments which can tolerate a "non-standard" kernel.
-          Only use this if you really know what you are doing.
+menu "Standard kernel features"
+	visible if EXPERT
 
 config UID16
-	bool "Enable 16-bit UID system calls" if EXPERT
+	bool "Enable 16-bit UID system calls"
 	depends on HAVE_UID16 && MULTIUSER
 	default y
 	help
 	  This enables the legacy 16-bit UID syscall wrappers.
 
 config MULTIUSER
-	bool "Multiple users, groups and capabilities support" if EXPERT
+	bool "Multiple users, groups and capabilities support"
 	default y
 	help
 	  This option enables support for non-root users, groups and
@@ -30,7 +23,7 @@ config MULTIUSER
 	  If unsure, say Y here.
 
 config SGETMASK_SYSCALL
-	bool "sgetmask/ssetmask syscalls support" if EXPERT
+	bool "sgetmask/ssetmask syscalls support"
 	def_bool PARISC || MN10300 || BLACKFIN || M68K || PPC || MIPS || X86 || SPARC || CRIS || MICROBLAZE || SUPERH
 	---help---
 	  sys_sgetmask and sys_ssetmask are obsolete system calls
@@ -40,7 +33,7 @@ config SGETMASK_SYSCALL
 	  If unsure, leave the default option here.
 
 config SYSFS_SYSCALL
-	bool "Sysfs syscall support" if EXPERT
+	bool "Sysfs syscall support"
 	default y
 	---help---
 	  sys_sysfs is an obsolete system call no longer supported in libc.
@@ -50,7 +43,7 @@ config SYSFS_SYSCALL
 	  If unsure say Y here.
 
 config SYSCTL_SYSCALL
-	bool "Sysctl syscall support" if EXPERT
+	bool "Sysctl syscall support"
 	depends on PROC_SYSCTL
 	default n
 	select SYSCTL
@@ -67,7 +60,7 @@ config SYSCTL_SYSCALL
 	  If unsure say N here.
 
 config KALLSYMS
-	 bool "Load all symbols for debugging/ksymoops" if EXPERT
+	 bool "Load all symbols for debugging/ksymoops"
 	 default y
 	 help
 	   Say Y here to let the kernel print out symbolic crash information and
@@ -93,7 +86,7 @@ config KALLSYMS_ALL
 
 config PRINTK
 	default y
-	bool "Enable support for printk" if EXPERT
+	bool "Enable support for printk"
 	select IRQ_WORK
 	help
 	  This option enables normal printk support. Removing it
@@ -103,7 +96,7 @@ config PRINTK
 	  strongly discouraged.
 
 config BUG
-	bool "BUG() support" if EXPERT
+	bool "BUG() support"
 	default y
 	help
           Disabling this option eliminates support for BUG and WARN, reducing
@@ -115,13 +108,13 @@ config BUG
 config ELF_CORE
 	depends on COREDUMP
 	default y
-	bool "Enable ELF core dumps" if EXPERT
+	bool "Enable ELF core dumps"
 	help
 	  Enable support for generating core dumps. Disabling saves about 4k.
 
 
 config PCSPKR_PLATFORM
-	bool "Enable PC-Speaker support" if EXPERT
+	bool "Enable PC-Speaker support"
 	depends on HAVE_PCSPKR_PLATFORM
 	select I8253_LOCK
 	default y
@@ -131,14 +124,14 @@ config PCSPKR_PLATFORM
 
 config BASE_FULL
 	default y
-	bool "Enable full-sized data structures for core" if EXPERT
+	bool "Enable full-sized data structures for core"
 	help
 	  Disabling this option reduces the size of miscellaneous core
 	  kernel data structures. This saves memory on small machines,
 	  but may reduce performance.
 
 config FUTEX
-	bool "Enable futex support" if EXPERT
+	bool "Enable futex support"
 	default y
 	select RT_MUTEXES
 	help
@@ -155,7 +148,7 @@ config HAVE_FUTEX_CMPXCHG
 	  checks.
 
 config EPOLL
-	bool "Enable eventpoll support" if EXPERT
+	bool "Enable eventpoll support"
 	default y
 	select ANON_INODES
 	help
@@ -163,7 +156,7 @@ config EPOLL
 	  support for epoll family of system calls.
 
 config SIGNALFD
-	bool "Enable signalfd() system call" if EXPERT
+	bool "Enable signalfd() system call"
 	select ANON_INODES
 	default y
 	help
@@ -173,7 +166,7 @@ config SIGNALFD
 	  If unsure, say Y.
 
 config TIMERFD
-	bool "Enable timerfd() system call" if EXPERT
+	bool "Enable timerfd() system call"
 	select ANON_INODES
 	default y
 	help
@@ -183,7 +176,7 @@ config TIMERFD
 	  If unsure, say Y.
 
 config EVENTFD
-	bool "Enable eventfd() system call" if EXPERT
+	bool "Enable eventfd() system call"
 	select ANON_INODES
 	default y
 	help
@@ -193,7 +186,7 @@ config EVENTFD
 	  If unsure, say Y.
 
 config SHMEM
-	bool "Use full shmem filesystem" if EXPERT
+	bool "Use full shmem filesystem"
 	default y
 	depends on MMU
 	help
@@ -204,7 +197,7 @@ config SHMEM
 	  which may be appropriate on small systems without swap.
 
 config AIO
-	bool "Enable AIO support" if EXPERT
+	bool "Enable AIO support"
 	default y
 	help
 	  This option enables POSIX asynchronous I/O which may by used
@@ -212,7 +205,7 @@ config AIO
 	  this option saves about 7k.
 
 config ADVISE_SYSCALLS
-	bool "Enable madvise/fadvise syscalls" if EXPERT
+	bool "Enable madvise/fadvise syscalls"
 	default y
 	help
 	  This option enables the madvise and fadvise syscalls, used by
@@ -223,9 +216,11 @@ config ADVISE_SYSCALLS
 
 config PCI_QUIRKS
 	default y
-	bool "Enable PCI quirk workarounds" if EXPERT
+	bool "Enable PCI quirk workarounds"
 	depends on PCI
 	help
 	  This enables workarounds for various PCI chipset
 	  bugs/quirks. Disable this only if your target machine is
 	  unaffected by PCI quirks.
+
+endmenu


^ permalink raw reply related	[flat|nested] 17+ messages in thread

* Re: [PATCH 2/1] init/Kconfig: Split expert menu into a separate file, init/Kconfig.expert
  2015-05-11 21:32       ` Paul Bolle
@ 2015-05-11 21:47         ` Josh Triplett
  2015-05-11 22:01           ` Paul Bolle
  0 siblings, 1 reply; 17+ messages in thread
From: Josh Triplett @ 2015-05-11 21:47 UTC (permalink / raw)
  To: Paul Bolle
  Cc: Ingo Molnar, Andrew Morton, Paul E. McKenney, Michal Hocko,
	Vladimir Davydov, Johannes Weiner, Geert Uytterhoeven,
	Andy Lutomirski, Bertrand Jacquin, Luis R. Rodriguez, Iulia Manda,
	Pranith Kumar, Clark Williams, Mel Gorman, Randy Dunlap,
	linux-kernel

On Mon, May 11, 2015 at 11:32:28PM +0200, Paul Bolle wrote:
> On Mon, 2015-05-11 at 14:18 -0700, Josh Triplett wrote:
> > However, that would produce *two* entries under the "General setup"
> > menu: a yes/no entry "Configure standard kernel features (expert users)"
> > with no submenu, and a "Some separate menu prompt here" entry with a
> > submenu but no '[ ]' for a yes/no option.  Integrating the two (without
> > using menuconfig's implicit "add stuff to submenu until an option's
> > prompt doesn't depend on this symbol" magic) requires new a kconfig
> > mechanism.
> 
> The diff pasted at the end of this message, which I quickly cobbled
> together an applies on top of this 2/1, generates these two lines in
> menuconfig (for EXPERT = 'y')
>     [*] Configure standard kernel features (expert users)
>         Standard kernel features  --->                   
> 
> Is squashing those two lines worth a new kconfig mechanism?

In my opinion, yes.  If you use the implicit (and error-prone)
menuconfig submenuing, you get a single entry with the '[ ]' and the
submenu.  There are currently 272 instances of "menuconfig" in Kconfig
files.  I'd like to have a less error-prone mechanism for people to use,
with an explicit "endmenu" at the end, and I don't want to leave any
incentive for people to need the more error-prone version.

I would be tempted to just make "menuconfig" require an endmenu, and
convert all users, but that would almost certainly break many
third-party users of kconfig.  So instead, I'm currently extending
"menu" (which already expects "endmenu") to allow the syntax
"menu config SYMBOL", which acts like a combination of "config SYMBOL"
and a menu with "visible if SYMBOL".  Diffstat for the patch I'm testing
right now:

 scripts/kconfig/zconf.y | 18 ++++++++++++++++++
  1 file changed, 18 insertions(+)

That seems worthwhile to have a less error-prone menu mechanism.

(The actual patch would also need to updated zconf.tab.c_shipped.)

(Also, the diff you posted would be smaller if you left "config EXPERT"
at the top of init/Kconfig.expert; why the move?)

- Josh Triplett

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH 2/1] init/Kconfig: Split expert menu into a separate file, init/Kconfig.expert
  2015-05-11 20:23 ` [PATCH 2/1] init/Kconfig: Split expert menu into a separate file, init/Kconfig.expert Josh Triplett
  2015-05-11 21:01   ` Paul Bolle
@ 2015-05-11 21:50   ` Paul Bolle
  2015-05-11 22:04     ` Josh Triplett
  2015-05-11 22:36   ` Luis R. Rodriguez
  2 siblings, 1 reply; 17+ messages in thread
From: Paul Bolle @ 2015-05-11 21:50 UTC (permalink / raw)
  To: Josh Triplett
  Cc: Ingo Molnar, Andrew Morton, Paul E. McKenney, Michal Hocko,
	Vladimir Davydov, Johannes Weiner, Geert Uytterhoeven,
	Andy Lutomirski, Bertrand Jacquin, Luis R. Rodriguez, Iulia Manda,
	Pranith Kumar, Clark Williams, Mel Gorman, Randy Dunlap,
	linux-kernel

On Mon, 2015-05-11 at 13:23 -0700, Josh Triplett wrote:
> --- /dev/null
> +++ b/init/Kconfig.expert
> @@ -0,0 +1,231 @@
> +menuconfig EXPERT
> +	bool "Configure standard kernel features (expert users)"
> +	# Unhide debug options, to make the on-by-default options visible
> +	select DEBUG_KERNEL
> +	help
> +	  This option allows certain base kernel options and settings
> +          to be disabled or tweaked. This is for specialized
> +          environments which can tolerate a "non-standard" kernel.
> +          Only use this if you really know what you are doing.

Comment here saying
	# All entries in this file must have "if EXPERT" after their prompt

or something to that effect (pending you patch, that is)?

> +config KALLSYMS
> +	 bool "Load all symbols for debugging/ksymoops" if EXPERT
> +	 default y
> +	 help
> +	   Say Y here to let the kernel print out symbolic crash information and
> +	   symbolic stack backtraces. This increases the size of the kernel
> +	   somewhat, as all symbols have to be loaded into the kernel image.
> +
> +config KALLSYMS_ALL
> +	bool "Include all symbols in kallsyms"

(For some reason this entry doesn't have if EXPERT but it seems to
behave as expected. Odd.)

> +	depends on DEBUG_KERNEL && KALLSYMS
> +	help
> +	   Normally kallsyms only contains the symbols of functions for nicer
> +	   OOPS messages and backtraces (i.e., symbols from the text and inittext
> +	   sections). This is sufficient for most cases. And only in very rare
> +	   cases (e.g., when a debugger is used) all symbols are required (e.g.,
> +	   names of variables from the data sections, etc).
> +
> +	   This option makes sure that all symbols are loaded into the kernel
> +	   image (i.e., symbols from all sections) in cost of increased kernel
> +	   size (depending on the kernel configuration, it may be 300KiB or
> +	   something like this).
> +
> +	   Say N unless you really need all symbols.
> +
> +config PRINTK
> +	default y
> +	bool "Enable support for printk" if EXPERT

Now you're touching this: bool [...] as the first line, please.

> +	select IRQ_WORK
> +	help
> +	  This option enables normal printk support. Removing it
> +	  eliminates most of the message strings from the kernel image
> +	  and makes the kernel more or less silent. As this makes it
> +	  very difficult to diagnose system problems, saying N here is
> +	  strongly discouraged.

> +config ELF_CORE
> +	depends on COREDUMP
> +	default y
> +	bool "Enable ELF core dumps" if EXPERT

Ditto.

> +	help
> +	  Enable support for generating core dumps. Disabling saves about 4k.

> +config BASE_FULL
> +	default y
> +	bool "Enable full-sized data structures for core" if EXPERT

Ditto.

> +	help
> +	  Disabling this option reduces the size of miscellaneous core
> +	  kernel data structures. This saves memory on small machines,
> +	  but may reduce performance.

> +config PCI_QUIRKS
> +	default y
> +	bool "Enable PCI quirk workarounds" if EXPERT

Ditto.

> +	depends on PCI
> +	help
> +	  This enables workarounds for various PCI chipset
> +	  bugs/quirks. Disable this only if your target machine is
> +	  unaffected by PCI quirks.


Paul Bolle


^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH 2/1] init/Kconfig: Split expert menu into a separate file, init/Kconfig.expert
  2015-05-11 21:47         ` Josh Triplett
@ 2015-05-11 22:01           ` Paul Bolle
  2015-05-11 22:40             ` Josh Triplett
  0 siblings, 1 reply; 17+ messages in thread
From: Paul Bolle @ 2015-05-11 22:01 UTC (permalink / raw)
  To: Josh Triplett
  Cc: Ingo Molnar, Andrew Morton, Paul E. McKenney, Michal Hocko,
	Vladimir Davydov, Johannes Weiner, Geert Uytterhoeven,
	Andy Lutomirski, Bertrand Jacquin, Luis R. Rodriguez, Iulia Manda,
	Pranith Kumar, Clark Williams, Mel Gorman, Randy Dunlap,
	linux-kernel

On Mon, 2015-05-11 at 14:47 -0700, Josh Triplett wrote:
> On Mon, May 11, 2015 at 11:32:28PM +0200, Paul Bolle wrote:
> > Is squashing those two lines worth a new kconfig mechanism?
> 
> In my opinion, yes.  If you use the implicit (and error-prone)
> menuconfig submenuing, you get a single entry with the '[ ]' and the
> submenu.  There are currently 272 instances of "menuconfig" in Kconfig
> files.

How many of those use the subtle trick EXPERT uses?

> I'd like to have a less error-prone mechanism for people to use,
> with an explicit "endmenu" at the end, and I don't want to leave any
> incentive for people to need the more error-prone version.
> 
> I would be tempted to just make "menuconfig" require an endmenu, and
> convert all users, but that would almost certainly break many
> third-party users of kconfig.  So instead, I'm currently extending
> "menu" (which already expects "endmenu") to allow the syntax
> "menu config SYMBOL", which acts like a combination of "config SYMBOL"
> and a menu with "visible if SYMBOL".

Bikeshedding (before I'm even convinced of the need of this extension):
"menu config" is far too similar to "menuconfig".

> Diffstat for the patch I'm testing
> right now:
> 
>  scripts/kconfig/zconf.y | 18 ++++++++++++++++++
>   1 file changed, 18 insertions(+)
> 
> That seems worthwhile to have a less error-prone menu mechanism.
> 
> (The actual patch would also need to updated zconf.tab.c_shipped.)

And some lines in Documentation/kbuild/kconfig-language.txt (speaking
from memory).

> (Also, the diff you posted would be smaller if you left "config EXPERT"
> at the top of init/Kconfig.expert; why the move?)

It was a quick hack. I didn't gave the move much thought, to be honest.

Thanks,


Paul Bolle


^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH 2/1] init/Kconfig: Split expert menu into a separate file, init/Kconfig.expert
  2015-05-11 21:50   ` Paul Bolle
@ 2015-05-11 22:04     ` Josh Triplett
  2015-05-11 22:15       ` Paul Bolle
  0 siblings, 1 reply; 17+ messages in thread
From: Josh Triplett @ 2015-05-11 22:04 UTC (permalink / raw)
  To: Paul Bolle
  Cc: Ingo Molnar, Andrew Morton, Paul E. McKenney, Michal Hocko,
	Vladimir Davydov, Johannes Weiner, Geert Uytterhoeven,
	Andy Lutomirski, Bertrand Jacquin, Luis R. Rodriguez, Iulia Manda,
	Pranith Kumar, Clark Williams, Mel Gorman, Randy Dunlap,
	linux-kernel

On Mon, May 11, 2015 at 11:50:21PM +0200, Paul Bolle wrote:
> On Mon, 2015-05-11 at 13:23 -0700, Josh Triplett wrote:
> > --- /dev/null
> > +++ b/init/Kconfig.expert
> > @@ -0,0 +1,231 @@
> > +menuconfig EXPERT
> > +	bool "Configure standard kernel features (expert users)"
> > +	# Unhide debug options, to make the on-by-default options visible
> > +	select DEBUG_KERNEL
> > +	help
> > +	  This option allows certain base kernel options and settings
> > +          to be disabled or tweaked. This is for specialized
> > +          environments which can tolerate a "non-standard" kernel.
> > +          Only use this if you really know what you are doing.
> 
> Comment here saying
> 	# All entries in this file must have "if EXPERT" after their prompt
> 
> or something to that effect (pending you patch, that is)?

Yeah, I should add such a comment.  I was hoping to make it obsolete via
kconfig changes, but in the interim, sure.

> > +config KALLSYMS
> > +	 bool "Load all symbols for debugging/ksymoops" if EXPERT
> > +	 default y
> > +	 help
> > +	   Say Y here to let the kernel print out symbolic crash information and
> > +	   symbolic stack backtraces. This increases the size of the kernel
> > +	   somewhat, as all symbols have to be loaded into the kernel image.
> > +
> > +config KALLSYMS_ALL
> > +	bool "Include all symbols in kallsyms"
> 
> (For some reason this entry doesn't have if EXPERT but it seems to
> behave as expected. Odd.)

Because it depends on KALLSYMS.  Magic!

> > +	depends on DEBUG_KERNEL && KALLSYMS
> > +	help
> > +	   Normally kallsyms only contains the symbols of functions for nicer
> > +	   OOPS messages and backtraces (i.e., symbols from the text and inittext
> > +	   sections). This is sufficient for most cases. And only in very rare
> > +	   cases (e.g., when a debugger is used) all symbols are required (e.g.,
> > +	   names of variables from the data sections, etc).
> > +
> > +	   This option makes sure that all symbols are loaded into the kernel
> > +	   image (i.e., symbols from all sections) in cost of increased kernel
> > +	   size (depending on the kernel configuration, it may be 300KiB or
> > +	   something like this).
> > +
> > +	   Say N unless you really need all symbols.
> > +
> > +config PRINTK
> > +	default y
> > +	bool "Enable support for printk" if EXPERT
> 
> Now you're touching this: bool [...] as the first line, please.

I'd like the commit moving these to their own file to very obviously
look like it's moving these lines unmodified, rather than making changes
in the process.  If you want this (and all the subsequent instances you
flagged) cleaned up, let's do that as a subsequent patch separate from
the move.

- Josh Triplett

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH 2/1] init/Kconfig: Split expert menu into a separate file, init/Kconfig.expert
  2015-05-11 22:04     ` Josh Triplett
@ 2015-05-11 22:15       ` Paul Bolle
  2015-05-28  8:13         ` Paul Bolle
  0 siblings, 1 reply; 17+ messages in thread
From: Paul Bolle @ 2015-05-11 22:15 UTC (permalink / raw)
  To: Josh Triplett
  Cc: Ingo Molnar, Andrew Morton, Paul E. McKenney, Michal Hocko,
	Vladimir Davydov, Johannes Weiner, Geert Uytterhoeven,
	Andy Lutomirski, Bertrand Jacquin, Luis R. Rodriguez, Iulia Manda,
	Pranith Kumar, Clark Williams, Mel Gorman, Randy Dunlap,
	linux-kernel

On Mon, 2015-05-11 at 15:04 -0700, Josh Triplett wrote:
> On Mon, May 11, 2015 at 11:50:21PM +0200, Paul Bolle wrote:
> > On Mon, 2015-05-11 at 13:23 -0700, Josh Triplett wrote:
> > > +config KALLSYMS_ALL
> > > +	bool "Include all symbols in kallsyms"
> > 
> > (For some reason this entry doesn't have if EXPERT but it seems to
> > behave as expected. Odd.)
> 
> Because it depends on KALLSYMS.  Magic!

Welcome to the land of Kconfig!

> > > +	depends on DEBUG_KERNEL && KALLSYMS
> > > +	help
> > > +	   Normally kallsyms only contains the symbols of functions for nicer
> > > +	   OOPS messages and backtraces (i.e., symbols from the text and inittext
> > > +	   sections). This is sufficient for most cases. And only in very rare
> > > +	   cases (e.g., when a debugger is used) all symbols are required (e.g.,
> > > +	   names of variables from the data sections, etc).
> > > +
> > > +	   This option makes sure that all symbols are loaded into the kernel
> > > +	   image (i.e., symbols from all sections) in cost of increased kernel
> > > +	   size (depending on the kernel configuration, it may be 300KiB or
> > > +	   something like this).
> > > +
> > > +	   Say N unless you really need all symbols.
> > > +
> > > +config PRINTK
> > > +	default y
> > > +	bool "Enable support for printk" if EXPERT
> > 
> > Now you're touching this: bool [...] as the first line, please.
> 
> I'd like the commit moving these to their own file to very obviously
> look like it's moving these lines unmodified, rather than making changes
> in the process.  If you want this (and all the subsequent instances you
> flagged) cleaned up, let's do that as a subsequent patch separate from
> the move.

That was one of my pet peeves leaving it's cage. Still, not sure if we'd
really notice if it's not a simple move. But a separate patch would
mainly add noise, so let's not do that.

Thanks,


Paul Bolle


^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH 2/1] init/Kconfig: Split expert menu into a separate file, init/Kconfig.expert
  2015-05-11 20:23 ` [PATCH 2/1] init/Kconfig: Split expert menu into a separate file, init/Kconfig.expert Josh Triplett
  2015-05-11 21:01   ` Paul Bolle
  2015-05-11 21:50   ` Paul Bolle
@ 2015-05-11 22:36   ` Luis R. Rodriguez
  2015-05-11 22:51     ` Josh Triplett
  2 siblings, 1 reply; 17+ messages in thread
From: Luis R. Rodriguez @ 2015-05-11 22:36 UTC (permalink / raw)
  To: Josh Triplett
  Cc: Ingo Molnar, Andrew Morton, Paul E. McKenney, Michal Hocko,
	Vladimir Davydov, Johannes Weiner, Geert Uytterhoeven,
	Andy Lutomirski, Bertrand Jacquin, Iulia Manda, Pranith Kumar,
	Clark Williams, Mel Gorman, Randy Dunlap, linux-kernel

On Mon, May 11, 2015 at 01:23:02PM -0700, Josh Triplett wrote:
>  init/Kconfig        | 232 +---------------------------------------------------
>  init/Kconfig.expert | 231 +++++++++++++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 232 insertions(+), 231 deletions(-)
>  create mode 100644 init/Kconfig.expert

I'm blinded by this patch, can you use git format-patch -M and also add:

[diff]                                                                          
        renamelimit=0

To your .gitconfig

  Luis

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH 2/1] init/Kconfig: Split expert menu into a separate file, init/Kconfig.expert
  2015-05-11 22:01           ` Paul Bolle
@ 2015-05-11 22:40             ` Josh Triplett
  0 siblings, 0 replies; 17+ messages in thread
From: Josh Triplett @ 2015-05-11 22:40 UTC (permalink / raw)
  To: Paul Bolle
  Cc: Ingo Molnar, Andrew Morton, Paul E. McKenney, Michal Hocko,
	Vladimir Davydov, Johannes Weiner, Geert Uytterhoeven,
	Andy Lutomirski, Bertrand Jacquin, Luis R. Rodriguez, Iulia Manda,
	Pranith Kumar, Clark Williams, Mel Gorman, Randy Dunlap,
	linux-kernel

On Tue, May 12, 2015 at 12:01:27AM +0200, Paul Bolle wrote:
> On Mon, 2015-05-11 at 14:47 -0700, Josh Triplett wrote:
> > On Mon, May 11, 2015 at 11:32:28PM +0200, Paul Bolle wrote:
> > > Is squashing those two lines worth a new kconfig mechanism?
> > 
> > In my opinion, yes.  If you use the implicit (and error-prone)
> > menuconfig submenuing, you get a single entry with the '[ ]' and the
> > submenu.  There are currently 272 instances of "menuconfig" in Kconfig
> > files.
> 
> How many of those use the subtle trick EXPERT uses?

A rough search suggests about 35, with the rest using an "if" right
after the menu (presumably because they want dependencies rather than
visibility).  (A couple of which currently have the same broken-menu
problem today's patch 1 fixed for EXPERT, and a couple of which are
using that trick *across input files*.)

> > I'd like to have a less error-prone mechanism for people to use,
> > with an explicit "endmenu" at the end, and I don't want to leave any
> > incentive for people to need the more error-prone version.
> > 
> > I would be tempted to just make "menuconfig" require an endmenu, and
> > convert all users, but that would almost certainly break many
> > third-party users of kconfig.  So instead, I'm currently extending
> > "menu" (which already expects "endmenu") to allow the syntax
> > "menu config SYMBOL", which acts like a combination of "config SYMBOL"
> > and a menu with "visible if SYMBOL".
> 
> Bikeshedding (before I'm even convinced of the need of this extension):
> "menu config" is far too similar to "menuconfig".

Yeah, agreed; given that people will still end up using "menuconfig" in
light of the above, I'll use another syntax.

> > Diffstat for the patch I'm testing
> > right now:
> > 
> >  scripts/kconfig/zconf.y | 18 ++++++++++++++++++
> >   1 file changed, 18 insertions(+)
> > 
> > That seems worthwhile to have a less error-prone menu mechanism.
> > 
> > (The actual patch would also need to updated zconf.tab.c_shipped.)
> 
> And some lines in Documentation/kbuild/kconfig-language.txt (speaking
> from memory).

Good point; this does need documentation.

- Josh Triplett

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH 2/1] init/Kconfig: Split expert menu into a separate file, init/Kconfig.expert
  2015-05-11 22:36   ` Luis R. Rodriguez
@ 2015-05-11 22:51     ` Josh Triplett
  0 siblings, 0 replies; 17+ messages in thread
From: Josh Triplett @ 2015-05-11 22:51 UTC (permalink / raw)
  To: Luis R. Rodriguez
  Cc: Ingo Molnar, Andrew Morton, Paul E. McKenney, Michal Hocko,
	Vladimir Davydov, Johannes Weiner, Geert Uytterhoeven,
	Andy Lutomirski, Bertrand Jacquin, Iulia Manda, Pranith Kumar,
	Clark Williams, Mel Gorman, Randy Dunlap, linux-kernel

On Tue, May 12, 2015 at 12:36:49AM +0200, Luis R. Rodriguez wrote:
> On Mon, May 11, 2015 at 01:23:02PM -0700, Josh Triplett wrote:
> >  init/Kconfig        | 232 +---------------------------------------------------
> >  init/Kconfig.expert | 231 +++++++++++++++++++++++++++++++++++++++++++++++++++
> >  2 files changed, 232 insertions(+), 231 deletions(-)
> >  create mode 100644 init/Kconfig.expert
> 
> I'm blinded by this patch, can you use git format-patch -M and also add:
> 
> [diff]                                                                          
>         renamelimit=0
> 
> To your .gitconfig

>From a quick test, doing what you suggest doesn't have any effect; if I
also use -C1 instead of -M, then the patch to init/Kconfig looks exactly
the same (deleting 231 lines and adding a "source" line), while the
patch to init/Kconfig.expert goes from adding 231 lines to copying
init/Kconfig and deleting 1800 lines.

That does not seem like an improvement.

- Josh Triplett

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH] init/Kconfig: Fix break in middle of EXPERT menu
  2015-05-11 18:13 [PATCH] init/Kconfig: Fix break in middle of EXPERT menu Josh Triplett
  2015-05-11 18:23 ` Randy Dunlap
  2015-05-11 20:23 ` [PATCH 2/1] init/Kconfig: Split expert menu into a separate file, init/Kconfig.expert Josh Triplett
@ 2015-05-12  7:04 ` Ingo Molnar
  2015-05-12  7:08   ` Josh Triplett
  2 siblings, 1 reply; 17+ messages in thread
From: Ingo Molnar @ 2015-05-12  7:04 UTC (permalink / raw)
  To: Josh Triplett
  Cc: Andrew Morton, Paul E. McKenney, Michal Hocko, Vladimir Davydov,
	Johannes Weiner, Geert Uytterhoeven, Andy Lutomirski,
	Bertrand Jacquin, Luis R. Rodriguez, Iulia Manda, Pranith Kumar,
	Clark Williams, Mel Gorman, linux-kernel


* Josh Triplett <josh@joshtriplett.org> wrote:

> Commit e1abf2cc8d5 ("bpf: Fix the build on BPF_SYSCALL=y &&
> !CONFIG_TRACING kernels, make it more configurable") made BPF_SYSCALL no
> longer hidden with !EXPERT, but left it in the middle of the EXPERT
> menu.  menuconfig stops putting config items under a submenu once it
> encounters an item that doesn't depend on the menu's config item, so
> this caused the remainder of the EXPERT menu to spill out into the
> containing menu around it.  Fix by moving BPF_SYSCALL before the EXPERT
> menu, next to BPF.
> 
> Fixes: e1abf2cc8d5 ("bpf: Fix the build on BPF_SYSCALL=y && !CONFIG_TRACING kernels, make it more configurable")
> Signed-off-by: Josh Triplett <josh@joshtriplett.org>
> ---
> 
> Ingo, do you want to take this through -tip?  Or should this go 
> through some other tree?

I can pick it up, but -mm might be better suited for this, if you do:

> I'm also thinking about splitting the entire EXPERT menu into a 
> separate Kconfig.expert and including it from init/Kconfig, to make 
> it clear that everything in that menu should only be visible if 
> EXPERT.  Right now, the long EXPERT menu blends into the longer 
> init/Kconfig, and issues like this happen every few kernel releases.

That's a good idea as well.

Thanks,

	Ingo

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH] init/Kconfig: Fix break in middle of EXPERT menu
  2015-05-12  7:04 ` [PATCH] init/Kconfig: Fix break in middle of EXPERT menu Ingo Molnar
@ 2015-05-12  7:08   ` Josh Triplett
  0 siblings, 0 replies; 17+ messages in thread
From: Josh Triplett @ 2015-05-12  7:08 UTC (permalink / raw)
  To: Ingo Molnar
  Cc: Andrew Morton, Paul E. McKenney, Michal Hocko, Vladimir Davydov,
	Johannes Weiner, Geert Uytterhoeven, Andy Lutomirski,
	Bertrand Jacquin, Luis R. Rodriguez, Iulia Manda, Pranith Kumar,
	Clark Williams, Mel Gorman, linux-kernel

On Tue, May 12, 2015 at 09:04:55AM +0200, Ingo Molnar wrote:
> 
> * Josh Triplett <josh@joshtriplett.org> wrote:
> 
> > Commit e1abf2cc8d5 ("bpf: Fix the build on BPF_SYSCALL=y &&
> > !CONFIG_TRACING kernels, make it more configurable") made BPF_SYSCALL no
> > longer hidden with !EXPERT, but left it in the middle of the EXPERT
> > menu.  menuconfig stops putting config items under a submenu once it
> > encounters an item that doesn't depend on the menu's config item, so
> > this caused the remainder of the EXPERT menu to spill out into the
> > containing menu around it.  Fix by moving BPF_SYSCALL before the EXPERT
> > menu, next to BPF.
> > 
> > Fixes: e1abf2cc8d5 ("bpf: Fix the build on BPF_SYSCALL=y && !CONFIG_TRACING kernels, make it more configurable")
> > Signed-off-by: Josh Triplett <josh@joshtriplett.org>
> > ---
> > 
> > Ingo, do you want to take this through -tip?  Or should this go 
> > through some other tree?
> 
> I can pick it up, but -mm might be better suited for this, if you do:

Fair enough.

> > I'm also thinking about splitting the entire EXPERT menu into a 
> > separate Kconfig.expert and including it from init/Kconfig, to make 
> > it clear that everything in that menu should only be visible if 
> > EXPERT.  Right now, the long EXPERT menu blends into the longer 
> > init/Kconfig, and issues like this happen every few kernel releases.
> 
> That's a good idea as well.

I did so, in a patch later in this thread.

- Josh Triplett

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH 2/1] init/Kconfig: Split expert menu into a separate file, init/Kconfig.expert
  2015-05-11 22:15       ` Paul Bolle
@ 2015-05-28  8:13         ` Paul Bolle
  0 siblings, 0 replies; 17+ messages in thread
From: Paul Bolle @ 2015-05-28  8:13 UTC (permalink / raw)
  To: Josh Triplett
  Cc: Ingo Molnar, Andrew Morton, Paul E. McKenney, Michal Hocko,
	Vladimir Davydov, Johannes Weiner, Geert Uytterhoeven,
	Andy Lutomirski, Bertrand Jacquin, Luis R. Rodriguez, Iulia Manda,
	Pranith Kumar, Clark Williams, Mel Gorman, Randy Dunlap,
	linux-kernel

[Yes, this patch is superseded by your series starting at
https://lkml.org/lkml/2015/5/14/447 . But I wanted to jot this down
somewhere.]

On Tue, 2015-05-12 at 00:15 +0200, Paul Bolle wrote:
> On Mon, 2015-05-11 at 15:04 -0700, Josh Triplett wrote:
> > On Mon, May 11, 2015 at 11:50:21PM +0200, Paul Bolle wrote:
> > > On Mon, 2015-05-11 at 13:23 -0700, Josh Triplett wrote:
> > > > +config KALLSYMS_ALL
> > > > +	bool "Include all symbols in kallsyms"
> > > 
> > > (For some reason this entry doesn't have if EXPERT but it seems to
> > > behave as expected. Odd.)
> > 
> > Because it depends on KALLSYMS.  Magic!
> 
> Welcome to the land of Kconfig!

It's even more subtle (at least currently, ie before this patch is
applied).

KALLSYMS_ALL's prompt doesn't depend on EXPERT. KALLSYMS's prompt does.
For some reason, perhaps because KALLSYMS_ALL depends on KALLSYMS, this
makes both entries visible under EXPERT's menu even if EXPERT is _not_
set.

But, in contrast to BPF_SYSCALL, this doesn't influence the following
symbols that do have a prompt that depends on EXPERT.

Adding "if EXPERT" to the prompt of KALLSYMS_ALL does hide that symbol
_and_ KALLSYMS (when EXPERT is not set, of course).

(This may be documented, or clearly commented in the code. I didn't
check.)

> > > > +	depends on DEBUG_KERNEL && KALLSYMS
> > > > +	help
> > > > +	   Normally kallsyms only contains the symbols of functions for nicer
> > > > +	   OOPS messages and backtraces (i.e., symbols from the text and inittext
> > > > +	   sections). This is sufficient for most cases. And only in very rare
> > > > +	   cases (e.g., when a debugger is used) all symbols are required (e.g.,
> > > > +	   names of variables from the data sections, etc).
> > > > +
> > > > +	   This option makes sure that all symbols are loaded into the kernel
> > > > +	   image (i.e., symbols from all sections) in cost of increased kernel
> > > > +	   size (depending on the kernel configuration, it may be 300KiB or
> > > > +	   something like this).
> > > > +
> > > > +	   Say N unless you really need all symbols.

Thanks,


Paul Bolle


^ permalink raw reply	[flat|nested] 17+ messages in thread

end of thread, other threads:[~2015-05-28  8:13 UTC | newest]

Thread overview: 17+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2015-05-11 18:13 [PATCH] init/Kconfig: Fix break in middle of EXPERT menu Josh Triplett
2015-05-11 18:23 ` Randy Dunlap
2015-05-11 20:23 ` [PATCH 2/1] init/Kconfig: Split expert menu into a separate file, init/Kconfig.expert Josh Triplett
2015-05-11 21:01   ` Paul Bolle
2015-05-11 21:18     ` Josh Triplett
2015-05-11 21:32       ` Paul Bolle
2015-05-11 21:47         ` Josh Triplett
2015-05-11 22:01           ` Paul Bolle
2015-05-11 22:40             ` Josh Triplett
2015-05-11 21:50   ` Paul Bolle
2015-05-11 22:04     ` Josh Triplett
2015-05-11 22:15       ` Paul Bolle
2015-05-28  8:13         ` Paul Bolle
2015-05-11 22:36   ` Luis R. Rodriguez
2015-05-11 22:51     ` Josh Triplett
2015-05-12  7:04 ` [PATCH] init/Kconfig: Fix break in middle of EXPERT menu Ingo Molnar
2015-05-12  7:08   ` Josh Triplett

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox