[PATCH 1/2] memcg: fix page_referencies cgroup filter on global reclaim

[PATCH 1/2] memcg: fix page_referencies cgroup filter on global reclaim

[Konstantin Khlebnikov]

Global memory reclaimer should't skip referencies for any pages,
even if they are shared between different cgroups.

This patch adds scan_control->current_mem_cgroup, which points to currently
shrinking sub-cgroup in hierarchy, at global reclaim it always NULL.

Signed-off-by: Konstantin Khlebnikov <khlebnikov@openvz.org>
---
 mm/vmscan.c |   18 ++++++++++++++----
 1 files changed, 14 insertions(+), 4 deletions(-)

diff --git a/mm/vmscan.c b/mm/vmscan.c
index 531abcc..b069fac 100644
--- a/mm/vmscan.c
+++ b/mm/vmscan.c
@@ -109,6 +109,12 @@ struct scan_control {
 	struct mem_cgroup *target_mem_cgroup;
 
 	/*
+	 * Currently reclaiming memory cgroup in hierarchy,
+	 * NULL for global reclaim.
+	 */
+	struct mem_cgroup *current_mem_cgroup;
+
+	/*
 	 * Nodemask of nodes allowed by the caller. If NULL, all nodes
 	 * are scanned.
 	 */
@@ -703,13 +709,13 @@ enum page_references {
 };
 
 static enum page_references page_check_references(struct page *page,
-						  struct mem_cgroup_zone *mz,
 						  struct scan_control *sc)
 {
 	int referenced_ptes, referenced_page;
 	unsigned long vm_flags;
 
-	referenced_ptes = page_referenced(page, 1, mz->mem_cgroup, &vm_flags);
+	referenced_ptes = page_referenced(page, 1,
+			sc->current_mem_cgroup, &vm_flags);
 	referenced_page = TestClearPageReferenced(page);
 
 	/* Lumpy reclaim - ignore references */
@@ -830,7 +836,7 @@ static unsigned long shrink_page_list(struct list_head *page_list,
 			}
 		}
 
-		references = page_check_references(page, mz, sc);
+		references = page_check_references(page, sc);
 		switch (references) {
 		case PAGEREF_ACTIVATE:
 			goto activate_locked;
@@ -1744,7 +1750,7 @@ static void shrink_active_list(unsigned long nr_to_scan,
 			continue;
 		}
 
-		if (page_referenced(page, 0, mz->mem_cgroup, &vm_flags)) {
+		if (page_referenced(page, 0, sc->current_mem_cgroup, &vm_flags)) {
 			nr_rotated += hpage_nr_pages(page);
 			/*
 			 * Identify referenced, file-backed active pages and
@@ -2163,6 +2169,9 @@ static void shrink_zone(int priority, struct zone *zone,
 			.zone = zone,
 		};
 
+		if (!global_reclaim(sc))
+			sc->current_mem_cgroup = memcg;
+
 		shrink_mem_cgroup_zone(priority, &mz, sc);
 		/*
 		 * Limit reclaim has historically picked one memcg and
@@ -2478,6 +2487,7 @@ unsigned long mem_cgroup_shrink_node_zone(struct mem_cgroup *memcg,
 		.may_swap = !noswap,
 		.order = 0,
 		.target_mem_cgroup = memcg,
+		.current_mem_cgroup = memcg,
 	};
 	struct mem_cgroup_zone mz = {
 		.mem_cgroup = memcg,

--
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/ .
Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

[PATCH 2/2] memcg: use vm_swappiness from current memcg

[Konstantin Khlebnikov]

At this point this is always the same cgroup, but it allows to drop one argument.

Signed-off-by: Konstantin Khlebnikov <khlebnikov@openvz.org>
---
 mm/vmscan.c |    9 ++++-----
 1 files changed, 4 insertions(+), 5 deletions(-)

diff --git a/mm/vmscan.c b/mm/vmscan.c
index b069fac..751fab3 100644
--- a/mm/vmscan.c
+++ b/mm/vmscan.c
@@ -1899,12 +1899,11 @@ static unsigned long shrink_list(enum lru_list lru, unsigned long nr_to_scan,
 	return shrink_inactive_list(nr_to_scan, mz, sc, priority, file);
 }
 
-static int vmscan_swappiness(struct mem_cgroup_zone *mz,
-			     struct scan_control *sc)
+static int vmscan_swappiness(struct scan_control *sc)
 {
 	if (global_reclaim(sc))
 		return vm_swappiness;
-	return mem_cgroup_swappiness(mz->mem_cgroup);
+	return mem_cgroup_swappiness(sc->current_mem_cgroup);
 }
 
 /*
@@ -1972,8 +1971,8 @@ static void get_scan_count(struct mem_cgroup_zone *mz, struct scan_control *sc,
 	 * With swappiness at 100, anonymous and file have the same priority.
 	 * This scanning priority is essentially the inverse of IO cost.
 	 */
-	anon_prio = vmscan_swappiness(mz, sc);
-	file_prio = 200 - vmscan_swappiness(mz, sc);
+	anon_prio = vmscan_swappiness(sc);
+	file_prio = 200 - vmscan_swappiness(sc);
 
 	/*
 	 * OK, so we have swap space and a fair amount of page cache

--
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/ .
Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH 2/2] memcg: use vm_swappiness from current memcg

[KAMEZAWA Hiroyuki]

On Wed, 15 Feb 2012 20:28:34 +0400
Konstantin Khlebnikov <khlebnikov@openvz.org> wrote:

> At this point this is always the same cgroup, but it allows to drop one argument.
> 
> Signed-off-by: Konstantin Khlebnikov <khlebnikov@openvz.org>

Do you mean "no logic change but clean up, dropping an argument" ?

I'm not sure using complicated sc-> is easier than passing an argument..

Acked-by: KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com>

--
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/ .
Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH 2/2] memcg: use vm_swappiness from current memcg

[Konstantin Khlebnikov]

KAMEZAWA Hiroyuki wrote:
> On Wed, 15 Feb 2012 20:28:34 +0400
> Konstantin Khlebnikov<khlebnikov@openvz.org>  wrote:
>
>> At this point this is always the same cgroup, but it allows to drop one argument.
>>
>> Signed-off-by: Konstantin Khlebnikov<khlebnikov@openvz.org>
>
> Do you mean "no logic change but clean up, dropping an argument" ?
>
> I'm not sure using complicated sc->  is easier than passing an argument..

It also was cleanup-preparation for "memory book keeping",
there I replace struct mem_cgroup_zone *mz with struct book *book,
struct book doesn't know anything about memory-cgroups.

>
> Acked-by: KAMEZAWA Hiroyuki<kamezawa.hiroyu@jp.fujitsu.com>
>
> --
> 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/ .
> Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/
> Don't email:<a href=mailto:"dont@kvack.org">  email@kvack.org</a>

--
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/ .
Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH 1/2] memcg: fix page_referencies cgroup filter on global reclaim

[KAMEZAWA Hiroyuki]

On Wed, 15 Feb 2012 20:28:30 +0400
Konstantin Khlebnikov <khlebnikov@openvz.org> wrote:

> Global memory reclaimer should't skip referencies for any pages,
> even if they are shared between different cgroups.
> 
> This patch adds scan_control->current_mem_cgroup, which points to currently
> shrinking sub-cgroup in hierarchy, at global reclaim it always NULL.
> 
> Signed-off-by: Konstantin Khlebnikov <khlebnikov@openvz.org>

seems ok to me. How about you, Johannes ?

Acked-by: KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com>


> ---
>  mm/vmscan.c |   18 ++++++++++++++----
>  1 files changed, 14 insertions(+), 4 deletions(-)
> 
> diff --git a/mm/vmscan.c b/mm/vmscan.c
> index 531abcc..b069fac 100644
> --- a/mm/vmscan.c
> +++ b/mm/vmscan.c
> @@ -109,6 +109,12 @@ struct scan_control {
>  	struct mem_cgroup *target_mem_cgroup;
>  
>  	/*
> +	 * Currently reclaiming memory cgroup in hierarchy,
> +	 * NULL for global reclaim.
> +	 */
> +	struct mem_cgroup *current_mem_cgroup;
> +
> +	/*
>  	 * Nodemask of nodes allowed by the caller. If NULL, all nodes
>  	 * are scanned.
>  	 */
> @@ -703,13 +709,13 @@ enum page_references {
>  };
>  
>  static enum page_references page_check_references(struct page *page,
> -						  struct mem_cgroup_zone *mz,
>  						  struct scan_control *sc)
>  {
>  	int referenced_ptes, referenced_page;
>  	unsigned long vm_flags;
>  
> -	referenced_ptes = page_referenced(page, 1, mz->mem_cgroup, &vm_flags);
> +	referenced_ptes = page_referenced(page, 1,
> +			sc->current_mem_cgroup, &vm_flags);
>  	referenced_page = TestClearPageReferenced(page);
>  
>  	/* Lumpy reclaim - ignore references */
> @@ -830,7 +836,7 @@ static unsigned long shrink_page_list(struct list_head *page_list,
>  			}
>  		}
>  
> -		references = page_check_references(page, mz, sc);
> +		references = page_check_references(page, sc);
>  		switch (references) {
>  		case PAGEREF_ACTIVATE:
>  			goto activate_locked;
> @@ -1744,7 +1750,7 @@ static void shrink_active_list(unsigned long nr_to_scan,
>  			continue;
>  		}
>  
> -		if (page_referenced(page, 0, mz->mem_cgroup, &vm_flags)) {
> +		if (page_referenced(page, 0, sc->current_mem_cgroup, &vm_flags)) {
>  			nr_rotated += hpage_nr_pages(page);
>  			/*
>  			 * Identify referenced, file-backed active pages and
> @@ -2163,6 +2169,9 @@ static void shrink_zone(int priority, struct zone *zone,
>  			.zone = zone,
>  		};
>  
> +		if (!global_reclaim(sc))
> +			sc->current_mem_cgroup = memcg;
> +
>  		shrink_mem_cgroup_zone(priority, &mz, sc);
>  		/*
>  		 * Limit reclaim has historically picked one memcg and
> @@ -2478,6 +2487,7 @@ unsigned long mem_cgroup_shrink_node_zone(struct mem_cgroup *memcg,
>  		.may_swap = !noswap,
>  		.order = 0,
>  		.target_mem_cgroup = memcg,
> +		.current_mem_cgroup = memcg,
>  	};
>  	struct mem_cgroup_zone mz = {
>  		.mem_cgroup = memcg,
> 
> --
> 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/ .
> Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/
> Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

--
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/ .
Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH 1/2] memcg: fix page_referencies cgroup filter on global reclaim

[Johannes Weiner]

On Wed, Feb 15, 2012 at 08:28:30PM +0400, Konstantin Khlebnikov wrote:
> Global memory reclaimer should't skip referencies for any pages,
> even if they are shared between different cgroups.

Agreed: if we reclaim from one memcg because of its limit, we want to
reclaim those pages that this group is not using.  If it's used by
someone else, it should be evicted and refaulted by the group that
needs it.

If we reclaim globally, all references are "true" because we want to
evict those pages that are not used by any cgroup.

But if we reclaim a hierarchical subgroup, we don't want to evict
pages that are shared among this hierarchy, either, even if the memcg
that has the page charged to it is not using it.  Bouncing the page
around the hierarchy is not sensible, because it does not solve the
problem of the parent hitting its limit when the sibling group will
refault it in a blink of an eye.  It should only be evicted if the
memcg that's not using it nears its own limit, because only in that
case would reclaiming the page remedy the situation.

> This patch adds scan_control->current_mem_cgroup, which points to currently
> shrinking sub-cgroup in hierarchy, at global reclaim it always NULL.

So to be consistent, I'm wondering if we should pass
sc->target_mem_cgroup - the limit-hitting hierarchy root - to
page_referenced() and then have mm_match_cgroup() do a
mem_cgroup_same_or_subtree() check to see if the vma is in the
hierarchy rooted at sc->target_mem_cgroup.

Global reclaim is handled automatically, because mm_match_cgroup() is
not checked when the passed memcg is NULL, which sc->target_mem_cgroup
is for global reclaim.

--
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/ .
Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH 1/2] memcg: fix page_referencies cgroup filter on global reclaim

[Konstantin Khlebnikov]

Johannes Weiner wrote:
> On Wed, Feb 15, 2012 at 08:28:30PM +0400, Konstantin Khlebnikov wrote:
>> Global memory reclaimer should't skip referencies for any pages,
>> even if they are shared between different cgroups.
>
> Agreed: if we reclaim from one memcg because of its limit, we want to
> reclaim those pages that this group is not using.  If it's used by
> someone else, it should be evicted and refaulted by the group that
> needs it.
>
> If we reclaim globally, all references are "true" because we want to
> evict those pages that are not used by any cgroup.
>
> But if we reclaim a hierarchical subgroup, we don't want to evict
> pages that are shared among this hierarchy, either, even if the memcg
> that has the page charged to it is not using it.  Bouncing the page
> around the hierarchy is not sensible, because it does not solve the
> problem of the parent hitting its limit when the sibling group will
> refault it in a blink of an eye.  It should only be evicted if the
> memcg that's not using it nears its own limit, because only in that
> case would reclaiming the page remedy the situation.
>
>> This patch adds scan_control->current_mem_cgroup, which points to currently
>> shrinking sub-cgroup in hierarchy, at global reclaim it always NULL.
>
> So to be consistent, I'm wondering if we should pass
> sc->target_mem_cgroup - the limit-hitting hierarchy root - to
> page_referenced() and then have mm_match_cgroup() do a
> mem_cgroup_same_or_subtree() check to see if the vma is in the
> hierarchy rooted at sc->target_mem_cgroup.
>
> Global reclaim is handled automatically, because mm_match_cgroup() is
> not checked when the passed memcg is NULL, which sc->target_mem_cgroup
> is for global reclaim.

Also we can try to recharge page to other cgroup, if we found in rmap another its user
outsize of currently shrinking hierarchy, page there is isolated, so at the end we will
insert page directly to its lru.

But the main purpose of this patch for me is killing mz->mem_cgroup dereference,
because I plan to replace mz with direct reference to lruvec, which will be memcg-free object.

--
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/ .
Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH 1/2] memcg: fix page_referencies cgroup filter on global reclaim

[Johannes Weiner]

On Tue, Feb 21, 2012 at 03:38:04PM +0400, Konstantin Khlebnikov wrote:
> Johannes Weiner wrote:
> >On Wed, Feb 15, 2012 at 08:28:30PM +0400, Konstantin Khlebnikov wrote:
> >>Global memory reclaimer should't skip referencies for any pages,
> >>even if they are shared between different cgroups.
> >
> >Agreed: if we reclaim from one memcg because of its limit, we want to
> >reclaim those pages that this group is not using.  If it's used by
> >someone else, it should be evicted and refaulted by the group that
> >needs it.
> >
> >If we reclaim globally, all references are "true" because we want to
> >evict those pages that are not used by any cgroup.
> >
> >But if we reclaim a hierarchical subgroup, we don't want to evict
> >pages that are shared among this hierarchy, either, even if the memcg
> >that has the page charged to it is not using it.  Bouncing the page
> >around the hierarchy is not sensible, because it does not solve the
> >problem of the parent hitting its limit when the sibling group will
> >refault it in a blink of an eye.  It should only be evicted if the
> >memcg that's not using it nears its own limit, because only in that
> >case would reclaiming the page remedy the situation.
> >
> >>This patch adds scan_control->current_mem_cgroup, which points to currently
> >>shrinking sub-cgroup in hierarchy, at global reclaim it always NULL.
> >
> >So to be consistent, I'm wondering if we should pass
> >sc->target_mem_cgroup - the limit-hitting hierarchy root - to
> >page_referenced() and then have mm_match_cgroup() do a
> >mem_cgroup_same_or_subtree() check to see if the vma is in the
> >hierarchy rooted at sc->target_mem_cgroup.
> >
> >Global reclaim is handled automatically, because mm_match_cgroup() is
> >not checked when the passed memcg is NULL, which sc->target_mem_cgroup
> >is for global reclaim.
> 
> Also we can try to recharge page to other cgroup, if we found in rmap another its user
> outsize of currently shrinking hierarchy, page there is isolated, so at the end we will
> insert page directly to its lru.

Not sure if it's worth the trouble, but that could work.

> But the main purpose of this patch for me is killing mz->mem_cgroup dereference,
> because I plan to replace mz with direct reference to lruvec, which will be memcg-free object.

I like that plan.  How about this patch then?

---
From: Johannes Weiner <hannes@cmpxchg.org>
Subject: mm: memcg: count pte references from every member of the reclaimed hierarchy

The rmap walker checking page table references has historically
ignored references from VMAs that were not part of the memcg that was
being reclaimed during memcg hard limit reclaim.

When transitioning global reclaim to memcg hierarchy reclaim, I missed
that bit and now references from outside a memcg are ignored even
during global reclaim.

Reverting back to traditional behaviour - count all references during
global reclaim and only mind references of the memcg being reclaimed
during limit reclaim would be one option.

However, the more generic idea is to ignore references exactly then
when they are outside the hierarchy that is currently under reclaim;
because only then will their reclamation be of any use to help the
pressure situation.  It makes no sense to ignore references from a
sibling memcg and then evict a page that will be immediately refaulted
by that sibling which contributes to the same usage of the common
ancestor under reclaim.

The solution: make the rmap walker ignore references from VMAs that
are not part of the hierarchy that is being reclaimed.

Flat limit reclaim will stay the same, hierarchical limit reclaim will
mind the references only to pages that the hierarchy owns.  Global
reclaim, since it reclaims from all memcgs, will be fixed to regard
all references.

Reported-by: Konstantin Khlebnikov <khlebnikov@openvz.org>
Signed-off-by: Johannes Weiner <hannes@cmpxchg.org>
---
 include/linux/memcontrol.h |    3 ++-
 mm/memcontrol.c            |    2 +-
 mm/vmscan.c                |    6 ++++--
 3 files changed, 7 insertions(+), 4 deletions(-)

diff --git a/include/linux/memcontrol.h b/include/linux/memcontrol.h
index 8537c5d..ba9455d 100644
--- a/include/linux/memcontrol.h
+++ b/include/linux/memcontrol.h
@@ -78,6 +78,7 @@ extern void mem_cgroup_uncharge_page(struct page *page);
 extern void mem_cgroup_uncharge_cache_page(struct page *page);
 
 extern void mem_cgroup_out_of_memory(struct mem_cgroup *memcg, gfp_t gfp_mask);
+bool mem_cgroup_same_or_subtree(const struct mem_cgroup *, struct mem_cgroup *);
 int task_in_mem_cgroup(struct task_struct *task, const struct mem_cgroup *memcg);
 
 extern struct mem_cgroup *try_get_mem_cgroup_from_page(struct page *page);
@@ -91,7 +92,7 @@ int mm_match_cgroup(const struct mm_struct *mm, const struct mem_cgroup *cgroup)
 	rcu_read_lock();
 	memcg = mem_cgroup_from_task(rcu_dereference((mm)->owner));
 	rcu_read_unlock();
-	return cgroup == memcg;
+	return mem_cgroup_same_or_subtree(cgroup, memcg);
 }
 
 extern struct cgroup_subsys_state *mem_cgroup_css(struct mem_cgroup *memcg);
diff --git a/mm/memcontrol.c b/mm/memcontrol.c
index e4be95a..f91d762 100644
--- a/mm/memcontrol.c
+++ b/mm/memcontrol.c
@@ -1044,7 +1044,7 @@ struct lruvec *mem_cgroup_lru_move_lists(struct zone *zone,
  * Checks whether given mem is same or in the root_mem_cgroup's
  * hierarchy subtree
  */
-static bool mem_cgroup_same_or_subtree(const struct mem_cgroup *root_memcg,
+bool mem_cgroup_same_or_subtree(const struct mem_cgroup *root_memcg,
 		struct mem_cgroup *memcg)
 {
 	if (root_memcg != memcg) {
diff --git a/mm/vmscan.c b/mm/vmscan.c
index c631234..120646e 100644
--- a/mm/vmscan.c
+++ b/mm/vmscan.c
@@ -708,7 +708,8 @@ static enum page_references page_check_references(struct page *page,
 	int referenced_ptes, referenced_page;
 	unsigned long vm_flags;
 
-	referenced_ptes = page_referenced(page, 1, mz->mem_cgroup, &vm_flags);
+	referenced_ptes = page_referenced(page, 1, sc->target_mem_cgroup,
+					  &vm_flags);
 	referenced_page = TestClearPageReferenced(page);
 
 	/* Lumpy reclaim - ignore references */
@@ -1710,7 +1711,8 @@ static void shrink_active_list(unsigned long nr_pages,
 			continue;
 		}
 
-		if (page_referenced(page, 0, mz->mem_cgroup, &vm_flags)) {
+		if (page_referenced(page, 0, sc->target_mem_cgroup,
+				    &vm_flags)) {
 			nr_rotated += hpage_nr_pages(page);
 			/*
 			 * Identify referenced, file-backed active pages and
-- 
1.7.7.6

--
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/ .
Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH 1/2] memcg: fix page_referencies cgroup filter on global reclaim

[Konstantin Khlebnikov]

Johannes Weiner wrote:
> On Tue, Feb 21, 2012 at 03:38:04PM +0400, Konstantin Khlebnikov wrote:
>> Johannes Weiner wrote:
>>> On Wed, Feb 15, 2012 at 08:28:30PM +0400, Konstantin Khlebnikov wrote:
>>>> Global memory reclaimer should't skip referencies for any pages,
>>>> even if they are shared between different cgroups.
>>>
>>> Agreed: if we reclaim from one memcg because of its limit, we want to
>>> reclaim those pages that this group is not using.  If it's used by
>>> someone else, it should be evicted and refaulted by the group that
>>> needs it.
>>>
>>> If we reclaim globally, all references are "true" because we want to
>>> evict those pages that are not used by any cgroup.
>>>
>>> But if we reclaim a hierarchical subgroup, we don't want to evict
>>> pages that are shared among this hierarchy, either, even if the memcg
>>> that has the page charged to it is not using it.  Bouncing the page
>>> around the hierarchy is not sensible, because it does not solve the
>>> problem of the parent hitting its limit when the sibling group will
>>> refault it in a blink of an eye.  It should only be evicted if the
>>> memcg that's not using it nears its own limit, because only in that
>>> case would reclaiming the page remedy the situation.
>>>
>>>> This patch adds scan_control->current_mem_cgroup, which points to currently
>>>> shrinking sub-cgroup in hierarchy, at global reclaim it always NULL.
>>>
>>> So to be consistent, I'm wondering if we should pass
>>> sc->target_mem_cgroup - the limit-hitting hierarchy root - to
>>> page_referenced() and then have mm_match_cgroup() do a
>>> mem_cgroup_same_or_subtree() check to see if the vma is in the
>>> hierarchy rooted at sc->target_mem_cgroup.
>>>
>>> Global reclaim is handled automatically, because mm_match_cgroup() is
>>> not checked when the passed memcg is NULL, which sc->target_mem_cgroup
>>> is for global reclaim.
>>
>> Also we can try to recharge page to other cgroup, if we found in rmap another its user
>> outsize of currently shrinking hierarchy, page there is isolated, so at the end we will
>> insert page directly to its lru.
>
> Not sure if it's worth the trouble, but that could work.
>
>> But the main purpose of this patch for me is killing mz->mem_cgroup dereference,
>> because I plan to replace mz with direct reference to lruvec, which will be memcg-free object.
>
> I like that plan.  How about this patch then?

I sent my version. See patch "[PATCH v3 02/21] memcg: make mm_match_cgroup() hirarchical"

one note below

>
> ---
> From: Johannes Weiner<hannes@cmpxchg.org>
> Subject: mm: memcg: count pte references from every member of the reclaimed hierarchy
>
> The rmap walker checking page table references has historically
> ignored references from VMAs that were not part of the memcg that was
> being reclaimed during memcg hard limit reclaim.
>
> When transitioning global reclaim to memcg hierarchy reclaim, I missed
> that bit and now references from outside a memcg are ignored even
> during global reclaim.
>
> Reverting back to traditional behaviour - count all references during
> global reclaim and only mind references of the memcg being reclaimed
> during limit reclaim would be one option.
>
> However, the more generic idea is to ignore references exactly then
> when they are outside the hierarchy that is currently under reclaim;
> because only then will their reclamation be of any use to help the
> pressure situation.  It makes no sense to ignore references from a
> sibling memcg and then evict a page that will be immediately refaulted
> by that sibling which contributes to the same usage of the common
> ancestor under reclaim.
>
> The solution: make the rmap walker ignore references from VMAs that
> are not part of the hierarchy that is being reclaimed.
>
> Flat limit reclaim will stay the same, hierarchical limit reclaim will
> mind the references only to pages that the hierarchy owns.  Global
> reclaim, since it reclaims from all memcgs, will be fixed to regard
> all references.
>
> Reported-by: Konstantin Khlebnikov<khlebnikov@openvz.org>
> Signed-off-by: Johannes Weiner<hannes@cmpxchg.org>
> ---
>   include/linux/memcontrol.h |    3 ++-
>   mm/memcontrol.c            |    2 +-
>   mm/vmscan.c                |    6 ++++--
>   3 files changed, 7 insertions(+), 4 deletions(-)
>
> diff --git a/include/linux/memcontrol.h b/include/linux/memcontrol.h
> index 8537c5d..ba9455d 100644
> --- a/include/linux/memcontrol.h
> +++ b/include/linux/memcontrol.h
> @@ -78,6 +78,7 @@ extern void mem_cgroup_uncharge_page(struct page *page);
>   extern void mem_cgroup_uncharge_cache_page(struct page *page);
>
>   extern void mem_cgroup_out_of_memory(struct mem_cgroup *memcg, gfp_t gfp_mask);
> +bool mem_cgroup_same_or_subtree(const struct mem_cgroup *, struct mem_cgroup *);
>   int task_in_mem_cgroup(struct task_struct *task, const struct mem_cgroup *memcg);
>
>   extern struct mem_cgroup *try_get_mem_cgroup_from_page(struct page *page);
> @@ -91,7 +92,7 @@ int mm_match_cgroup(const struct mm_struct *mm, const struct mem_cgroup *cgroup)
>   	rcu_read_lock();
>   	memcg = mem_cgroup_from_task(rcu_dereference((mm)->owner));
>   	rcu_read_unlock();
> -	return cgroup == memcg;
> +	return mem_cgroup_same_or_subtree(cgroup, memcg);
>   }

seems like this memcg pointer is rcu-protected, so it seems broken.

>
>   extern struct cgroup_subsys_state *mem_cgroup_css(struct mem_cgroup *memcg);
> diff --git a/mm/memcontrol.c b/mm/memcontrol.c
> index e4be95a..f91d762 100644
> --- a/mm/memcontrol.c
> +++ b/mm/memcontrol.c
> @@ -1044,7 +1044,7 @@ struct lruvec *mem_cgroup_lru_move_lists(struct zone *zone,
>    * Checks whether given mem is same or in the root_mem_cgroup's
>    * hierarchy subtree
>    */
> -static bool mem_cgroup_same_or_subtree(const struct mem_cgroup *root_memcg,
> +bool mem_cgroup_same_or_subtree(const struct mem_cgroup *root_memcg,
>   		struct mem_cgroup *memcg)
>   {
>   	if (root_memcg != memcg) {
> diff --git a/mm/vmscan.c b/mm/vmscan.c
> index c631234..120646e 100644
> --- a/mm/vmscan.c
> +++ b/mm/vmscan.c
> @@ -708,7 +708,8 @@ static enum page_references page_check_references(struct page *page,
>   	int referenced_ptes, referenced_page;
>   	unsigned long vm_flags;
>
> -	referenced_ptes = page_referenced(page, 1, mz->mem_cgroup,&vm_flags);
> +	referenced_ptes = page_referenced(page, 1, sc->target_mem_cgroup,
> +					&vm_flags);
>   	referenced_page = TestClearPageReferenced(page);
>
>   	/* Lumpy reclaim - ignore references */
> @@ -1710,7 +1711,8 @@ static void shrink_active_list(unsigned long nr_pages,
>   			continue;
>   		}
>
> -		if (page_referenced(page, 0, mz->mem_cgroup,&vm_flags)) {
> +		if (page_referenced(page, 0, sc->target_mem_cgroup,
> +				&vm_flags)) {
>   			nr_rotated += hpage_nr_pages(page);
>   			/*
>   			 * Identify referenced, file-backed active pages and

--
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/ .
Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>