* Avoid high order memory allocating with kmalloc, when read large seq file
@ 2013-01-29 6:14 xtu4
2013-01-29 21:49 ` David Rientjes
` (2 more replies)
0 siblings, 3 replies; 9+ messages in thread
From: xtu4 @ 2013-01-29 6:14 UTC (permalink / raw)
To: linux-kernel, guifang.tang, linX.z.chen, akpm
Subject: [PATCH] [SEQ_FILE] Avoid high order memory allocating with kmalloc
when read large seq file
currently, when dumpstate access /proc/xxx/binder , this binder include
lots of info,
it will use seq_read in kernel, in this function, it will trigger high
order memory alloc,
when read binder info or other large file, this will cause memory
presure when system
don't have contious high order memory, it will lead to high kswap
workload to reclaim the
page. so change kmalloc to vmalloc, it can avoid contiously high order
memory allocating.
[ 4356.532357] dumpstate: page allocation failure: order:4, mode:0x40d0
[ 4356.532400] Pid: 18256, comm: dumpstate Tainted: G C
3.0.34-141128-g4be7088 #1
[ 4356.532416] Call Trace:
[ 4356.532443] [<c185c836>] ? printk+0x1d/0x1f
[ 4356.532467] [<c12cde1f>] warn_alloc_failed+0xbf/0xf0
[ 4356.532491] [<c12d0aba>] __alloc_pages_nodemask+0x4ba/0x6a0
[ 4356.532521] [<c12d0d1c>] __get_free_pages+0x1c/0x30
[ 4356.532541] [<c12f51e1>] kmalloc_order_trace+0x21/0xd0
[ 4356.532561] [<c131d0f7>] ? seq_read+0x137/0x390
[ 4356.532579] [<c12f549a>] __kmalloc+0x20a/0x230
[ 4356.532596] [<c131d0f7>] ? seq_read+0x137/0x390
[ 4356.532616] [<c12d343c>] ? put_page+0x2c/0x40
[ 4356.532634] [<c12f4a7d>] ? kfree+0xcd/0x160
[ 4356.532655] [<c18656cd>] ? mutex_unlock+0xd/0x10
[ 4356.532675] [<c131d109>] seq_read+0x149/0x390
[ 4356.532697] [<c130062c>] vfs_read+0x8c/0x160
[ 4356.532716] [<c131cfc0>] ? seq_lseek+0x180/0x180
[ 4356.532735] [<c130073d>] sys_read+0x3d/0x70
[ 4356.532755] [<c1866e91>] syscall_call+0x7/0xb
[ 4356.532777] [<c1860000>] ? log_dir_items+0x33d/0x40c
Signed-off-by: xiaobing tu <xiaobing.tu@intel.com>
Signed-off-by: linX z chen <linX.z.chen@intel.com>
Signed-off-by: guifang tang <guifang.tang@intel.com>
Change-Id: I892c97d02cf25e59b23c9bc68dff754ea01c1d56
---
fs/seq_file.c | 18 +++++++++++++++---
1 files changed, 15 insertions(+), 3 deletions(-)
diff --git a/fs/seq_file.c b/fs/seq_file.c
index dba43c3..20b8e36 100644
--- a/fs/seq_file.c
+++ b/fs/seq_file.c
@@ -209,8 +209,17 @@ ssize_t seq_read(struct file *file, char __user
*buf, size_t size, loff_t *ppos)
if (m->count < m->size)
goto Fill;
m->op->stop(m, p);
- kfree(m->buf);
- m->buf = kmalloc(m->size <<= 1, GFP_KERNEL);
+ if (m->size > 2 * PAGE_SIZE) {
+ vfree(m->buf);
+ } else
+ kfree(m->buf);
+ m->size <<= 1;
+ if (m->size > 2 * PAGE_SIZE) {
+ m->buf = vmalloc(m->size);
+ } else
+ m->buf = kmalloc(m->size <<= 1, GFP_KERNEL);
+
+
if (!m->buf)
goto Enomem;
m->count = 0;
@@ -325,7 +334,10 @@ EXPORT_SYMBOL(seq_lseek);
int seq_release(struct inode *inode, struct file *file)
{
struct seq_file *m = file->private_data;
- kfree(m->buf);
+ if (m->size > 2 * PAGE_SIZE) {
+ vfree(m->buf);
+ } else
+ kfree(m->buf);
kfree(m);
return 0;
}
--
1.7.6
^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: Avoid high order memory allocating with kmalloc, when read large seq file
2013-01-29 6:14 Avoid high order memory allocating with kmalloc, when read large seq file xtu4
@ 2013-01-29 21:49 ` David Rientjes
2013-01-31 6:19 ` Tu, Xiaobing
2013-01-30 0:24 ` Andrew Morton
2013-01-31 6:11 ` resend----[PATCH] " xtu4
2 siblings, 1 reply; 9+ messages in thread
From: David Rientjes @ 2013-01-29 21:49 UTC (permalink / raw)
To: xtu4; +Cc: linux-kernel, guifang.tang, linX.z.chen, Andrew Morton
On Tue, 29 Jan 2013, xtu4 wrote:
> Subject: [PATCH] [SEQ_FILE] Avoid high order memory allocating with kmalloc
> when read large seq file
>
> currently, when dumpstate access /proc/xxx/binder , this binder include lots
> of info,
> it will use seq_read in kernel, in this function, it will trigger high order
> memory alloc,
> when read binder info or other large file, this will cause memory presure when
> system
> don't have contious high order memory, it will lead to high kswap workload to
> reclaim the
> page. so change kmalloc to vmalloc, it can avoid contiously high order memory
> allocating.
> [ 4356.532357] dumpstate: page allocation failure: order:4, mode:0x40d0
> [ 4356.532400] Pid: 18256, comm: dumpstate Tainted: G C
> 3.0.34-141128-g4be7088 #1
> [ 4356.532416] Call Trace:
> [ 4356.532443] [<c185c836>] ? printk+0x1d/0x1f
> [ 4356.532467] [<c12cde1f>] warn_alloc_failed+0xbf/0xf0
> [ 4356.532491] [<c12d0aba>] __alloc_pages_nodemask+0x4ba/0x6a0
> [ 4356.532521] [<c12d0d1c>] __get_free_pages+0x1c/0x30
> [ 4356.532541] [<c12f51e1>] kmalloc_order_trace+0x21/0xd0
> [ 4356.532561] [<c131d0f7>] ? seq_read+0x137/0x390
> [ 4356.532579] [<c12f549a>] __kmalloc+0x20a/0x230
> [ 4356.532596] [<c131d0f7>] ? seq_read+0x137/0x390
> [ 4356.532616] [<c12d343c>] ? put_page+0x2c/0x40
> [ 4356.532634] [<c12f4a7d>] ? kfree+0xcd/0x160
> [ 4356.532655] [<c18656cd>] ? mutex_unlock+0xd/0x10
> [ 4356.532675] [<c131d109>] seq_read+0x149/0x390
> [ 4356.532697] [<c130062c>] vfs_read+0x8c/0x160
> [ 4356.532716] [<c131cfc0>] ? seq_lseek+0x180/0x180
> [ 4356.532735] [<c130073d>] sys_read+0x3d/0x70
> [ 4356.532755] [<c1866e91>] syscall_call+0x7/0xb
> [ 4356.532777] [<c1860000>] ? log_dir_items+0x33d/0x40c
> Signed-off-by: xiaobing tu <xiaobing.tu@intel.com>
> Signed-off-by: linX z chen <linX.z.chen@intel.com>
> Signed-off-by: guifang tang <guifang.tang@intel.com>
> Change-Id: I892c97d02cf25e59b23c9bc68dff754ea01c1d56
> ---
> fs/seq_file.c | 18 +++++++++++++++---
> 1 files changed, 15 insertions(+), 3 deletions(-)
>
> diff --git a/fs/seq_file.c b/fs/seq_file.c
> index dba43c3..20b8e36 100644
> --- a/fs/seq_file.c
> +++ b/fs/seq_file.c
> @@ -209,8 +209,17 @@ ssize_t seq_read(struct file *file, char __user *buf,
> size_t size, loff_t *ppos)
> if (m->count < m->size)
> goto Fill;
> m->op->stop(m, p);
> - kfree(m->buf);
> - m->buf = kmalloc(m->size <<= 1, GFP_KERNEL);
> + if (m->size > 2 * PAGE_SIZE) {
> + vfree(m->buf);
> + } else
> + kfree(m->buf);
> + m->size <<= 1;
> + if (m->size > 2 * PAGE_SIZE) {
> + m->buf = vmalloc(m->size);
> + } else
> + m->buf = kmalloc(m->size <<= 1, GFP_KERNEL);
I thought m->size was already shifted.
Your patch is severely mangled, please check out
Documentation/SubmittingPatches and Documentation/email-clients.txt.
> +
> +
> if (!m->buf)
> goto Enomem;
> m->count = 0;
> @@ -325,7 +334,10 @@ EXPORT_SYMBOL(seq_lseek);
> int seq_release(struct inode *inode, struct file *file)
> {
> struct seq_file *m = file->private_data;
> - kfree(m->buf);
> + if (m->size > 2 * PAGE_SIZE) {
> + vfree(m->buf);
> + } else
> + kfree(m->buf);
> kfree(m);
> return 0;
> }
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: Avoid high order memory allocating with kmalloc, when read large seq file
2013-01-29 6:14 Avoid high order memory allocating with kmalloc, when read large seq file xtu4
2013-01-29 21:49 ` David Rientjes
@ 2013-01-30 0:24 ` Andrew Morton
2013-01-31 6:19 ` Tu, Xiaobing
2013-01-31 6:11 ` resend----[PATCH] " xtu4
2 siblings, 1 reply; 9+ messages in thread
From: Andrew Morton @ 2013-01-30 0:24 UTC (permalink / raw)
To: xtu4; +Cc: linux-kernel, guifang.tang, linX.z.chen, Arve Hjønnevåg
On Tue, 29 Jan 2013 14:14:14 +0800
xtu4 <xiaobing.tu@intel.com> wrote:
> @@ -209,8 +209,17 @@ ssize_t seq_read(struct file *file, char __user
> *buf, size_t size, loff_t *ppos)
> if (m->count < m->size)
> goto Fill;
> m->op->stop(m, p);
> - kfree(m->buf);
> - m->buf = kmalloc(m->size <<= 1, GFP_KERNEL);
> + if (m->size > 2 * PAGE_SIZE) {
> + vfree(m->buf);
> + } else
> + kfree(m->buf);
> + m->size <<= 1;
> + if (m->size > 2 * PAGE_SIZE) {
> + m->buf = vmalloc(m->size);
> + } else
> + m->buf = kmalloc(m->size <<= 1, GFP_KERNEL);
> +
> +
> if (!m->buf)
> goto Enomem;
> m->count = 0;
> @@ -325,7 +334,10 @@ EXPORT_SYMBOL(seq_lseek);
The conventional way of doing this is to attempt the kmalloc with
__GFP_NOWARN and if that failed, fall back to vmalloc().
Using vmalloc is generally not a good thing, mainly because of
fragmentation issues, but for short-lived allocations like this, that
shouldn't be too bad.
But really, the binder code is being obnoxious here and it would be
best to fix it up. Please identify with some care which part of the
binder code is causing this problem. binder_stats_show(), from a
guess? It looks like that function's output size is proportional to
the number of processes on binder_procs? If so, there is no upper
bound, is there? Problem!
btw, binder_debug_no_lock should just go away. That list needs
locking.
^ permalink raw reply [flat|nested] 9+ messages in thread
* resend----[PATCH] Avoid high order memory allocating with kmalloc, when read large seq file
2013-01-29 6:14 Avoid high order memory allocating with kmalloc, when read large seq file xtu4
2013-01-29 21:49 ` David Rientjes
2013-01-30 0:24 ` Andrew Morton
@ 2013-01-31 6:11 ` xtu4
2013-01-31 21:27 ` Andrew Morton
` (2 more replies)
2 siblings, 3 replies; 9+ messages in thread
From: xtu4 @ 2013-01-31 6:11 UTC (permalink / raw)
To: linux-kernel, guifang.tang, linX.z.chen, akpm
[SEQ_FILE] Avoid high order memory allocating with kmalloc
when read large seq file
currently, when dumpstate access /proc/xxx/binder , this binder include
lots of info,
it will use seq_read in kernel, in this function, it will trigger high
order memory alloc,
when read binder info or other large file, this will cause memory
presure when system
don't have contious high order memory, it will lead to high kswap
workload to reclaim the
page. so change kmalloc to vmalloc, it can avoid contiously high order
memory allocating.
[ 4356.532357] dumpstate: page allocation failure: order:4, mode:0x40d0
[ 4356.532400] Pid: 18256, comm: dumpstate Tainted: G C
3.0.34-141128-g4be7088 #1
[ 4356.532416] Call Trace:
[ 4356.532443] [<c185c836>] ? printk+0x1d/0x1f
[ 4356.532467] [<c12cde1f>] warn_alloc_failed+0xbf/0xf0
[ 4356.532491] [<c12d0aba>] __alloc_pages_nodemask+0x4ba/0x6a0
[ 4356.532521] [<c12d0d1c>] __get_free_pages+0x1c/0x30
[ 4356.532541] [<c12f51e1>] kmalloc_order_trace+0x21/0xd0
[ 4356.532561] [<c131d0f7>] ? seq_read+0x137/0x390
[ 4356.532579] [<c12f549a>] __kmalloc+0x20a/0x230
[ 4356.532596] [<c131d0f7>] ? seq_read+0x137/0x390
[ 4356.532616] [<c12d343c>] ? put_page+0x2c/0x40
[ 4356.532634] [<c12f4a7d>] ? kfree+0xcd/0x160
[ 4356.532655] [<c18656cd>] ? mutex_unlock+0xd/0x10
[ 4356.532675] [<c131d109>] seq_read+0x149/0x390
[ 4356.532697] [<c130062c>] vfs_read+0x8c/0x160
[ 4356.532716] [<c131cfc0>] ? seq_lseek+0x180/0x180
[ 4356.532735] [<c130073d>] sys_read+0x3d/0x70
[ 4356.532755] [<c1866e91>] syscall_call+0x7/0xb
[ 4356.532777] [<c1860000>] ? log_dir_items+0x33d/0x40c
the m->size is very huge
<3>[ 1185.457656, 1] xiaobing >> seq_read: m->size 8192
<3>[ 1185.463462, 1] xiaobing >> seq_read: m->size 16384
<3>[ 1185.470472, 1] xiaobing >> seq_read: m->size 32768
<3>[ 1185.481201, 0] xiaobing >> seq_read: m->size 8192
<3>[ 1185.488071, 0] xiaobing >> seq_read: m->size 16384
<3>[ 1185.495892, 0] xiaobing >> seq_read: m->size 32768
<3>[ 1185.504841, 0] xiaobing >> seq_read: m->size 65536
<3>[ 1185.517180, 0] xiaobing >> seq_read: m->size 131072
<3>[ 1185.536286, 0] xiaobing >> seq_read: m->size 262144
some times even more then 262144 byte
Signed-off-by: xiaobing tu <xiaobing.tu@intel.com>
Change-Id: I892c97d02cf25e59b23c9bc68dff754ea01c1d56
---
fs/seq_file.c | 22 +++++++++++++++++-----
1 files changed, 17 insertions(+), 5 deletions(-)
diff --git a/fs/seq_file.c b/fs/seq_file.c
index dba43c3..19df826 100644
--- a/fs/seq_file.c
+++ b/fs/seq_file.c
@@ -12,7 +12,7 @@
#include <asm/uaccess.h>
#include <asm/page.h>
-
+#include <linux/mm.h>
/**
* seq_open - initialize sequential file
* @file: file we initialize
@@ -116,7 +116,13 @@ static int traverse(struct seq_file *m, loff_t offset)
Eoverflow:
m->op->stop(m, p);
kfree(m->buf);
- m->buf = kmalloc(m->size <<= 1, GFP_KERNEL);
+
+ is_vmalloc_addr(m->buf) ? vfree(m->buf) : kfree(m->buf);
+ m->size <<= 1;
+ if (m->size <= (2 * PAGE_SIZE))
+ m->buf = kmalloc(m->size, GFP_KERNEL);
+ else
+ m->buf = vmalloc(m->size);
return !m->buf ? -ENOMEM : -EAGAIN;
}
@@ -209,8 +215,14 @@ ssize_t seq_read(struct file *file, char __user
*buf, size_t size, loff_t *ppos)
if (m->count < m->size)
goto Fill;
m->op->stop(m, p);
- kfree(m->buf);
- m->buf = kmalloc(m->size <<= 1, GFP_KERNEL);
+ is_vmalloc_addr(m->buf) ? vfree(m->buf) : kfree(m->buf);
+ m->size <<= 1;
+ if (m->size > 2 * PAGE_SIZE)
+ m->buf = vmalloc(m->size);
+ else
+ m->buf = kmalloc(m->size, GFP_KERNEL);
+
+
if (!m->buf)
goto Enomem;
m->count = 0;
@@ -325,7 +337,7 @@ EXPORT_SYMBOL(seq_lseek);
int seq_release(struct inode *inode, struct file *file)
{
struct seq_file *m = file->private_data;
- kfree(m->buf);
+ is_vmalloc_addr(m->buf) ? vfree(m->buf) : kfree(m->buf);
kfree(m);
return 0;
}
--
1.7.6
^ permalink raw reply related [flat|nested] 9+ messages in thread
* RE: Avoid high order memory allocating with kmalloc, when read large seq file
2013-01-30 0:24 ` Andrew Morton
@ 2013-01-31 6:19 ` Tu, Xiaobing
0 siblings, 0 replies; 9+ messages in thread
From: Tu, Xiaobing @ 2013-01-31 6:19 UTC (permalink / raw)
To: Andrew Morton
Cc: linux-kernel@vger.kernel.org, Tang, Guifang, Chen, LinX Z,
Arve Hj?nnev?g
Hi Morton
Thank you very much for your kindly info, In android system,, when you read /sys/kernel/debug/binder/proc/xxx, xxx is the process id , it will trigger high order kmalloc.
But we can't limit the size of binder info, because we need this to debug the binder related issue.
I had re-send the patch, how do you think for about using vmalloc instaed of kmalloc when malloc high order allocating? Memory gragment should not be the issue, because this is very quick to free such memory.
Br
Xiaobing
-----Original Message-----
From: Andrew Morton [mailto:akpm@linux-foundation.org]
Sent: Wednesday, January 30, 2013 8:25 AM
To: Tu, Xiaobing
Cc: linux-kernel@vger.kernel.org; Tang, Guifang; Chen, LinX Z; Arve Hjønnevåg
Subject: Re: Avoid high order memory allocating with kmalloc, when read large seq file
On Tue, 29 Jan 2013 14:14:14 +0800
xtu4 <xiaobing.tu@intel.com> wrote:
> @@ -209,8 +209,17 @@ ssize_t seq_read(struct file *file, char __user
> *buf, size_t size, loff_t *ppos)
> if (m->count < m->size)
> goto Fill;
> m->op->stop(m, p);
> - kfree(m->buf);
> - m->buf = kmalloc(m->size <<= 1, GFP_KERNEL);
> + if (m->size > 2 * PAGE_SIZE) {
> + vfree(m->buf);
> + } else
> + kfree(m->buf);
> + m->size <<= 1;
> + if (m->size > 2 * PAGE_SIZE) {
> + m->buf = vmalloc(m->size);
> + } else
> + m->buf = kmalloc(m->size <<= 1, GFP_KERNEL);
> +
> +
> if (!m->buf)
> goto Enomem;
> m->count = 0;
> @@ -325,7 +334,10 @@ EXPORT_SYMBOL(seq_lseek);
The conventional way of doing this is to attempt the kmalloc with __GFP_NOWARN and if that failed, fall back to vmalloc().
Using vmalloc is generally not a good thing, mainly because of fragmentation issues, but for short-lived allocations like this, that shouldn't be too bad.
But really, the binder code is being obnoxious here and it would be best to fix it up. Please identify with some care which part of the binder code is causing this problem. binder_stats_show(), from a guess? It looks like that function's output size is proportional to the number of processes on binder_procs? If so, there is no upper bound, is there? Problem!
btw, binder_debug_no_lock should just go away. That list needs locking.
^ permalink raw reply [flat|nested] 9+ messages in thread
* RE: Avoid high order memory allocating with kmalloc, when read large seq file
2013-01-29 21:49 ` David Rientjes
@ 2013-01-31 6:19 ` Tu, Xiaobing
0 siblings, 0 replies; 9+ messages in thread
From: Tu, Xiaobing @ 2013-01-31 6:19 UTC (permalink / raw)
To: David Rientjes
Cc: linux-kernel@vger.kernel.org, Tang, Guifang, Chen, LinX Z,
Andrew Morton
Hi Rientjes
Thanks a lot for your info, yes, you are right, I just re-send the patch, could you please review?
Br
Xiaobing
-----Original Message-----
From: David Rientjes [mailto:rientjes@google.com]
Sent: Wednesday, January 30, 2013 5:49 AM
To: Tu, Xiaobing
Cc: linux-kernel@vger.kernel.org; Tang, Guifang; Chen, LinX Z; Andrew Morton
Subject: Re: Avoid high order memory allocating with kmalloc, when read large seq file
On Tue, 29 Jan 2013, xtu4 wrote:
> Subject: [PATCH] [SEQ_FILE] Avoid high order memory allocating with
> kmalloc when read large seq file
>
> currently, when dumpstate access /proc/xxx/binder , this binder
> include lots of info, it will use seq_read in kernel, in this
> function, it will trigger high order memory alloc, when read binder
> info or other large file, this will cause memory presure when system
> don't have contious high order memory, it will lead to high kswap
> workload to reclaim the page. so change kmalloc to vmalloc, it can
> avoid contiously high order memory allocating.
> [ 4356.532357] dumpstate: page allocation failure: order:4, mode:0x40d0
> [ 4356.532400] Pid: 18256, comm: dumpstate Tainted: G C
> 3.0.34-141128-g4be7088 #1
> [ 4356.532416] Call Trace:
> [ 4356.532443] [<c185c836>] ? printk+0x1d/0x1f [ 4356.532467]
> [<c12cde1f>] warn_alloc_failed+0xbf/0xf0 [ 4356.532491] [<c12d0aba>]
> __alloc_pages_nodemask+0x4ba/0x6a0
> [ 4356.532521] [<c12d0d1c>] __get_free_pages+0x1c/0x30 [ 4356.532541]
> [<c12f51e1>] kmalloc_order_trace+0x21/0xd0 [ 4356.532561]
> [<c131d0f7>] ? seq_read+0x137/0x390 [ 4356.532579] [<c12f549a>]
> __kmalloc+0x20a/0x230 [ 4356.532596] [<c131d0f7>] ?
> seq_read+0x137/0x390 [ 4356.532616] [<c12d343c>] ? put_page+0x2c/0x40
> [ 4356.532634] [<c12f4a7d>] ? kfree+0xcd/0x160 [ 4356.532655]
> [<c18656cd>] ? mutex_unlock+0xd/0x10 [ 4356.532675] [<c131d109>]
> seq_read+0x149/0x390 [ 4356.532697] [<c130062c>] vfs_read+0x8c/0x160
> [ 4356.532716] [<c131cfc0>] ? seq_lseek+0x180/0x180 [ 4356.532735]
> [<c130073d>] sys_read+0x3d/0x70 [ 4356.532755] [<c1866e91>]
> syscall_call+0x7/0xb [ 4356.532777] [<c1860000>] ?
> log_dir_items+0x33d/0x40c
> Signed-off-by: xiaobing tu <xiaobing.tu@intel.com>
> Signed-off-by: linX z chen <linX.z.chen@intel.com>
> Signed-off-by: guifang tang <guifang.tang@intel.com>
> Change-Id: I892c97d02cf25e59b23c9bc68dff754ea01c1d56
> ---
> fs/seq_file.c | 18 +++++++++++++++---
> 1 files changed, 15 insertions(+), 3 deletions(-)
>
> diff --git a/fs/seq_file.c b/fs/seq_file.c index dba43c3..20b8e36
> 100644
> --- a/fs/seq_file.c
> +++ b/fs/seq_file.c
> @@ -209,8 +209,17 @@ ssize_t seq_read(struct file *file, char __user
> *buf, size_t size, loff_t *ppos)
> if (m->count < m->size)
> goto Fill;
> m->op->stop(m, p);
> - kfree(m->buf);
> - m->buf = kmalloc(m->size <<= 1, GFP_KERNEL);
> + if (m->size > 2 * PAGE_SIZE) {
> + vfree(m->buf);
> + } else
> + kfree(m->buf);
> + m->size <<= 1;
> + if (m->size > 2 * PAGE_SIZE) {
> + m->buf = vmalloc(m->size);
> + } else
> + m->buf = kmalloc(m->size <<= 1, GFP_KERNEL);
I thought m->size was already shifted.
Your patch is severely mangled, please check out Documentation/SubmittingPatches and Documentation/email-clients.txt.
> +
> +
> if (!m->buf)
> goto Enomem;
> m->count = 0;
> @@ -325,7 +334,10 @@ EXPORT_SYMBOL(seq_lseek); int seq_release(struct
> inode *inode, struct file *file) {
> struct seq_file *m = file->private_data;
> - kfree(m->buf);
> + if (m->size > 2 * PAGE_SIZE) {
> + vfree(m->buf);
> + } else
> + kfree(m->buf);
> kfree(m);
> return 0;
> }
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: resend----[PATCH] Avoid high order memory allocating with kmalloc, when read large seq file
2013-01-31 6:11 ` resend----[PATCH] " xtu4
@ 2013-01-31 21:27 ` Andrew Morton
2013-02-01 3:30 ` xtu4
2013-02-01 3:31 ` xtu4
2 siblings, 0 replies; 9+ messages in thread
From: Andrew Morton @ 2013-01-31 21:27 UTC (permalink / raw)
To: xtu4; +Cc: linux-kernel, guifang.tang, linX.z.chen
On Thu, 31 Jan 2013 14:11:32 +0800
xtu4 <xiaobing.tu@intel.com> wrote:
> [SEQ_FILE] Avoid high order memory allocating with kmalloc
> when read large seq file
The patch is still horridly mangled by your email client.
The implementation cheerfully ignores my earlier comment: "The
conventional way of doing this is to attempt the kmalloc with
__GFP_NOWARN and if that failed, fall back to vmalloc()."
This code has existed for ten years and this is the first time anyone
has encountered this problem. Rather than mucking around with the
seq_file code to attempt to fix this, it would be better to fix the
calling code.
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: resend----[PATCH] Avoid high order memory allocating with kmalloc, when read large seq file
2013-01-31 6:11 ` resend----[PATCH] " xtu4
2013-01-31 21:27 ` Andrew Morton
@ 2013-02-01 3:30 ` xtu4
2013-02-01 3:31 ` xtu4
2 siblings, 0 replies; 9+ messages in thread
From: xtu4 @ 2013-02-01 3:30 UTC (permalink / raw)
To: linux-kernel, guifang.tang, linX.z.chen, akpm, yanmin.zhang
On 01/31/2013 02:11 PM, xtu4 wrote:
> [SEQ_FILE] Avoid high order memory allocating with kmalloc
> when read large seq file
>
> currently, when dumpstate access /proc/xxx/binder , this binder
> include lots of info,
> it will use seq_read in kernel, in this function, it will trigger high
> order memory alloc,
> when read binder info or other large file, this will cause memory
> presure when system
> don't have contious high order memory, it will lead to high kswap
> workload to reclaim the
> page. so change kmalloc to vmalloc, it can avoid contiously high order
> memory allocating.
> [ 4356.532357] dumpstate: page allocation failure: order:4, mode:0x40d0
> [ 4356.532400] Pid: 18256, comm: dumpstate Tainted: G C
> 3.0.34-141128-g4be7088 #1
> [ 4356.532416] Call Trace:
> [ 4356.532443] [<c185c836>] ? printk+0x1d/0x1f
> [ 4356.532467] [<c12cde1f>] warn_alloc_failed+0xbf/0xf0
> [ 4356.532491] [<c12d0aba>] __alloc_pages_nodemask+0x4ba/0x6a0
> [ 4356.532521] [<c12d0d1c>] __get_free_pages+0x1c/0x30
> [ 4356.532541] [<c12f51e1>] kmalloc_order_trace+0x21/0xd0
> [ 4356.532561] [<c131d0f7>] ? seq_read+0x137/0x390
> [ 4356.532579] [<c12f549a>] __kmalloc+0x20a/0x230
> [ 4356.532596] [<c131d0f7>] ? seq_read+0x137/0x390
> [ 4356.532616] [<c12d343c>] ? put_page+0x2c/0x40
> [ 4356.532634] [<c12f4a7d>] ? kfree+0xcd/0x160
> [ 4356.532655] [<c18656cd>] ? mutex_unlock+0xd/0x10
> [ 4356.532675] [<c131d109>] seq_read+0x149/0x390
> [ 4356.532697] [<c130062c>] vfs_read+0x8c/0x160
> [ 4356.532716] [<c131cfc0>] ? seq_lseek+0x180/0x180
> [ 4356.532735] [<c130073d>] sys_read+0x3d/0x70
> [ 4356.532755] [<c1866e91>] syscall_call+0x7/0xb
> [ 4356.532777] [<c1860000>] ? log_dir_items+0x33d/0x40c
>
> the m->size is very huge
> <3>[ 1185.457656, 1] xiaobing >> seq_read: m->size 8192
> <3>[ 1185.463462, 1] xiaobing >> seq_read: m->size 16384
> <3>[ 1185.470472, 1] xiaobing >> seq_read: m->size 32768
> <3>[ 1185.481201, 0] xiaobing >> seq_read: m->size 8192
> <3>[ 1185.488071, 0] xiaobing >> seq_read: m->size 16384
> <3>[ 1185.495892, 0] xiaobing >> seq_read: m->size 32768
> <3>[ 1185.504841, 0] xiaobing >> seq_read: m->size 65536
> <3>[ 1185.517180, 0] xiaobing >> seq_read: m->size 131072
> <3>[ 1185.536286, 0] xiaobing >> seq_read: m->size 262144
>
> some times even more then 262144 byte
>
> Signed-off-by: xiaobing tu <xiaobing.tu@intel.com>
> Change-Id: I892c97d02cf25e59b23c9bc68dff754ea01c1d56
> ---
> fs/seq_file.c | 22 +++++++++++++++++-----
> 1 files changed, 17 insertions(+), 5 deletions(-)
>
> diff --git a/fs/seq_file.c b/fs/seq_file.c
> index dba43c3..19df826 100644
> --- a/fs/seq_file.c
> +++ b/fs/seq_file.c
> @@ -12,7 +12,7 @@
>
> #include <asm/uaccess.h>
> #include <asm/page.h>
> -
> +#include <linux/mm.h>
> /**
> * seq_open - initialize sequential file
> * @file: file we initialize
> @@ -116,7 +116,13 @@ static int traverse(struct seq_file *m, loff_t
> offset)
> Eoverflow:
> m->op->stop(m, p);
> kfree(m->buf);
> - m->buf = kmalloc(m->size <<= 1, GFP_KERNEL);
> +
> + is_vmalloc_addr(m->buf) ? vfree(m->buf) : kfree(m->buf);
> + m->size <<= 1;
> + if (m->size <= (2 * PAGE_SIZE))
> + m->buf = kmalloc(m->size, GFP_KERNEL);
> + else
> + m->buf = vmalloc(m->size);
> return !m->buf ? -ENOMEM : -EAGAIN;
> }
>
> @@ -209,8 +215,14 @@ ssize_t seq_read(struct file *file, char __user
> *buf, size_t size, loff_t *ppos)
> if (m->count < m->size)
> goto Fill;
> m->op->stop(m, p);
> - kfree(m->buf);
> - m->buf = kmalloc(m->size <<= 1, GFP_KERNEL);
> + is_vmalloc_addr(m->buf) ? vfree(m->buf) : kfree(m->buf);
> + m->size <<= 1;
> + if (m->size > 2 * PAGE_SIZE)
> + m->buf = vmalloc(m->size);
> + else
> + m->buf = kmalloc(m->size, GFP_KERNEL);
> +
> +
> if (!m->buf)
> goto Enomem;
> m->count = 0;
> @@ -325,7 +337,7 @@ EXPORT_SYMBOL(seq_lseek);
> int seq_release(struct inode *inode, struct file *file)
> {
> struct seq_file *m = file->private_data;
> - kfree(m->buf);
> + is_vmalloc_addr(m->buf) ? vfree(m->buf) : kfree(m->buf);
> kfree(m);
> return 0;
> }
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: resend----[PATCH] Avoid high order memory allocating with kmalloc, when read large seq file
2013-01-31 6:11 ` resend----[PATCH] " xtu4
2013-01-31 21:27 ` Andrew Morton
2013-02-01 3:30 ` xtu4
@ 2013-02-01 3:31 ` xtu4
2 siblings, 0 replies; 9+ messages in thread
From: xtu4 @ 2013-02-01 3:31 UTC (permalink / raw)
To: linux-kernel, guifang.tang, linX.z.chen, akpm, yanmin.zhang
On 01/31/2013 02:11 PM, xtu4 wrote:
> [SEQ_FILE] Avoid high order memory allocating with kmalloc
> when read large seq file
>
> currently, when dumpstate access /proc/xxx/binder , this binder
> include lots of info,
> it will use seq_read in kernel, in this function, it will trigger high
> order memory alloc,
> when read binder info or other large file, this will cause memory
> presure when system
> don't have contious high order memory, it will lead to high kswap
> workload to reclaim the
> page. so change kmalloc to vmalloc, it can avoid contiously high order
> memory allocating.
> [ 4356.532357] dumpstate: page allocation failure: order:4, mode:0x40d0
> [ 4356.532400] Pid: 18256, comm: dumpstate Tainted: G C
> 3.0.34-141128-g4be7088 #1
> [ 4356.532416] Call Trace:
> [ 4356.532443] [<c185c836>] ? printk+0x1d/0x1f
> [ 4356.532467] [<c12cde1f>] warn_alloc_failed+0xbf/0xf0
> [ 4356.532491] [<c12d0aba>] __alloc_pages_nodemask+0x4ba/0x6a0
> [ 4356.532521] [<c12d0d1c>] __get_free_pages+0x1c/0x30
> [ 4356.532541] [<c12f51e1>] kmalloc_order_trace+0x21/0xd0
> [ 4356.532561] [<c131d0f7>] ? seq_read+0x137/0x390
> [ 4356.532579] [<c12f549a>] __kmalloc+0x20a/0x230
> [ 4356.532596] [<c131d0f7>] ? seq_read+0x137/0x390
> [ 4356.532616] [<c12d343c>] ? put_page+0x2c/0x40
> [ 4356.532634] [<c12f4a7d>] ? kfree+0xcd/0x160
> [ 4356.532655] [<c18656cd>] ? mutex_unlock+0xd/0x10
> [ 4356.532675] [<c131d109>] seq_read+0x149/0x390
> [ 4356.532697] [<c130062c>] vfs_read+0x8c/0x160
> [ 4356.532716] [<c131cfc0>] ? seq_lseek+0x180/0x180
> [ 4356.532735] [<c130073d>] sys_read+0x3d/0x70
> [ 4356.532755] [<c1866e91>] syscall_call+0x7/0xb
> [ 4356.532777] [<c1860000>] ? log_dir_items+0x33d/0x40c
>
> the m->size is very huge
> <3>[ 1185.457656, 1] xiaobing >> seq_read: m->size 8192
> <3>[ 1185.463462, 1] xiaobing >> seq_read: m->size 16384
> <3>[ 1185.470472, 1] xiaobing >> seq_read: m->size 32768
> <3>[ 1185.481201, 0] xiaobing >> seq_read: m->size 8192
> <3>[ 1185.488071, 0] xiaobing >> seq_read: m->size 16384
> <3>[ 1185.495892, 0] xiaobing >> seq_read: m->size 32768
> <3>[ 1185.504841, 0] xiaobing >> seq_read: m->size 65536
> <3>[ 1185.517180, 0] xiaobing >> seq_read: m->size 131072
> <3>[ 1185.536286, 0] xiaobing >> seq_read: m->size 262144
>
> some times even more then 262144 byte
>
> Signed-off-by: xiaobing tu <xiaobing.tu@intel.com>
> Change-Id: I892c97d02cf25e59b23c9bc68dff754ea01c1d56
> ---
> fs/seq_file.c | 22 +++++++++++++++++-----
> 1 files changed, 17 insertions(+), 5 deletions(-)
>
> diff --git a/fs/seq_file.c b/fs/seq_file.c
> index dba43c3..19df826 100644
> --- a/fs/seq_file.c
> +++ b/fs/seq_file.c
> @@ -12,7 +12,7 @@
>
> #include <asm/uaccess.h>
> #include <asm/page.h>
> -
> +#include <linux/mm.h>
> /**
> * seq_open - initialize sequential file
> * @file: file we initialize
> @@ -116,7 +116,13 @@ static int traverse(struct seq_file *m, loff_t
> offset)
> Eoverflow:
> m->op->stop(m, p);
> kfree(m->buf);
> - m->buf = kmalloc(m->size <<= 1, GFP_KERNEL);
> +
> + is_vmalloc_addr(m->buf) ? vfree(m->buf) : kfree(m->buf);
> + m->size <<= 1;
> + if (m->size <= (2 * PAGE_SIZE))
> + m->buf = kmalloc(m->size, GFP_KERNEL);
> + else
> + m->buf = vmalloc(m->size);
> return !m->buf ? -ENOMEM : -EAGAIN;
> }
>
> @@ -209,8 +215,14 @@ ssize_t seq_read(struct file *file, char __user
> *buf, size_t size, loff_t *ppos)
> if (m->count < m->size)
> goto Fill;
> m->op->stop(m, p);
> - kfree(m->buf);
> - m->buf = kmalloc(m->size <<= 1, GFP_KERNEL);
> + is_vmalloc_addr(m->buf) ? vfree(m->buf) : kfree(m->buf);
> + m->size <<= 1;
> + if (m->size > 2 * PAGE_SIZE)
> + m->buf = vmalloc(m->size);
> + else
> + m->buf = kmalloc(m->size, GFP_KERNEL);
> +
> +
> if (!m->buf)
> goto Enomem;
> m->count = 0;
> @@ -325,7 +337,7 @@ EXPORT_SYMBOL(seq_lseek);
> int seq_release(struct inode *inode, struct file *file)
> {
> struct seq_file *m = file->private_data;
> - kfree(m->buf);
> + is_vmalloc_addr(m->buf) ? vfree(m->buf) : kfree(m->buf);
> kfree(m);
> return 0;
> }
^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2013-02-01 3:31 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-01-29 6:14 Avoid high order memory allocating with kmalloc, when read large seq file xtu4
2013-01-29 21:49 ` David Rientjes
2013-01-31 6:19 ` Tu, Xiaobing
2013-01-30 0:24 ` Andrew Morton
2013-01-31 6:19 ` Tu, Xiaobing
2013-01-31 6:11 ` resend----[PATCH] " xtu4
2013-01-31 21:27 ` Andrew Morton
2013-02-01 3:30 ` xtu4
2013-02-01 3:31 ` xtu4
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).