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

Groups > linux.kernel > #1182117 > unrolled thread

[PATCH 0/3] zsmalloc: small compaction improvements

Back to article view | Back to linux.kernel

Contents

  [PATCH 0/3] zsmalloc: small compaction improvements Sergey Senozhatsky <sergey.senozhatsky@gmail.com> - 2015-07-11 11:50 +0200
    [PATCH 1/3] zsmalloc: factor out zs_pages_to_compact() Sergey Senozhatsky <sergey.senozhatsky@gmail.com> - 2015-07-11 11:50 +0200
    [PATCH 3/3] zsmalloc: do not take class lock in zs_pages_to_compact() Sergey Senozhatsky <sergey.senozhatsky@gmail.com> - 2015-07-11 11:50 +0200
    [PATCH 2/3] zram: make compact a read-write sysfs node Sergey Senozhatsky <sergey.senozhatsky@gmail.com> - 2015-07-11 11:50 +0200
    Re: [PATCH 0/3] zsmalloc: small compaction improvements Minchan Kim <minchan@kernel.org> - 2015-07-15 02:30 +0200
      Re: [PATCH 0/3] zsmalloc: small compaction improvements Sergey Senozhatsky <sergey.senozhatsky@gmail.com> - 2015-07-15 13:20 +0200
    Re: [PATCH 0/3] zsmalloc: small compaction improvements Sergey Senozhatsky <sergey.senozhatsky.work@gmail.com> - 2015-07-15 02:30 +0200

#1182117 — [PATCH 0/3] zsmalloc: small compaction improvements

Hello,

First two patches introduce new zsmalloc zs_pages_to_compact()
symbol and change zram's `compact' sysfs attribute to be
read-write:
-- write triggers compaction, no changes
-- read returns the number of pages that compaction can
   potentially free

This lets user space to make a bit better decisions and to
avoid unneeded (which will not result in any significant
memory savings) compaction calls:

Example:

      if [ `cat /sys/block/zram<id>/compact` -gt 10 ]; then
          echo 1 > /sys/block/zram<id>/compact;
      fi

Up until now user space could not tell whether compaction
will result in any gain.

The third patch removes class locking around zs_can_compact()
in zs_pages_to_compact(), the motivation and details are
provided in the commit message.

Sergey Senozhatsky (3):
  zsmalloc: factor out zs_pages_to_compact()
  zram: make compact a read-write sysfs node
  zsmalloc: do not take class lock in zs_pages_to_compact()

 Documentation/ABI/testing/sysfs-block-zram |  7 +++---
 Documentation/blockdev/zram.txt            |  4 +++-
 drivers/block/zram/zram_drv.c              | 16 ++++++++++++-
 include/linux/zsmalloc.h                   |  1 +
 mm/zsmalloc.c                              | 37 +++++++++++++++++-------------
 5 files changed, 44 insertions(+), 21 deletions(-)

-- 
2.4.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]

#1182118 — [PATCH 1/3] zsmalloc: factor out zs_pages_to_compact()

Factor out the code that calculates how many pages compaction
can free into zs_pages_to_compact() function and export it
as zsmalloc API symbol. We still use it in zs_shrinker_count(),
just like we did before, and at the same time we now let zram
know this number (and provide it to user space) so user space
can make better assumptions about manual compaction effectiveness.

Signed-off-by: Sergey Senozhatsky <sergey.senozhatsky@gmail.com>
---
 include/linux/zsmalloc.h |  1 +
 mm/zsmalloc.c            | 39 +++++++++++++++++++++++----------------
 2 files changed, 24 insertions(+), 16 deletions(-)

diff --git a/include/linux/zsmalloc.h b/include/linux/zsmalloc.h
index 6398dfa..8f4de78 100644
--- a/include/linux/zsmalloc.h
+++ b/include/linux/zsmalloc.h
@@ -53,6 +53,7 @@ void zs_unmap_object(struct zs_pool *pool, unsigned long handle);
 
 unsigned long zs_get_total_pages(struct zs_pool *pool);
 unsigned long zs_compact(struct zs_pool *pool);
+unsigned long zs_pages_to_compact(struct zs_pool *pool);
 
 void zs_pool_stats(struct zs_pool *pool, struct zs_pool_stats *stats);
 #endif
diff --git a/mm/zsmalloc.c b/mm/zsmalloc.c
index c10885c..b10a228 100644
--- a/mm/zsmalloc.c
+++ b/mm/zsmalloc.c
@@ -1798,6 +1798,28 @@ void zs_pool_stats(struct zs_pool *pool, struct zs_pool_stats *stats)
 }
 EXPORT_SYMBOL_GPL(zs_pool_stats);
 
+unsigned long zs_pages_to_compact(struct zs_pool *pool)
+{
+	unsigned long pages_to_free = 0;
+	int i;
+	struct size_class *class;
+
+	for (i = zs_size_classes - 1; i >= 0; i--) {
+		class = pool->size_class[i];
+		if (!class)
+			continue;
+		if (class->index != i)
+			continue;
+
+		spin_lock(&class->lock);
+		pages_to_free += zs_can_compact(class);
+		spin_unlock(&class->lock);
+	}
+
+	return pages_to_free;
+}
+EXPORT_SYMBOL_GPL(zs_pages_to_compact);
+
 static unsigned long zs_shrinker_scan(struct shrinker *shrinker,
 		struct shrink_control *sc)
 {
@@ -1819,28 +1841,13 @@ static unsigned long zs_shrinker_scan(struct shrinker *shrinker,
 static unsigned long zs_shrinker_count(struct shrinker *shrinker,
 		struct shrink_control *sc)
 {
-	int i;
-	struct size_class *class;
-	unsigned long pages_to_free = 0;
 	struct zs_pool *pool = container_of(shrinker, struct zs_pool,
 			shrinker);
 
 	if (!pool->shrinker_enabled)
 		return 0;
 
-	for (i = zs_size_classes - 1; i >= 0; i--) {
-		class = pool->size_class[i];
-		if (!class)
-			continue;
-		if (class->index != i)
-			continue;
-
-		spin_lock(&class->lock);
-		pages_to_free += zs_can_compact(class);
-		spin_unlock(&class->lock);
-	}
-
-	return pages_to_free;
+	return zs_pages_to_compact(pool);
 }
 
 static void zs_unregister_shrinker(struct zs_pool *pool)
-- 
2.4.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] | [prev] | [next] | [standalone]

#1182119 — [PATCH 3/3] zsmalloc: do not take class lock in zs_pages_to_compact()

We can avoid taking class ->lock around zs_can_compact() in
zs_pages_to_compact(), because the number that we return back
is outdated in general case, by design. We have different
source that are able to change class's state right after we
return from zs_can_compact() -- ongoing IO operations, manually
triggered compaction or automatic compaction, or all three
simultaneously.

We re-do this calculations during compaction on a per class basis
anyway.

zs_unregister_shrinker() will not return until we have an active
shrinker, so classes won't unexpectedly disappear while
zs_pages_to_compact(), invoked by zs_shrinker_count(), iterates
them.

When called from zram, we are protected by zram's ->init_lock,
so, again, classes will be there until zs_pages_to_compact()
iterates them.

Signed-off-by: Sergey Senozhatsky <sergey.senozhatsky@gmail.com>
---
 mm/zsmalloc.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/mm/zsmalloc.c b/mm/zsmalloc.c
index b10a228..824c182 100644
--- a/mm/zsmalloc.c
+++ b/mm/zsmalloc.c
@@ -1811,9 +1811,7 @@ unsigned long zs_pages_to_compact(struct zs_pool *pool)
 		if (class->index != i)
 			continue;
 
-		spin_lock(&class->lock);
 		pages_to_free += zs_can_compact(class);
-		spin_unlock(&class->lock);
 	}
 
 	return pages_to_free;
-- 
2.4.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] | [prev] | [next] | [standalone]

#1182120 — [PATCH 2/3] zram: make compact a read-write sysfs node

Change zram's `compact' sysfs node to be a read-write attribute.
Write triggers zsmalloc compaction, just as before, read returns
the number of pages that zsmalloc can potentially compact.

User space now has a chance to estimate possible compaction memory
savings and avoid unnecessary compactions.

Example:

  if [ `cat /sys/block/zram<id>/compact` -gt 10 ]; then
      echo 1 > /sys/block/zram<id>/compact;
  fi

Signed-off-by: Sergey Senozhatsky <sergey.senozhatsky@gmail.com>
---
 Documentation/ABI/testing/sysfs-block-zram |  7 ++++---
 Documentation/blockdev/zram.txt            |  4 +++-
 drivers/block/zram/zram_drv.c              | 16 +++++++++++++++-
 3 files changed, 22 insertions(+), 5 deletions(-)

diff --git a/Documentation/ABI/testing/sysfs-block-zram b/Documentation/ABI/testing/sysfs-block-zram
index 2e69e83..0093998 100644
--- a/Documentation/ABI/testing/sysfs-block-zram
+++ b/Documentation/ABI/testing/sysfs-block-zram
@@ -146,9 +146,10 @@ What:		/sys/block/zram<id>/compact
 Date:		August 2015
 Contact:	Minchan Kim <minchan@kernel.org>
 Description:
-		The compact file is write-only and trigger compaction for
-		allocator zrm uses. The allocator moves some objects so that
-		it could free fragment space.
+		The compact file is read/write. Write triggers underlying
+		allocator's memory compaction, which may result in memory
+		savings. Read returns the number of pages that compaction
+		can potentially (but not guaranteed to) free.
 
 What:		/sys/block/zram<id>/io_stat
 Date:		August 2015
diff --git a/Documentation/blockdev/zram.txt b/Documentation/blockdev/zram.txt
index 62435bb..1854f62 100644
--- a/Documentation/blockdev/zram.txt
+++ b/Documentation/blockdev/zram.txt
@@ -146,7 +146,9 @@ mem_limit         RW    the maximum amount of memory ZRAM can use to store
                         the compressed data
 pages_compacted   RO    the number of pages freed during compaction
                         (available only via zram<id>/mm_stat node)
-compact           WO    trigger memory compaction
+compact           RW    write triggers memory compaction, read shows how many
+                        pages can potentially (but not necessarily will) be
+                        compacted
 
 WARNING
 =======
diff --git a/drivers/block/zram/zram_drv.c b/drivers/block/zram/zram_drv.c
index f5ef9e0..def9b8a 100644
--- a/drivers/block/zram/zram_drv.c
+++ b/drivers/block/zram/zram_drv.c
@@ -404,6 +404,20 @@ static ssize_t compact_store(struct device *dev,
 	return len;
 }
 
+static ssize_t compact_show(struct device *dev,
+		struct device_attribute *attr, char *buf)
+{
+	struct zram *zram = dev_to_zram(dev);
+	unsigned long num_pages = 0;
+
+	down_read(&zram->init_lock);
+	if (init_done(zram))
+		num_pages = zs_pages_to_compact(zram->meta->mem_pool);
+	up_read(&zram->init_lock);
+
+	return scnprintf(buf, PAGE_SIZE, "%lu\n", num_pages);
+}
+
 static ssize_t io_stat_show(struct device *dev,
 		struct device_attribute *attr, char *buf)
 {
@@ -1145,7 +1159,7 @@ static const struct block_device_operations zram_devops = {
 	.owner = THIS_MODULE
 };
 
-static DEVICE_ATTR_WO(compact);
+static DEVICE_ATTR_RW(compact);
 static DEVICE_ATTR_RW(disksize);
 static DEVICE_ATTR_RO(initstate);
 static DEVICE_ATTR_WO(reset);
-- 
2.4.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] | [prev] | [next] | [standalone]

#1184154

On Wed, Jul 15, 2015 at 09:21:06AM +0900, Sergey Senozhatsky wrote:
> On (07/15/15 01:52), Minchan Kim wrote:
> > > alrighty... again...
> > > 
> > > > > 
> > > > > /sys/block/zram<id>/compact is a black box. We provide it, we don't
> > > > > throttle it in the kernel, and user space is absolutely clueless when
> > > > > it invokes compaction. From some remote (or alternative) point of
> > > > 
> > > > But we have zs_can_compact so it can effectively skip the class if it
> > > > is not proper class.
> > > 
> > > user triggered compaction can compact too much.
> > > in its current state triggering a compaction from user space is like
> > > playing a lottery or a russian roulette.
> > 
> > We were on different page.
> 
> > I thought the motivation from this patchset is to prevent compaction
> > overhead by frequent user-driven compaction request because user
> > don't know how they can get free pages by compaction so they should
> > ask compact frequently with blind.
> 
> this is exactly the motivation for this patchset. seriously.

User should rely on the auto-compaction.

> 
> whatever.
> 
> 	-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]

#1184693

On (07/15/15 09:24), Minchan Kim wrote:
> On Wed, Jul 15, 2015 at 09:21:06AM +0900, Sergey Senozhatsky wrote:
> > On (07/15/15 01:52), Minchan Kim wrote:
> > > > alrighty... again...
> > > > 
> > > > > > 
> > > > > > /sys/block/zram<id>/compact is a black box. We provide it, we don't
> > > > > > throttle it in the kernel, and user space is absolutely clueless when
> > > > > > it invokes compaction. From some remote (or alternative) point of
> > > > > 
> > > > > But we have zs_can_compact so it can effectively skip the class if it
> > > > > is not proper class.
> > > > 
> > > > user triggered compaction can compact too much.
> > > > in its current state triggering a compaction from user space is like
> > > > playing a lottery or a russian roulette.
> > > 
> > > We were on different page.
> > 
> > > I thought the motivation from this patchset is to prevent compaction
> > > overhead by frequent user-driven compaction request because user
> > > don't know how they can get free pages by compaction so they should
> > > ask compact frequently with blind.
> > 
> > this is exactly the motivation for this patchset. seriously.
> 
> User should rely on the auto-compaction.

yep, which will be available in 5-6 months... right behind the corner.

	-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]

#1184157

On (07/15/15 01:52), Minchan Kim wrote:
> > alrighty... again...
> > 
> > > > 
> > > > /sys/block/zram<id>/compact is a black box. We provide it, we don't
> > > > throttle it in the kernel, and user space is absolutely clueless when
> > > > it invokes compaction. From some remote (or alternative) point of
> > > 
> > > But we have zs_can_compact so it can effectively skip the class if it
> > > is not proper class.
> > 
> > user triggered compaction can compact too much.
> > in its current state triggering a compaction from user space is like
> > playing a lottery or a russian roulette.
> 
> We were on different page.

> I thought the motivation from this patchset is to prevent compaction
> overhead by frequent user-driven compaction request because user
> don't know how they can get free pages by compaction so they should
> ask compact frequently with blind.

this is exactly the motivation for this patchset. seriously.

whatever.

	-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] | [standalone]

Back to top | Article view | linux.kernel

csiph-web