* Re: seq_file: procfs: kmalloc_slab WARNING
2013-08-06 13:26 seq_file: procfs: kmalloc_slab WARNING Wladislav Wiebe
@ 2013-08-12 11:07 ` Wladislav Wiebe
2013-08-29 21:54 ` Andrew Morton
1 sibling, 0 replies; 4+ messages in thread
From: Wladislav Wiebe @ 2013-08-12 11:07 UTC (permalink / raw)
To: viro, linux-fsdevel
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
>
^ permalink raw reply related [flat|nested] 4+ messages in thread* Re: seq_file: procfs: kmalloc_slab WARNING
2013-08-06 13:26 seq_file: procfs: kmalloc_slab WARNING Wladislav Wiebe
2013-08-12 11:07 ` Wladislav Wiebe
@ 2013-08-29 21:54 ` Andrew Morton
2013-08-30 7:10 ` Wladislav Wiebe
1 sibling, 1 reply; 4+ messages in thread
From: Andrew Morton @ 2013-08-29 21:54 UTC (permalink / raw)
To: Wladislav Wiebe; +Cc: viro, linux-fsdevel
On Tue, 06 Aug 2013 15:26:23 +0200 Wladislav Wiebe <wladislav.kw@gmail.com> 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?
kmalloc_large() only exists in slub, so this will break slab and slob
builds.
Did you try adding __GFP_NOWARN? That's appropriate in this case.
^ permalink raw reply [flat|nested] 4+ messages in thread