[patch] mm: memcontrol: default hierarchy interface for memory fix - high reclaim

[patch] mm: memcontrol: default hierarchy interface for memory fix - high reclaim

[Johannes Weiner]

High limit reclaim can currently overscan in proportion to how many
charges are happening concurrently.  Tone it down such that charges
don't target the entire high-boundary excess, but instead only the
pages they charged themselves when excess is detected.

Reported-by: Michal Hocko <mhocko@suse.cz>
Signed-off-by: Johannes Weiner <hannes@cmpxchg.org>
---
 mm/memcontrol.c | 16 +++++-----------
 1 file changed, 5 insertions(+), 11 deletions(-)

diff --git a/mm/memcontrol.c b/mm/memcontrol.c
index 323a01fa1833..7adccee9fecb 100644
--- a/mm/memcontrol.c
+++ b/mm/memcontrol.c
@@ -2348,19 +2348,13 @@ done_restock:
 		refill_stock(memcg, batch - nr_pages);
 	/*
 	 * If the hierarchy is above the normal consumption range,
-	 * make the charging task trim the excess.
+	 * make the charging task trim their excess contribution.
 	 */
 	do {
-		unsigned long nr_pages = page_counter_read(&memcg->memory);
-		unsigned long high = ACCESS_ONCE(memcg->high);
-
-		if (nr_pages > high) {
-			mem_cgroup_events(memcg, MEMCG_HIGH, 1);
-
-			try_to_free_mem_cgroup_pages(memcg, nr_pages - high,
-						     gfp_mask, true);
-		}
-
+		if (page_counter_read(&memcg->memory) <= memcg->high)
+			continue;
+		mem_cgroup_events(memcg, MEMCG_HIGH, 1);
+		try_to_free_mem_cgroup_pages(memcg, nr_pages, gfp_mask, true);
 	} while ((memcg = parent_mem_cgroup(memcg)));
 done:
 	return ret;
-- 
2.2.0

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [patch] mm: memcontrol: default hierarchy interface for memory fix - high reclaim

[Michal Hocko]

On Sat 17-01-15 10:21:19, Johannes Weiner wrote:
> High limit reclaim can currently overscan in proportion to how many
> charges are happening concurrently.  Tone it down such that charges
> don't target the entire high-boundary excess, but instead only the
> pages they charged themselves when excess is detected.
> 
> Reported-by: Michal Hocko <mhocko@suse.cz>
> Signed-off-by: Johannes Weiner <hannes@cmpxchg.org>

I certainly agree with this approach.
Acked-by: Michal Hocko <mhocko@suse.cz>

Is this planned to be folded into the original patch or go on its own. I
am OK with both ways, maybe having it separate would be better from
documentation POV.

> ---
>  mm/memcontrol.c | 16 +++++-----------
>  1 file changed, 5 insertions(+), 11 deletions(-)
> 
> diff --git a/mm/memcontrol.c b/mm/memcontrol.c
> index 323a01fa1833..7adccee9fecb 100644
> --- a/mm/memcontrol.c
> +++ b/mm/memcontrol.c
> @@ -2348,19 +2348,13 @@ done_restock:
>  		refill_stock(memcg, batch - nr_pages);
>  	/*
>  	 * If the hierarchy is above the normal consumption range,
> -	 * make the charging task trim the excess.
> +	 * make the charging task trim their excess contribution.
>  	 */
>  	do {
> -		unsigned long nr_pages = page_counter_read(&memcg->memory);
> -		unsigned long high = ACCESS_ONCE(memcg->high);
> -
> -		if (nr_pages > high) {
> -			mem_cgroup_events(memcg, MEMCG_HIGH, 1);
> -
> -			try_to_free_mem_cgroup_pages(memcg, nr_pages - high,
> -						     gfp_mask, true);
> -		}
> -
> +		if (page_counter_read(&memcg->memory) <= memcg->high)
> +			continue;
> +		mem_cgroup_events(memcg, MEMCG_HIGH, 1);
> +		try_to_free_mem_cgroup_pages(memcg, nr_pages, gfp_mask, true);
>  	} while ((memcg = parent_mem_cgroup(memcg)));
>  done:
>  	return ret;
> -- 
> 2.2.0
> 

-- 
Michal Hocko
SUSE Labs

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [patch] mm: memcontrol: default hierarchy interface for memory fix - high reclaim

[Johannes Weiner]

On Tue, Jan 20, 2015 at 02:25:19PM +0100, Michal Hocko wrote:
> On Sat 17-01-15 10:21:19, Johannes Weiner wrote:
> > High limit reclaim can currently overscan in proportion to how many
> > charges are happening concurrently.  Tone it down such that charges
> > don't target the entire high-boundary excess, but instead only the
> > pages they charged themselves when excess is detected.
> > 
> > Reported-by: Michal Hocko <mhocko@suse.cz>
> > Signed-off-by: Johannes Weiner <hannes@cmpxchg.org>
> 
> I certainly agree with this approach.
> Acked-by: Michal Hocko <mhocko@suse.cz>

Thanks!

> Is this planned to be folded into the original patch or go on its own. I
> am OK with both ways, maybe having it separate would be better from
> documentation POV.

I submitted them to be folded in.  Which aspect would you like to see
documented?

Re: [patch] mm: memcontrol: default hierarchy interface for memory fix - high reclaim

[Michal Hocko]

On Tue 20-01-15 09:16:28, Johannes Weiner wrote:
> On Tue, Jan 20, 2015 at 02:25:19PM +0100, Michal Hocko wrote:
[...]
> > Is this planned to be folded into the original patch or go on its own. I
> > am OK with both ways, maybe having it separate would be better from
> > documentation POV.
> 
> I submitted them to be folded in.  Which aspect would you like to see
> documented?

That the excess target reclaim has been attempted and changed with a
patch which explains why. So this was kind of "git log as a
documentation" thing.

-- 
Michal Hocko
SUSE Labs

Re: [patch] mm: memcontrol: default hierarchy interface for memory fix - high reclaim

[Johannes Weiner]

On Tue, Jan 20, 2015 at 03:31:19PM +0100, Michal Hocko wrote:
> On Tue 20-01-15 09:16:28, Johannes Weiner wrote:
> > On Tue, Jan 20, 2015 at 02:25:19PM +0100, Michal Hocko wrote:
> [...]
> > > Is this planned to be folded into the original patch or go on its own. I
> > > am OK with both ways, maybe having it separate would be better from
> > > documentation POV.
> > 
> > I submitted them to be folded in.  Which aspect would you like to see
> > documented?
> 
> That the excess target reclaim has been attempted and changed with a
> patch which explains why. So this was kind of "git log as a
> documentation" thing.

I agreed to soften it because you had reasonable concerns and it was
still strong enough for my tests.  But we hardly "attempted" this
version.  Should this turn out to be too weak for other users in
practice we have to reconsider the stronger approach and actually put
your theory to the test and see if it holds up in practice.  There is
no knowledge to record at this point, we just have speculation and no
real need to push the envelope right now.