* memory-controller patch fails to boot in qemu [mmotm]
@ 2009-08-01 14:07 Jiri Slaby
2009-08-01 14:08 ` Jiri Slaby
2009-08-01 20:02 ` Balbir Singh
0 siblings, 2 replies; 9+ messages in thread
From: Jiri Slaby @ 2009-08-01 14:07 UTC (permalink / raw)
To: Andrew Morton
Cc: Linux kernel mailing list, Balbir Singh, KAMEZAWA Hiroyuki,
Li Zefan, KOSAKI Motohiro
Hi,
in mmotm-2009-07-30-05-01, the patch named
memory-controller-soft-limit-organize-cgroups-v9.patch
causes qemu fail to boot with tons of:
BUG: scheduling while atomic: async/2/480/0x10000002
Modules linked in:
Pid: 480, comm: async/2 Tainted: G AW 2.6.31-rc4-mm1-bh #13
Call Trace:
[<ffffffff81036b6c>] __schedule_bug+0x5c/0x70
[<ffffffff8140491b>] thread_return+0x5c1/0x786
[<ffffffff8103dd30>] __cond_resched+0x20/0x50
[<ffffffff81404b9d>] _cond_resched+0x2d/0x40
[<ffffffff81096694>] truncate_inode_pages_range+0x224/0x450
[<ffffffff8106dfa1>] ? smp_call_function_many+0x1e1/0x210
[<ffffffff810e50d0>] ? invalidate_bh_lru+0x0/0x90
[<ffffffff810e514b>] ? invalidate_bh_lru+0x7b/0x90
[<ffffffff810e50d0>] ? invalidate_bh_lru+0x0/0x90
[<ffffffff810968d0>] truncate_inode_pages+0x10/0x20
[<ffffffff810ea875>] kill_bdev+0x35/0x40
[<ffffffff810eba18>] __blkdev_put+0xa8/0x190
[<ffffffff810ebb0b>] blkdev_put+0xb/0x10
[<ffffffff81116f62>] register_disk+0x172/0x180
[<ffffffff8115bca5>] add_disk+0x85/0x150
[<ffffffff812398cf>] sd_probe_async+0x12f/0x200
[<ffffffff810616ca>] async_thread+0x10a/0x270
[<ffffffff8103f7a0>] ? default_wake_function+0x0/0x10
[<ffffffff810615c0>] ? async_thread+0x0/0x270
[<ffffffff8105ac66>] kthread+0x96/0xa0
[<ffffffff8100ceaa>] child_rip+0xa/0x20
[<ffffffff8105abd0>] ? kthread+0x0/0xa0
[<ffffffff8100cea0>] ? child_rip+0x0/0x20
Looks like an omitted unlock. I don't see anything suspicious in the
patch though.
The W taint flag is from:
WARNING: at /home/latest/bh/drivers/ata/libata-core.c:4980
ata_qc_complete+0x205/0x220()
Hardware name:
Modules linked in:
Pid: 226, comm: ata/0 Tainted: G A 2.6.31-rc4-mm1-bh #13
Call Trace:
[<ffffffff810441d8>] warn_slowpath_common+0x78/0xb0
[<ffffffff8104421f>] warn_slowpath_null+0xf/0x20
[<ffffffff8123e975>] ata_qc_complete+0x205/0x220
[<ffffffff8124ac8b>] ata_do_link_abort+0xeb/0x110
[<ffffffff8124acbb>] ata_port_abort+0xb/0x10
[<ffffffff8124acea>] ata_port_freeze+0x2a/0x50
[<ffffffff8124ea34>] ata_hsm_qc_complete+0x114/0x120
[<ffffffff8124fbe2>] ata_sff_hsm_move+0x192/0x760
[<ffffffff81250567>] ata_pio_task+0x37/0x130
[<ffffffff81250530>] ? ata_pio_task+0x0/0x130
[<ffffffff81056651>] worker_thread+0x151/0x250
[<ffffffff8105afe0>] ? autoremove_wake_function+0x0/0x40
[<ffffffff81056500>] ? worker_thread+0x0/0x250
[<ffffffff8105ac66>] kthread+0x96/0xa0
[<ffffffff8100ceaa>] child_rip+0xa/0x20
[<ffffffff8105abd0>] ? kthread+0x0/0xa0
[<ffffffff8100cea0>] ? child_rip+0x0/0x20
---[ end trace 4afa694ee13ac545 ]---
and is unrelated (it was always there).
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: memory-controller patch fails to boot in qemu [mmotm]
2009-08-01 14:07 memory-controller patch fails to boot in qemu [mmotm] Jiri Slaby
@ 2009-08-01 14:08 ` Jiri Slaby
2009-08-01 20:02 ` Balbir Singh
1 sibling, 0 replies; 9+ messages in thread
From: Jiri Slaby @ 2009-08-01 14:08 UTC (permalink / raw)
To: Andrew Morton
Cc: Linux kernel mailing list, Balbir Singh, KAMEZAWA Hiroyuki,
Li Zefan, KOSAKI Motohiro
On 08/01/2009 04:07 PM, Jiri Slaby wrote:
> Looks like an omitted unlock. I don't see anything suspicious in the
> patch though.
CONFIG_TREE_RCU=y
btw
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: memory-controller patch fails to boot in qemu [mmotm]
2009-08-01 14:07 memory-controller patch fails to boot in qemu [mmotm] Jiri Slaby
2009-08-01 14:08 ` Jiri Slaby
@ 2009-08-01 20:02 ` Balbir Singh
2009-08-01 22:09 ` Hugh Dickins
1 sibling, 1 reply; 9+ messages in thread
From: Balbir Singh @ 2009-08-01 20:02 UTC (permalink / raw)
To: Jiri Slaby
Cc: Andrew Morton, Linux kernel mailing list, KAMEZAWA Hiroyuki,
Li Zefan, KOSAKI Motohiro
* Jiri Slaby <jirislaby@gmail.com> [2009-08-01 16:07:38]:
> Hi,
>
> in mmotm-2009-07-30-05-01, the patch named
> memory-controller-soft-limit-organize-cgroups-v9.patch
> causes qemu fail to boot with tons of:
> BUG: scheduling while atomic: async/2/480/0x10000002
> Modules linked in:
> Pid: 480, comm: async/2 Tainted: G AW 2.6.31-rc4-mm1-bh #13
> Call Trace:
> [<ffffffff81036b6c>] __schedule_bug+0x5c/0x70
> [<ffffffff8140491b>] thread_return+0x5c1/0x786
> [<ffffffff8103dd30>] __cond_resched+0x20/0x50
> [<ffffffff81404b9d>] _cond_resched+0x2d/0x40
> [<ffffffff81096694>] truncate_inode_pages_range+0x224/0x450
> [<ffffffff8106dfa1>] ? smp_call_function_many+0x1e1/0x210
> [<ffffffff810e50d0>] ? invalidate_bh_lru+0x0/0x90
> [<ffffffff810e514b>] ? invalidate_bh_lru+0x7b/0x90
> [<ffffffff810e50d0>] ? invalidate_bh_lru+0x0/0x90
> [<ffffffff810968d0>] truncate_inode_pages+0x10/0x20
> [<ffffffff810ea875>] kill_bdev+0x35/0x40
> [<ffffffff810eba18>] __blkdev_put+0xa8/0x190
> [<ffffffff810ebb0b>] blkdev_put+0xb/0x10
> [<ffffffff81116f62>] register_disk+0x172/0x180
> [<ffffffff8115bca5>] add_disk+0x85/0x150
> [<ffffffff812398cf>] sd_probe_async+0x12f/0x200
> [<ffffffff810616ca>] async_thread+0x10a/0x270
> [<ffffffff8103f7a0>] ? default_wake_function+0x0/0x10
> [<ffffffff810615c0>] ? async_thread+0x0/0x270
> [<ffffffff8105ac66>] kthread+0x96/0xa0
> [<ffffffff8100ceaa>] child_rip+0xa/0x20
> [<ffffffff8105abd0>] ? kthread+0x0/0xa0
> [<ffffffff8100cea0>] ? child_rip+0x0/0x20
>
> Looks like an omitted unlock. I don't see anything suspicious in the
> patch though.
Thanks for the report, did you bisect the mmotm series to identify the
root cause? What does your .config look like? I tried kvm with the
patches (mmotm 30th July) and qemu-kvm (30th-july) with a Fedora 11
guest image and the system booted just fine for me.
Could you share your command line as well?
--
Balbir
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: memory-controller patch fails to boot in qemu [mmotm]
2009-08-01 20:02 ` Balbir Singh
@ 2009-08-01 22:09 ` Hugh Dickins
2009-08-01 22:26 ` Balbir Singh
2009-08-01 22:44 ` Balbir Singh
0 siblings, 2 replies; 9+ messages in thread
From: Hugh Dickins @ 2009-08-01 22:09 UTC (permalink / raw)
To: Balbir Singh
Cc: Jiri Slaby, Andrew Morton, Linux kernel mailing list,
KAMEZAWA Hiroyuki, Li Zefan, KOSAKI Motohiro
On Sun, 2 Aug 2009, Balbir Singh wrote:
> * Jiri Slaby <jirislaby@gmail.com> [2009-08-01 16:07:38]:
> >
> > in mmotm-2009-07-30-05-01, the patch named
> > memory-controller-soft-limit-organize-cgroups-v9.patch
> > causes qemu fail to boot with tons of:
> > BUG: scheduling while atomic: async/2/480/0x10000002
> > Modules linked in:
> > Pid: 480, comm: async/2 Tainted: G AW 2.6.31-rc4-mm1-bh #13
> > Call Trace:
> > [<ffffffff81036b6c>] __schedule_bug+0x5c/0x70
> > [<ffffffff8140491b>] thread_return+0x5c1/0x786
> > [<ffffffff8103dd30>] __cond_resched+0x20/0x50
> > [<ffffffff81404b9d>] _cond_resched+0x2d/0x40
> > [<ffffffff81096694>] truncate_inode_pages_range+0x224/0x450
> > [<ffffffff8106dfa1>] ? smp_call_function_many+0x1e1/0x210
> > [<ffffffff810e50d0>] ? invalidate_bh_lru+0x0/0x90
> > [<ffffffff810e514b>] ? invalidate_bh_lru+0x7b/0x90
> > [<ffffffff810e50d0>] ? invalidate_bh_lru+0x0/0x90
> > [<ffffffff810968d0>] truncate_inode_pages+0x10/0x20
> > [<ffffffff810ea875>] kill_bdev+0x35/0x40
> > [<ffffffff810eba18>] __blkdev_put+0xa8/0x190
> > [<ffffffff810ebb0b>] blkdev_put+0xb/0x10
> > [<ffffffff81116f62>] register_disk+0x172/0x180
> > [<ffffffff8115bca5>] add_disk+0x85/0x150
> > [<ffffffff812398cf>] sd_probe_async+0x12f/0x200
> > [<ffffffff810616ca>] async_thread+0x10a/0x270
> > [<ffffffff8103f7a0>] ? default_wake_function+0x0/0x10
> > [<ffffffff810615c0>] ? async_thread+0x0/0x270
> > [<ffffffff8105ac66>] kthread+0x96/0xa0
> > [<ffffffff8100ceaa>] child_rip+0xa/0x20
> > [<ffffffff8105abd0>] ? kthread+0x0/0xa0
> > [<ffffffff8100cea0>] ? child_rip+0x0/0x20
> >
> > Looks like an omitted unlock. I don't see anything suspicious in the
> > patch though.
>
>
> Thanks for the report, did you bisect the mmotm series to identify the
> root cause? What does your .config look like? I tried kvm with the
> patches (mmotm 30th July) and qemu-kvm (30th-july) with a Fedora 11
> guest image and the system booted just fine for me.
>
> Could you share your command line as well?
I've just finished chasing something similar (without qemu),
and was about to post this:
[PATCH mmotm] memory controller: soft limit organize cgroups v9 fix
CONFIG_CGROUP_MEM_RES_CTLR=y CONFIG_PREEMPT=y mmotm fails to boot:
Kernel panic - not syncing: No init found; after lots of scheduling
while atomics, starting from when async_thread does sd_probe_async.
mem_cgroup_soft_limit_check() was doing an unbalanced get_cpu():
don't get_cpu if we won't need it, and put_cpu if we did get_cpu.
Hmm, this a weird function, passed an argument just to tell it to do
nothing. Perhaps a placeholder for something more sensible to come?
Signed-off-by: Hugh Dickins <hugh.dickins@tiscali.co.uk>
---
Fix to memory-controller-soft-limit-organize-cgroups-v9.patch
mm/memcontrol.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
--- mmotm/mm/memcontrol.c 2009-08-01 05:48:08.000000000 +0100
+++ linux/mm/memcontrol.c 2009-08-01 21:45:37.000000000 +0100
@@ -375,19 +375,21 @@ static bool mem_cgroup_soft_limit_check(
bool over_soft_limit)
{
bool ret = false;
- int cpu = get_cpu();
+ int cpu;
s64 val;
struct mem_cgroup_stat_cpu *cpustat;
if (!over_soft_limit)
return ret;
+ cpu = get_cpu();
cpustat = &mem->stat.cpustat[cpu];
val = __mem_cgroup_stat_read_local(cpustat, MEM_CGROUP_STAT_EVENTS);
if (unlikely(val > SOFTLIMIT_EVENTS_THRESH)) {
__mem_cgroup_stat_reset_safe(cpustat, MEM_CGROUP_STAT_EVENTS);
ret = true;
}
+ put_cpu();
return ret;
}
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: memory-controller patch fails to boot in qemu [mmotm]
2009-08-01 22:09 ` Hugh Dickins
@ 2009-08-01 22:26 ` Balbir Singh
2009-08-02 16:12 ` Hugh Dickins
2009-08-01 22:44 ` Balbir Singh
1 sibling, 1 reply; 9+ messages in thread
From: Balbir Singh @ 2009-08-01 22:26 UTC (permalink / raw)
To: Hugh Dickins
Cc: Jiri Slaby, Andrew Morton, Linux kernel mailing list,
KAMEZAWA Hiroyuki, Li Zefan, KOSAKI Motohiro
* Hugh Dickins <hugh.dickins@tiscali.co.uk> [2009-08-01 23:09:09]:
> On Sun, 2 Aug 2009, Balbir Singh wrote:
> > * Jiri Slaby <jirislaby@gmail.com> [2009-08-01 16:07:38]:
> > >
> > > in mmotm-2009-07-30-05-01, the patch named
> > > memory-controller-soft-limit-organize-cgroups-v9.patch
> > > causes qemu fail to boot with tons of:
> > > BUG: scheduling while atomic: async/2/480/0x10000002
> > > Modules linked in:
> > > Pid: 480, comm: async/2 Tainted: G AW 2.6.31-rc4-mm1-bh #13
> > > Call Trace:
> > > [<ffffffff81036b6c>] __schedule_bug+0x5c/0x70
> > > [<ffffffff8140491b>] thread_return+0x5c1/0x786
> > > [<ffffffff8103dd30>] __cond_resched+0x20/0x50
> > > [<ffffffff81404b9d>] _cond_resched+0x2d/0x40
> > > [<ffffffff81096694>] truncate_inode_pages_range+0x224/0x450
> > > [<ffffffff8106dfa1>] ? smp_call_function_many+0x1e1/0x210
> > > [<ffffffff810e50d0>] ? invalidate_bh_lru+0x0/0x90
> > > [<ffffffff810e514b>] ? invalidate_bh_lru+0x7b/0x90
> > > [<ffffffff810e50d0>] ? invalidate_bh_lru+0x0/0x90
> > > [<ffffffff810968d0>] truncate_inode_pages+0x10/0x20
> > > [<ffffffff810ea875>] kill_bdev+0x35/0x40
> > > [<ffffffff810eba18>] __blkdev_put+0xa8/0x190
> > > [<ffffffff810ebb0b>] blkdev_put+0xb/0x10
> > > [<ffffffff81116f62>] register_disk+0x172/0x180
> > > [<ffffffff8115bca5>] add_disk+0x85/0x150
> > > [<ffffffff812398cf>] sd_probe_async+0x12f/0x200
> > > [<ffffffff810616ca>] async_thread+0x10a/0x270
> > > [<ffffffff8103f7a0>] ? default_wake_function+0x0/0x10
> > > [<ffffffff810615c0>] ? async_thread+0x0/0x270
> > > [<ffffffff8105ac66>] kthread+0x96/0xa0
> > > [<ffffffff8100ceaa>] child_rip+0xa/0x20
> > > [<ffffffff8105abd0>] ? kthread+0x0/0xa0
> > > [<ffffffff8100cea0>] ? child_rip+0x0/0x20
> > >
> > > Looks like an omitted unlock. I don't see anything suspicious in the
> > > patch though.
> >
> >
> > Thanks for the report, did you bisect the mmotm series to identify the
> > root cause? What does your .config look like? I tried kvm with the
> > patches (mmotm 30th July) and qemu-kvm (30th-july) with a Fedora 11
> > guest image and the system booted just fine for me.
> >
> > Could you share your command line as well?
>
> I've just finished chasing something similar (without qemu),
> and was about to post this:
>
> [PATCH mmotm] memory controller: soft limit organize cgroups v9 fix
>
> CONFIG_CGROUP_MEM_RES_CTLR=y CONFIG_PREEMPT=y mmotm fails to boot:
> Kernel panic - not syncing: No init found; after lots of scheduling
> while atomics, starting from when async_thread does sd_probe_async.
>
> mem_cgroup_soft_limit_check() was doing an unbalanced get_cpu():
> don't get_cpu if we won't need it, and put_cpu if we did get_cpu.
>
> Hmm, this a weird function, passed an argument just to tell it to do
> nothing. Perhaps a placeholder for something more sensible to come?
The argument is passed a result of a function, It no-ops quite
frequently for the root cgroup.
>
> Signed-off-by: Hugh Dickins <hugh.dickins@tiscali.co.uk>
> ---
> Fix to memory-controller-soft-limit-organize-cgroups-v9.patch
>
> mm/memcontrol.c | 4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
>
> --- mmotm/mm/memcontrol.c 2009-08-01 05:48:08.000000000 +0100
> +++ linux/mm/memcontrol.c 2009-08-01 21:45:37.000000000 +0100
> @@ -375,19 +375,21 @@ static bool mem_cgroup_soft_limit_check(
> bool over_soft_limit)
> {
> bool ret = false;
> - int cpu = get_cpu();
> + int cpu;
> s64 val;
> struct mem_cgroup_stat_cpu *cpustat;
>
> if (!over_soft_limit)
> return ret;
>
> + cpu = get_cpu();
> cpustat = &mem->stat.cpustat[cpu];
> val = __mem_cgroup_stat_read_local(cpustat, MEM_CGROUP_STAT_EVENTS);
> if (unlikely(val > SOFTLIMIT_EVENTS_THRESH)) {
> __mem_cgroup_stat_reset_safe(cpustat, MEM_CGROUP_STAT_EVENTS);
> ret = true;
> }
> + put_cpu();
> return ret;
> }
>
Thanks, my bad, I should have spotted the missing put_cpu(). I'll test
this with CONFIG_PREEMPT, CONFIG_PREEMPT_DEBUG and report back. The
patch obviously looks correct, but I'll test it as well.
--
Balbir
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: memory-controller patch fails to boot in qemu [mmotm]
2009-08-01 22:09 ` Hugh Dickins
2009-08-01 22:26 ` Balbir Singh
@ 2009-08-01 22:44 ` Balbir Singh
1 sibling, 0 replies; 9+ messages in thread
From: Balbir Singh @ 2009-08-01 22:44 UTC (permalink / raw)
To: Hugh Dickins
Cc: Jiri Slaby, Andrew Morton, Linux kernel mailing list,
KAMEZAWA Hiroyuki, Li Zefan, KOSAKI Motohiro
* Hugh Dickins <hugh.dickins@tiscali.co.uk> [2009-08-01 23:09:09]:
> I've just finished chasing something similar (without qemu),
> and was about to post this:
>
> [PATCH mmotm] memory controller: soft limit organize cgroups v9 fix
>
> CONFIG_CGROUP_MEM_RES_CTLR=y CONFIG_PREEMPT=y mmotm fails to boot:
> Kernel panic - not syncing: No init found; after lots of scheduling
> while atomics, starting from when async_thread does sd_probe_async.
>
> mem_cgroup_soft_limit_check() was doing an unbalanced get_cpu():
> don't get_cpu if we won't need it, and put_cpu if we did get_cpu.
>
> Hmm, this a weird function, passed an argument just to tell it to do
> nothing. Perhaps a placeholder for something more sensible to come?
>
> Signed-off-by: Hugh Dickins <hugh.dickins@tiscali.co.uk>
> ---
> Fix to memory-controller-soft-limit-organize-cgroups-v9.patch
>
> mm/memcontrol.c | 4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
>
> --- mmotm/mm/memcontrol.c 2009-08-01 05:48:08.000000000 +0100
> +++ linux/mm/memcontrol.c 2009-08-01 21:45:37.000000000 +0100
> @@ -375,19 +375,21 @@ static bool mem_cgroup_soft_limit_check(
> bool over_soft_limit)
> {
> bool ret = false;
> - int cpu = get_cpu();
> + int cpu;
> s64 val;
> struct mem_cgroup_stat_cpu *cpustat;
>
> if (!over_soft_limit)
> return ret;
>
> + cpu = get_cpu();
> cpustat = &mem->stat.cpustat[cpu];
> val = __mem_cgroup_stat_read_local(cpustat, MEM_CGROUP_STAT_EVENTS);
> if (unlikely(val > SOFTLIMIT_EVENTS_THRESH)) {
> __mem_cgroup_stat_reset_safe(cpustat, MEM_CGROUP_STAT_EVENTS);
> ret = true;
> }
> + put_cpu();
> return ret;
> }
>
I can confirm that this fixes the problem with CONFIG_PREEMPT enabled.
Tested-by: Balbir Singh <balbir@linux.vnet.ibm.com>
Acked-by: Balbir Singh <balbir@linux.vnet.ibm.com>
--
Balbir
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: memory-controller patch fails to boot in qemu [mmotm]
2009-08-01 22:26 ` Balbir Singh
@ 2009-08-02 16:12 ` Hugh Dickins
2009-08-02 16:18 ` KAMEZAWA Hiroyuki
2009-08-02 16:27 ` Balbir Singh
0 siblings, 2 replies; 9+ messages in thread
From: Hugh Dickins @ 2009-08-02 16:12 UTC (permalink / raw)
To: Balbir Singh
Cc: Jiri Slaby, Andrew Morton, Linux kernel mailing list,
KAMEZAWA Hiroyuki, Li Zefan, KOSAKI Motohiro
On Sun, 2 Aug 2009, Balbir Singh wrote:
> * Hugh Dickins <hugh.dickins@tiscali.co.uk> [2009-08-01 23:09:09]:
> >
> > Hmm, this a weird function, passed an argument just to tell it to do
> > nothing. Perhaps a placeholder for something more sensible to come?
>
> The argument is passed a result of a function, It no-ops quite
> frequently for the root cgroup.
The more often it no-ops, the sillier it is to be called in
the first place: here's an updated patch which fixes that too.
[PATCH mmotm] memory controller: soft limit organize cgroups v9 fix
CONFIG_CGROUP_MEM_RES_CTLR=y CONFIG_PREEMPT=y mmotm fails to boot:
Kernel panic - not syncing: No init found; after lots of scheduling
while atomics, starting from when async_thread does sd_probe_async.
mem_cgroup_soft_limit_check() was doing an unbalanced get_cpu():
don't get_cpu if we won't need it, and put_cpu if we did get_cpu.
And fix the silliness of passing it an "over_soft_limit" argument
that just tells it to return false when false.
Signed-off-by: Hugh Dickins <hugh.dickins@tiscali.co.uk>
---
Fix to memory-controller-soft-limit-organize-cgroups-v9.patch
mm/memcontrol.c | 14 ++++++--------
1 file changed, 6 insertions(+), 8 deletions(-)
--- mmotm/mm/memcontrol.c 2009-08-01 05:48:08.000000000 +0100
+++ linux/mm/memcontrol.c 2009-08-02 16:56:02.000000000 +0100
@@ -371,23 +371,21 @@ mem_cgroup_remove_exceeded(struct mem_cg
spin_unlock(&mctz->lock);
}
-static bool mem_cgroup_soft_limit_check(struct mem_cgroup *mem,
- bool over_soft_limit)
+static bool mem_cgroup_soft_limit_check(struct mem_cgroup *mem)
{
bool ret = false;
- int cpu = get_cpu();
+ int cpu;
s64 val;
struct mem_cgroup_stat_cpu *cpustat;
- if (!over_soft_limit)
- return ret;
-
+ cpu = get_cpu();
cpustat = &mem->stat.cpustat[cpu];
val = __mem_cgroup_stat_read_local(cpustat, MEM_CGROUP_STAT_EVENTS);
if (unlikely(val > SOFTLIMIT_EVENTS_THRESH)) {
__mem_cgroup_stat_reset_safe(cpustat, MEM_CGROUP_STAT_EVENTS);
ret = true;
}
+ put_cpu();
return ret;
}
@@ -1342,7 +1340,7 @@ static int __mem_cgroup_try_charge(struc
if (soft_fail_res) {
mem_over_soft_limit =
mem_cgroup_from_res_counter(soft_fail_res, res);
- if (mem_cgroup_soft_limit_check(mem_over_soft_limit, true))
+ if (mem_cgroup_soft_limit_check(mem_over_soft_limit))
mem_cgroup_update_tree(mem_over_soft_limit, page);
}
return 0;
@@ -1873,7 +1871,7 @@ __mem_cgroup_uncharge_common(struct page
mz = page_cgroup_zoneinfo(pc);
unlock_page_cgroup(pc);
- if (mem_cgroup_soft_limit_check(mem, soft_limit_excess))
+ if (soft_limit_excess && mem_cgroup_soft_limit_check(mem))
mem_cgroup_update_tree(mem, page);
/* at swapout, this memcg will be accessed to record to swap */
if (ctype != MEM_CGROUP_CHARGE_TYPE_SWAPOUT)
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: memory-controller patch fails to boot in qemu [mmotm]
2009-08-02 16:12 ` Hugh Dickins
@ 2009-08-02 16:18 ` KAMEZAWA Hiroyuki
2009-08-02 16:27 ` Balbir Singh
1 sibling, 0 replies; 9+ messages in thread
From: KAMEZAWA Hiroyuki @ 2009-08-02 16:18 UTC (permalink / raw)
To: Hugh Dickins
Cc: Balbir Singh, Jiri Slaby, Andrew Morton,
Linux kernel mailing list, KAMEZAWA Hiroyuki, Li Zefan,
KOSAKI Motohiro
Hugh Dickins wrote:
> On Sun, 2 Aug 2009, Balbir Singh wrote:
>> * Hugh Dickins <hugh.dickins@tiscali.co.uk> [2009-08-01 23:09:09]:
>> >
>> > Hmm, this a weird function, passed an argument just to tell it to do
>> > nothing. Perhaps a placeholder for something more sensible to come?
>>
>> The argument is passed a result of a function, It no-ops quite
>> frequently for the root cgroup.
>
> The more often it no-ops, the sillier it is to be called in
> the first place: here's an updated patch which fixes that too.
>
>
> [PATCH mmotm] memory controller: soft limit organize cgroups v9 fix
>
> CONFIG_CGROUP_MEM_RES_CTLR=y CONFIG_PREEMPT=y mmotm fails to boot:
> Kernel panic - not syncing: No init found; after lots of scheduling
> while atomics, starting from when async_thread does sd_probe_async.
>
> mem_cgroup_soft_limit_check() was doing an unbalanced get_cpu():
> don't get_cpu if we won't need it, and put_cpu if we did get_cpu.
>
> And fix the silliness of passing it an "over_soft_limit" argument
> that just tells it to return false when false.
>
> Signed-off-by: Hugh Dickins <hugh.dickins@tiscali.co.uk>
nice fix, thank you very much.
Reviewed-by: KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com>
> ---
> Fix to memory-controller-soft-limit-organize-cgroups-v9.patch
>
> mm/memcontrol.c | 14 ++++++--------
> 1 file changed, 6 insertions(+), 8 deletions(-)
>
> --- mmotm/mm/memcontrol.c 2009-08-01 05:48:08.000000000 +0100
> +++ linux/mm/memcontrol.c 2009-08-02 16:56:02.000000000 +0100
> @@ -371,23 +371,21 @@ mem_cgroup_remove_exceeded(struct mem_cg
> spin_unlock(&mctz->lock);
> }
>
> -static bool mem_cgroup_soft_limit_check(struct mem_cgroup *mem,
> - bool over_soft_limit)
> +static bool mem_cgroup_soft_limit_check(struct mem_cgroup *mem)
> {
> bool ret = false;
> - int cpu = get_cpu();
> + int cpu;
> s64 val;
> struct mem_cgroup_stat_cpu *cpustat;
>
> - if (!over_soft_limit)
> - return ret;
> -
> + cpu = get_cpu();
> cpustat = &mem->stat.cpustat[cpu];
> val = __mem_cgroup_stat_read_local(cpustat, MEM_CGROUP_STAT_EVENTS);
> if (unlikely(val > SOFTLIMIT_EVENTS_THRESH)) {
> __mem_cgroup_stat_reset_safe(cpustat, MEM_CGROUP_STAT_EVENTS);
> ret = true;
> }
> + put_cpu();
> return ret;
> }
>
> @@ -1342,7 +1340,7 @@ static int __mem_cgroup_try_charge(struc
> if (soft_fail_res) {
> mem_over_soft_limit =
> mem_cgroup_from_res_counter(soft_fail_res, res);
> - if (mem_cgroup_soft_limit_check(mem_over_soft_limit, true))
> + if (mem_cgroup_soft_limit_check(mem_over_soft_limit))
> mem_cgroup_update_tree(mem_over_soft_limit, page);
> }
> return 0;
> @@ -1873,7 +1871,7 @@ __mem_cgroup_uncharge_common(struct page
> mz = page_cgroup_zoneinfo(pc);
> unlock_page_cgroup(pc);
>
> - if (mem_cgroup_soft_limit_check(mem, soft_limit_excess))
> + if (soft_limit_excess && mem_cgroup_soft_limit_check(mem))
> mem_cgroup_update_tree(mem, page);
> /* at swapout, this memcg will be accessed to record to swap */
> if (ctype != MEM_CGROUP_CHARGE_TYPE_SWAPOUT)
>
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: memory-controller patch fails to boot in qemu [mmotm]
2009-08-02 16:12 ` Hugh Dickins
2009-08-02 16:18 ` KAMEZAWA Hiroyuki
@ 2009-08-02 16:27 ` Balbir Singh
1 sibling, 0 replies; 9+ messages in thread
From: Balbir Singh @ 2009-08-02 16:27 UTC (permalink / raw)
To: Hugh Dickins
Cc: Jiri Slaby, Andrew Morton, Linux kernel mailing list,
KAMEZAWA Hiroyuki, Li Zefan, KOSAKI Motohiro
* Hugh Dickins <hugh.dickins@tiscali.co.uk> [2009-08-02 17:12:53]:
> On Sun, 2 Aug 2009, Balbir Singh wrote:
> > * Hugh Dickins <hugh.dickins@tiscali.co.uk> [2009-08-01 23:09:09]:
> > >
> > > Hmm, this a weird function, passed an argument just to tell it to do
> > > nothing. Perhaps a placeholder for something more sensible to come?
> >
> > The argument is passed a result of a function, It no-ops quite
> > frequently for the root cgroup.
>
> The more often it no-ops, the sillier it is to be called in
> the first place: here's an updated patch which fixes that too.
>
>
> [PATCH mmotm] memory controller: soft limit organize cgroups v9 fix
>
> CONFIG_CGROUP_MEM_RES_CTLR=y CONFIG_PREEMPT=y mmotm fails to boot:
> Kernel panic - not syncing: No init found; after lots of scheduling
> while atomics, starting from when async_thread does sd_probe_async.
>
> mem_cgroup_soft_limit_check() was doing an unbalanced get_cpu():
> don't get_cpu if we won't need it, and put_cpu if we did get_cpu.
>
> And fix the silliness of passing it an "over_soft_limit" argument
> that just tells it to return false when false.
>
> Signed-off-by: Hugh Dickins <hugh.dickins@tiscali.co.uk>
Fair enough (I guess my intention of keeping if blocks outside for
each call site, does not pay sufficiently enough), Thanks for the cleanup
Reviewed-by: Balbir Singh <balbir@linux.vnet.ibm.com>
--
Balbir
^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2009-08-02 16:27 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2009-08-01 14:07 memory-controller patch fails to boot in qemu [mmotm] Jiri Slaby
2009-08-01 14:08 ` Jiri Slaby
2009-08-01 20:02 ` Balbir Singh
2009-08-01 22:09 ` Hugh Dickins
2009-08-01 22:26 ` Balbir Singh
2009-08-02 16:12 ` Hugh Dickins
2009-08-02 16:18 ` KAMEZAWA Hiroyuki
2009-08-02 16:27 ` Balbir Singh
2009-08-01 22:44 ` Balbir Singh
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.