linux-kernel - Re: [PATCH 1/7] dynamic debug v2

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: <20080809010741.GA26036@kroah.com>
Date:	Fri, 8 Aug 2008 18:07:41 -0700
From:	Greg KH <greg@...ah.com>
To:	Jason Baron <jbaron@...hat.com>
Cc:	linux-kernel@...r.kernel.org, akpm@...ux-foundation.org,
	joe@...ches.com, nick@...k-andrew.net, randy.dunlap@...cle.com
Subject: Re: [PATCH 1/7] dynamic debug v2 - infrastructure

On Fri, Aug 08, 2008 at 05:51:53PM -0400, Jason Baron wrote:
> 
> ok, here's a patch that implements just the basic core functionality that ties
> into pr_debug() and dev_dbg(). That is, any users of those two functions can
> be dynamically enabled/disabled at runtime if CONFIG_DYNAMIC_PRINTK_DEBUG is
> set at compile time. We can add the more complex flag/level debugging later as
> you suggest...

Very nice, just as I'm writing patches to dynamically make
CONFIG_USB_DEBUG be selectable on the fly :)

Don't worry, that can easily be moved over to your version once it is
done and in the tree...

> Few notes...there is still one control file: <debugfs>/dynamic_printk/modules
> We can split this up now or later, but I kind of like being able to see all
> the controls in one file. Also, i've used a djb2 hash function in the code,
> which i'm not sure is under the correct license/copyright, so i just wanted
> to point that out as well. see: scripts/basic/hash.c. If its an issue, i can
> find another hash function. 

Why use that hash function?  Actually, it looks like you are using 2
different ones, right?  Why?

> --- /dev/null
> +++ b/include/linux/dynamic_printk.h
> @@ -0,0 +1,94 @@
> +#ifndef _DYNAMIC_PRINTK_H
> +#define _DYNAMIC_PRINTK_H
> +
> +#ifdef __KERNEL__

Shouldn't be needed.

> +#include <linux/string.h>
> +#include <linux/hash.h>
> +
> +#define DYNAMIC_DEBUG_HASH_BITS 6
> +#define DEBUG_HASH_TABLE_SIZE (1 << DYNAMIC_DEBUG_HASH_BITS)
> +
> +#define TYPE_BOOLEAN 1

What's this for?

> +#define DYNAMIC_ENABLED_ALL 0
> +#define DYNAMIC_ENABLED_NONE 1
> +#define DYNAMIC_ENABLED_SOME 2
> +
> +extern int dynamic_enabled;
> +extern long long dynamic_printk_enabled;
> +extern long long dynamic_printk_enabled2;

What's up with the "2" versions?

> +config DYNAMIC_PRINTK_DEBUG
> +	bool "Enable dynamic printk() call support"
> +	default n
> +	depends on PRINTK
> +	select PRINTK_DEBUG
> +	help
> +	 
> +	  Compiles debug level messages into the kernel, which would not
> +	  otherwise be available at runtime. These messages can then be
> +	  enabled/disabled on a per module basis. This mechanism, implicitly 
> +	  enables all pr_debug() and dev_dbg() calls. The impact of this 
> +	  compile option is a larger kernel text size ~2%.

Trailing spaces :(

It's looking very good so far.

thanks,

greg k-h
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/