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

Groups > linux.kernel > #1181160 > unrolled thread

[PATCH] zsmalloc: consider ZS_ALMOST_FULL as migrate source

Started byMinchan Kim <minchan@kernel.org>
First post2015-07-10 03:40 +0200
Last post2015-07-10 04:40 +0200
Articles 5 — 2 participants

Back to article view | Back to linux.kernel

Contents

  [PATCH] zsmalloc: consider ZS_ALMOST_FULL as migrate source Minchan Kim <minchan@kernel.org> - 2015-07-10 03:40 +0200
    Re: [PATCH] zsmalloc: consider ZS_ALMOST_FULL as migrate source Sergey Senozhatsky <sergey.senozhatsky.work@gmail.com> - 2015-07-10 04:00 +0200
      Re: [PATCH] zsmalloc: consider ZS_ALMOST_FULL as migrate source Minchan Kim <minchan@kernel.org> - 2015-07-10 04:40 +0200
    Re: [PATCH] zsmalloc: consider ZS_ALMOST_FULL as migrate source Sergey Senozhatsky <sergey.senozhatsky.work@gmail.com> - 2015-07-10 04:10 +0200
      Re: [PATCH] zsmalloc: consider ZS_ALMOST_FULL as migrate source Minchan Kim <minchan@kernel.org> - 2015-07-10 04:40 +0200

#1181160 — [PATCH] zsmalloc: consider ZS_ALMOST_FULL as migrate source

FromMinchan Kim <minchan@kernel.org>
Date2015-07-10 03:40 +0200
Subject[PATCH] zsmalloc: consider ZS_ALMOST_FULL as migrate source
Message-ID<pKvse-7NE-17@gated-at.bofh.it>
There is no reason to prevent select ZS_ALMOST_FULL as migration
source if we cannot find source from ZS_ALMOST_EMPTY.

With this patch, zs_can_compact will return more exact result.

Signed-off-by: Minchan Kim <minchan@kernel.org>
---
 mm/zsmalloc.c |   19 ++++++++++++-------
 1 file changed, 12 insertions(+), 7 deletions(-)

diff --git a/mm/zsmalloc.c b/mm/zsmalloc.c
index 8c78bcb..7bd7dde 100644
--- a/mm/zsmalloc.c
+++ b/mm/zsmalloc.c
@@ -1687,12 +1687,20 @@ static enum fullness_group putback_zspage(struct zs_pool *pool,
 static struct page *isolate_source_page(struct size_class *class)
 {
 	struct page *page;
+	int i;
+	bool found = false;
 
-	page = class->fullness_list[ZS_ALMOST_EMPTY];
-	if (page)
-		remove_zspage(page, class, ZS_ALMOST_EMPTY);
+	for (i = ZS_ALMOST_EMPTY; i >= ZS_ALMOST_FULL; i--) {
+		page = class->fullness_list[i];
+		if (!page)
+			continue;
 
-	return page;
+		remove_zspage(page, class, i);
+		found = true;
+		break;
+	}
+
+	return found ? page : NULL;
 }
 
 /*
@@ -1706,9 +1714,6 @@ static unsigned long zs_can_compact(struct size_class *class)
 {
 	unsigned long obj_wasted;
 
-	if (!zs_stat_get(class, CLASS_ALMOST_EMPTY))
-		return 0;
-
 	obj_wasted = zs_stat_get(class, OBJ_ALLOCATED) -
 		zs_stat_get(class, OBJ_USED);
 
-- 
1.7.9.5

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [next] | [standalone]

#1181165

FromSergey Senozhatsky <sergey.senozhatsky.work@gmail.com>
Date2015-07-10 04:00 +0200
Message-ID<pKvLA-7UM-3@gated-at.bofh.it>
In reply to#1181160
On (07/10/15 10:32), Minchan Kim wrote:
> There is no reason to prevent select ZS_ALMOST_FULL as migration
> source if we cannot find source from ZS_ALMOST_EMPTY.
> 
> With this patch, zs_can_compact will return more exact result.
> 

wouldn't that be too aggresive?

drainig 'only ZS_ALMOST_EMPTY classes' sounds safer than draining
'ZS_ALMOST_EMPTY and ZS_ALMOST_FULL clases'. you seemed to be worried
that compaction can leave no unused objects in classes, which will
result in zspage allocation happening right after compaction. it looks
like here the chances to cause zspage allocation are even higher. don't
you think so?

> Signed-off-by: Minchan Kim <minchan@kernel.org>
> ---
>  mm/zsmalloc.c |   19 ++++++++++++-------
>  1 file changed, 12 insertions(+), 7 deletions(-)
> 
> diff --git a/mm/zsmalloc.c b/mm/zsmalloc.c
> index 8c78bcb..7bd7dde 100644
> --- a/mm/zsmalloc.c
> +++ b/mm/zsmalloc.c
> @@ -1687,12 +1687,20 @@ static enum fullness_group putback_zspage(struct zs_pool *pool,
>  static struct page *isolate_source_page(struct size_class *class)
>  {
>  	struct page *page;
> +	int i;
> +	bool found = false;
>  
> -	page = class->fullness_list[ZS_ALMOST_EMPTY];
> -	if (page)
> -		remove_zspage(page, class, ZS_ALMOST_EMPTY);
> +	for (i = ZS_ALMOST_EMPTY; i >= ZS_ALMOST_FULL; i--) {
> +		page = class->fullness_list[i];
> +		if (!page)
> +			continue;
>  
> -	return page;
> +		remove_zspage(page, class, i);
> +		found = true;
> +		break;
> +	}
> +
> +	return found ? page : NULL;
>  }
>  
>  /*
> @@ -1706,9 +1714,6 @@ static unsigned long zs_can_compact(struct size_class *class)
>  {
>  	unsigned long obj_wasted;
>  
> -	if (!zs_stat_get(class, CLASS_ALMOST_EMPTY))
> -		return 0;
> -

well, you asked to add this check like a week or two ago (it's not even
in -next yet) and now you remove it.

>  	obj_wasted = zs_stat_get(class, OBJ_ALLOCATED) -
>  		zs_stat_get(class, OBJ_USED);
>  

	-ss
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

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

#1181180

FromMinchan Kim <minchan@kernel.org>
Date2015-07-10 04:40 +0200
Message-ID<pKwoh-8tG-3@gated-at.bofh.it>
In reply to#1181165
Hi Sergey,

On Fri, Jul 10, 2015 at 10:58:28AM +0900, Sergey Senozhatsky wrote:
> On (07/10/15 10:32), Minchan Kim wrote:
> > There is no reason to prevent select ZS_ALMOST_FULL as migration
> > source if we cannot find source from ZS_ALMOST_EMPTY.
> > 
> > With this patch, zs_can_compact will return more exact result.
> > 
> 
> wouldn't that be too aggresive?
> 
> drainig 'only ZS_ALMOST_EMPTY classes' sounds safer than draining
> 'ZS_ALMOST_EMPTY and ZS_ALMOST_FULL clases'. you seemed to be worried
> that compaction can leave no unused objects in classes, which will
> result in zspage allocation happening right after compaction. it looks
> like here the chances to cause zspage allocation are even higher. don't
> you think so?

Good question.

My worry was failure of order-0 page allocation in zram-swap path
when memory presssure is really heavy but I didn't insist to you
from sometime. The reason I changed my mind was

1. It's almost dead system if there is no order-0 page
2. If old might be working well, it's not our design, just luck.


> 
> > Signed-off-by: Minchan Kim <minchan@kernel.org>
> > ---
> >  mm/zsmalloc.c |   19 ++++++++++++-------
> >  1 file changed, 12 insertions(+), 7 deletions(-)
> > 
> > diff --git a/mm/zsmalloc.c b/mm/zsmalloc.c
> > index 8c78bcb..7bd7dde 100644
> > --- a/mm/zsmalloc.c
> > +++ b/mm/zsmalloc.c
> > @@ -1687,12 +1687,20 @@ static enum fullness_group putback_zspage(struct zs_pool *pool,
> >  static struct page *isolate_source_page(struct size_class *class)
> >  {
> >  	struct page *page;
> > +	int i;
> > +	bool found = false;
> >  
> > -	page = class->fullness_list[ZS_ALMOST_EMPTY];
> > -	if (page)
> > -		remove_zspage(page, class, ZS_ALMOST_EMPTY);
> > +	for (i = ZS_ALMOST_EMPTY; i >= ZS_ALMOST_FULL; i--) {
> > +		page = class->fullness_list[i];
> > +		if (!page)
> > +			continue;
> >  
> > -	return page;
> > +		remove_zspage(page, class, i);
> > +		found = true;
> > +		break;
> > +	}
> > +
> > +	return found ? page : NULL;
> >  }
> >  
> >  /*
> > @@ -1706,9 +1714,6 @@ static unsigned long zs_can_compact(struct size_class *class)
> >  {
> >  	unsigned long obj_wasted;
> >  
> > -	if (!zs_stat_get(class, CLASS_ALMOST_EMPTY))
> > -		return 0;
> > -
> 
> well, you asked to add this check like a week or two ago (it's not even
> in -next yet) and now you remove it.

The reason I wanted to check CLASS_ALMOST_EMPTY is to make zs_can_compact exact.
But with this patch, we can achieve it without above.

> 
> >  	obj_wasted = zs_stat_get(class, OBJ_ALLOCATED) -
> >  		zs_stat_get(class, OBJ_USED);
> >  
> 
> 	-ss

-- 
Kind regards,
Minchan Kim
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

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

#1181168

FromSergey Senozhatsky <sergey.senozhatsky.work@gmail.com>
Date2015-07-10 04:10 +0200
Message-ID<pKvVf-8dZ-5@gated-at.bofh.it>
In reply to#1181160
On (07/10/15 10:32), Minchan Kim wrote:
>  static struct page *isolate_source_page(struct size_class *class)
>  {
>  	struct page *page;
> +	int i;
> +	bool found = false;
>  

why use 'bool found'? just return `page', which will be either NULL
or !NULL?

	-ss

> -	page = class->fullness_list[ZS_ALMOST_EMPTY];
> -	if (page)
> -		remove_zspage(page, class, ZS_ALMOST_EMPTY);
> +	for (i = ZS_ALMOST_EMPTY; i >= ZS_ALMOST_FULL; i--) {
> +		page = class->fullness_list[i];
> +		if (!page)
> +			continue;
>  
> -	return page;
> +		remove_zspage(page, class, i);
> +		found = true;
> +		break;
> +	}
> +
> +	return found ? page : NULL;
>  }

	-ss
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

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

#1181181

FromMinchan Kim <minchan@kernel.org>
Date2015-07-10 04:40 +0200
Message-ID<pKwoh-8tG-5@gated-at.bofh.it>
In reply to#1181168
On Fri, Jul 10, 2015 at 11:06:24AM +0900, Sergey Senozhatsky wrote:
> On (07/10/15 10:32), Minchan Kim wrote:
> >  static struct page *isolate_source_page(struct size_class *class)
> >  {
> >  	struct page *page;
> > +	int i;
> > +	bool found = false;
> >  
> 
> why use 'bool found'? just return `page', which will be either NULL
> or !NULL?

It seems my old version which had a bug during test. :(
I will resend with the fix.

Thanks, Sergey!

> 
> 	-ss
> 
> > -	page = class->fullness_list[ZS_ALMOST_EMPTY];
> > -	if (page)
> > -		remove_zspage(page, class, ZS_ALMOST_EMPTY);
> > +	for (i = ZS_ALMOST_EMPTY; i >= ZS_ALMOST_FULL; i--) {
> > +		page = class->fullness_list[i];
> > +		if (!page)
> > +			continue;
> >  
> > -	return page;
> > +		remove_zspage(page, class, i);
> > +		found = true;
> > +		break;
> > +	}
> > +
> > +	return found ? page : NULL;
> >  }
> 
> 	-ss

-- 
Kind regards,
Minchan Kim
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web