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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <c8101a7e-97a2-a32f-39d0-e6a229153398@infradead.org>
Date:   Tue, 25 Feb 2020 20:26:20 -0800
From:   Randy Dunlap <rdunlap@...radead.org>
To:     Luigi Rizzo <lrizzo@...gle.com>, linux-kernel@...r.kernel.org,
        mhiramat@...nel.org, akpm@...ux-foundation.org,
        gregkh@...uxfoundation.org, naveen.n.rao@...ux.ibm.com,
        changbin.du@...el.com, ardb@...nel.org, rizzo@....unipi.it,
        pabeni@...hat.com, toke@...hat.com, hawk@...nel.org
Subject: Re: [PATCH 1/2] quickstats, kernel sample collector

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@...gle.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