Re: [PATCH v2 1/7] mm/page_ext: resurrect struct page extending code for debugging

From: Andrew Morton
Date: Fri Nov 21 2014 - 18:37:40 EST


On Fri, 21 Nov 2014 17:14:00 +0900 Joonsoo Kim <iamjoonsoo.kim@xxxxxxx> wrote:

> When we debug something, we'd like to insert some information to
> every page. For this purpose, we sometimes modify struct page itself.
> But, this has drawbacks. First, it requires re-compile. This makes us
> hesitate to use the powerful debug feature so development process is
> slowed down. And, second, sometimes it is impossible to rebuild the kernel
> due to third party module dependency. At third, system behaviour would be
> largely different after re-compile, because it changes size of struct
> page greatly and this structure is accessed by every part of kernel.
> Keeping this as it is would be better to reproduce errornous situation.
>
> This feature is intended to overcome above mentioned problems. This feature
> allocates memory for extended data per page in certain place rather than
> the struct page itself. This memory can be accessed by the accessor
> functions provided by this code. During the boot process, it checks whether
> allocation of huge chunk of memory is needed or not. If not, it avoids
> allocating memory at all. With this advantage, we can include this feature
> into the kernel in default and can avoid rebuild and solve related problems.
>
> Until now, memcg uses this technique. But, now, memcg decides to embed
> their variable to struct page itself and it's code to extend struct page
> has been removed. I'd like to use this code to develop debug feature,
> so this patch resurrect it.
>
> To help these things to work well, this patch introduces two callbacks
> for clients. One is the need callback which is mandatory if user wants
> to avoid useless memory allocation at boot-time. The other is optional,
> init callback, which is used to do proper initialization after memory
> is allocated. Detailed explanation about purpose of these functions is
> in code comment. Please refer it.
>
> Others are completely same with previous extension code in memcg.
>
> ...
>
> +static bool __init invoke_need_callbacks(void)
> +{
> + int i;
> + int entries = ARRAY_SIZE(page_ext_ops);
> +
> + for (i = 0; i < entries; i++) {
> + if (page_ext_ops[i]->need && page_ext_ops[i]->need())
> + return true;
> + }
> +
> + return false;
> +}
> +
> +static void __init invoke_init_callbacks(void)
> +{
> + int i;
> + int entries = sizeof(page_ext_ops) / sizeof(page_ext_ops[0]);

ARRAY_SIZE()

> + for (i = 0; i < entries; i++) {
> + if (page_ext_ops[i]->init)
> + page_ext_ops[i]->init();
> + }
> +}
> +
>
> ...
>
> +void __init page_ext_init_flatmem(void)
> +{
> +
> + int nid, fail;
> +
> + if (!invoke_need_callbacks)
> + return;
> +
> + for_each_online_node(nid) {
> + fail = alloc_node_page_ext(nid);
> + if (fail)
> + goto fail;
> + }
> + pr_info("allocated %ld bytes of page_ext\n", total_usage);
> + invoke_init_callbacks();
> + return;
> +
> +fail:
> + pr_crit("allocation of page_ext failed.\n");
> + panic("Out of memory");

Did we really need to panic the machine? The situation should be
pretty easily recoverable by disabling the clients. I guess it's OK as
long as page_ext is being used for kernel developer debug things.

> +}
> +

We'll need this to fix the build. I'll queue it up.


From: Andrew Morton <akpm@xxxxxxxxxxxxxxxxxxxx>
Subject: include/linux/kmemleak.h: needs slab.h

include/linux/kmemleak.h: In function 'kmemleak_alloc_recursive':
include/linux/kmemleak.h:43: error: 'SLAB_NOLEAKTRACE' undeclared (first use in this function)

--- a/include/linux/kmemleak.h~include-linux-kmemleakh-needs-slabh
+++ a/include/linux/kmemleak.h
@@ -21,6 +21,8 @@
#ifndef __KMEMLEAK_H
#define __KMEMLEAK_H

+#include <linux/slab.h>
+
#ifdef CONFIG_DEBUG_KMEMLEAK

extern void kmemleak_init(void) __ref;



And here are a couple of tweaks for this patch:

From: Andrew Morton <akpm@xxxxxxxxxxxxxxxxxxxx>
Subject: mm-page_ext-resurrect-struct-page-extending-code-for-debugging-fix

use ARRAY_SIZE, clean up 80-col tricks

--- a/mm/page_ext.c~mm-page_ext-resurrect-struct-page-extending-code-for-debugging-fix
+++ a/mm/page_ext.c
@@ -71,7 +71,7 @@ static bool __init invoke_need_callbacks
static void __init invoke_init_callbacks(void)
{
int i;
- int entries = sizeof(page_ext_ops) / sizeof(page_ext_ops[0]);
+ int entries = ARRAY_SIZE(page_ext_ops);

for (i = 0; i < entries; i++) {
if (page_ext_ops[i]->init)
@@ -81,7 +81,6 @@ static void __init invoke_init_callbacks

#if !defined(CONFIG_SPARSEMEM)

-
void __meminit pgdat_page_ext_init(struct pglist_data *pgdat)
{
pgdat->node_page_ext = NULL;
@@ -232,8 +231,9 @@ static void free_page_ext(void *addr)
vfree(addr);
} else {
struct page *page = virt_to_page(addr);
- size_t table_size =
- sizeof(struct page_ext) * PAGES_PER_SECTION;
+ size_t table_size;
+
+ table_size = sizeof(struct page_ext) * PAGES_PER_SECTION;

BUG_ON(PageReserved(page));
free_pages_exact(addr, table_size);

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