From mboxrd@z Thu Jan 1 00:00:00 1970 From: Kamezawa Hiroyuki Subject: Re: [PATCH V2 0/3] memcg: simply lock of page stat accounting Date: Tue, 14 May 2013 09:41:42 +0900 Message-ID: <51918846.7090006@jp.fujitsu.com> References: <1368421410-4795-1-git-send-email-handai.szj@taobao.com> Mime-Version: 1.0 Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <1368421410-4795-1-git-send-email-handai.szj-3b8fjiQLQpfQT0dZR+AlfA@public.gmane.org> Sender: cgroups-owner-u79uwXL29TY76Z2rM5mHXA@public.gmane.org List-ID: Content-Type: text/plain; charset="us-ascii" To: Sha Zhengju Cc: cgroups-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, linux-mm-Bw31MaZKKs3YtjvyW6yDsg@public.gmane.org, mhocko-AlSwsSmVLrQ@public.gmane.org, akpm-de/tnXTf+JLsfHDXvbKv3WD2FQJk+8+b@public.gmane.org, hughd-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org, gthelen-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org, Sha Zhengju If you want to rewrite all things and make memcg cleaner, I don't stop it. But, how about starting with this simeple one for your 1st purpose ? doesn't work ? dirty ? == this patch is untested. == >From 95e405451f56933c4777e64bb02326ec0462f7a7 Mon Sep 17 00:00:00 2001 From: KAMEZAWA Hiroyuki Date: Tue, 14 May 2013 09:40:55 +0900 Subject: [PATCH] Allow nesting lock of memcg's page stat accouting. Sha Zhengju and Michal Hocko pointed out that mem_cgroup_begin/end_update_page_stat() should be nested lock. https://lkml.org/lkml/2013/1/2/48 page_remove_rmap mem_cgroup_begin_update_page_stat <<< 1 set_page_dirty __set_page_dirty_buffers __set_page_dirty mem_cgroup_begin_update_page_stat <<< 2 move_lock_mem_cgroup spin_lock_irqsave(&memcg->move_lock, *flags); This patch add a nesting functionality with per-thread counter. Signed-off-by: KAMEZAWA Hiroyuki --- include/linux/sched.h | 1 + mm/memcontrol.c | 22 +++++++++++++++++++++- 2 files changed, 22 insertions(+), 1 deletions(-) diff --git a/include/linux/sched.h b/include/linux/sched.h index 84ceef5..cca3229 100644 --- a/include/linux/sched.h +++ b/include/linux/sched.h @@ -1402,6 +1402,7 @@ struct task_struct { unsigned long memsw_nr_pages; /* uncharged mem+swap usage */ } memcg_batch; unsigned int memcg_kmem_skip_account; + unsigned int memcg_page_stat_accounting; #endif #ifdef CONFIG_HAVE_HW_BREAKPOINT atomic_t ptrace_bp_refcnt; diff --git a/mm/memcontrol.c b/mm/memcontrol.c index 357371a..152f8df 100644 --- a/mm/memcontrol.c +++ b/mm/memcontrol.c @@ -2352,12 +2352,30 @@ again: */ if (!mem_cgroup_stolen(memcg)) return; + /* + * In some case, we need nested lock of this. + * page_remove_rmap + * mem_cgroup_begin_update_page_stat <<< 1 + * set_page_dirty + * __set_page_dirty_buffers + * __set_page_dirty + * mem_cgroup_begin_update_page_stat <<< 2 + * move_lock_mem_cgroup + * spin_lock_irqsave(&memcg->move_lock, *flags); + * + * We avoid this deadlock by having per thread counter. + */ + if (current->memcg_page_stat_accounting > 0) { + current->memcg_page_stat_accounting++; + return; + } move_lock_mem_cgroup(memcg, flags); if (memcg != pc->mem_cgroup || !PageCgroupUsed(pc)) { move_unlock_mem_cgroup(memcg, flags); goto again; } + current->memcg_page_stat_accounting = 1; *locked = true; } @@ -2370,7 +2388,9 @@ void __mem_cgroup_end_update_page_stat(struct page *page, unsigned long *flags) * lock is held because a routine modifies pc->mem_cgroup * should take move_lock_mem_cgroup(). */ - move_unlock_mem_cgroup(pc->mem_cgroup, flags); + current->memcg_page_stat_accounting--; + if (!current->memcg_page_stat_accounting) + move_unlock_mem_cgroup(pc->mem_cgroup, flags); } void mem_cgroup_update_page_stat(struct page *page, -- 1.7.4.1 From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from psmtp.com (na3sys010amx160.postini.com [74.125.245.160]) by kanga.kvack.org (Postfix) with SMTP id C199B6B0033 for ; Mon, 13 May 2013 20:42:10 -0400 (EDT) Received: from m2.gw.fujitsu.co.jp (unknown [10.0.50.72]) by fgwmail5.fujitsu.co.jp (Postfix) with ESMTP id D48173EE0C0 for ; Tue, 14 May 2013 09:42:08 +0900 (JST) Received: from smail (m2 [127.0.0.1]) by outgoing.m2.gw.fujitsu.co.jp (Postfix) with ESMTP id C413145DE55 for ; Tue, 14 May 2013 09:42:08 +0900 (JST) Received: from s2.gw.fujitsu.co.jp (s2.gw.fujitsu.co.jp [10.0.50.92]) by m2.gw.fujitsu.co.jp (Postfix) with ESMTP id A1B4C45DE53 for ; Tue, 14 May 2013 09:42:08 +0900 (JST) Received: from s2.gw.fujitsu.co.jp (localhost.localdomain [127.0.0.1]) by s2.gw.fujitsu.co.jp (Postfix) with ESMTP id 925CC1DB8038 for ; Tue, 14 May 2013 09:42:08 +0900 (JST) Received: from ml14.s.css.fujitsu.com (ml14.s.css.fujitsu.com [10.240.81.134]) by s2.gw.fujitsu.co.jp (Postfix) with ESMTP id E5B7B1DB803C for ; Tue, 14 May 2013 09:42:07 +0900 (JST) Message-ID: <51918846.7090006@jp.fujitsu.com> Date: Tue, 14 May 2013 09:41:42 +0900 From: Kamezawa Hiroyuki MIME-Version: 1.0 Subject: Re: [PATCH V2 0/3] memcg: simply lock of page stat accounting References: <1368421410-4795-1-git-send-email-handai.szj@taobao.com> In-Reply-To: <1368421410-4795-1-git-send-email-handai.szj@taobao.com> Content-Type: text/plain; charset=ISO-2022-JP Content-Transfer-Encoding: 7bit Sender: owner-linux-mm@kvack.org List-ID: To: Sha Zhengju Cc: cgroups@vger.kernel.org, linux-mm@kvack.org, mhocko@suse.cz, akpm@linux-foundation.org, hughd@google.com, gthelen@google.com, Sha Zhengju If you want to rewrite all things and make memcg cleaner, I don't stop it. But, how about starting with this simeple one for your 1st purpose ? doesn't work ? dirty ? == this patch is untested. ==