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

Groups > linux.kernel > #1317796 > unrolled thread

[PATCH] zram: export the number of available comp streams

Back to article view | Back to linux.kernel

Contents

  [PATCH] zram: export the number of available comp streams Sergey Senozhatsky <sergey.senozhatsky@gmail.com> - 2016-01-26 13:10 +0100
    Re: [PATCH] zram: export the number of available comp streams Andrew Morton <akpm@linux-foundation.org> - 2016-01-26 22:20 +0100
      Re: [PATCH] zram: export the number of available comp streams Sergey Senozhatsky <sergey.senozhatsky.work@gmail.com> - 2016-01-27 01:40 +0100

#1317796 — [PATCH] zram: export the number of available comp streams

I've been asked several very simple questions:
a) How can I ensure that zram uses (or used) several compression
   streams?
b) What is the current number of comp streams (how much memory
   does zram *actually* use for compression streams, if there are
   more than one stream)?

zram, indeed, does not provide any info and does not answer
these questions. Reading from `max_comp_streams' let to estimate
only theoretical comp streams memory consumption, which assumes
that zram will allocate max_comp_streams. However, it's possible
that the real number of compression streams will never reach that
max value, due to various reasons, e.g. max_comp_streams is too
high, etc.

The patch adds `avail_streams' column to the /sys/block/zram<id>/mm_stat
device file. For a single compression stream backend it's always 1,
for a multi stream backend - it shows the actual ->avail_strm value.

The number of allocated compression streams answers several
questions:
a) the current `level of concurrency' that the device has
   experienced
b) the amount of memory used by compression streams (by multiplying
   the `avail_streams' column value, ->buffer size and algorithm's
   specific scratch buffer size; the last are easy to find out,
   unlike `avail_streams').

Signed-off-by: Sergey Senozhatsky <sergey.senozhatsky@gmail.com>
---
 Documentation/blockdev/zram.txt |  9 +++++++++
 drivers/block/zram/zcomp.c      | 24 ++++++++++++++++++++++++
 drivers/block/zram/zcomp.h      |  2 ++
 drivers/block/zram/zram_drv.c   |  7 +++++--
 4 files changed, 40 insertions(+), 2 deletions(-)

diff --git a/Documentation/blockdev/zram.txt b/Documentation/blockdev/zram.txt
index 5bda503..95bad79 100644
--- a/Documentation/blockdev/zram.txt
+++ b/Documentation/blockdev/zram.txt
@@ -227,6 +227,15 @@ line of text and contains the following stats separated by whitespace:
 	mem_used_max
 	zero_pages
 	num_migrated
+	avail_streams
+
+`avail_streams' column shows the current number of available compression
+streams, which is not necessarily equal to the number of max compression
+streams. The number of max compression streams can be set too high and be
+unreachable (depending on the load and the usage pattern, of course).
+`avail_streams' let to find out the real 'level of concurrency' that
+a particular zram device saw and to calculate the real memory consumption
+by allocated compression streams, not the theoretical maximum value.
 
 9) Deactivate:
 	swapoff /dev/zram0
diff --git a/drivers/block/zram/zcomp.c b/drivers/block/zram/zcomp.c
index 3ef42e5..83ee2a4 100644
--- a/drivers/block/zram/zcomp.c
+++ b/drivers/block/zram/zcomp.c
@@ -183,6 +183,18 @@ static bool zcomp_strm_multi_set_max_streams(struct zcomp *comp, int num_strm)
 	return true;
 }
 
+static int zcomp_strm_multi_num_avail_streams(struct zcomp *comp)
+{
+	int avail;
+	struct zcomp_strm_multi *zs = comp->stream;
+
+	spin_lock(&zs->strm_lock);
+	avail = zs->avail_strm;
+	spin_unlock(&zs->strm_lock);
+
+	return avail;
+}
+
 static void zcomp_strm_multi_destroy(struct zcomp *comp)
 {
 	struct zcomp_strm_multi *zs = comp->stream;
@@ -206,6 +218,7 @@ static int zcomp_strm_multi_create(struct zcomp *comp, int max_strm)
 	comp->strm_find = zcomp_strm_multi_find;
 	comp->strm_release = zcomp_strm_multi_release;
 	comp->set_max_streams = zcomp_strm_multi_set_max_streams;
+	comp->num_avail_streams = zcomp_strm_multi_num_avail_streams;
 	zs = kmalloc(sizeof(struct zcomp_strm_multi), GFP_KERNEL);
 	if (!zs)
 		return -ENOMEM;
@@ -246,6 +259,11 @@ static bool zcomp_strm_single_set_max_streams(struct zcomp *comp, int num_strm)
 	return false;
 }
 
+static int zcomp_strm_single_num_avail_streams(struct zcomp *comp)
+{
+	return 1;
+}
+
 static void zcomp_strm_single_destroy(struct zcomp *comp)
 {
 	struct zcomp_strm_single *zs = comp->stream;
@@ -261,6 +279,7 @@ static int zcomp_strm_single_create(struct zcomp *comp)
 	comp->strm_find = zcomp_strm_single_find;
 	comp->strm_release = zcomp_strm_single_release;
 	comp->set_max_streams = zcomp_strm_single_set_max_streams;
+	comp->num_avail_streams = zcomp_strm_single_num_avail_streams;
 	zs = kmalloc(sizeof(struct zcomp_strm_single), GFP_KERNEL);
 	if (!zs)
 		return -ENOMEM;
@@ -304,6 +323,11 @@ bool zcomp_set_max_streams(struct zcomp *comp, int num_strm)
 	return comp->set_max_streams(comp, num_strm);
 }
 
+int zcomp_num_avail_streams(struct zcomp *comp)
+{
+	return comp->num_avail_streams(comp);
+}
+
 struct zcomp_strm *zcomp_strm_find(struct zcomp *comp)
 {
 	return comp->strm_find(comp);
diff --git a/drivers/block/zram/zcomp.h b/drivers/block/zram/zcomp.h
index b7d2a4b..a69a3ca 100644
--- a/drivers/block/zram/zcomp.h
+++ b/drivers/block/zram/zcomp.h
@@ -47,6 +47,7 @@ struct zcomp {
 	struct zcomp_strm *(*strm_find)(struct zcomp *comp);
 	void (*strm_release)(struct zcomp *comp, struct zcomp_strm *zstrm);
 	bool (*set_max_streams)(struct zcomp *comp, int num_strm);
+	int (*num_avail_streams)(struct zcomp *comp);
 	void (*destroy)(struct zcomp *comp);
 };
 
@@ -66,4 +67,5 @@ int zcomp_decompress(struct zcomp *comp, const unsigned char *src,
 		size_t src_len, unsigned char *dst);
 
 bool zcomp_set_max_streams(struct zcomp *comp, int num_strm);
+int zcomp_num_avail_streams(struct zcomp *comp);
 #endif /* _ZCOMP_H_ */
diff --git a/drivers/block/zram/zram_drv.c b/drivers/block/zram/zram_drv.c
index 370c2f7..46055db 100644
--- a/drivers/block/zram/zram_drv.c
+++ b/drivers/block/zram/zram_drv.c
@@ -429,6 +429,7 @@ static ssize_t mm_stat_show(struct device *dev,
 	struct zs_pool_stats pool_stats;
 	u64 orig_size, mem_used = 0;
 	long max_used;
+	int avail_streams = 0;
 	ssize_t ret;
 
 	memset(&pool_stats, 0x00, sizeof(struct zs_pool_stats));
@@ -437,20 +438,22 @@ static ssize_t mm_stat_show(struct device *dev,
 	if (init_done(zram)) {
 		mem_used = zs_get_total_pages(zram->meta->mem_pool);
 		zs_pool_stats(zram->meta->mem_pool, &pool_stats);
+		avail_streams = zcomp_num_avail_streams(zram->comp);
 	}
 
 	orig_size = atomic64_read(&zram->stats.pages_stored);
 	max_used = atomic_long_read(&zram->stats.max_used_pages);
 
 	ret = scnprintf(buf, PAGE_SIZE,
-			"%8llu %8llu %8llu %8lu %8ld %8llu %8lu\n",
+			"%8llu %8llu %8llu %8lu %8ld %8llu %8lu %8d\n",
 			orig_size << PAGE_SHIFT,
 			(u64)atomic64_read(&zram->stats.compr_data_size),
 			mem_used << PAGE_SHIFT,
 			zram->limit_pages << PAGE_SHIFT,
 			max_used << PAGE_SHIFT,
 			(u64)atomic64_read(&zram->stats.zero_pages),
-			pool_stats.pages_compacted);
+			pool_stats.pages_compacted,
+			avail_streams);
 	up_read(&zram->init_lock);
 
 	return ret;
-- 
2.7.0.75.g3ee1e0f

[toc] | [next] | [standalone]

#1318395

On Tue, 26 Jan 2016 21:03:59 +0900 Sergey Senozhatsky <sergey.senozhatsky@gmail.com> wrote:

> I've been asked several very simple questions:
> a) How can I ensure that zram uses (or used) several compression
>    streams?
> b) What is the current number of comp streams (how much memory
>    does zram *actually* use for compression streams, if there are
>    more than one stream)?
> 
> zram, indeed, does not provide any info and does not answer
> these questions. Reading from `max_comp_streams' let to estimate
> only theoretical comp streams memory consumption, which assumes
> that zram will allocate max_comp_streams. However, it's possible
> that the real number of compression streams will never reach that
> max value, due to various reasons, e.g. max_comp_streams is too
> high, etc.
> 
> The patch adds `avail_streams' column to the /sys/block/zram<id>/mm_stat
> device file. For a single compression stream backend it's always 1,
> for a multi stream backend - it shows the actual ->avail_strm value.
> 
> The number of allocated compression streams answers several
> questions:
> a) the current `level of concurrency' that the device has
>    experienced
> b) the amount of memory used by compression streams (by multiplying
>    the `avail_streams' column value, ->buffer size and algorithm's
>    specific scratch buffer size; the last are easy to find out,
>    unlike `avail_streams').
> 
> --- a/Documentation/blockdev/zram.txt
> +++ b/Documentation/blockdev/zram.txt
> @@ -227,6 +227,15 @@ line of text and contains the following stats separated by whitespace:
>  	mem_used_max
>  	zero_pages
>  	num_migrated
> +	avail_streams
> +
> +`avail_streams' column shows the current number of available compression
> +streams, which is not necessarily equal to the number of max compression
> +streams. The number of max compression streams can be set too high and be
> +unreachable (depending on the load and the usage pattern, of course).
> +`avail_streams' let to find out the real 'level of concurrency' that
> +a particular zram device saw and to calculate the real memory consumption
> +by allocated compression streams, not the theoretical maximum value.
>

"number of max compression streams" doesn't make a lot of sense.  It
should be "max number of compression streams", yes"

--- a/Documentation/blockdev/zram.txt~zram-export-the-number-of-available-comp-streams-fix
+++ a/Documentation/blockdev/zram.txt
@@ -230,12 +230,13 @@ line of text and contains the following
 	avail_streams
 
 `avail_streams' column shows the current number of available compression
-streams, which is not necessarily equal to the number of max compression
-streams. The number of max compression streams can be set too high and be
-unreachable (depending on the load and the usage pattern, of course).
-`avail_streams' let to find out the real 'level of concurrency' that
-a particular zram device saw and to calculate the real memory consumption
-by allocated compression streams, not the theoretical maximum value.
+streams, which is not necessarily equal to the max number of compression
+streams.  The max number of compression streams can be set too high and
+can be unreachable (depending on the load and the usage pattern, of
+course).  `avail_streams' permits finding out the real 'level of
+concurrency' that a particular zram device saw and to calculate the real
+memory consumption by allocated compression streams, not the theoretical
+maximum value.
 
 9) Deactivate:
 	swapoff /dev/zram0

> ...
>
> --- a/drivers/block/zram/zcomp.c
> +++ b/drivers/block/zram/zcomp.c
> @@ -183,6 +183,18 @@ static bool zcomp_strm_multi_set_max_streams(struct zcomp *comp, int num_strm)
>  	return true;
>  }
>  
> +static int zcomp_strm_multi_num_avail_streams(struct zcomp *comp)
> +{
> +	int avail;
> +	struct zcomp_strm_multi *zs = comp->stream;
> +
> +	spin_lock(&zs->strm_lock);
> +	avail = zs->avail_strm;
> +	spin_unlock(&zs->strm_lock);
> +
> +	return avail;
> +}

The spin_lock() doesn't do anything very useful here - we're simply
reading an `int' and it could be omitted.  I guess it's OK for
documentary reasons (and perhaps for the memory barrier).

>
> ...
>

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

#1318501

On (01/26/16 13:13), Andrew Morton wrote:
[..]
> > +`avail_streams' column shows the current number of available compression
> > +streams, which is not necessarily equal to the number of max compression
> > +streams. The number of max compression streams can be set too high and be
> > +unreachable (depending on the load and the usage pattern, of course).
> > +`avail_streams' let to find out the real 'level of concurrency' that
> > +a particular zram device saw and to calculate the real memory consumption
> > +by allocated compression streams, not the theoretical maximum value.
> >
> 
> "number of max compression streams" doesn't make a lot of sense.  It
> should be "max number of compression streams", yes"

Thank you! much better this way.

[..]
> > +static int zcomp_strm_multi_num_avail_streams(struct zcomp *comp)
> > +{
> > +	int avail;
> > +	struct zcomp_strm_multi *zs = comp->stream;
> > +
> > +	spin_lock(&zs->strm_lock);
> > +	avail = zs->avail_strm;
> > +	spin_unlock(&zs->strm_lock);
> > +
> > +	return avail;
> > +}
> 
> The spin_lock() doesn't do anything very useful here - we're simply
> reading an `int' and it could be omitted.  I guess it's OK for
> documentary reasons (and perhaps for the memory barrier).

yes, agree, that was exactly my thinking. it's fine to have it here
for barrier, but at the same it can be scratched and replaced with
a "yes, this is racy. don't send a patch. `avail_streams' is not so
important" comment.

let's hear from Minchan, if he hates it then I'll just send a v2.

	-ss

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web