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

[Andrew Morton <akpm@linux-foundation.org>]

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).

>
> ...
>