Re: [Xen-devel] [PATCH v2 6/7] build: convert perfc{, _arrays} to Kconfig

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 03.05.16 at 16:29, <cardoe@xxxxxxxxxx> wrote:
> Convert the 'perfc' and 'perfc_arrays' options to Kconfig as
> CONFIG_PERF_COUNTERS and CONFIG_PERF_ARRAYS to minimize code changes.
> 
> Signed-off-by: Doug Goldstein <cardoe@xxxxxxxxxx>
> ---
> CC: Jan Beulich <jbeulich@xxxxxxxx>
> CC: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>

Again too narrow a Cc list.

> --- a/xen/Kconfig.debug
> +++ b/xen/Kconfig.debug
> @@ -28,6 +28,21 @@ config FRAME_POINTER
>         maybe slower, but it gives very useful debugging information
>         in case of any Xen bugs.
>  
> +config PERF_ARRAYS
> +     bool "Performance Counter Array Histograms"
> +     default n
> +     depends on PERF_COUNTERS
> +     ---help---
> +       Enables software performance counter array histograms.
> +
> +config PERF_COUNTERS
> +     bool "Performance Counters"
> +     default n
> +     ---help---
> +       Enables software performance counters that allows you to analyze
> +       bottlenecks in the system. To access this data you must use the
> +       'xenperf' tool.

I guess you put them in this order because that's how they sort.
But for any of the menu config interfaces this will result in the
former not properly becoming a sub-item of the latter. IOW
dependent should always (directly) follow their master options.

Also - is "default n" really useful here? In one of the earlier
patches you had omitted it, and that's what I'd prefer unless
doing so has some bad side effect.

>  ifneq ($(origin kexec),undefined)
>  $(error "You must use 'make menuconfig' to enable/disable kexec now.")
>  endif

Considering this context - no similar addition here?

> --- a/xen/arch/x86/x86_64/asm-offsets.c
> +++ b/xen/arch/x86/x86_64/asm-offsets.c
> @@ -151,7 +151,7 @@ void __dummy__(void)
>      OFFSET(TRAPBOUNCE_eip, struct trap_bounce, eip);
>      BLANK();
>  
> -#if PERF_COUNTERS
> +#if CONFIG_PERF_COUNTERS

#ifdef

> --- a/xen/common/Makefile
> +++ b/xen/common/Makefile
> @@ -64,7 +64,7 @@ obj-$(CONFIG_XSPLICE) += xsplice_elf.o
>  
>  obj-bin-$(CONFIG_X86) += $(foreach n,decompress bunzip2 unxz unlzma unlzo 
> unlz4 earlycpio,$(n).init.o)
>  
> -obj-$(perfc)       += perfc.o
> +obj-$(CONFIG_PERF_COUNTERS)       += perfc.o

Please move this up at once, too, like you did in one of the earlier
patches.

Jan


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel