[PATCH 0/6] new zram statistics reporting scheme

[PATCH 0/6] new zram statistics reporting scheme

[Sergey Senozhatsky]

Hello,

This patch introduces rework to zram stats. We have per-stat sysfs
nodes, and it makes things a bit hard to use in user space: it doesn't
give an immediate stats 'snapshot', it requires user space to use
more syscals -- open, read, close for every stat file, with 
appropriate error checks on every step, etc.

First, zram now accounts block layer statistics. available in
/sys/block/zram<id>/stat and /proc/diskstats files. So some new
stats are available (see Documentation/block/stat.txt), besides,
zram's activities are now can be monitored by sysstat's iostat
or similar tools.

Example:
cat /sys/block/zram0/stat
248     0    1984    0   251029     0  2008232   5120   0   5116   5116



Second, group currently exported on per-stat basis nodes into two
categories (files):

-- zram<id>/io_stat
accumulates device's IO stats, that are not accounted by block layer,
and contains:
        failed_reads
        failed_writes
        invalid_io
        notify_free

Example:
cat /sys/block/zram0/io_stat
0        0        0   652572


-- zram<id>/mm_stat
accumulates zram mm stats and contains:
        orig_data_size
        compr_data_size
        mem_used_total
        mem_limit
        mem_used_max
        zero_pages
        num_migrated

Example:
cat /sys/block/zram0/mm_stat
434634752 270288572 279158784        0 579895296    15060        0


per-stat sysfs nodes are now considered to be deprecated and we plan
to remove them (and clean up some of the existing stat code) in two
years (as of now, there is no warning printed to syslog about deprecated
stats being used). user space is advised to use the above mentioned 3
files.


note:
util-linux mailing list is not Cc-ed into this series. once we settle
it down, I'll write to Karel. (we have several months ahead until 4.1
will be released).


Sergey Senozhatsky (6):
  zram: remove `num_migrated' device attr
  zram: move compact_store() to sysfs functions area
  zram: use generic start/end io accounting
  zram: describe device attrs in documentation
  zram: export new 'io_stat' sysfs attrs
  zram: export new 'mm_stat' sysfs attrs

 Documentation/ABI/testing/sysfs-block-zram |  18 +++--
 Documentation/blockdev/zram.txt            |  79 ++++++++++++++++++----
 drivers/block/zram/zram_drv.c              | 101 ++++++++++++++++++++++-------
 3 files changed, 157 insertions(+), 41 deletions(-)

-- 
2.3.2.209.gd67f9d5

[PATCH 1/6] zram: remove `num_migrated' device attr

[Sergey Senozhatsky]

Remove sysfs `num_migrated' attribute. We are moving away from
per-stat device attrs towards 3 stat files that will accumulate
io and mm stats in a format similar to block layer statistics in
/sys/block/<dev>/stat. That will be easier to use in user space,
and reduce the number of syscalls needed to read zram device
statistics.

`num_migrated' will return back in zram<id>/mm_stat file.

Signed-off-by: Sergey Senozhatsky <sergey.senozhatsky@gmail.com>
---
 Documentation/ABI/testing/sysfs-block-zram | 7 -------
 drivers/block/zram/zram_drv.c              | 2 --
 2 files changed, 9 deletions(-)

diff --git a/Documentation/ABI/testing/sysfs-block-zram b/Documentation/ABI/testing/sysfs-block-zram
index bede902..91ad707 100644
--- a/Documentation/ABI/testing/sysfs-block-zram
+++ b/Documentation/ABI/testing/sysfs-block-zram
@@ -149,10 +149,3 @@ 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.
-
-What:		/sys/block/zram<id>/num_migrated
-Date:		August 2015
-Contact:	Minchan Kim <minchan@kernel.org>
-Description:
-		The compact file is read-only and shows how many object
-		migrated by compaction.
diff --git a/drivers/block/zram/zram_drv.c b/drivers/block/zram/zram_drv.c
index 87f6671..2009a5a 100644
--- a/drivers/block/zram/zram_drv.c
+++ b/drivers/block/zram/zram_drv.c
@@ -392,7 +392,6 @@ ZRAM_ATTR_RO(invalid_io);
 ZRAM_ATTR_RO(notify_free);
 ZRAM_ATTR_RO(zero_pages);
 ZRAM_ATTR_RO(compr_data_size);
-ZRAM_ATTR_RO(num_migrated);
 
 static inline bool zram_meta_get(struct zram *zram)
 {
@@ -1069,7 +1068,6 @@ static struct attribute *zram_disk_attrs[] = {
 	&dev_attr_num_writes.attr,
 	&dev_attr_failed_reads.attr,
 	&dev_attr_failed_writes.attr,
-	&dev_attr_num_migrated.attr,
 	&dev_attr_compact.attr,
 	&dev_attr_invalid_io.attr,
 	&dev_attr_notify_free.attr,
-- 
2.3.2.209.gd67f9d5

Re: [PATCH 1/6] zram: remove `num_migrated' device attr

[Minchan Kim]

On Wed, Mar 11, 2015 at 12:08:29AM +0900, Sergey Senozhatsky wrote:
> Remove sysfs `num_migrated' attribute. We are moving away from
> per-stat device attrs towards 3 stat files that will accumulate
> io and mm stats in a format similar to block layer statistics in
> /sys/block/<dev>/stat. That will be easier to use in user space,
> and reduce the number of syscalls needed to read zram device
> statistics.
> 
> `num_migrated' will return back in zram<id>/mm_stat file.
> 
> Signed-off-by: Sergey Senozhatsky <sergey.senozhatsky@gmail.com>

Acked-by: Minchan Kim <minchan@kernel.org>

> ---
>  Documentation/ABI/testing/sysfs-block-zram | 7 -------
>  drivers/block/zram/zram_drv.c              | 2 --
>  2 files changed, 9 deletions(-)
> 
> diff --git a/Documentation/ABI/testing/sysfs-block-zram b/Documentation/ABI/testing/sysfs-block-zram
> index bede902..91ad707 100644
> --- a/Documentation/ABI/testing/sysfs-block-zram
> +++ b/Documentation/ABI/testing/sysfs-block-zram
> @@ -149,10 +149,3 @@ 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.
> -
> -What:		/sys/block/zram<id>/num_migrated
> -Date:		August 2015
> -Contact:	Minchan Kim <minchan@kernel.org>
> -Description:
> -		The compact file is read-only and shows how many object
                     ^^^^^

Argh, I believe you will correct it in later patch.

Thanks.

-- 
Kind regards,
Minchan Kim

Re: [PATCH 1/6] zram: remove `num_migrated' device attr

[Sergey Senozhatsky]

Hello,

On (03/12/15 10:16), Minchan Kim wrote:
> > -What:		/sys/block/zram<id>/num_migrated
> > -Date:		August 2015
> > -Contact:	Minchan Kim <minchan@kernel.org>
> > -Description:
> > -		The compact file is read-only and shows how many object
>                      ^^^^^
> 
> Argh, I believe you will correct it in later patch.
> 

it's documented as "num_migrated  --  the number of objects migrated by compaction"
in later patch.

	-ss

[PATCH 2/6] zram: move compact_store() to sysfs functions area

[Sergey Senozhatsky]

A cosmetic change. We have a new code layout and keep zram per-device
sysfs store and show functions in one place. Move compact_store() to
that handlers block to conform to current layout.

Signed-off-by: Sergey Senozhatsky <sergey.senozhatsky@gmail.com>
---
 drivers/block/zram/zram_drv.c | 42 +++++++++++++++++++++---------------------
 1 file changed, 21 insertions(+), 21 deletions(-)

diff --git a/drivers/block/zram/zram_drv.c b/drivers/block/zram/zram_drv.c
index 2009a5a..472c40c 100644
--- a/drivers/block/zram/zram_drv.c
+++ b/drivers/block/zram/zram_drv.c
@@ -67,27 +67,6 @@ static inline struct zram *dev_to_zram(struct device *dev)
 	return (struct zram *)dev_to_disk(dev)->private_data;
 }
 
-static ssize_t compact_store(struct device *dev,
-		struct device_attribute *attr, const char *buf, size_t len)
-{
-	unsigned long nr_migrated;
-	struct zram *zram = dev_to_zram(dev);
-	struct zram_meta *meta;
-
-	down_read(&zram->init_lock);
-	if (!init_done(zram)) {
-		up_read(&zram->init_lock);
-		return -EINVAL;
-	}
-
-	meta = zram->meta;
-	nr_migrated = zs_compact(meta->mem_pool);
-	atomic64_add(nr_migrated, &zram->stats.num_migrated);
-	up_read(&zram->init_lock);
-
-	return len;
-}
-
 /* flag operations require table entry bit_spin_lock() being held */
 static int zram_test_flag(struct zram_meta *meta, u32 index,
 			enum zram_pageflags flag)
@@ -384,6 +363,27 @@ static ssize_t comp_algorithm_store(struct device *dev,
 	return len;
 }
 
+static ssize_t compact_store(struct device *dev,
+		struct device_attribute *attr, const char *buf, size_t len)
+{
+	unsigned long nr_migrated;
+	struct zram *zram = dev_to_zram(dev);
+	struct zram_meta *meta;
+
+	down_read(&zram->init_lock);
+	if (!init_done(zram)) {
+		up_read(&zram->init_lock);
+		return -EINVAL;
+	}
+
+	meta = zram->meta;
+	nr_migrated = zs_compact(meta->mem_pool);
+	atomic64_add(nr_migrated, &zram->stats.num_migrated);
+	up_read(&zram->init_lock);
+
+	return len;
+}
+
 ZRAM_ATTR_RO(num_reads);
 ZRAM_ATTR_RO(num_writes);
 ZRAM_ATTR_RO(failed_reads);
-- 
2.3.2.209.gd67f9d5

Re: [PATCH 2/6] zram: move compact_store() to sysfs functions area

[Minchan Kim]

On Wed, Mar 11, 2015 at 12:08:30AM +0900, Sergey Senozhatsky wrote:
> A cosmetic change. We have a new code layout and keep zram per-device
> sysfs store and show functions in one place. Move compact_store() to
> that handlers block to conform to current layout.
> 
> Signed-off-by: Sergey Senozhatsky <sergey.senozhatsky@gmail.com>
Acked-by: Minchan Kim <minchan@kernel.org>

I should have done by myself.

Thanks, Sergey!

-- 
Kind regards,
Minchan Kim

[PATCH 3/6] zram: use generic start/end io accounting

[Sergey Senozhatsky]

Use bio generic_start_io_acct() and generic_end_io_acct() to account
device's block layer statistics. This will let users to monitor zram
activities using sysstat and similar packages/tools.

Apart from the usual per-stat sysfs attr, zram IO stats are now also
available in '/sys/block/zram<id>/stat' and '/proc/diskstats' files.

We will slowly get rid of per-stat sysfs files.

Signed-off-by: Sergey Senozhatsky <sergey.senozhatsky@gmail.com>
---
 drivers/block/zram/zram_drv.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/drivers/block/zram/zram_drv.c b/drivers/block/zram/zram_drv.c
index 472c40c..45319eb9 100644
--- a/drivers/block/zram/zram_drv.c
+++ b/drivers/block/zram/zram_drv.c
@@ -743,8 +743,12 @@ static void zram_bio_discard(struct zram *zram, u32 index,
 static int zram_bvec_rw(struct zram *zram, struct bio_vec *bvec, u32 index,
 			int offset, int rw)
 {
+	unsigned long start_time = jiffies;
 	int ret;
 
+	generic_start_io_acct(rw, bvec->bv_len >> SECTOR_SHIFT,
+			&zram->disk->part0);
+
 	if (rw == READ) {
 		atomic64_inc(&zram->stats.num_reads);
 		ret = zram_bvec_read(zram, bvec, index, offset);
@@ -753,6 +757,8 @@ static int zram_bvec_rw(struct zram *zram, struct bio_vec *bvec, u32 index,
 		ret = zram_bvec_write(zram, bvec, index, offset);
 	}
 
+	generic_end_io_acct(rw, &zram->disk->part0, start_time);
+
 	if (unlikely(ret)) {
 		if (rw == READ)
 			atomic64_inc(&zram->stats.failed_reads);
-- 
2.3.2.209.gd67f9d5

Re: [PATCH 3/6] zram: use generic start/end io accounting

[Minchan Kim]

On Wed, Mar 11, 2015 at 12:08:31AM +0900, Sergey Senozhatsky wrote:
> Use bio generic_start_io_acct() and generic_end_io_acct() to account
> device's block layer statistics. This will let users to monitor zram
> activities using sysstat and similar packages/tools.

Yay, Thanks!

> 
> Apart from the usual per-stat sysfs attr, zram IO stats are now also
> available in '/sys/block/zram<id>/stat' and '/proc/diskstats' files.
> 
> We will slowly get rid of per-stat sysfs files.
> 
> Signed-off-by: Sergey Senozhatsky <sergey.senozhatsky@gmail.com>
Acked-by: Minchan Kim <minchan@kernel.org>

-- 
Kind regards,
Minchan Kim

[PATCH 4/6] zram: describe device attrs in documentation

[Sergey Senozhatsky]

Briefly describe exported device stat attrs in zram documentation.
We will eventually get rid of per-stat sysfs nodes and, thus,
clean up Documentation/ABI/testing/sysfs-block-zram file, which is
the only source of information about device sysfs nodes.

Add `num_migrated' description, since there is no independent
`num_migrated' sysfs node (and no corresponding sysfs-block-zram
entry), it will be exported via zram<id>/mm_stat file.

At this point we can provide minimal attrs description, because
sysfs-block-zram still contains detailed information.

Signed-off-by: Sergey Senozhatsky <sergey.senozhatsky@gmail.com>
---
 Documentation/blockdev/zram.txt | 49 +++++++++++++++++++++++++++++------------
 1 file changed, 35 insertions(+), 14 deletions(-)

diff --git a/Documentation/blockdev/zram.txt b/Documentation/blockdev/zram.txt
index 902c97c..149b49a 100644
--- a/Documentation/blockdev/zram.txt
+++ b/Documentation/blockdev/zram.txt
@@ -117,20 +117,41 @@ execute
 	echo X > /sys/class/zram-control/zram_remove
 
 8) Stats:
-	Per-device statistics are exported as various nodes under
-	/sys/block/zram<id>/
-		disksize
-		num_reads
-		num_writes
-		failed_reads
-		failed_writes
-		invalid_io
-		notify_free
-		zero_pages
-		orig_data_size
-		compr_data_size
-		mem_used_total
-		mem_used_max
+Per-device statistics are exported as various nodes under /sys/block/zram<id>/
+
+A brief description of exported device attritbutes. For more details please
+read Documentation/ABI/testing/sysfs-block-zram.
+
+Name             mode             description
+----            ------            -----------
+disksize          RO    disk size
+initstate         RO    shows the initialization state of the device
+reset             WO    trigger device reset
+num_reads         RO    the number of reads
+failed_reads      RO    the number of failed reads
+num_write         RO    the number of writes
+failed_writes     RO    the number of failed writes
+invalid_io        RO    the number of non-page-size-aligned I/O requests
+max_comp_streams  RW    the number of possible concurrent compress operations
+comp_algorithm    RW    show and change the compression algorithm
+notify_free       RO    the number of notifications to free pages (either
+                        slot free notifications or REQ_DISCARD requests)
+zero_pages        RO    the number of zero filled pages written to this disk
+orig_data_size    RO    uncompressed size of data stored in this disk
+compr_data_size   RO    compressed size of data stored in this disk
+mem_used_total    RO    the amount of memory allocated for this disk
+mem_used_max      RW    the maximum amount of memory zram have consumed to
+                        store compressed data
+mem_limit         RW    the maximum amount of memory ZRAM can use to store
+                        the compressed data
+num_migrated      RO    the number of objects migrated by compaction
+compact           WO    trigger memory compaction
+
+
+File /sys/block/zram<id>/stat
+
+Represents block layer statistics. Read Documentation/block/stat.txt for
+details.
 
 9) Deactivate:
 	swapoff /dev/zram0
-- 
2.3.2.209.gd67f9d5

Re: [PATCH 4/6] zram: describe device attrs in documentation

[Minchan Kim]

On Wed, Mar 11, 2015 at 12:08:32AM +0900, Sergey Senozhatsky wrote:
> Briefly describe exported device stat attrs in zram documentation.
> We will eventually get rid of per-stat sysfs nodes and, thus,
> clean up Documentation/ABI/testing/sysfs-block-zram file, which is
> the only source of information about device sysfs nodes.
> 
> Add `num_migrated' description, since there is no independent
> `num_migrated' sysfs node (and no corresponding sysfs-block-zram
> entry), it will be exported via zram<id>/mm_stat file.
> 
> At this point we can provide minimal attrs description, because
> sysfs-block-zram still contains detailed information.
> 
> Signed-off-by: Sergey Senozhatsky <sergey.senozhatsky@gmail.com>

FYI, git-am got failed based on next-2015311 but patch tool worked.

> ---
>  Documentation/blockdev/zram.txt | 49 +++++++++++++++++++++++++++++------------
>  1 file changed, 35 insertions(+), 14 deletions(-)
> 
> diff --git a/Documentation/blockdev/zram.txt b/Documentation/blockdev/zram.txt
> index 902c97c..149b49a 100644
> --- a/Documentation/blockdev/zram.txt
> +++ b/Documentation/blockdev/zram.txt
> @@ -117,20 +117,41 @@ execute
>  	echo X > /sys/class/zram-control/zram_remove
>  
>  8) Stats:
> -	Per-device statistics are exported as various nodes under
> -	/sys/block/zram<id>/
> -		disksize
> -		num_reads
> -		num_writes
> -		failed_reads
> -		failed_writes
> -		invalid_io
> -		notify_free
> -		zero_pages
> -		orig_data_size
> -		compr_data_size
> -		mem_used_total
> -		mem_used_max
> +Per-device statistics are exported as various nodes under /sys/block/zram<id>/
> +
> +A brief description of exported device attritbutes. For more details please
> +read Documentation/ABI/testing/sysfs-block-zram.
> +
> +Name             mode             description
> +----            ------            -----------
> +disksize          RO    disk size

disksize is RW

> +initstate         RO    shows the initialization state of the device
> +reset             WO    trigger device reset
> +num_reads         RO    the number of reads
> +failed_reads      RO    the number of failed reads
> +num_write         RO    the number of writes
> +failed_writes     RO    the number of failed writes
> +invalid_io        RO    the number of non-page-size-aligned I/O requests
> +max_comp_streams  RW    the number of possible concurrent compress operations
> +comp_algorithm    RW    show and change the compression algorithm
> +notify_free       RO    the number of notifications to free pages (either
> +                        slot free notifications or REQ_DISCARD requests)
> +zero_pages        RO    the number of zero filled pages written to this disk
> +orig_data_size    RO    uncompressed size of data stored in this disk
> +compr_data_size   RO    compressed size of data stored in this disk
> +mem_used_total    RO    the amount of memory allocated for this disk
> +mem_used_max      RW    the maximum amount of memory zram have consumed to
> +                        store compressed data
> +mem_limit         RW    the maximum amount of memory ZRAM can use to store
> +                        the compressed data
> +num_migrated      RO    the number of objects migrated by compaction
> +compact           WO    trigger memory compaction

Otherwise, looks cleaner!
Thanks!
 

-- 
Kind regards,
Minchan Kim

Re: [PATCH 4/6] zram: describe device attrs in documentation

[Sergey Senozhatsky]

On (03/12/15 10:33), Minchan Kim wrote:
> FYI, git-am got failed based on next-2015311 but patch tool worked.
> 

hm... oh, do you have this one applied

	From: Sergey Senozhatsky <sergey.senozhatsky@gmail.com>
	Subject: zram: do not let user enforce new device dev_id

	This patch forbids user to enforce device ids for newly added zram
	devices, as suggested by Minchan Kim.  There seems to be a little interest
	in this functionality and its use-cases are rather non-obvious.
	[..]

?

it does some documentation "rollbacks"

     Documentation/blockdev/zram.txt            |   20 +-

that is probably the reason.



> > +----            ------            -----------
> > +disksize          RO    disk size
> 
> disksize is RW
> 

ah, good catch! will send a trivial patch later.


Thanks for review!

	-ss

[PATCH 5/6] zram: export new 'io_stat' sysfs attrs

[Sergey Senozhatsky]

Per-device `zram<id>/io_stat' file provides accumulated I/O statistics
of particular zram device in a format similar to block layer statistics.
The file consists of a single line and represents the following stats
(separated by whitespace):
	failed_reads
	failed_writes
	invalid_io
	notify_free

Signed-off-by: Sergey Senozhatsky <sergey.senozhatsky@gmail.com>
---
 Documentation/ABI/testing/sysfs-block-zram |  9 +++++++++
 Documentation/blockdev/zram.txt            | 12 ++++++++++++
 drivers/block/zram/zram_drv.c              | 20 ++++++++++++++++++++
 3 files changed, 41 insertions(+)

diff --git a/Documentation/ABI/testing/sysfs-block-zram b/Documentation/ABI/testing/sysfs-block-zram
index 91ad707..a7f622f 100644
--- a/Documentation/ABI/testing/sysfs-block-zram
+++ b/Documentation/ABI/testing/sysfs-block-zram
@@ -149,3 +149,12 @@ 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.
+
+What:		/sys/block/zram<id>/io_stat
+Date:		August 2015
+Contact:	Sergey Senozhatsky <sergey.senozhatsky@gmail.com>
+Description:
+		The io_stat file is read-only and accumulates device's I/O
+		statistics not accounted by block layer. For example,
+		failed_reads, failed_writes, etc. File format is similar to
+		block layer statistics file format.
diff --git a/Documentation/blockdev/zram.txt b/Documentation/blockdev/zram.txt
index 149b49a..eb62792 100644
--- a/Documentation/blockdev/zram.txt
+++ b/Documentation/blockdev/zram.txt
@@ -153,6 +153,18 @@ File /sys/block/zram<id>/stat
 Represents block layer statistics. Read Documentation/block/stat.txt for
 details.
 
+
+File /sys/block/zram<id>/io_stat
+
+The stat file represents device's I/O statistics not accounted by block
+layer and, thus, not available in zram<id>/stat file. It consists of a
+single line of text and contains the following stats separated by
+whitespace:
+	failed_reads
+	failed_writes
+	invalid_io
+	notify_free
+
 9) Deactivate:
 	swapoff /dev/zram0
 	umount /dev/zram1
diff --git a/drivers/block/zram/zram_drv.c b/drivers/block/zram/zram_drv.c
index 45319eb9..c02121f 100644
--- a/drivers/block/zram/zram_drv.c
+++ b/drivers/block/zram/zram_drv.c
@@ -384,6 +384,25 @@ static ssize_t compact_store(struct device *dev,
 	return len;
 }
 
+static ssize_t io_stat_show(struct device *dev,
+		struct device_attribute *attr, char *buf)
+{
+	struct zram *zram = dev_to_zram(dev);
+	ssize_t ret;
+
+	down_read(&zram->init_lock);
+	ret = scnprintf(buf, PAGE_SIZE,
+			"%8llu %8llu %8llu %8llu\n",
+			(u64)atomic64_read(&zram->stats.failed_reads),
+			(u64)atomic64_read(&zram->stats.failed_writes),
+			(u64)atomic64_read(&zram->stats.invalid_io),
+			(u64)atomic64_read(&zram->stats.notify_free));
+	up_read(&zram->init_lock);
+
+	return ret;
+}
+
+static DEVICE_ATTR_RO(io_stat);
 ZRAM_ATTR_RO(num_reads);
 ZRAM_ATTR_RO(num_writes);
 ZRAM_ATTR_RO(failed_reads);
@@ -1085,6 +1104,7 @@ static struct attribute *zram_disk_attrs[] = {
 	&dev_attr_mem_used_max.attr,
 	&dev_attr_max_comp_streams.attr,
 	&dev_attr_comp_algorithm.attr,
+	&dev_attr_io_stat.attr,
 	NULL,
 };
 
-- 
2.3.2.209.gd67f9d5

Re: [PATCH 5/6] zram: export new 'io_stat' sysfs attrs

[Minchan Kim]

On Wed, Mar 11, 2015 at 12:08:33AM +0900, Sergey Senozhatsky wrote:
> Per-device `zram<id>/io_stat' file provides accumulated I/O statistics
> of particular zram device in a format similar to block layer statistics.
> The file consists of a single line and represents the following stats
> (separated by whitespace):
> 	failed_reads
> 	failed_writes
> 	invalid_io
> 	notify_free
> 
> Signed-off-by: Sergey Senozhatsky <sergey.senozhatsky@gmail.com>
Acked-by: Minchan Kim <minchan@kernel.org>

-- 
Kind regards,
Minchan Kim

[PATCH 6/6] zram: export new 'mm_stat' sysfs attrs

[Sergey Senozhatsky]

Per-device `zram<id>/mm_stat' file provides accumulated mm statistics
of particular zram device in a format similar to block layer statistics.
The file consists of a single line and represents the following stats
(separated by whitespace):
	orig_data_size
	compr_data_size
	mem_used_total
	mem_limit
	mem_used_max
	zero_pages
	num_migrated

Since now we have three stat files (block layer zram<id>/stat,
zram<id>/io_stat and zram<id>/mm_stat) document WARNING about
per-stat sysfs nodes being deprecated.

Signed-off-by: Sergey Senozhatsky <sergey.senozhatsky@gmail.com>
---
 Documentation/ABI/testing/sysfs-block-zram |  8 ++++++++
 Documentation/blockdev/zram.txt            | 18 +++++++++++++++++
 drivers/block/zram/zram_drv.c              | 31 ++++++++++++++++++++++++++++++
 3 files changed, 57 insertions(+)

diff --git a/Documentation/ABI/testing/sysfs-block-zram b/Documentation/ABI/testing/sysfs-block-zram
index a7f622f..8114c81 100644
--- a/Documentation/ABI/testing/sysfs-block-zram
+++ b/Documentation/ABI/testing/sysfs-block-zram
@@ -158,3 +158,11 @@ Description:
 		statistics not accounted by block layer. For example,
 		failed_reads, failed_writes, etc. File format is similar to
 		block layer statistics file format.
+
+What:		/sys/block/zram<id>/mm_stat
+Date:		August 2015
+Contact:	Sergey Senozhatsky <sergey.senozhatsky@gmail.com>
+Description:
+		The mm_stat file is read-only and accumulates device's mm
+		statistics (orig_data_size, compr_data_size, etc.) in a format
+		similar to block layer statistics file format.
diff --git a/Documentation/blockdev/zram.txt b/Documentation/blockdev/zram.txt
index eb62792..faf6270 100644
--- a/Documentation/blockdev/zram.txt
+++ b/Documentation/blockdev/zram.txt
@@ -148,6 +148,10 @@ num_migrated      RO    the number of objects migrated by compaction
 compact           WO    trigger memory compaction
 
 
+WARNING, per-stat sysfs attributes are considered to be deprecated and will
+eventually be removed. User space is advised to use the following files to
+read the device statistics.
+
 File /sys/block/zram<id>/stat
 
 Represents block layer statistics. Read Documentation/block/stat.txt for
@@ -165,6 +169,20 @@ whitespace:
 	invalid_io
 	notify_free
 
+
+File /sys/block/zram<id>/mm_stat
+
+The stat file represents device's mm statistics. It consists of a single
+line of text and contains the following stats separated by whitespace:
+	orig_data_size
+	compr_data_size
+	mem_used_total
+	mem_limit
+	mem_used_max
+	zero_pages
+	num_migrated
+
+
 9) Deactivate:
 	swapoff /dev/zram0
 	umount /dev/zram1
diff --git a/drivers/block/zram/zram_drv.c b/drivers/block/zram/zram_drv.c
index c02121f..7493096 100644
--- a/drivers/block/zram/zram_drv.c
+++ b/drivers/block/zram/zram_drv.c
@@ -402,7 +402,37 @@ static ssize_t io_stat_show(struct device *dev,
 	return ret;
 }
 
+static ssize_t mm_stat_show(struct device *dev,
+		struct device_attribute *attr, char *buf)
+{
+	struct zram *zram = dev_to_zram(dev);
+	u64 orig_size, mem_used = 0;
+	long max_used;
+	ssize_t ret;
+
+	down_read(&zram->init_lock);
+	if (init_done(zram))
+		mem_used = zs_get_total_pages(zram->meta->mem_pool);
+
+	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 %8llu\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),
+			(u64)atomic64_read(&zram->stats.num_migrated));
+	up_read(&zram->init_lock);
+
+	return ret;
+}
+
 static DEVICE_ATTR_RO(io_stat);
+static DEVICE_ATTR_RO(mm_stat);
 ZRAM_ATTR_RO(num_reads);
 ZRAM_ATTR_RO(num_writes);
 ZRAM_ATTR_RO(failed_reads);
@@ -1105,6 +1135,7 @@ static struct attribute *zram_disk_attrs[] = {
 	&dev_attr_max_comp_streams.attr,
 	&dev_attr_comp_algorithm.attr,
 	&dev_attr_io_stat.attr,
+	&dev_attr_mm_stat.attr,
 	NULL,
 };
 
-- 
2.3.2.209.gd67f9d5

Re: [PATCH 6/6] zram: export new 'mm_stat' sysfs attrs

[Minchan Kim]

On Wed, Mar 11, 2015 at 12:08:34AM +0900, Sergey Senozhatsky wrote:
> Per-device `zram<id>/mm_stat' file provides accumulated mm statistics
> of particular zram device in a format similar to block layer statistics.
> The file consists of a single line and represents the following stats
> (separated by whitespace):
> 	orig_data_size
> 	compr_data_size
> 	mem_used_total
> 	mem_limit
> 	mem_used_max
> 	zero_pages
> 	num_migrated
> 
> Since now we have three stat files (block layer zram<id>/stat,
> zram<id>/io_stat and zram<id>/mm_stat) document WARNING about
> per-stat sysfs nodes being deprecated.

Any user doesn't take care of document. I think we should add
pr_warn_once to notify the user if he tried deprecated interface.
In addition, we should add deprecated interface in Documentation/ABI/obsolete/

> 
> Signed-off-by: Sergey Senozhatsky <sergey.senozhatsky@gmail.com>
> ---
>  Documentation/ABI/testing/sysfs-block-zram |  8 ++++++++
>  Documentation/blockdev/zram.txt            | 18 +++++++++++++++++
>  drivers/block/zram/zram_drv.c              | 31 ++++++++++++++++++++++++++++++
>  3 files changed, 57 insertions(+)
> 
> diff --git a/Documentation/ABI/testing/sysfs-block-zram b/Documentation/ABI/testing/sysfs-block-zram
> index a7f622f..8114c81 100644
> --- a/Documentation/ABI/testing/sysfs-block-zram
> +++ b/Documentation/ABI/testing/sysfs-block-zram
> @@ -158,3 +158,11 @@ Description:
>  		statistics not accounted by block layer. For example,
>  		failed_reads, failed_writes, etc. File format is similar to
>  		block layer statistics file format.
> +
> +What:		/sys/block/zram<id>/mm_stat
> +Date:		August 2015
> +Contact:	Sergey Senozhatsky <sergey.senozhatsky@gmail.com>
> +Description:
> +		The mm_stat file is read-only and accumulates device's mm
> +		statistics (orig_data_size, compr_data_size, etc.) in a format

Every field in mm_stat doesn't mean accumulation.

-- 
Kind regards,
Minchan Kim

Re: [PATCH 6/6] zram: export new 'mm_stat' sysfs attrs

[Sergey Senozhatsky]

On (03/12/15 10:41), Minchan Kim wrote:
> Any user doesn't take care of document. I think we should add
> pr_warn_once to notify the user if he tried deprecated interface.

yes, that was something I didn't want to include into this patch.
I agree that some sort of a warning should go into the logs. do
you want me to add pr_warn_once() to every _show() function?

having this in documentation will not do any harm, so let's keep it
there as well.

> In addition, we should add deprecated interface in Documentation/ABI/obsolete/
> 

didn't know about that. ok, will send "deprecate old attrs" patch that will
add deprecated attrs to Documentation/ABI/obsolete/ and add pr_warn_once().


> > 
> > Signed-off-by: Sergey Senozhatsky <sergey.senozhatsky@gmail.com>
> > ---
> >  Documentation/ABI/testing/sysfs-block-zram |  8 ++++++++
> >  Documentation/blockdev/zram.txt            | 18 +++++++++++++++++
> >  drivers/block/zram/zram_drv.c              | 31 ++++++++++++++++++++++++++++++
> >  3 files changed, 57 insertions(+)
> > 
> > diff --git a/Documentation/ABI/testing/sysfs-block-zram b/Documentation/ABI/testing/sysfs-block-zram
> > index a7f622f..8114c81 100644
> > --- a/Documentation/ABI/testing/sysfs-block-zram
> > +++ b/Documentation/ABI/testing/sysfs-block-zram
> > @@ -158,3 +158,11 @@ Description:
> >  		statistics not accounted by block layer. For example,
> >  		failed_reads, failed_writes, etc. File format is similar to
> >  		block layer statistics file format.
> > +
> > +What:		/sys/block/zram<id>/mm_stat
> > +Date:		August 2015
> > +Contact:	Sergey Senozhatsky <sergey.senozhatsky@gmail.com>
> > +Description:
> > +		The mm_stat file is read-only and accumulates device's mm
> > +		statistics (orig_data_size, compr_data_size, etc.) in a format
> 
> Every field in mm_stat doesn't mean accumulation.
> 

agree. bad wording. only num_migrated shows the accumulated number.
will send a trivial patch later.

	-ss

Re: [PATCH 0/6] new zram statistics reporting scheme

[Minchan Kim]

Hi Sergey,

On Wed, Mar 11, 2015 at 12:08:28AM +0900, Sergey Senozhatsky wrote:
> Hello,
> 
> This patch introduces rework to zram stats. We have per-stat sysfs
> nodes, and it makes things a bit hard to use in user space: it doesn't
> give an immediate stats 'snapshot', it requires user space to use
> more syscals -- open, read, close for every stat file, with 
> appropriate error checks on every step, etc.
> 
> First, zram now accounts block layer statistics. available in
> /sys/block/zram<id>/stat and /proc/diskstats files. So some new
> stats are available (see Documentation/block/stat.txt), besides,
> zram's activities are now can be monitored by sysstat's iostat
> or similar tools.
> 
> Example:
> cat /sys/block/zram0/stat
> 248     0    1984    0   251029     0  2008232   5120   0   5116   5116
> 
> 
> 
> Second, group currently exported on per-stat basis nodes into two
> categories (files):
> 
> -- zram<id>/io_stat
> accumulates device's IO stats, that are not accounted by block layer,
> and contains:
>         failed_reads
>         failed_writes
>         invalid_io
>         notify_free
> 
> Example:
> cat /sys/block/zram0/io_stat
> 0        0        0   652572
> 
> 
> -- zram<id>/mm_stat
> accumulates zram mm stats and contains:
>         orig_data_size
>         compr_data_size
>         mem_used_total
>         mem_limit
>         mem_used_max
>         zero_pages
>         num_migrated
> 
> Example:
> cat /sys/block/zram0/mm_stat
> 434634752 270288572 279158784        0 579895296    15060        0
> 
> 
> per-stat sysfs nodes are now considered to be deprecated and we plan
> to remove them (and clean up some of the existing stat code) in two
> years (as of now, there is no warning printed to syslog about deprecated
> stats being used). user space is advised to use the above mentioned 3
> files.
> 
> 
> note:
> util-linux mailing list is not Cc-ed into this series. once we settle
> it down, I'll write to Karel. (we have several months ahead until 4.1
> will be released).

I really appreciate you enhance stat functions, esp, working with iostat!

One thing I want to discuss is sometime we could remove RO fields
in /sys/block/zram/ but we couldn't remove RW fields because
io_stat/mm_stat doesn't have any writable option now so users will
have two options to read stat. For exmaple,

        cat /sys/block/zram/mem_used_max
        cat /sys/block/zram/mm_stat | awk friend

How about changing only writeable, not readable for duplicated stats
in /sys/block/zram? So, user will have writeable stat to set some
options in /sys/block/zram and readable stat to get some data in
/sys/block/zram/[io|mm]_stat if the stat is duplicated in both.


-- 
Kind regards,
Minchan Kim

Re: [PATCH 0/6] new zram statistics reporting scheme

[Sergey Senozhatsky]

On (03/12/15 10:55), Minchan Kim wrote:
> I really appreciate you enhance stat functions, esp, working with iostat!

thanks! my pleasure.

> One thing I want to discuss is sometime we could remove RO fields
> in /sys/block/zram/ but we couldn't remove RW fields because
> io_stat/mm_stat doesn't have any writable option now so users will
> have two options to read stat. For exmaple,

I played with CONFIG_ZRAM_OLD_STATS option, which turns RW attrs into
WO attrs (where possible/needed). but it turned out to be a rather
ugly patch and I eventually decided that I don't want to have these
#ifdef-s in zram code for the next two years. so providing both RW/RO
old stats (with a warning in the logs) and RO [mm|io]_stat sound like
a better plan.

>         cat /sys/block/zram/mem_used_max
>         cat /sys/block/zram/mm_stat | awk friend
> 
> How about changing only writeable, not readable for duplicated stats
> in /sys/block/zram? So, user will have writeable stat to set some
> options in /sys/block/zram and readable stat to get some data in
> /sys/block/zram/[io|mm]_stat if the stat is duplicated in both.

Sorry, I probably didn't drink enough coffee today, can you please
rephrase or give a trivial example?

		-ss

Re: [PATCH 0/6] new zram statistics reporting scheme

[Sergey Senozhatsky]

On (03/12/15 11:16), Sergey Senozhatsky wrote:
> >         cat /sys/block/zram/mem_used_max
> >         cat /sys/block/zram/mm_stat | awk friend
> > 
> > How about changing only writeable, not readable for duplicated stats
> > in /sys/block/zram? So, user will have writeable stat to set some
> > options in /sys/block/zram and readable stat to get some data in
> > /sys/block/zram/[io|mm]_stat if the stat is duplicated in both.
> 

did you mean:

-- attrs that are currently RW will eventually turn into WO. attr show()
   will be done by [mm|io]_stat file.

-- attrs that are currently RO will be removed. attr show() will be
   done by [mm|io]_stat file.

?

so, iow, rather than removing all duplicated attrs we remove only RO attrs
and keep previously RW attrs in write-only mode.

	-ss

Re: [PATCH 0/6] new zram statistics reporting scheme

[Minchan Kim]

On Thu, Mar 12, 2015 at 11:16:09AM +0900, Sergey Senozhatsky wrote:
> On (03/12/15 10:55), Minchan Kim wrote:
> > I really appreciate you enhance stat functions, esp, working with iostat!
> 
> thanks! my pleasure.
> 
> > One thing I want to discuss is sometime we could remove RO fields
> > in /sys/block/zram/ but we couldn't remove RW fields because
> > io_stat/mm_stat doesn't have any writable option now so users will
> > have two options to read stat. For exmaple,
> 
> I played with CONFIG_ZRAM_OLD_STATS option, which turns RW attrs into
> WO attrs (where possible/needed). but it turned out to be a rather
> ugly patch and I eventually decided that I don't want to have these
> #ifdef-s in zram code for the next two years. so providing both RW/RO
> old stats (with a warning in the logs) and RO [mm|io]_stat sound like
> a better plan.

I think we don't need CONFIG_ZRAM_OLD_STATS.
For example, for mem_used_max, we could add pr_warn_once in
*mem_used_max_show* but not *mem_used_max_store*. so, old users will see
deprecated message when they try to *read* the vaule old stat while they
will write new value.
One or two year later, we could remove mem_used_max_show then everyone
should read the vaule /sys/block/zram0/mm_stat.

> 
> >         cat /sys/block/zram/mem_used_max
> >         cat /sys/block/zram/mm_stat | awk friend
> > 
> > How about changing only writeable, not readable for duplicated stats
> > in /sys/block/zram? So, user will have writeable stat to set some
> > options in /sys/block/zram and readable stat to get some data in
> > /sys/block/zram/[io|mm]_stat if the stat is duplicated in both.
> 
> Sorry, I probably didn't drink enough coffee today, can you please
> rephrase or give a trivial example?

Sorry, I was vague. I meant users can read the vaule for
mem_used_max and mem_limit in two places(ie, /sys/block/zram/
and /sys/block/zram/mm_stat). If we feel it's handy for user,
i am not against that. But if there is no reason, I want to
make /sys/block/zram/ stat write-only if it is possible stat
like mem_used_max. I guess It's easier rule for user.


> 
> 		-ss

-- 
Kind regards,
Minchan Kim

Re: [PATCH 0/6] new zram statistics reporting scheme

[Sergey Senozhatsky]

On (03/12/15 14:11), Minchan Kim wrote:
> I think we don't need CONFIG_ZRAM_OLD_STATS.
> For example, for mem_used_max, we could add pr_warn_once in
> *mem_used_max_show* but not *mem_used_max_store*. so, old users will see
> deprecated message when they try to *read* the vaule old stat while they
> will write new value.
> One or two year later, we could remove mem_used_max_show then everyone
> should read the vaule /sys/block/zram0/mm_stat.
> 
> > 
> > >         cat /sys/block/zram/mem_used_max
> > >         cat /sys/block/zram/mm_stat | awk friend
> > > 
> > > How about changing only writeable, not readable for duplicated stats
> > > in /sys/block/zram? So, user will have writeable stat to set some
> > > options in /sys/block/zram and readable stat to get some data in
> > > /sys/block/zram/[io|mm]_stat if the stat is duplicated in both.
> > 
> > Sorry, I probably didn't drink enough coffee today, can you please
> > rephrase or give a trivial example?
> 
> Sorry, I was vague. I meant users can read the vaule for
> mem_used_max and mem_limit in two places(ie, /sys/block/zram/
> and /sys/block/zram/mm_stat). If we feel it's handy for user,
> i am not against that. But if there is no reason, I want to
> make /sys/block/zram/ stat write-only if it is possible stat
> like mem_used_max. I guess It's easier rule for user.

in a long term -- yes. duplication must go away. we provide [mm|io]_stat
nodes to do stats show.

so,
-- RW will be downgraded to WO
-- RO will be removed

(I'm talking about those attrs that are currently visible in two
places -- the original per-stat sysfs node and new [mm|io]_stat node).


and this is what CONFIG_ZRAM_OLD_STATS was doing: downgrade of RW to WO,
removal of some RO attrs. but I'd prefer to go with warn once plan and
do this downgrade/removal manually two years later.

	-ss