Re: seq_file: procfs: kmalloc_slab WARNING

[Wladislav Wiebe <wladislav.kw@gmail.com>]

Hi guys,

we got the real root cause of the allocation issue:

Subject: [PATCH 1/1] of: fdt: fix memory initialization for expanded DT

Already existing property flags are filled wrong for properties created from
initial FDT. This could cause problems if this DYNAMIC device-tree functions
are used later, i.e. properties are attached/detached/replaced. Simply dumping
flags from the running system show, that some initial static (not allocated via
kzmalloc()) nodes are marked as dynamic.

I putted some debug extensions to property_proc_show(..) :
..
+       if (OF_IS_DYNAMIC(pp))
+               pr_err("DEBUG: xxx : OF_IS_DYNAMIC\n");
+       if (OF_IS_DETACHED(pp))
+               pr_err("DEBUG: xxx : OF_IS_DETACHED\n");

when you operate on the nodes (e.g.: ~$ cat /proc/device-tree/*some_node*) you
will see that those flags are filled wrong, basically in most cases it will dump
a DYNAMIC or DETACHED status, which is in not true.
(BTW. this OF_IS_DETACHED is a own define for debug purposes which which just
make a test_bit(OF_DETACHED, &x->_flags)

If nodes are dynamic kernel is allowed to kfree() them. But it will crash
attempting to do so on the nodes from FDT -- they are not allocated via
kzmalloc().

Signed-off-by: Wladislav Wiebe <wladislav.kw@gmail.com>
---
 drivers/of/fdt.c |    2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/drivers/of/fdt.c b/drivers/of/fdt.c
index 6bb7cf2..b10ba00 100644
--- a/drivers/of/fdt.c
+++ b/drivers/of/fdt.c
@@ -392,6 +392,8 @@ static void __unflatten_device_tree(struct boot_param_header *blob,
 	mem = (unsigned long)
 		dt_alloc(size + 4, __alignof__(struct device_node));

+	memset((void *)mem, 0, size);
+
 	((__be32 *)mem)[size / 4] = cpu_to_be32(0xdeadbeef);

 	pr_debug("  unflattening %lx...\n", mem);
-- 1.7.1

This is committed to the mainline - hope it comes in soon.

Thanks & BR,
Wladislav Wiebe

On 06/08/13 15:26, Wladislav Wiebe wrote:
> Hello FS community,
> 
> on a PPC 32-Bit board with a Linux Kernel v3.10.0 I see that seq_read tries to allocate
> for a procfs entry 8 MB (max. should be 4 MB). It's basically a Device Tree entry (/proc/device-tree/localbus@5000/flash@0/partition@1/reg)
> and why it needs 8 MB now, I don't know - the same Device Tree works fine on previous Kernel releases.
> 
> When it's happen it throw warnings from with kmalloc_slab.
> I added some debug functions to seq_read, to see when allocating 8 MB or more
> (Marked "DEBUG xxx ...")
> 
> ..
> UBI: background thread "ubi_bgt0d" started, PID 895
> DEBUG xxx seq_read: m->size = 8388608
> DEBUG xxx seq_read: path = /proc/device-tree/localbus@5000/flash@0/partition@1/reg
> ------------[ cut here ]------------
> WARNING: at /var/fpwork/wiebe/newfsm/bld/bld-kernelsources-linux/results/linux/mm/slab_common.c:377
> Modules linked in: ubi mddg_post(O) mddg_rpram(O) mddg_system_driver(O) mddg_watchdog(O)
> CPU: 0 PID: 903 Comm: hexdump Tainted: G           O 3.10.0-0-sampleversion-fcmd #60
> task: cf97c230 ti: cfbf0000 task.ti: cfbf0000
> NIP: c0098d98 LR: c00acf1c CTR: 00000000
> REGS: cfbf1da0 TRAP: 0700   Tainted: G           O  (3.10.0-0-sampleversion-fcmd)
> MSR: 00029000 <CE,EE,ME>  CR: 22008444  XER: 00000000
> 
> GPR00: c00d2100 cfbf1e50 cf97c230 00800000 000000d0 00000000 ce7ffffc 00000000
> GPR08: 00400000 c0380000 00000001 cfbf1e50 22008444 100a24dc 00000001 c030e2d0
> GPR16: cfacac88 c030e2b0 007fffff fffff000 c028c80c 00000000 cfbf1e80 00000400
> GPR24: 4801c000 cfbdf4e0 00000000 00000000 c00d2100 00000000 000000d0 cfacac80
> NIP [c0098d98] kmalloc_slab+0x24/0xb0
> LR [c00acf1c] __kmalloc+0x1c/0x154
> Call Trace:
> [cfbf1e50] [c00ac4d0] kfree+0x6c/0x120 (unreliable)
> [cfbf1e70] [c00d2100] seq_read+0x328/0x5d4
> [cfbf1ee0] [c00fb8b8] proc_reg_read+0x5c/0x90
> [cfbf1ef0] [c00b27b8] vfs_read+0xa4/0x150
> [cfbf1f10] [c00b29a8] SyS_read+0x4c/0x84
> [cfbf1f40] [c000be3c] ret_from_syscall+0x0/0x3c
> --- Exception: c01 at 0xfe63934
>     LR = 0xfe1a6a8
> Instruction dump:
> 7c631910 7c6300d0 4e800020 3d200040 7f834840 409d003c 708a0200 39200000
> 40a20094 3d20c038 8949fe89 694a0001 <0f0a0000> 2f8a0000 39200000 41be0078
> ---[ end trace d2ede23251d61cd0 ]---
> DEBUG xxx seq_read: m->size = 8388608
> DEBUG xxx seq_read: path = /proc/device-tree/localbus@5000/flash@0/partition@1/reg
> UBIFS: recovery needed
> ..
> 
> 
> Basically we discussed this a bit with MM guys,
> at the end there was a proposal from Christoph Lameter <cl@linux.com> of using kmalloc_large instead of kmalloc.
> ####################
> There is no point in using the slab allocation functions for
> large page order allocation. Use kmalloc_large().
> 
> This fixes the warning about large allocs but it will still cause
> large contiguous allocs that could fail because of memory fragmentation.
> 
> Signed-off-by: Christoph Lameter <cl@linux.com>
> 
> Index: linux/fs/seq_file.c
> ===================================================================
> --- linux.orig/fs/seq_file.c	2013-07-31 10:39:03.050472030 -0500
> +++ linux/fs/seq_file.c	2013-07-31 10:39:03.050472030 -0500
> @@ -136,7 +136,7 @@ static int traverse(struct seq_file *m,
>  Eoverflow:
>  	m->op->stop(m, p);
>  	kfree(m->buf);
> -	m->buf = kmalloc(m->size <<= 1, GFP_KERNEL);
> +	m->buf = kmalloc_large(m->size <<= 1, GFP_KERNEL);
>  	return !m->buf ? -ENOMEM : -EAGAIN;
>  }
> 
> @@ -232,7 +232,7 @@ ssize_t seq_read(struct file *file, char
>  			goto Fill;
>  		m->op->stop(m, p);
>  		kfree(m->buf);
> -		m->buf = kmalloc(m->size <<= 1, GFP_KERNEL);
> +		m->buf = kmalloc_large(m->size <<= 1, GFP_KERNEL);
>  		if (!m->buf)
>  			goto Enomem;
>  		m->count = 0;
> ####################
> 
> 
> What do you think about this, or do we have other alternatives?
> Because with current Kernel releases is this kmalloc_large function not accessible in a easy "include and use" way.
> 
> you can follow the MM thread here:
> http://linuxppc.10917.n7.nabble.com/mm-slab-ppc-ubi-kmalloc-slab-WARNING-PPC-UBI-driver-td74404.html
> 
> 
> Thanks & BR,
> Wladislav Wiebe
>