[PATCH] mm: add a new command-line kmemcheck value

[Xishi Qiu]

Add a new command-line kmemcheck value: kmemcheck=3 (disable the feature),
this is the same effect as CONFIG_KMEMCHECK disabled.
After doing this, we can enable/disable kmemcheck feature in one vmlinux.

Signed-off-by: Xishi Qiu <qiux...@huawei.com>
---
arch/x86/mm/init.c | 11 ++++++
arch/x86/mm/kmemcheck/kmemcheck.c | 62 ++++++++++++++++++++++++++-----------
arch/x86/mm/kmemcheck/shadow.c | 13 +++++---
include/linux/kmemcheck.h | 2 +
mm/kmemcheck.c | 12 +++++--
mm/page_alloc.c | 2 +
6 files changed, 76 insertions(+), 26 deletions(-)

diff --git a/arch/x86/mm/init.c b/arch/x86/mm/init.c
index f971306..cd7d75f 100644
--- a/arch/x86/mm/init.c
+++ b/arch/x86/mm/init.c
@@ -135,6 +135,15 @@ static void __init probe_page_size_mask(void)
page_size_mask |= 1 << PG_LEVEL_2M;
#endif

+#if defined(CONFIG_KMEMCHECK)
+ if (!kmemcheck_on) {
+ if (direct_gbpages)
+ page_size_mask |= 1 << PG_LEVEL_1G;
+ if (cpu_has_pse)
+ page_size_mask |= 1 << PG_LEVEL_2M;
+ }
+#endif
+
/* Enable PSE if available */
if (cpu_has_pse)
set_in_cr4(X86_CR4_PSE);
@@ -331,6 +340,8 @@ bool pfn_range_is_mapped(unsigned long start_pfn, unsigned long end_pfn)
return false;
}

+extern int kmemcheck_on;
+
/*
* Setup the direct mapping of the physical memory at PAGE_OFFSET.
* This runs before bootmem is initialized and gets pages directly from
diff --git a/arch/x86/mm/kmemcheck/kmemcheck.c b/arch/x86/mm/kmemcheck/kmemcheck.c
index d87dd6d..d686ee0 100644
--- a/arch/x86/mm/kmemcheck/kmemcheck.c
+++ b/arch/x86/mm/kmemcheck/kmemcheck.c
@@ -44,30 +44,35 @@
#ifdef CONFIG_KMEMCHECK_ONESHOT_BY_DEFAULT
# define KMEMCHECK_ENABLED 2
#endif
+#define KMEMCHECK_CLOSED 3

-int kmemcheck_enabled = KMEMCHECK_ENABLED;
+int kmemcheck_enabled = KMEMCHECK_CLOSED;
+int kmemcheck_on = 0;

int __init kmemcheck_init(void)
{
+ if (kmemcheck_on) {
#ifdef CONFIG_SMP
- /*
- * Limit SMP to use a single CPU. We rely on the fact that this code
- * runs before SMP is set up.
- */
- if (setup_max_cpus > 1) {
- printk(KERN_INFO
- "kmemcheck: Limiting number of CPUs to 1.\n");
- setup_max_cpus = 1;
- }
+ /*
+ * Limit SMP to use a single CPU. We rely on the fact that this code
+ * runs before SMP is set up.
+ */
+ if (setup_max_cpus > 1) {
+ printk(KERN_INFO
+ "kmemcheck: Limiting number of CPUs to 1.\n");
+ setup_max_cpus = 1;
+ }
#endif

- if (!kmemcheck_selftest()) {
- printk(KERN_INFO "kmemcheck: self-tests failed; disabling\n");
- kmemcheck_enabled = 0;
- return -EINVAL;
+ if (!kmemcheck_selftest()) {
+ printk(KERN_INFO "kmemcheck: self-tests failed; disabling\n");
+ kmemcheck_enabled = 0;
+ return -EINVAL;
+ }
+
+ printk(KERN_INFO "kmemcheck: Initialized\n");
}

- printk(KERN_INFO "kmemcheck: Initialized\n");
return 0;
}

@@ -82,6 +87,12 @@ static int __init param_kmemcheck(char *str)
return -EINVAL;

sscanf(str, "%d", &kmemcheck_enabled);
+
+ if ((kmemcheck_enabled >= KMEMCHECK_CLOSED) || (kmemcheck_enabled < 0))
+ kmemcheck_on = 0;
+ else
+ kmemcheck_on = 1;
+
return 0;
}

@@ -134,9 +145,12 @@ static DEFINE_PER_CPU(struct kmemcheck_context, kmemcheck_context);

bool kmemcheck_active(struct pt_regs *regs)
{
- struct kmemcheck_context *data = &__get_cpu_var(kmemcheck_context);
+ if (kmemcheck_on) {
+ struct kmemcheck_context *data = &__get_cpu_var(kmemcheck_context);
+ return data->balance > 0;
+ }

- return data->balance > 0;
+ return false;
}

/* Save an address that needs to be shown/hidden */
@@ -223,6 +237,9 @@ void kmemcheck_hide(struct pt_regs *regs)
struct kmemcheck_context *data = &__get_cpu_var(kmemcheck_context);
int n;

+ if (!kmemcheck_on)
+ return;
+
BUG_ON(!irqs_disabled());

if (unlikely(data->balance != 1)) {
@@ -278,6 +295,9 @@ void kmemcheck_show_pages(struct page *p, unsigned int n)

bool kmemcheck_page_is_tracked(struct page *p)
{
+ if (!kmemcheck_on)
+ return false;
+
/* This will also check the "hidden" flag of the PTE. */
return kmemcheck_pte_lookup((unsigned long) page_address(p));
}
@@ -333,6 +353,9 @@ bool kmemcheck_is_obj_initialized(unsigned long addr, size_t size)
enum kmemcheck_shadow status;
void *shadow;

+ if (!kmemcheck_on)
+ return true;
+
shadow = kmemcheck_shadow_lookup(addr);
if (!shadow)
return true;
@@ -616,6 +639,9 @@ bool kmemcheck_fault(struct pt_regs *regs, unsigned long address,
{
pte_t *pte;

+ if (!kmemcheck_on)
+ return false;
+
/*
* XXX: Is it safe to assume that memory accesses from virtual 86
* mode or non-kernel code segments will _never_ access kernel
@@ -644,7 +670,7 @@ bool kmemcheck_fault(struct pt_regs *regs, unsigned long address,

bool kmemcheck_trap(struct pt_regs *regs)
{
- if (!kmemcheck_active(regs))
+ if (!kmemcheck_on || !kmemcheck_active(regs))
return false;

/* We're done. */
diff --git a/arch/x86/mm/kmemcheck/shadow.c b/arch/x86/mm/kmemcheck/shadow.c
index aec1242..26e461d 100644
--- a/arch/x86/mm/kmemcheck/shadow.c
+++ b/arch/x86/mm/kmemcheck/shadow.c
@@ -90,7 +90,8 @@ void kmemcheck_mark_uninitialized(void *address, unsigned int n)
*/
void kmemcheck_mark_initialized(void *address, unsigned int n)
{
- mark_shadow(address, n, KMEMCHECK_SHADOW_INITIALIZED);
+ if (kmemcheck_on)
+ mark_shadow(address, n, KMEMCHECK_SHADOW_INITIALIZED);
}
EXPORT_SYMBOL_GPL(kmemcheck_mark_initialized);

@@ -103,16 +104,18 @@ void kmemcheck_mark_unallocated_pages(struct page *p, unsigned int n)
{
unsigned int i;

- for (i = 0; i < n; ++i)
- kmemcheck_mark_unallocated(page_address(&p[i]), PAGE_SIZE);
+ if (kmemcheck_on)
+ for (i = 0; i < n; ++i)
+ kmemcheck_mark_unallocated(page_address(&p[i]), PAGE_SIZE);
}

void kmemcheck_mark_uninitialized_pages(struct page *p, unsigned int n)
{
unsigned int i;

- for (i = 0; i < n; ++i)
- kmemcheck_mark_uninitialized(page_address(&p[i]), PAGE_SIZE);
+ if (kmemcheck_on)
+ for (i = 0; i < n; ++i)
+ kmemcheck_mark_uninitialized(page_address(&p[i]), PAGE_SIZE);
}

void kmemcheck_mark_initialized_pages(struct page *p, unsigned int n)
diff --git a/include/linux/kmemcheck.h b/include/linux/kmemcheck.h
index 39f8453..13d15e1 100644
--- a/include/linux/kmemcheck.h
+++ b/include/linux/kmemcheck.h
@@ -6,6 +6,7 @@

#ifdef CONFIG_KMEMCHECK
extern int kmemcheck_enabled;
+extern int kmemcheck_on;

/* The slab-related functions. */
void kmemcheck_alloc_shadow(struct page *page, int order, gfp_t flags, int node);
@@ -88,6 +89,7 @@ bool kmemcheck_is_obj_initialized(unsigned long addr, size_t size);

#else
#define kmemcheck_enabled 0
+#define kmemcheck_on 0

static inline void
kmemcheck_alloc_shadow(struct page *page, int order, gfp_t flags, int node)
diff --git a/mm/kmemcheck.c b/mm/kmemcheck.c
index fd814fd..fd89146 100644
--- a/mm/kmemcheck.c
+++ b/mm/kmemcheck.c
@@ -10,6 +10,9 @@ void kmemcheck_alloc_shadow(struct page *page, int order, gfp_t flags, int node)
int pages;
int i;

+ if (!kmemcheck_on)
+ return;
+
pages = 1 << order;

/*
@@ -41,6 +44,9 @@ void kmemcheck_free_shadow(struct page *page, int order)
int pages;
int i;

+ if (!kmemcheck_on)
+ return;
+
if (!kmemcheck_page_is_tracked(page))
return;

@@ -63,7 +69,7 @@ void kmemcheck_slab_alloc(struct kmem_cache *s, gfp_t gfpflags, void *object,
* Has already been memset(), which initializes the shadow for us
* as well.
*/
- if (gfpflags & __GFP_ZERO)
+ if (!kmemcheck_on || gfpflags & __GFP_ZERO)
return;

/* No need to initialize the shadow of a non-tracked slab. */
@@ -92,7 +98,7 @@ void kmemcheck_slab_alloc(struct kmem_cache *s, gfp_t gfpflags, void *object,
void kmemcheck_slab_free(struct kmem_cache *s, void *object, size_t size)
{
/* TODO: RCU freeing is unsupported for now; hide false positives. */
- if (!s->ctor && !(s->flags & SLAB_DESTROY_BY_RCU))
+ if (kmemcheck_on && !s->ctor && !(s->flags & SLAB_DESTROY_BY_RCU))
kmemcheck_mark_freed(object, size);
}

@@ -101,7 +107,7 @@ void kmemcheck_pagealloc_alloc(struct page *page, unsigned int order,
{
int pages;

- if (gfpflags & (__GFP_HIGHMEM | __GFP_NOTRACK))
+ if (!kmemcheck_on || (gfpflags & (__GFP_HIGHMEM | __GFP_NOTRACK)))
return;

pages = 1 << order;
diff --git a/mm/page_alloc.c b/mm/page_alloc.c
index f861d02..0b2bbf2 100644
--- a/mm/page_alloc.c
+++ b/mm/page_alloc.c
@@ -2716,6 +2716,8 @@ retry_cpuset:
page = get_page_from_freelist(gfp_mask|__GFP_HARDWALL, nodemask, order,
zonelist, high_zoneidx, alloc_flags,
preferred_zone, migratetype);
+ if (kmemcheck_on && kmemcheck_enabled && page)
+ kmemcheck_pagealloc_alloc(page, order, gfp_mask);
if (unlikely(!page)) {
/*
* Runtime PM, block IO and its error handling path
--
1.7.1



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

[Vegard Nossum]

(Oops, resend to restore Cc.)

Hi,

On 31 December 2013 09:32, Xishi Qiu <qiux...@huawei.com> wrote:
> Add a new command-line kmemcheck value: kmemcheck=3 (disable the feature),
> this is the same effect as CONFIG_KMEMCHECK disabled.
> After doing this, we can enable/disable kmemcheck feature in one vmlinux.

Could you please explain what exactly the difference is between the
existing kmemcheck=0 parameter and the new kmemcheck=3?

Thanks,


Vegard

[Xishi Qiu]

On 2013/12/31 18:12, Vegard Nossum wrote:

> (Oops, resend to restore Cc.)
>
> Hi,
>
> On 31 December 2013 09:32, Xishi Qiu <qiux...@huawei.com> wrote:
>> Add a new command-line kmemcheck value: kmemcheck=3 (disable the feature),
>> this is the same effect as CONFIG_KMEMCHECK disabled.
>> After doing this, we can enable/disable kmemcheck feature in one vmlinux.
>
> Could you please explain what exactly the difference is between the
> existing kmemcheck=0 parameter and the new kmemcheck=3?
>
> Thanks,
>
>
> Vegard
>

Hi Vegard,

kmemcheck=0: enable kmemcheck feature, but don't check the memory.
and the OS use only one cpu.(setup_max_cpus = 1)
kmemcheck=3: disable kmemcheck feature.

this is the same effect as CONFIG_KMEMCHECK disabled.

OS will use cpus as many as possible.

Thanks,
Xishi Qiu

[Vegard Nossum]

On 2 January 2014 02:34, Xishi Qiu <qiux...@huawei.com> wrote:
> On 2013/12/31 18:12, Vegard Nossum wrote:
>> On 31 December 2013 09:32, Xishi Qiu <qiux...@huawei.com> wrote:
>>> Add a new command-line kmemcheck value: kmemcheck=3 (disable the feature),
>>> this is the same effect as CONFIG_KMEMCHECK disabled.
>>> After doing this, we can enable/disable kmemcheck feature in one vmlinux.
>>
>> Could you please explain what exactly the difference is between the
>> existing kmemcheck=0 parameter and the new kmemcheck=3?
>

> kmemcheck=0: enable kmemcheck feature, but don't check the memory.
> and the OS use only one cpu.(setup_max_cpus = 1)
> kmemcheck=3: disable kmemcheck feature.
> this is the same effect as CONFIG_KMEMCHECK disabled.
> OS will use cpus as many as possible.
>

I see. In that case, I think it's better to allow all the CPUs to keep
running while kmemcheck is disabled with kmemcheck=0 boot parameter,
and offline them when/if kmemcheck is reenabled via
/proc/sys/kernel/kmemcheck.


Vegard

[Xishi Qiu]

On 2014/1/11 0:02, Vegard Nossum wrote:

> On 2 January 2014 02:34, Xishi Qiu <qiux...@huawei.com> wrote:
>> On 2013/12/31 18:12, Vegard Nossum wrote:
>>> On 31 December 2013 09:32, Xishi Qiu <qiux...@huawei.com> wrote:
>>>> Add a new command-line kmemcheck value: kmemcheck=3 (disable the feature),
>>>> this is the same effect as CONFIG_KMEMCHECK disabled.
>>>> After doing this, we can enable/disable kmemcheck feature in one vmlinux.
>>>
>>> Could you please explain what exactly the difference is between the
>>> existing kmemcheck=0 parameter and the new kmemcheck=3?
>>
>> kmemcheck=0: enable kmemcheck feature, but don't check the memory.
>> and the OS use only one cpu.(setup_max_cpus = 1)
>> kmemcheck=3: disable kmemcheck feature.
>> this is the same effect as CONFIG_KMEMCHECK disabled.
>> OS will use cpus as many as possible.
>>
>
> I see. In that case, I think it's better to allow all the CPUs to keep
> running while kmemcheck is disabled with kmemcheck=0 boot parameter,
> and offline them when/if kmemcheck is reenabled via
> /proc/sys/kernel/kmemcheck.
>
>
> Vegard
>
>

Hi Vegard,

In some scenes, user want to check memory dynamicly, this "dynamically"
means we can turn on/off the feature at boottime, not runtime. Without
this patch, if user want to use this feature, he should change config
and build the kernel, then reboot. This is impossilbe if user has no
kernel code or he don't know how to build the kernel.

boottime: kmemcheck=0/1/2/3 (command-line)
runtime: kmemcheck=0/1/2 (/proc/sys/kernel/kmemcheck)

The main different between kmemcheck=0 and 3 is the used memory. Kmemcheck
will use about twice as much memory as normal.

Thanks,
Xishi Qiu

[Xishi Qiu]

On 2014/1/22 9:22, Xishi Qiu wrote:

>
> Hi Vegard,
>
> In some scenes, user want to check memory dynamicly, this "dynamically"
> means we can turn on/off the feature at boottime, not runtime. Without
> this patch, if user want to use this feature, he should change config
> and build the kernel, then reboot. This is impossilbe if user has no
> kernel code or he don't know how to build the kernel.
>
> boottime: kmemcheck=0/1/2/3 (command-line)
> runtime: kmemcheck=0/1/2 (/proc/sys/kernel/kmemcheck)
>
> The main different between kmemcheck=0 and 3 is the used memory. Kmemcheck
> will use about twice as much memory as normal.
>
> Thanks,
> Xishi Qiu
>
> --

Hi Vegard,

What do you think of this feature?

Add a command-line "kmemcheck=3", then the kernel runs as the same as CONFIG_KMEMCHECK=off
even CONFIG_KMEMCHECK is turn on. "kmemcheck=0/1/2" is the same as originally.
In another word, "kmemcheck=3" is the same as:
1) turn off CONFIG_KMEMCHECK
2) rebuild the kernel
3) reboot
The different between kmemcheck=0 and 3 is the used memory and nr_cpus.
Also kmemcheck=0 can used in runtime, and kmemcheck=3 is only used in boot.

I think this feature can help users to debug the kernel quickly, It is no
need to open CONFIG_KMEMCHECK and rebuild it. Especially sometimes users don't
have the kernel source code or the code is different from www.kernel.org.
e.g. some private features were added to the kernel source code, and usually
users can not have the source code.

[Xishi Qiu]

If we want to debug the kernel memory, we should turn on CONFIG_KMEMCHECK
and rebuild the kernel. This always takes a long time and sometimes
impossible, e.g. users don't have the kernel source code or the code
is different from "www.kernel.org" (private features may be added to the
kernel, and usually users can not get the whole code).

This patch adds a new command-line "kmemcheck=3", then the kernel will run

as the same as CONFIG_KMEMCHECK=off even CONFIG_KMEMCHECK is turn on.

"kmemcheck=0/1/2" is the same as originally. This means we can always turn
on CONFIG_KMEMCHECK, and use "kmemcheck=3" to control it on/off with out
rebuild the kernel.

In another word, "kmemcheck=3" is equivalent:

1) turn off CONFIG_KMEMCHECK
2) rebuild the kernel
3) reboot

The different between kmemcheck=0 and 3 is the used memory and nr_cpus.
Also kmemcheck=0 can used in runtime, and kmemcheck=3 is only used in boot.

boottime: kmemcheck=0/1/2/3 (command-line)
runtime: kmemcheck=0/1/2 (/proc/sys/kernel/kmemcheck)

[Vegard Nossum]

On 17 February 2014 03:34, Xishi Qiu <qiux...@huawei.com> wrote:
> If we want to debug the kernel memory, we should turn on CONFIG_KMEMCHECK
> and rebuild the kernel. This always takes a long time and sometimes
> impossible, e.g. users don't have the kernel source code or the code
> is different from "www.kernel.org" (private features may be added to the
> kernel, and usually users can not get the whole code).
>
> This patch adds a new command-line "kmemcheck=3", then the kernel will run
> as the same as CONFIG_KMEMCHECK=off even CONFIG_KMEMCHECK is turn on.
> "kmemcheck=0/1/2" is the same as originally. This means we can always turn
> on CONFIG_KMEMCHECK, and use "kmemcheck=3" to control it on/off with out
> rebuild the kernel.
>
> In another word, "kmemcheck=3" is equivalent:
> 1) turn off CONFIG_KMEMCHECK
> 2) rebuild the kernel
> 3) reboot
>
> The different between kmemcheck=0 and 3 is the used memory and nr_cpus.
> Also kmemcheck=0 can used in runtime, and kmemcheck=3 is only used in boot.
> boottime: kmemcheck=0/1/2/3 (command-line)
> runtime: kmemcheck=0/1/2 (/proc/sys/kernel/kmemcheck)

This is not the right way to do what you want.

The behaviour that we want is:

- CONFIG_KMEMCHECK=y + kmemcheck=0 (boot parameter) should have a
minimal runtime impact and not limit the number of CPUs
- CONFIG_KMEMCHECK=y + kmemcheck=1 should limit the number of CPUs during boot
- setting kmemcheck to 1 via /proc/sys/kernel/kmemcheck should
probably return an error if more than 1 CPU is online


Vegard

[Xishi Qiu]

Hi Vegard,

Thank you for your reply. If we only use "kmemcheck=0" to control, how about
the used memory? Will it use about twice as much memory as normal?

Thanks,
Xishi Qiu

> .

[Dave Hansen]

On 02/16/2014 06:34 PM, Xishi Qiu wrote:
> diff --git a/arch/x86/mm/init.c b/arch/x86/mm/init.c
> index f971306..cd7d75f 100644
> --- a/arch/x86/mm/init.c
> +++ b/arch/x86/mm/init.c
> @@ -135,6 +135,15 @@ static void __init probe_page_size_mask(void)
> page_size_mask |= 1 << PG_LEVEL_2M;
> #endif
>
> +#if defined(CONFIG_KMEMCHECK)
> + if (!kmemcheck_on) {
> + if (direct_gbpages)
> + page_size_mask |= 1 << PG_LEVEL_1G;
> + if (cpu_has_pse)
> + page_size_mask |= 1 << PG_LEVEL_2M;
> + }
> +#endif

This is a copy-n-paste from just above which is inside a:

#if !defined(CONFIG_DEBUG_PAGEALLOC) && !defined(CONFIG_KMEMCHECK)

This gets really confusing to figure out which one of these options will
rule. Maybe it's just time to add a kmemcheck_active() function which
gets #ifdef'd to 0 if the config option is off.

> /* Enable PSE if available */
> if (cpu_has_pse)
> set_in_cr4(X86_CR4_PSE);
> @@ -331,6 +340,8 @@ bool pfn_range_is_mapped(unsigned long start_pfn, unsigned long end_pfn)
> return false;
> }
>
> +extern int kmemcheck_on;

Didn't you _just_ reference this? Either it's unnecessary, or this code
doesn't compile.

> /*
> * Setup the direct mapping of the physical memory at PAGE_OFFSET.
> * This runs before bootmem is initialized and gets pages directly from
> diff --git a/arch/x86/mm/kmemcheck/kmemcheck.c b/arch/x86/mm/kmemcheck/kmemcheck.c
> index d87dd6d..d686ee0 100644
> --- a/arch/x86/mm/kmemcheck/kmemcheck.c
> +++ b/arch/x86/mm/kmemcheck/kmemcheck.c
> @@ -44,30 +44,35 @@
> #ifdef CONFIG_KMEMCHECK_ONESHOT_BY_DEFAULT
> # define KMEMCHECK_ENABLED 2
> #endif
> +#define KMEMCHECK_CLOSED 3
>
> -int kmemcheck_enabled = KMEMCHECK_ENABLED;
> +int kmemcheck_enabled = KMEMCHECK_CLOSED;
> +int kmemcheck_on = 0;

This is pretty confusing. If I see "kmemcheck_on" and
"kmemcheck_enabled" in the code, it's hard to figure out which one to
trust and infer what they were _supposed_ to be doing.

Please add some documentation for these, at least. The commit message
isn't enough.

I'd also suggest breaking this up in to at least two pieces: one which
adds the functions to check at runtime if we want to use kmemcheck, and
then a second one to actually add this tunable.