Re: [PATCH 1/2] quickstats, kernel sample collector

[Randy Dunlap <rdunlap@infradead.org>]

Hi,

(Aside: in patch 0, is "abube" a typo?
Data are exported or controlled as abube.
)

For both patch 1 & patch 2, please use the expected/documented multi-line
comment style (in Documentation/process/coding-style.rst):

The preferred style for long (multi-line) comments is:
{except in net/ and drivers/net/}

	/*
	 * This is the preferred style for multi-line
	 * comments in the Linux kernel source code.
	 * Please use it consistently.
	 *
	 * Description:  A column of asterisks on the left side,
	 * with beginning and ending almost-blank lines.
	 */

On 2/25/20 6:30 PM, Luigi Rizzo wrote:
> quickstats is a helper to accumulate in-kernel samples (timestamps,
> sizes, etc) and show distributions through debugfs. Compiled as a module
> by default; set CONFIG_QUICKSTATS=y in your config to make it compiled in.

For inclusion in the kernel source tree, it should default to n, not m.

[snip]


> Signed-off-by: Luigi Rizzo <lrizzo@google.com>
> ---
>  include/linux/kstats.h |  61 +++++++++
>  lib/Kconfig.debug      |   7 +
>  lib/Makefile           |   1 +
>  lib/kstats.c           | 303 +++++++++++++++++++++++++++++++++++++++++
>  4 files changed, 372 insertions(+)
>  create mode 100644 include/linux/kstats.h
>  create mode 100644 lib/kstats.c
> 

> diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug
> index 69def4a9df009..d581ad075d438 100644
> --- a/lib/Kconfig.debug
> +++ b/lib/Kconfig.debug
> @@ -1452,6 +1452,13 @@ config LATENCYTOP
>  	  Enable this option if you want to use the LatencyTOP tool
>  	  to find out which userspace is blocking on what kernel operations.
>  
> +config QUICKSTATS
> +	tristate "collect percpu metrics and export distributions through debugfs"
> +	default m

Drop the "default m".
Add:	depends on DEBUG_FS

This code should build OK even when CONFIG_DEBUG_FS is not set/enabled,
but it seems that it would be useless in that case.


> +	help
> +	  Helper library to collect percpu kernel metrics, exporting
> +	  distributions through debugfs. See kernel/kstats.c
> +
>  source "kernel/trace/Kconfig"
>  
>  config PROVIDE_OHCI1394_DMA_INIT


thanks.
-- 
~Randy