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.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
