linux-kernel - Re: [PATCH] memcg: put soft limit reclaim out of way if the excess tree is empty

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20160801150343.GA7603@cmpxchg.org>
Date:	Mon, 1 Aug 2016 11:03:43 -0400
From:	Johannes Weiner <hannes@...xchg.org>
To:	Michal Hocko <mhocko@...nel.org>
Cc:	Vladimir Davydov <vdavydov@...tuozzo.com>,
	Andrew Morton <akpm@...ux-foundation.org>, linux-mm@...ck.org,
	LKML <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH] memcg: put soft limit reclaim out of way if the excess
 tree is empty

On Mon, Aug 01, 2016 at 04:12:28PM +0200, Michal Hocko wrote:
> From: Michal Hocko <mhocko@...e.com>
> Date: Mon, 1 Aug 2016 10:42:06 +0200
> Subject: [PATCH] memcg: put soft limit reclaim out of way if the excess tree
>  is empty
> 
> We've had a report about soft lockups caused by lock bouncing in the
> soft reclaim path:
> 
> [331404.849734] BUG: soft lockup - CPU#0 stuck for 22s! [kav4proxy-kavic:3128]
> [331404.849920] RIP: 0010:[<ffffffff81469798>]  [<ffffffff81469798>] _raw_spin_lock+0x18/0x20
> [331404.849997] Call Trace:
> [331404.850010]  [<ffffffff811557ea>] mem_cgroup_soft_limit_reclaim+0x25a/0x280
> [331404.850020]  [<ffffffff8111041d>] shrink_zones+0xed/0x200
> [331404.850027]  [<ffffffff81111a94>] do_try_to_free_pages+0x74/0x320
> [331404.850034]  [<ffffffff81112072>] try_to_free_pages+0x112/0x180
> [331404.850042]  [<ffffffff81104a6f>] __alloc_pages_slowpath+0x3ff/0x820
> [331404.850049]  [<ffffffff81105079>] __alloc_pages_nodemask+0x1e9/0x200
> [331404.850056]  [<ffffffff81141e01>] alloc_pages_vma+0xe1/0x290
> [331404.850064]  [<ffffffff8112402f>] do_wp_page+0x19f/0x840
> [331404.850071]  [<ffffffff811257cd>] handle_pte_fault+0x1cd/0x230
> [331404.850079]  [<ffffffff8146d3ed>] do_page_fault+0x1fd/0x4c0
> [331404.850087]  [<ffffffff81469ec5>] page_fault+0x25/0x30
> 
> There are no memcgs created so there cannot be any in the soft limit
> excess obviously:
> [...]
> memory  0       1       1
> 
> so all this just seems to be mem_cgroup_largest_soft_limit_node
> trying to get spin_lock_irq(&mctz->lock) just to find out that the soft
> limit excess tree is empty. This is just pointless waisting of cycles
> and cache line bouncing during heavy parallel reclaim on large machines.
> The particular machine wasn't very healthy and most probably suffering
> from a memory leak which just caused the memory reclaim to trash
> heavily. But bouncing on the lock certainly didn't help...
> 
> Introduce soft_limit_tree_empty which does the optimistic lockless check
> and bail out early if the tree is empty. This is theoretically racy but
> that shouldn't matter all that much. First of all soft limit is a best
> effort feature and it is slowly getting deprecated and its usage should
> be really scarce. Bouncing on a lock without a good reason is surely
> much bigger problem, especially on large CPU machines.
> 
> Signed-off-by: Michal Hocko <mhocko@...e.com>
> ---
>  mm/memcontrol.c | 8 ++++++++
>  1 file changed, 8 insertions(+)
> 
> diff --git a/mm/memcontrol.c b/mm/memcontrol.c
> index c265212bec8c..c0b57b6a194e 100644
> --- a/mm/memcontrol.c
> +++ b/mm/memcontrol.c
> @@ -2543,6 +2543,11 @@ static int mem_cgroup_resize_memsw_limit(struct mem_cgroup *memcg,
>  	return ret;
>  }
>  
> +static inline bool soft_limit_tree_empty(struct mem_cgroup_tree_per_node *mctz)
> +{
> +	return RB_EMPTY_ROOT(&mctz->rb_root);
> +}

Can you please fold this into the caller? It should be obvious enough.

Other than that, this patch makes sense to me.

Acked-by: Johannes Weiner <hannes@...xchg.org>