Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1655802 > unrolled thread

[PATCH] memcg: refactor mem_cgroup_resize_limit()

Back to article view | Back to linux.kernel

Contents

  [PATCH] memcg: refactor mem_cgroup_resize_limit() Yu Zhao <yuzhao@google.com> - 2017-06-02 01:10 +0200
    Re: [PATCH] memcg: refactor mem_cgroup_resize_limit() Nikolay Borisov <n.borisov.lkml@gmail.com> - 2017-06-02 09:40 +0200
      Re: [PATCH] memcg: refactor mem_cgroup_resize_limit() Yu Zhao <yuzhao@google.com> - 2017-06-04 21:50 +0200
    Re: [PATCH] memcg: refactor mem_cgroup_resize_limit() Vladimir Davydov <vdavydov.dev@gmail.com> - 2017-06-03 17:20 +0200
    [PATCH v2] memcg: refactor mem_cgroup_resize_limit() Yu Zhao <yuzhao@google.com> - 2017-06-04 22:10 +0200
      Re: [PATCH v2] memcg: refactor mem_cgroup_resize_limit() Vladimir Davydov <vdavydov.dev@gmail.com> - 2017-06-04 22:20 +0200
        Re: [PATCH v2] memcg: refactor mem_cgroup_resize_limit() Yu Zhao <yuzhao@google.com> - 2017-06-04 22:40 +0200
    [PATCH v3] memcg: refactor mem_cgroup_resize_limit() Yu Zhao <yuzhao@google.com> - 2017-06-04 23:20 +0200

#1655802 — [PATCH] memcg: refactor mem_cgroup_resize_limit()

mem_cgroup_resize_limit() and mem_cgroup_resize_memsw_limit() have
identical logics. Refactor code so we don't need to keep two pieces
of code that does same thing.

Signed-off-by: Yu Zhao <yuzhao@google.com>
---
 mm/memcontrol.c | 71 +++++++++------------------------------------------------
 1 file changed, 11 insertions(+), 60 deletions(-)

diff --git a/mm/memcontrol.c b/mm/memcontrol.c
index 94172089f52f..a4f0daaff704 100644
--- a/mm/memcontrol.c
+++ b/mm/memcontrol.c
@@ -2422,13 +2422,14 @@ static inline int mem_cgroup_move_swap_account(swp_entry_t entry,
 static DEFINE_MUTEX(memcg_limit_mutex);
 
 static int mem_cgroup_resize_limit(struct mem_cgroup *memcg,
-				   unsigned long limit)
+				   unsigned long limit, bool memsw)
 {
 	unsigned long curusage;
 	unsigned long oldusage;
 	bool enlarge = false;
 	int retry_count;
 	int ret;
+	struct page_counter *counter = memsw ? &memcg->memsw : &memcg->memory;
 
 	/*
 	 * For keeping hierarchical_reclaim simple, how long we should retry
@@ -2438,58 +2439,7 @@ static int mem_cgroup_resize_limit(struct mem_cgroup *memcg,
 	retry_count = MEM_CGROUP_RECLAIM_RETRIES *
 		      mem_cgroup_count_children(memcg);
 
-	oldusage = page_counter_read(&memcg->memory);
-
-	do {
-		if (signal_pending(current)) {
-			ret = -EINTR;
-			break;
-		}
-
-		mutex_lock(&memcg_limit_mutex);
-		if (limit > memcg->memsw.limit) {
-			mutex_unlock(&memcg_limit_mutex);
-			ret = -EINVAL;
-			break;
-		}
-		if (limit > memcg->memory.limit)
-			enlarge = true;
-		ret = page_counter_limit(&memcg->memory, limit);
-		mutex_unlock(&memcg_limit_mutex);
-
-		if (!ret)
-			break;
-
-		try_to_free_mem_cgroup_pages(memcg, 1, GFP_KERNEL, true);
-
-		curusage = page_counter_read(&memcg->memory);
-		/* Usage is reduced ? */
-		if (curusage >= oldusage)
-			retry_count--;
-		else
-			oldusage = curusage;
-	} while (retry_count);
-
-	if (!ret && enlarge)
-		memcg_oom_recover(memcg);
-
-	return ret;
-}
-
-static int mem_cgroup_resize_memsw_limit(struct mem_cgroup *memcg,
-					 unsigned long limit)
-{
-	unsigned long curusage;
-	unsigned long oldusage;
-	bool enlarge = false;
-	int retry_count;
-	int ret;
-
-	/* see mem_cgroup_resize_res_limit */
-	retry_count = MEM_CGROUP_RECLAIM_RETRIES *
-		      mem_cgroup_count_children(memcg);
-
-	oldusage = page_counter_read(&memcg->memsw);
+	oldusage = page_counter_read(counter);
 
 	do {
 		if (signal_pending(current)) {
@@ -2498,22 +2448,23 @@ static int mem_cgroup_resize_memsw_limit(struct mem_cgroup *memcg,
 		}
 
 		mutex_lock(&memcg_limit_mutex);
-		if (limit < memcg->memory.limit) {
+		if (memsw ? limit < memcg->memory.limit :
+			    limit > memcg->memsw.limit) {
 			mutex_unlock(&memcg_limit_mutex);
 			ret = -EINVAL;
 			break;
 		}
-		if (limit > memcg->memsw.limit)
+		if (limit > counter->limit)
 			enlarge = true;
-		ret = page_counter_limit(&memcg->memsw, limit);
+		ret = page_counter_limit(counter, limit);
 		mutex_unlock(&memcg_limit_mutex);
 
 		if (!ret)
 			break;
 
-		try_to_free_mem_cgroup_pages(memcg, 1, GFP_KERNEL, false);
+		try_to_free_mem_cgroup_pages(memcg, 1, GFP_KERNEL, !memsw);
 
-		curusage = page_counter_read(&memcg->memsw);
+		curusage = page_counter_read(counter);
 		/* Usage is reduced ? */
 		if (curusage >= oldusage)
 			retry_count--;
@@ -2975,10 +2926,10 @@ static ssize_t mem_cgroup_write(struct kernfs_open_file *of,
 		}
 		switch (MEMFILE_TYPE(of_cft(of)->private)) {
 		case _MEM:
-			ret = mem_cgroup_resize_limit(memcg, nr_pages);
+			ret = mem_cgroup_resize_limit(memcg, nr_pages, false);
 			break;
 		case _MEMSWAP:
-			ret = mem_cgroup_resize_memsw_limit(memcg, nr_pages);
+			ret = mem_cgroup_resize_limit(memcg, nr_pages, true);
 			break;
 		case _KMEM:
 			ret = memcg_update_kmem_limit(memcg, nr_pages);
-- 
2.13.0.506.g27d5fe0cd-goog

[toc] | [next] | [standalone]

#1655983

On  2.06.2017 02:02, Yu Zhao wrote:
> mem_cgroup_resize_limit() and mem_cgroup_resize_memsw_limit() have
> identical logics. Refactor code so we don't need to keep two pieces
> of code that does same thing.
> 
> Signed-off-by: Yu Zhao <yuzhao@google.com>
> ---
>  mm/memcontrol.c | 71 +++++++++------------------------------------------------
>  1 file changed, 11 insertions(+), 60 deletions(-)
> 
> diff --git a/mm/memcontrol.c b/mm/memcontrol.c
> index 94172089f52f..a4f0daaff704 100644
> --- a/mm/memcontrol.c
> +++ b/mm/memcontrol.c
> @@ -2422,13 +2422,14 @@ static inline int mem_cgroup_move_swap_account(swp_entry_t entry,
>  static DEFINE_MUTEX(memcg_limit_mutex);
>  
>  static int mem_cgroup_resize_limit(struct mem_cgroup *memcg,
> -				   unsigned long limit)
> +				   unsigned long limit, bool memsw)
>  {
>  	unsigned long curusage;
>  	unsigned long oldusage;
>  	bool enlarge = false;
>  	int retry_count;
>  	int ret;
> +	struct page_counter *counter = memsw ? &memcg->memsw : &memcg->memory;
>  
>  	/*
>  	 * For keeping hierarchical_reclaim simple, how long we should retry
> @@ -2438,58 +2439,7 @@ static int mem_cgroup_resize_limit(struct mem_cgroup *memcg,
>  	retry_count = MEM_CGROUP_RECLAIM_RETRIES *
>  		      mem_cgroup_count_children(memcg);
>  
> -	oldusage = page_counter_read(&memcg->memory);
> -
> -	do {
> -		if (signal_pending(current)) {
> -			ret = -EINTR;
> -			break;
> -		}
> -
> -		mutex_lock(&memcg_limit_mutex);
> -		if (limit > memcg->memsw.limit) {
> -			mutex_unlock(&memcg_limit_mutex);
> -			ret = -EINVAL;
> -			break;
> -		}
> -		if (limit > memcg->memory.limit)
> -			enlarge = true;
> -		ret = page_counter_limit(&memcg->memory, limit);
> -		mutex_unlock(&memcg_limit_mutex);
> -
> -		if (!ret)
> -			break;
> -
> -		try_to_free_mem_cgroup_pages(memcg, 1, GFP_KERNEL, true);
> -
> -		curusage = page_counter_read(&memcg->memory);
> -		/* Usage is reduced ? */
> -		if (curusage >= oldusage)
> -			retry_count--;
> -		else
> -			oldusage = curusage;
> -	} while (retry_count);
> -
> -	if (!ret && enlarge)
> -		memcg_oom_recover(memcg);
> -
> -	return ret;
> -}
> -
> -static int mem_cgroup_resize_memsw_limit(struct mem_cgroup *memcg,
> -					 unsigned long limit)
> -{
> -	unsigned long curusage;
> -	unsigned long oldusage;
> -	bool enlarge = false;
> -	int retry_count;
> -	int ret;
> -
> -	/* see mem_cgroup_resize_res_limit */
> -	retry_count = MEM_CGROUP_RECLAIM_RETRIES *
> -		      mem_cgroup_count_children(memcg);
> -
> -	oldusage = page_counter_read(&memcg->memsw);
> +	oldusage = page_counter_read(counter);
>  
>  	do {
>  		if (signal_pending(current)) {
> @@ -2498,22 +2448,23 @@ static int mem_cgroup_resize_memsw_limit(struct mem_cgroup *memcg,
>  		}
>  
>  		mutex_lock(&memcg_limit_mutex);
> -		if (limit < memcg->memory.limit) {
> +		if (memsw ? limit < memcg->memory.limit :
> +			    limit > memcg->memsw.limit) {

No, just no. Please createa a local variable and use that. Using the
ternary operator in an 'if' statement is just ugly!

>  			mutex_unlock(&memcg_limit_mutex);
>  			ret = -EINVAL;
>  			break;
>  		}
> -		if (limit > memcg->memsw.limit)
> +		if (limit > counter->limit)
>  			enlarge = true;
> -		ret = page_counter_limit(&memcg->memsw, limit);
> +		ret = page_counter_limit(counter, limit);
>  		mutex_unlock(&memcg_limit_mutex);
>  
>  		if (!ret)
>  			break;
>  
> -		try_to_free_mem_cgroup_pages(memcg, 1, GFP_KERNEL, false);
> +		try_to_free_mem_cgroup_pages(memcg, 1, GFP_KERNEL, !memsw);
>  
> -		curusage = page_counter_read(&memcg->memsw);
> +		curusage = page_counter_read(counter);
>  		/* Usage is reduced ? */
>  		if (curusage >= oldusage)
>  			retry_count--;
> @@ -2975,10 +2926,10 @@ static ssize_t mem_cgroup_write(struct kernfs_open_file *of,
>  		}
>  		switch (MEMFILE_TYPE(of_cft(of)->private)) {
>  		case _MEM:
> -			ret = mem_cgroup_resize_limit(memcg, nr_pages);
> +			ret = mem_cgroup_resize_limit(memcg, nr_pages, false);
>  			break;
>  		case _MEMSWAP:
> -			ret = mem_cgroup_resize_memsw_limit(memcg, nr_pages);
> +			ret = mem_cgroup_resize_limit(memcg, nr_pages, true);
>  			break;
>  		case _KMEM:
>  			ret = memcg_update_kmem_limit(memcg, nr_pages);
> 

[toc] | [prev] | [next] | [standalone]

#1657148

On Fri, Jun 02, 2017 at 10:32:52AM +0300, Nikolay Borisov wrote:
> 
> 
> On  2.06.2017 02:02, Yu Zhao wrote:
> > mem_cgroup_resize_limit() and mem_cgroup_resize_memsw_limit() have
> > identical logics. Refactor code so we don't need to keep two pieces
> > of code that does same thing.
> > 
> > Signed-off-by: Yu Zhao <yuzhao@google.com>
> > ---
> >  mm/memcontrol.c | 71 +++++++++------------------------------------------------
> >  1 file changed, 11 insertions(+), 60 deletions(-)
> > 
> > diff --git a/mm/memcontrol.c b/mm/memcontrol.c
> > index 94172089f52f..a4f0daaff704 100644
> > --- a/mm/memcontrol.c
> > +++ b/mm/memcontrol.c
> > @@ -2422,13 +2422,14 @@ static inline int mem_cgroup_move_swap_account(swp_entry_t entry,
> >  static DEFINE_MUTEX(memcg_limit_mutex);
> >  
> >  static int mem_cgroup_resize_limit(struct mem_cgroup *memcg,
> > -				   unsigned long limit)
> > +				   unsigned long limit, bool memsw)
> >  {
> >  	unsigned long curusage;
> >  	unsigned long oldusage;
> >  	bool enlarge = false;
> >  	int retry_count;
> >  	int ret;
> > +	struct page_counter *counter = memsw ? &memcg->memsw : &memcg->memory;
> >  
> >  	/*
> >  	 * For keeping hierarchical_reclaim simple, how long we should retry
> > @@ -2438,58 +2439,7 @@ static int mem_cgroup_resize_limit(struct mem_cgroup *memcg,
> >  	retry_count = MEM_CGROUP_RECLAIM_RETRIES *
> >  		      mem_cgroup_count_children(memcg);
> >  
> > -	oldusage = page_counter_read(&memcg->memory);
> > -
> > -	do {
> > -		if (signal_pending(current)) {
> > -			ret = -EINTR;
> > -			break;
> > -		}
> > -
> > -		mutex_lock(&memcg_limit_mutex);
> > -		if (limit > memcg->memsw.limit) {
> > -			mutex_unlock(&memcg_limit_mutex);
> > -			ret = -EINVAL;
> > -			break;
> > -		}
> > -		if (limit > memcg->memory.limit)
> > -			enlarge = true;
> > -		ret = page_counter_limit(&memcg->memory, limit);
> > -		mutex_unlock(&memcg_limit_mutex);
> > -
> > -		if (!ret)
> > -			break;
> > -
> > -		try_to_free_mem_cgroup_pages(memcg, 1, GFP_KERNEL, true);
> > -
> > -		curusage = page_counter_read(&memcg->memory);
> > -		/* Usage is reduced ? */
> > -		if (curusage >= oldusage)
> > -			retry_count--;
> > -		else
> > -			oldusage = curusage;
> > -	} while (retry_count);
> > -
> > -	if (!ret && enlarge)
> > -		memcg_oom_recover(memcg);
> > -
> > -	return ret;
> > -}
> > -
> > -static int mem_cgroup_resize_memsw_limit(struct mem_cgroup *memcg,
> > -					 unsigned long limit)
> > -{
> > -	unsigned long curusage;
> > -	unsigned long oldusage;
> > -	bool enlarge = false;
> > -	int retry_count;
> > -	int ret;
> > -
> > -	/* see mem_cgroup_resize_res_limit */
> > -	retry_count = MEM_CGROUP_RECLAIM_RETRIES *
> > -		      mem_cgroup_count_children(memcg);
> > -
> > -	oldusage = page_counter_read(&memcg->memsw);
> > +	oldusage = page_counter_read(counter);
> >  
> >  	do {
> >  		if (signal_pending(current)) {
> > @@ -2498,22 +2448,23 @@ static int mem_cgroup_resize_memsw_limit(struct mem_cgroup *memcg,
> >  		}
> >  
> >  		mutex_lock(&memcg_limit_mutex);
> > -		if (limit < memcg->memory.limit) {
> > +		if (memsw ? limit < memcg->memory.limit :
> > +			    limit > memcg->memsw.limit) {
> 
> No, just no. Please createa a local variable and use that. Using the
> ternary operator in an 'if' statement is just ugly!

Thanks. It is uncommon but seems no aesthetic difference to me. I'll
replace it with an extra variable and resend the patch.

[toc] | [prev] | [next] | [standalone]

#1656854

On Thu, Jun 01, 2017 at 04:02:12PM -0700, Yu Zhao wrote:
> mem_cgroup_resize_limit() and mem_cgroup_resize_memsw_limit() have
> identical logics. Refactor code so we don't need to keep two pieces
> of code that does same thing.
> 
> Signed-off-by: Yu Zhao <yuzhao@google.com>
> ---
>  mm/memcontrol.c | 71 +++++++++------------------------------------------------
>  1 file changed, 11 insertions(+), 60 deletions(-)

Makes sense to me.

Acked-by: Vladimir Davydov <vdavydov.dev@gmail.com>

[toc] | [prev] | [next] | [standalone]

#1657150 — [PATCH v2] memcg: refactor mem_cgroup_resize_limit()

mem_cgroup_resize_limit() and mem_cgroup_resize_memsw_limit() have
identical logics. Refactor code so we don't need to keep two pieces
of code that does same thing.

Signed-off-by: Yu Zhao <yuzhao@google.com>
Acked-by: Vladimir Davydov <vdavydov.dev@gmail.com>
---
Changelog since v1:
* minor style change

 mm/memcontrol.c | 73 ++++++++++-----------------------------------------------
 1 file changed, 13 insertions(+), 60 deletions(-)

diff --git a/mm/memcontrol.c b/mm/memcontrol.c
index 94172089f52f..08a97a381b2a 100644
--- a/mm/memcontrol.c
+++ b/mm/memcontrol.c
@@ -2422,13 +2422,15 @@ static inline int mem_cgroup_move_swap_account(swp_entry_t entry,
 static DEFINE_MUTEX(memcg_limit_mutex);
 
 static int mem_cgroup_resize_limit(struct mem_cgroup *memcg,
-				   unsigned long limit)
+				   unsigned long limit, bool memsw)
 {
 	unsigned long curusage;
 	unsigned long oldusage;
 	bool enlarge = false;
 	int retry_count;
 	int ret;
+	bool inverted;
+	struct page_counter *counter = memsw ? &memcg->memsw : &memcg->memory;
 
 	/*
 	 * For keeping hierarchical_reclaim simple, how long we should retry
@@ -2438,58 +2440,7 @@ static int mem_cgroup_resize_limit(struct mem_cgroup *memcg,
 	retry_count = MEM_CGROUP_RECLAIM_RETRIES *
 		      mem_cgroup_count_children(memcg);
 
-	oldusage = page_counter_read(&memcg->memory);
-
-	do {
-		if (signal_pending(current)) {
-			ret = -EINTR;
-			break;
-		}
-
-		mutex_lock(&memcg_limit_mutex);
-		if (limit > memcg->memsw.limit) {
-			mutex_unlock(&memcg_limit_mutex);
-			ret = -EINVAL;
-			break;
-		}
-		if (limit > memcg->memory.limit)
-			enlarge = true;
-		ret = page_counter_limit(&memcg->memory, limit);
-		mutex_unlock(&memcg_limit_mutex);
-
-		if (!ret)
-			break;
-
-		try_to_free_mem_cgroup_pages(memcg, 1, GFP_KERNEL, true);
-
-		curusage = page_counter_read(&memcg->memory);
-		/* Usage is reduced ? */
-		if (curusage >= oldusage)
-			retry_count--;
-		else
-			oldusage = curusage;
-	} while (retry_count);
-
-	if (!ret && enlarge)
-		memcg_oom_recover(memcg);
-
-	return ret;
-}
-
-static int mem_cgroup_resize_memsw_limit(struct mem_cgroup *memcg,
-					 unsigned long limit)
-{
-	unsigned long curusage;
-	unsigned long oldusage;
-	bool enlarge = false;
-	int retry_count;
-	int ret;
-
-	/* see mem_cgroup_resize_res_limit */
-	retry_count = MEM_CGROUP_RECLAIM_RETRIES *
-		      mem_cgroup_count_children(memcg);
-
-	oldusage = page_counter_read(&memcg->memsw);
+	oldusage = page_counter_read(counter);
 
 	do {
 		if (signal_pending(current)) {
@@ -2498,22 +2449,24 @@ static int mem_cgroup_resize_memsw_limit(struct mem_cgroup *memcg,
 		}
 
 		mutex_lock(&memcg_limit_mutex);
-		if (limit < memcg->memory.limit) {
+		inverted = memsw ? limit < memcg->memory.limit :
+				   limit > memcg->memsw.limit;
+		if (inverted)
 			mutex_unlock(&memcg_limit_mutex);
 			ret = -EINVAL;
 			break;
 		}
-		if (limit > memcg->memsw.limit)
+		if (limit > counter->limit)
 			enlarge = true;
-		ret = page_counter_limit(&memcg->memsw, limit);
+		ret = page_counter_limit(counter, limit);
 		mutex_unlock(&memcg_limit_mutex);
 
 		if (!ret)
 			break;
 
-		try_to_free_mem_cgroup_pages(memcg, 1, GFP_KERNEL, false);
+		try_to_free_mem_cgroup_pages(memcg, 1, GFP_KERNEL, !memsw);
 
-		curusage = page_counter_read(&memcg->memsw);
+		curusage = page_counter_read(counter);
 		/* Usage is reduced ? */
 		if (curusage >= oldusage)
 			retry_count--;
@@ -2975,10 +2928,10 @@ static ssize_t mem_cgroup_write(struct kernfs_open_file *of,
 		}
 		switch (MEMFILE_TYPE(of_cft(of)->private)) {
 		case _MEM:
-			ret = mem_cgroup_resize_limit(memcg, nr_pages);
+			ret = mem_cgroup_resize_limit(memcg, nr_pages, false);
 			break;
 		case _MEMSWAP:
-			ret = mem_cgroup_resize_memsw_limit(memcg, nr_pages);
+			ret = mem_cgroup_resize_limit(memcg, nr_pages, true);
 			break;
 		case _KMEM:
 			ret = memcg_update_kmem_limit(memcg, nr_pages);
-- 
2.13.0.506.g27d5fe0cd-goog

[toc] | [prev] | [next] | [standalone]

#1657157 — Re: [PATCH v2] memcg: refactor mem_cgroup_resize_limit()

On Sun, Jun 04, 2017 at 01:04:37PM -0700, Yu Zhao wrote:
> @@ -2498,22 +2449,24 @@ static int mem_cgroup_resize_memsw_limit(struct mem_cgroup *memcg,
>  		}
>  
>  		mutex_lock(&memcg_limit_mutex);
> -		if (limit < memcg->memory.limit) {
> +		inverted = memsw ? limit < memcg->memory.limit :
> +				   limit > memcg->memsw.limit;
> +		if (inverted)
>  			mutex_unlock(&memcg_limit_mutex);
>  			ret = -EINVAL;
>  			break;
>  		}

For some reason, I liked this patch more without this extra variable :-)

[toc] | [prev] | [next] | [standalone]

#1657165 — Re: [PATCH v2] memcg: refactor mem_cgroup_resize_limit()

On Sun, Jun 04, 2017 at 11:09:42PM +0300, Vladimir Davydov wrote:
> On Sun, Jun 04, 2017 at 01:04:37PM -0700, Yu Zhao wrote:
> > @@ -2498,22 +2449,24 @@ static int mem_cgroup_resize_memsw_limit(struct mem_cgroup *memcg,
> >  		}
> >  
> >  		mutex_lock(&memcg_limit_mutex);
> > -		if (limit < memcg->memory.limit) {
> > +		inverted = memsw ? limit < memcg->memory.limit :
> > +				   limit > memcg->memsw.limit;
> > +		if (inverted)
> >  			mutex_unlock(&memcg_limit_mutex);
> >  			ret = -EINVAL;
> >  			break;
> >  		}
> 
> For some reason, I liked this patch more without this extra variable :-)
Well, I'll refrain myself from commenting more because we are now at
the risk of starting a coding style war over this.

[toc] | [prev] | [next] | [standalone]

#1657175 — [PATCH v3] memcg: refactor mem_cgroup_resize_limit()

mem_cgroup_resize_limit() and mem_cgroup_resize_memsw_limit() have
identical logics. Refactor code so we don't need to keep two pieces
of code that does same thing.

Signed-off-by: Yu Zhao <yuzhao@google.com>
Acked-by: Vladimir Davydov <vdavydov.dev@gmail.com>
---
Changelog since v1:
* minor style change
Changelog since v2:
* fix build error

 mm/memcontrol.c | 73 ++++++++++-----------------------------------------------
 1 file changed, 13 insertions(+), 60 deletions(-)

diff --git a/mm/memcontrol.c b/mm/memcontrol.c
index 94172089f52f..08a97a381b2a 100644
--- a/mm/memcontrol.c
+++ b/mm/memcontrol.c
@@ -2422,13 +2422,15 @@ static inline int mem_cgroup_move_swap_account(swp_entry_t entry,
 static DEFINE_MUTEX(memcg_limit_mutex);
 
 static int mem_cgroup_resize_limit(struct mem_cgroup *memcg,
-				   unsigned long limit)
+				   unsigned long limit, bool memsw)
 {
 	unsigned long curusage;
 	unsigned long oldusage;
 	bool enlarge = false;
 	int retry_count;
 	int ret;
+	bool inverted;
+	struct page_counter *counter = memsw ? &memcg->memsw : &memcg->memory;
 
 	/*
 	 * For keeping hierarchical_reclaim simple, how long we should retry
@@ -2438,58 +2440,7 @@ static int mem_cgroup_resize_limit(struct mem_cgroup *memcg,
 	retry_count = MEM_CGROUP_RECLAIM_RETRIES *
 		      mem_cgroup_count_children(memcg);
 
-	oldusage = page_counter_read(&memcg->memory);
-
-	do {
-		if (signal_pending(current)) {
-			ret = -EINTR;
-			break;
-		}
-
-		mutex_lock(&memcg_limit_mutex);
-		if (limit > memcg->memsw.limit) {
-			mutex_unlock(&memcg_limit_mutex);
-			ret = -EINVAL;
-			break;
-		}
-		if (limit > memcg->memory.limit)
-			enlarge = true;
-		ret = page_counter_limit(&memcg->memory, limit);
-		mutex_unlock(&memcg_limit_mutex);
-
-		if (!ret)
-			break;
-
-		try_to_free_mem_cgroup_pages(memcg, 1, GFP_KERNEL, true);
-
-		curusage = page_counter_read(&memcg->memory);
-		/* Usage is reduced ? */
-		if (curusage >= oldusage)
-			retry_count--;
-		else
-			oldusage = curusage;
-	} while (retry_count);
-
-	if (!ret && enlarge)
-		memcg_oom_recover(memcg);
-
-	return ret;
-}
-
-static int mem_cgroup_resize_memsw_limit(struct mem_cgroup *memcg,
-					 unsigned long limit)
-{
-	unsigned long curusage;
-	unsigned long oldusage;
-	bool enlarge = false;
-	int retry_count;
-	int ret;
-
-	/* see mem_cgroup_resize_res_limit */
-	retry_count = MEM_CGROUP_RECLAIM_RETRIES *
-		      mem_cgroup_count_children(memcg);
-
-	oldusage = page_counter_read(&memcg->memsw);
+	oldusage = page_counter_read(counter);
 
 	do {
 		if (signal_pending(current)) {
@@ -2498,22 +2449,24 @@ static int mem_cgroup_resize_memsw_limit(struct mem_cgroup *memcg,
 		}
 
 		mutex_lock(&memcg_limit_mutex);
-		if (limit < memcg->memory.limit) {
+		inverted = memsw ? limit < memcg->memory.limit :
+				   limit > memcg->memsw.limit;
+		if (inverted) {
 			mutex_unlock(&memcg_limit_mutex);
 			ret = -EINVAL;
 			break;
 		}
-		if (limit > memcg->memsw.limit)
+		if (limit > counter->limit)
 			enlarge = true;
-		ret = page_counter_limit(&memcg->memsw, limit);
+		ret = page_counter_limit(counter, limit);
 		mutex_unlock(&memcg_limit_mutex);
 
 		if (!ret)
 			break;
 
-		try_to_free_mem_cgroup_pages(memcg, 1, GFP_KERNEL, false);
+		try_to_free_mem_cgroup_pages(memcg, 1, GFP_KERNEL, !memsw);
 
-		curusage = page_counter_read(&memcg->memsw);
+		curusage = page_counter_read(counter);
 		/* Usage is reduced ? */
 		if (curusage >= oldusage)
 			retry_count--;
@@ -2975,10 +2928,10 @@ static ssize_t mem_cgroup_write(struct kernfs_open_file *of,
 		}
 		switch (MEMFILE_TYPE(of_cft(of)->private)) {
 		case _MEM:
-			ret = mem_cgroup_resize_limit(memcg, nr_pages);
+			ret = mem_cgroup_resize_limit(memcg, nr_pages, false);
 			break;
 		case _MEMSWAP:
-			ret = mem_cgroup_resize_memsw_limit(memcg, nr_pages);
+			ret = mem_cgroup_resize_limit(memcg, nr_pages, true);
 			break;
 		case _KMEM:
 			ret = memcg_update_kmem_limit(memcg, nr_pages);
-- 
2.13.0.506.g27d5fe0cd-goog

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web