linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCHv3 0/8] zram stats rework and code cleanup
@ 2014-01-16 13:12 Sergey Senozhatsky
  2014-01-16 13:12 ` [PATCHv3 1/8] zram: drop `init_done' struct zram member Sergey Senozhatsky
                   ` (8 more replies)
  0 siblings, 9 replies; 22+ messages in thread
From: Sergey Senozhatsky @ 2014-01-16 13:12 UTC (permalink / raw)
  To: Andrew Morton
  Cc: Minchan Kim, Jerome Marchand, Nitin Gupta, linux-kernel,
	Sergey Senozhatsky

This patchset includes zram stats clean up and enhancements.

Sergey Senozhatsky (8):
  zram: drop `init_done' struct zram member
  zram: do not pass rw argument to __zram_make_request()
  zram: remove good and bad compress stats
  zram: use atomic64_t for all zram stats
  zram: remove zram stats code duplication
  zram: report failed read and write stats
  zram: drop not used table `count' member
  zram: move zram size warning to documentation

 Documentation/blockdev/zram.txt |   5 ++
 drivers/block/zram/zram_drv.c   | 175 +++++++++++++---------------------------
 drivers/block/zram/zram_drv.h   |  10 +--
 3 files changed, 64 insertions(+), 126 deletions(-)

-- 
1.8.5.3.493.gb139ac2


^ permalink raw reply	[flat|nested] 22+ messages in thread

* [PATCHv3 1/8] zram: drop `init_done' struct zram member
  2014-01-16 13:12 [PATCHv3 0/8] zram stats rework and code cleanup Sergey Senozhatsky
@ 2014-01-16 13:12 ` Sergey Senozhatsky
  2014-01-16 13:12 ` [PATCHv3 2/8] zram: do not pass rw argument to __zram_make_request() Sergey Senozhatsky
                   ` (7 subsequent siblings)
  8 siblings, 0 replies; 22+ messages in thread
From: Sergey Senozhatsky @ 2014-01-16 13:12 UTC (permalink / raw)
  To: Andrew Morton
  Cc: Minchan Kim, Jerome Marchand, Nitin Gupta, linux-kernel,
	Sergey Senozhatsky

Introduce init_done() helper function which allows us to drop
`init_done' struct zram member. init_done() uses the fact that
->init_done == 1 equals to ->meta != NULL.

Acked-by: Minchan Kim <minchan@kernel.org>
Acked-by: Jerome Marchand <jmarchan@redhat.com>
Signed-off-by: Sergey Senozhatsky <sergey.senozhatsky@gmail.com>
---
 drivers/block/zram/zram_drv.c | 21 +++++++++++----------
 drivers/block/zram/zram_drv.h |  1 -
 2 files changed, 11 insertions(+), 11 deletions(-)

diff --git a/drivers/block/zram/zram_drv.c b/drivers/block/zram/zram_drv.c
index 011e55d..c323e05 100644
--- a/drivers/block/zram/zram_drv.c
+++ b/drivers/block/zram/zram_drv.c
@@ -42,6 +42,11 @@ static struct zram *zram_devices;
 /* Module params (documentation at end) */
 static unsigned int num_devices = 1;
 
+static inline int init_done(struct zram *zram)
+{
+	return zram->meta != NULL;
+}
+
 static inline struct zram *dev_to_zram(struct device *dev)
 {
 	return (struct zram *)dev_to_disk(dev)->private_data;
@@ -60,7 +65,7 @@ static ssize_t initstate_show(struct device *dev,
 {
 	struct zram *zram = dev_to_zram(dev);
 
-	return sprintf(buf, "%u\n", zram->init_done);
+	return sprintf(buf, "%u\n", init_done(zram));
 }
 
 static ssize_t num_reads_show(struct device *dev,
@@ -133,7 +138,7 @@ static ssize_t mem_used_total_show(struct device *dev,
 	struct zram_meta *meta = zram->meta;
 
 	down_read(&zram->init_lock);
-	if (zram->init_done)
+	if (init_done(zram))
 		val = zs_get_total_size_bytes(meta->mem_pool);
 	up_read(&zram->init_lock);
 
@@ -546,14 +551,12 @@ static void zram_reset_device(struct zram *zram, bool reset_capacity)
 	struct zram_meta *meta;
 
 	down_write(&zram->init_lock);
-	if (!zram->init_done) {
+	if (!init_done(zram)) {
 		up_write(&zram->init_lock);
 		return;
 	}
 
 	meta = zram->meta;
-	zram->init_done = 0;
-
 	/* Free all pages that are still in this zram device */
 	for (index = 0; index < zram->disksize >> PAGE_SHIFT; index++) {
 		unsigned long handle = meta->table[index].handle;
@@ -594,8 +597,6 @@ static void zram_init_device(struct zram *zram, struct zram_meta *meta)
 	queue_flag_set_unlocked(QUEUE_FLAG_NONROT, zram->disk->queue);
 
 	zram->meta = meta;
-	zram->init_done = 1;
-
 	pr_debug("Initialization done!\n");
 }
 
@@ -613,7 +614,7 @@ static ssize_t disksize_store(struct device *dev,
 	disksize = PAGE_ALIGN(disksize);
 	meta = zram_meta_alloc(disksize);
 	down_write(&zram->init_lock);
-	if (zram->init_done) {
+	if (init_done(zram)) {
 		up_write(&zram->init_lock);
 		zram_meta_free(meta);
 		pr_info("Cannot change disksize for initialized device\n");
@@ -734,7 +735,7 @@ static void zram_make_request(struct request_queue *queue, struct bio *bio)
 	struct zram *zram = queue->queuedata;
 
 	down_read(&zram->init_lock);
-	if (unlikely(!zram->init_done))
+	if (unlikely(!init_done(zram)))
 		goto error;
 
 	if (!valid_io_request(zram, bio)) {
@@ -857,7 +858,7 @@ static int create_device(struct zram *zram, int device_id)
 		goto out_free_disk;
 	}
 
-	zram->init_done = 0;
+	zram->meta = NULL;
 	return 0;
 
 out_free_disk:
diff --git a/drivers/block/zram/zram_drv.h b/drivers/block/zram/zram_drv.h
index ad8aa35..e81e9cd 100644
--- a/drivers/block/zram/zram_drv.h
+++ b/drivers/block/zram/zram_drv.h
@@ -95,7 +95,6 @@ struct zram {
 	struct zram_meta *meta;
 	struct request_queue *queue;
 	struct gendisk *disk;
-	int init_done;
 	/* Prevent concurrent execution of device init, reset and R/W request */
 	struct rw_semaphore init_lock;
 	/*
-- 
1.8.5.3.493.gb139ac2


^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [PATCHv3 2/8] zram: do not pass rw argument to __zram_make_request()
  2014-01-16 13:12 [PATCHv3 0/8] zram stats rework and code cleanup Sergey Senozhatsky
  2014-01-16 13:12 ` [PATCHv3 1/8] zram: drop `init_done' struct zram member Sergey Senozhatsky
@ 2014-01-16 13:12 ` Sergey Senozhatsky
  2014-01-16 13:12 ` [PATCHv3 3/8] zram: remove good and bad compress stats Sergey Senozhatsky
                   ` (6 subsequent siblings)
  8 siblings, 0 replies; 22+ messages in thread
From: Sergey Senozhatsky @ 2014-01-16 13:12 UTC (permalink / raw)
  To: Andrew Morton
  Cc: Minchan Kim, Jerome Marchand, Nitin Gupta, linux-kernel,
	Sergey Senozhatsky

Do not pass rw argument down the __zram_make_request() -> zram_bvec_rw()
chain, decode it in zram_bvec_rw() instead. Besides, this is the place
where we distinguish READ and WRITE bio data directions, so account zram
RW stats here, instead of __zram_make_request(). This also allows to account
a real number of zram READ/WRITE operations, not just requests (single RW
request may cause a number of zram RW ops with separate locking,
compression/decompression, etc).

v2: do not check for READA return code. noted by Jerome Marchand.

Acked-by: Minchan Kim <minchan@kernel.org>
Acked-by: Jerome Marchand <jmarchan@redhat.com>
Signed-off-by: Sergey Senozhatsky <sergey.senozhatsky@gmail.com>
---
 drivers/block/zram/zram_drv.c | 30 ++++++++++++------------------
 1 file changed, 12 insertions(+), 18 deletions(-)

diff --git a/drivers/block/zram/zram_drv.c b/drivers/block/zram/zram_drv.c
index c323e05..b0bcb7e 100644
--- a/drivers/block/zram/zram_drv.c
+++ b/drivers/block/zram/zram_drv.c
@@ -533,14 +533,18 @@ out:
 }
 
 static int zram_bvec_rw(struct zram *zram, struct bio_vec *bvec, u32 index,
-			int offset, struct bio *bio, int rw)
+			int offset, struct bio *bio)
 {
 	int ret;
+	int rw = bio_data_dir(bio);
 
-	if (rw == READ)
+	if (rw == READ) {
+		atomic64_inc(&zram->stats.num_reads);
 		ret = zram_bvec_read(zram, bvec, index, offset, bio);
-	else
+	} else {
+		atomic64_inc(&zram->stats.num_writes);
 		ret = zram_bvec_write(zram, bvec, index, offset);
+	}
 
 	return ret;
 }
@@ -670,22 +674,13 @@ out:
 	return ret;
 }
 
-static void __zram_make_request(struct zram *zram, struct bio *bio, int rw)
+static void __zram_make_request(struct zram *zram, struct bio *bio)
 {
 	int offset;
 	u32 index;
 	struct bio_vec bvec;
 	struct bvec_iter iter;
 
-	switch (rw) {
-	case READ:
-		atomic64_inc(&zram->stats.num_reads);
-		break;
-	case WRITE:
-		atomic64_inc(&zram->stats.num_writes);
-		break;
-	}
-
 	index = bio->bi_iter.bi_sector >> SECTORS_PER_PAGE_SHIFT;
 	offset = (bio->bi_iter.bi_sector &
 		  (SECTORS_PER_PAGE - 1)) << SECTOR_SHIFT;
@@ -704,16 +699,15 @@ static void __zram_make_request(struct zram *zram, struct bio *bio, int rw)
 			bv.bv_len = max_transfer_size;
 			bv.bv_offset = bvec.bv_offset;
 
-			if (zram_bvec_rw(zram, &bv, index, offset, bio, rw) < 0)
+			if (zram_bvec_rw(zram, &bv, index, offset, bio) < 0)
 				goto out;
 
 			bv.bv_len = bvec.bv_len - max_transfer_size;
 			bv.bv_offset += max_transfer_size;
-			if (zram_bvec_rw(zram, &bv, index+1, 0, bio, rw) < 0)
+			if (zram_bvec_rw(zram, &bv, index + 1, 0, bio) < 0)
 				goto out;
 		} else
-			if (zram_bvec_rw(zram, &bvec, index, offset, bio, rw)
-			    < 0)
+			if (zram_bvec_rw(zram, &bvec, index, offset, bio) < 0)
 				goto out;
 
 		update_position(&index, &offset, &bvec);
@@ -743,7 +737,7 @@ static void zram_make_request(struct request_queue *queue, struct bio *bio)
 		goto error;
 	}
 
-	__zram_make_request(zram, bio, bio_data_dir(bio));
+	__zram_make_request(zram, bio);
 	up_read(&zram->init_lock);
 
 	return;
-- 
1.8.5.3.493.gb139ac2


^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [PATCHv3 3/8] zram: remove good and bad compress stats
  2014-01-16 13:12 [PATCHv3 0/8] zram stats rework and code cleanup Sergey Senozhatsky
  2014-01-16 13:12 ` [PATCHv3 1/8] zram: drop `init_done' struct zram member Sergey Senozhatsky
  2014-01-16 13:12 ` [PATCHv3 2/8] zram: do not pass rw argument to __zram_make_request() Sergey Senozhatsky
@ 2014-01-16 13:12 ` Sergey Senozhatsky
  2014-01-16 13:44   ` Jerome Marchand
  2014-01-17  6:48   ` Minchan Kim
  2014-01-16 13:12 ` [PATCHv3 4/8] zram: use atomic64_t for all zram stats Sergey Senozhatsky
                   ` (5 subsequent siblings)
  8 siblings, 2 replies; 22+ messages in thread
From: Sergey Senozhatsky @ 2014-01-16 13:12 UTC (permalink / raw)
  To: Andrew Morton
  Cc: Minchan Kim, Jerome Marchand, Nitin Gupta, linux-kernel,
	Sergey Senozhatsky

Remove `good' and `bad' compressed sub-requests stats. RW request may
cause a number of RW sub-requests. zram used to account `good' compressed
sub-queries (with compressed size less than 50% of original size), `bad'
compressed sub-queries (with compressed size greater that 75% of original
size), leaving sub-requests with compression size between 50% and 75% of
original size not accounted and not reported. zram already accounts each
sub-request's compression size so we can calculate real device compression
ratio.

Signed-off-by: Sergey Senozhatsky <sergey.senozhatsky@gmail.com>
---
 drivers/block/zram/zram_drv.c | 11 -----------
 drivers/block/zram/zram_drv.h |  2 --
 2 files changed, 13 deletions(-)

diff --git a/drivers/block/zram/zram_drv.c b/drivers/block/zram/zram_drv.c
index b0bcb7e..c7c7789 100644
--- a/drivers/block/zram/zram_drv.c
+++ b/drivers/block/zram/zram_drv.c
@@ -293,7 +293,6 @@ static void zram_free_page(struct zram *zram, size_t index)
 {
 	struct zram_meta *meta = zram->meta;
 	unsigned long handle = meta->table[index].handle;
-	u16 size = meta->table[index].size;
 
 	if (unlikely(!handle)) {
 		/*
@@ -307,14 +306,8 @@ static void zram_free_page(struct zram *zram, size_t index)
 		return;
 	}
 
-	if (unlikely(size > max_zpage_size))
-		atomic_dec(&zram->stats.bad_compress);
-
 	zs_free(meta->mem_pool, handle);
 
-	if (size <= PAGE_SIZE / 2)
-		atomic_dec(&zram->stats.good_compress);
-
 	atomic64_sub(meta->table[index].size, &zram->stats.compr_size);
 	atomic_dec(&zram->stats.pages_stored);
 
@@ -478,7 +471,6 @@ static int zram_bvec_write(struct zram *zram, struct bio_vec *bvec, u32 index,
 	}
 
 	if (unlikely(clen > max_zpage_size)) {
-		atomic_inc(&zram->stats.bad_compress);
 		clen = PAGE_SIZE;
 		src = NULL;
 		if (is_partial_io(bvec))
@@ -518,9 +510,6 @@ static int zram_bvec_write(struct zram *zram, struct bio_vec *bvec, u32 index,
 	/* Update stats */
 	atomic64_add(clen, &zram->stats.compr_size);
 	atomic_inc(&zram->stats.pages_stored);
-	if (clen <= PAGE_SIZE / 2)
-		atomic_inc(&zram->stats.good_compress);
-
 out:
 	if (locked)
 		mutex_unlock(&meta->buffer_lock);
diff --git a/drivers/block/zram/zram_drv.h b/drivers/block/zram/zram_drv.h
index e81e9cd..2f173cb 100644
--- a/drivers/block/zram/zram_drv.h
+++ b/drivers/block/zram/zram_drv.h
@@ -78,8 +78,6 @@ struct zram_stats {
 	atomic64_t notify_free;	/* no. of swap slot free notifications */
 	atomic_t pages_zero;		/* no. of zero filled pages */
 	atomic_t pages_stored;	/* no. of pages currently stored */
-	atomic_t good_compress;	/* % of pages with compression ratio<=50% */
-	atomic_t bad_compress;	/* % of pages with compression ratio>=75% */
 };
 
 struct zram_meta {
-- 
1.8.5.3.493.gb139ac2


^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [PATCHv3 4/8] zram: use atomic64_t for all zram stats
  2014-01-16 13:12 [PATCHv3 0/8] zram stats rework and code cleanup Sergey Senozhatsky
                   ` (2 preceding siblings ...)
  2014-01-16 13:12 ` [PATCHv3 3/8] zram: remove good and bad compress stats Sergey Senozhatsky
@ 2014-01-16 13:12 ` Sergey Senozhatsky
  2014-01-16 13:58   ` Jerome Marchand
  2014-01-17  6:54   ` Minchan Kim
  2014-01-16 13:12 ` [PATCHv3 5/8] zram: remove zram stats code duplication Sergey Senozhatsky
                   ` (4 subsequent siblings)
  8 siblings, 2 replies; 22+ messages in thread
From: Sergey Senozhatsky @ 2014-01-16 13:12 UTC (permalink / raw)
  To: Andrew Morton
  Cc: Minchan Kim, Jerome Marchand, Nitin Gupta, linux-kernel,
	Sergey Senozhatsky

This is a preparation patch for stats code duplication removal.

1) use atomic64_t for `pages_zero' and `pages_stored' zram stats.
2) `compr_size' and `pages_zero' struct zram_stats members did not
follow the existing device attr naming scheme: zram_stats.ATTR has
ATTR_show() function. rename them:
-- compr_size -> compr_data_size
-- pages_zero -> zero_pages

Minchan Kim's note:
 If we really have trouble with atomic stat operation, we could
 change it with percpu_counter so that it could solve atomic overhead and
 unnecessary memory space by introducing unsigned long instead of 64bit
 atomic_t.

Signed-off-by: Sergey Senozhatsky <sergey.senozhatsky@gmail.com>
---
 drivers/block/zram/zram_drv.c | 18 +++++++++---------
 drivers/block/zram/zram_drv.h |  6 +++---
 2 files changed, 12 insertions(+), 12 deletions(-)

diff --git a/drivers/block/zram/zram_drv.c b/drivers/block/zram/zram_drv.c
index c7c7789..0035d39 100644
--- a/drivers/block/zram/zram_drv.c
+++ b/drivers/block/zram/zram_drv.c
@@ -109,7 +109,7 @@ static ssize_t zero_pages_show(struct device *dev,
 {
 	struct zram *zram = dev_to_zram(dev);
 
-	return sprintf(buf, "%u\n", atomic_read(&zram->stats.pages_zero));
+	return sprintf(buf, "%llu\n", (u64)atomic64_read(&zram->stats.zero_pages));
 }
 
 static ssize_t orig_data_size_show(struct device *dev,
@@ -118,7 +118,7 @@ static ssize_t orig_data_size_show(struct device *dev,
 	struct zram *zram = dev_to_zram(dev);
 
 	return sprintf(buf, "%llu\n",
-		(u64)(atomic_read(&zram->stats.pages_stored)) << PAGE_SHIFT);
+		(u64)(atomic64_read(&zram->stats.pages_stored)) << PAGE_SHIFT);
 }
 
 static ssize_t compr_data_size_show(struct device *dev,
@@ -127,7 +127,7 @@ static ssize_t compr_data_size_show(struct device *dev,
 	struct zram *zram = dev_to_zram(dev);
 
 	return sprintf(buf, "%llu\n",
-			(u64)atomic64_read(&zram->stats.compr_size));
+			(u64)atomic64_read(&zram->stats.compr_data_size));
 }
 
 static ssize_t mem_used_total_show(struct device *dev,
@@ -301,15 +301,15 @@ static void zram_free_page(struct zram *zram, size_t index)
 		 */
 		if (zram_test_flag(meta, index, ZRAM_ZERO)) {
 			zram_clear_flag(meta, index, ZRAM_ZERO);
-			atomic_dec(&zram->stats.pages_zero);
+			atomic64_dec(&zram->stats.zero_pages);
 		}
 		return;
 	}
 
 	zs_free(meta->mem_pool, handle);
 
-	atomic64_sub(meta->table[index].size, &zram->stats.compr_size);
-	atomic_dec(&zram->stats.pages_stored);
+	atomic64_sub(meta->table[index].size, &zram->stats.compr_data_size);
+	atomic64_dec(&zram->stats.pages_stored);
 
 	meta->table[index].handle = 0;
 	meta->table[index].size = 0;
@@ -452,7 +452,7 @@ static int zram_bvec_write(struct zram *zram, struct bio_vec *bvec, u32 index,
 		zram_set_flag(meta, index, ZRAM_ZERO);
 		write_unlock(&zram->meta->tb_lock);
 
-		atomic_inc(&zram->stats.pages_zero);
+		atomic64_inc(&zram->stats.zero_pages);
 		ret = 0;
 		goto out;
 	}
@@ -508,8 +508,8 @@ static int zram_bvec_write(struct zram *zram, struct bio_vec *bvec, u32 index,
 	write_unlock(&zram->meta->tb_lock);
 
 	/* Update stats */
-	atomic64_add(clen, &zram->stats.compr_size);
-	atomic_inc(&zram->stats.pages_stored);
+	atomic64_add(clen, &zram->stats.compr_data_size);
+	atomic64_inc(&zram->stats.pages_stored);
 out:
 	if (locked)
 		mutex_unlock(&meta->buffer_lock);
diff --git a/drivers/block/zram/zram_drv.h b/drivers/block/zram/zram_drv.h
index 2f173cb..58d4ac5 100644
--- a/drivers/block/zram/zram_drv.h
+++ b/drivers/block/zram/zram_drv.h
@@ -69,15 +69,15 @@ struct table {
 } __aligned(4);
 
 struct zram_stats {
-	atomic64_t compr_size;	/* compressed size of pages stored */
+	atomic64_t compr_data_size;	/* compressed size of pages stored */
 	atomic64_t num_reads;	/* failed + successful */
 	atomic64_t num_writes;	/* --do-- */
 	atomic64_t failed_reads;	/* should NEVER! happen */
 	atomic64_t failed_writes;	/* can happen when memory is too low */
 	atomic64_t invalid_io;	/* non-page-aligned I/O requests */
 	atomic64_t notify_free;	/* no. of swap slot free notifications */
-	atomic_t pages_zero;		/* no. of zero filled pages */
-	atomic_t pages_stored;	/* no. of pages currently stored */
+	atomic64_t zero_pages;		/* no. of zero filled pages */
+	atomic64_t pages_stored;	/* no. of pages currently stored */
 };
 
 struct zram_meta {
-- 
1.8.5.3.493.gb139ac2


^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [PATCHv3 5/8] zram: remove zram stats code duplication
  2014-01-16 13:12 [PATCHv3 0/8] zram stats rework and code cleanup Sergey Senozhatsky
                   ` (3 preceding siblings ...)
  2014-01-16 13:12 ` [PATCHv3 4/8] zram: use atomic64_t for all zram stats Sergey Senozhatsky
@ 2014-01-16 13:12 ` Sergey Senozhatsky
  2014-01-17  6:54   ` Minchan Kim
  2014-01-16 13:12 ` [PATCHv3 6/8] zram: report failed read and write stats Sergey Senozhatsky
                   ` (3 subsequent siblings)
  8 siblings, 1 reply; 22+ messages in thread
From: Sergey Senozhatsky @ 2014-01-16 13:12 UTC (permalink / raw)
  To: Andrew Morton
  Cc: Minchan Kim, Jerome Marchand, Nitin Gupta, linux-kernel,
	Sergey Senozhatsky

Introduce ZRAM_ATTR_RO macro that generates device_attribute
and default ATTR show() function for existing atomic64_t zram
stats.

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

diff --git a/drivers/block/zram/zram_drv.c b/drivers/block/zram/zram_drv.c
index 0035d39..e7102f1 100644
--- a/drivers/block/zram/zram_drv.c
+++ b/drivers/block/zram/zram_drv.c
@@ -42,6 +42,17 @@ static struct zram *zram_devices;
 /* Module params (documentation at end) */
 static unsigned int num_devices = 1;
 
+#define ZRAM_ATTR_RO(name)						\
+static ssize_t zram_attr_##name##_show(struct device *d,		\
+				struct device_attribute *attr, char *b)	\
+{									\
+	struct zram *zram = dev_to_zram(d);				\
+	return sprintf(b, "%llu\n",					\
+		(u64)atomic64_read(&zram->stats.name));			\
+}									\
+static struct device_attribute dev_attr_##name =			\
+	__ATTR(name, S_IRUGO, zram_attr_##name##_show, NULL);
+
 static inline int init_done(struct zram *zram)
 {
 	return zram->meta != NULL;
@@ -63,53 +74,14 @@ static ssize_t disksize_show(struct device *dev,
 static ssize_t initstate_show(struct device *dev,
 		struct device_attribute *attr, char *buf)
 {
+	u32 val;
 	struct zram *zram = dev_to_zram(dev);
 
-	return sprintf(buf, "%u\n", init_done(zram));
-}
-
-static ssize_t num_reads_show(struct device *dev,
-		struct device_attribute *attr, char *buf)
-{
-	struct zram *zram = dev_to_zram(dev);
-
-	return sprintf(buf, "%llu\n",
-			(u64)atomic64_read(&zram->stats.num_reads));
-}
-
-static ssize_t num_writes_show(struct device *dev,
-		struct device_attribute *attr, char *buf)
-{
-	struct zram *zram = dev_to_zram(dev);
-
-	return sprintf(buf, "%llu\n",
-			(u64)atomic64_read(&zram->stats.num_writes));
-}
-
-static ssize_t invalid_io_show(struct device *dev,
-		struct device_attribute *attr, char *buf)
-{
-	struct zram *zram = dev_to_zram(dev);
-
-	return sprintf(buf, "%llu\n",
-			(u64)atomic64_read(&zram->stats.invalid_io));
-}
-
-static ssize_t notify_free_show(struct device *dev,
-		struct device_attribute *attr, char *buf)
-{
-	struct zram *zram = dev_to_zram(dev);
-
-	return sprintf(buf, "%llu\n",
-			(u64)atomic64_read(&zram->stats.notify_free));
-}
-
-static ssize_t zero_pages_show(struct device *dev,
-		struct device_attribute *attr, char *buf)
-{
-	struct zram *zram = dev_to_zram(dev);
+	down_read(&zram->init_lock);
+	val = init_done(zram);
+	up_read(&zram->init_lock);
 
-	return sprintf(buf, "%llu\n", (u64)atomic64_read(&zram->stats.zero_pages));
+	return sprintf(buf, "%u\n", val);
 }
 
 static ssize_t orig_data_size_show(struct device *dev,
@@ -121,15 +93,6 @@ static ssize_t orig_data_size_show(struct device *dev,
 		(u64)(atomic64_read(&zram->stats.pages_stored)) << PAGE_SHIFT);
 }
 
-static ssize_t compr_data_size_show(struct device *dev,
-		struct device_attribute *attr, char *buf)
-{
-	struct zram *zram = dev_to_zram(dev);
-
-	return sprintf(buf, "%llu\n",
-			(u64)atomic64_read(&zram->stats.compr_data_size));
-}
-
 static ssize_t mem_used_total_show(struct device *dev,
 		struct device_attribute *attr, char *buf)
 {
@@ -760,15 +723,16 @@ static DEVICE_ATTR(disksize, S_IRUGO | S_IWUSR,
 		disksize_show, disksize_store);
 static DEVICE_ATTR(initstate, S_IRUGO, initstate_show, NULL);
 static DEVICE_ATTR(reset, S_IWUSR, NULL, reset_store);
-static DEVICE_ATTR(num_reads, S_IRUGO, num_reads_show, NULL);
-static DEVICE_ATTR(num_writes, S_IRUGO, num_writes_show, NULL);
-static DEVICE_ATTR(invalid_io, S_IRUGO, invalid_io_show, NULL);
-static DEVICE_ATTR(notify_free, S_IRUGO, notify_free_show, NULL);
-static DEVICE_ATTR(zero_pages, S_IRUGO, zero_pages_show, NULL);
 static DEVICE_ATTR(orig_data_size, S_IRUGO, orig_data_size_show, NULL);
-static DEVICE_ATTR(compr_data_size, S_IRUGO, compr_data_size_show, NULL);
 static DEVICE_ATTR(mem_used_total, S_IRUGO, mem_used_total_show, NULL);
 
+ZRAM_ATTR_RO(num_reads);
+ZRAM_ATTR_RO(num_writes);
+ZRAM_ATTR_RO(invalid_io);
+ZRAM_ATTR_RO(notify_free);
+ZRAM_ATTR_RO(zero_pages);
+ZRAM_ATTR_RO(compr_data_size);
+
 static struct attribute *zram_disk_attrs[] = {
 	&dev_attr_disksize.attr,
 	&dev_attr_initstate.attr,
-- 
1.8.5.3.493.gb139ac2


^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [PATCHv3 6/8] zram: report failed read and write stats
  2014-01-16 13:12 [PATCHv3 0/8] zram stats rework and code cleanup Sergey Senozhatsky
                   ` (4 preceding siblings ...)
  2014-01-16 13:12 ` [PATCHv3 5/8] zram: remove zram stats code duplication Sergey Senozhatsky
@ 2014-01-16 13:12 ` Sergey Senozhatsky
  2014-02-04 22:18   ` Andrew Morton
  2014-01-16 13:12 ` [PATCHv3 7/8] zram: drop not used table `count' member Sergey Senozhatsky
                   ` (2 subsequent siblings)
  8 siblings, 1 reply; 22+ messages in thread
From: Sergey Senozhatsky @ 2014-01-16 13:12 UTC (permalink / raw)
  To: Andrew Morton
  Cc: Minchan Kim, Jerome Marchand, Nitin Gupta, linux-kernel,
	Sergey Senozhatsky

zram accounted but did not report numbers of failed read
and write queries. make these stats available as failed_reads
and failed_writes attrs.

Acked-by: Minchan Kim <minchan@kernel.org>
Acked-by: Jerome Marchand <jmarchan@redhat.com>
Signed-off-by: Sergey Senozhatsky <sergey.senozhatsky@gmail.com>
---
 drivers/block/zram/zram_drv.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/block/zram/zram_drv.c b/drivers/block/zram/zram_drv.c
index e7102f1..9d67fbf 100644
--- a/drivers/block/zram/zram_drv.c
+++ b/drivers/block/zram/zram_drv.c
@@ -728,6 +728,8 @@ static DEVICE_ATTR(mem_used_total, S_IRUGO, mem_used_total_show, NULL);
 
 ZRAM_ATTR_RO(num_reads);
 ZRAM_ATTR_RO(num_writes);
+ZRAM_ATTR_RO(failed_reads);
+ZRAM_ATTR_RO(failed_writes);
 ZRAM_ATTR_RO(invalid_io);
 ZRAM_ATTR_RO(notify_free);
 ZRAM_ATTR_RO(zero_pages);
@@ -739,6 +741,8 @@ static struct attribute *zram_disk_attrs[] = {
 	&dev_attr_reset.attr,
 	&dev_attr_num_reads.attr,
 	&dev_attr_num_writes.attr,
+	&dev_attr_failed_reads.attr,
+	&dev_attr_failed_writes.attr,
 	&dev_attr_invalid_io.attr,
 	&dev_attr_notify_free.attr,
 	&dev_attr_zero_pages.attr,
-- 
1.8.5.3.493.gb139ac2


^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [PATCHv3 7/8] zram: drop not used table `count' member
  2014-01-16 13:12 [PATCHv3 0/8] zram stats rework and code cleanup Sergey Senozhatsky
                   ` (5 preceding siblings ...)
  2014-01-16 13:12 ` [PATCHv3 6/8] zram: report failed read and write stats Sergey Senozhatsky
@ 2014-01-16 13:12 ` Sergey Senozhatsky
  2014-01-16 13:52   ` Jerome Marchand
  2014-01-17  7:03   ` Minchan Kim
  2014-01-16 13:12 ` [PATCHv3 8/8] zram: move zram size warning to documentation Sergey Senozhatsky
  2014-01-17  7:09 ` [PATCHv3 0/8] zram stats rework and code cleanup Minchan Kim
  8 siblings, 2 replies; 22+ messages in thread
From: Sergey Senozhatsky @ 2014-01-16 13:12 UTC (permalink / raw)
  To: Andrew Morton
  Cc: Minchan Kim, Jerome Marchand, Nitin Gupta, linux-kernel,
	Sergey Senozhatsky

struct table `count' member is not used.

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

diff --git a/drivers/block/zram/zram_drv.h b/drivers/block/zram/zram_drv.h
index 58d4ac5..1d5b1f5 100644
--- a/drivers/block/zram/zram_drv.h
+++ b/drivers/block/zram/zram_drv.h
@@ -64,7 +64,6 @@ enum zram_pageflags {
 struct table {
 	unsigned long handle;
 	u16 size;	/* object size (excluding header) */
-	u8 count;	/* object ref count (not yet used) */
 	u8 flags;
 } __aligned(4);
 
-- 
1.8.5.3.493.gb139ac2


^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [PATCHv3 8/8] zram: move zram size warning to documentation
  2014-01-16 13:12 [PATCHv3 0/8] zram stats rework and code cleanup Sergey Senozhatsky
                   ` (6 preceding siblings ...)
  2014-01-16 13:12 ` [PATCHv3 7/8] zram: drop not used table `count' member Sergey Senozhatsky
@ 2014-01-16 13:12 ` Sergey Senozhatsky
  2014-01-17  7:08   ` Minchan Kim
  2014-01-17  7:09 ` [PATCHv3 0/8] zram stats rework and code cleanup Minchan Kim
  8 siblings, 1 reply; 22+ messages in thread
From: Sergey Senozhatsky @ 2014-01-16 13:12 UTC (permalink / raw)
  To: Andrew Morton
  Cc: Minchan Kim, Jerome Marchand, Nitin Gupta, linux-kernel,
	Sergey Senozhatsky

Move zram warning about disksize and size of memory correlation
to zram documentation.

Signed-off-by: Sergey Senozhatsky <sergey.senozhatsky@gmail.com>
---
 Documentation/blockdev/zram.txt |  5 +++++
 drivers/block/zram/zram_drv.c   | 15 ---------------
 2 files changed, 5 insertions(+), 15 deletions(-)

diff --git a/Documentation/blockdev/zram.txt b/Documentation/blockdev/zram.txt
index 2eccddf..393541be 100644
--- a/Documentation/blockdev/zram.txt
+++ b/Documentation/blockdev/zram.txt
@@ -33,6 +33,11 @@ Following shows a typical sequence of steps for using zram.
             echo 512M > /sys/block/zram0/disksize
             echo 1G > /sys/block/zram0/disksize
 
+Note:
+There is little point creating a zram of greater than twice the size of memory
+since we expect a 2:1 compression ratio. Note that zram uses about 0.1% of the
+size of the disk when not in use so a huge zram is wasteful.
+
 3) Activate:
 	mkswap /dev/zram0
 	swapon /dev/zram0
diff --git a/drivers/block/zram/zram_drv.c b/drivers/block/zram/zram_drv.c
index 9d67fbf..5ec61be 100644
--- a/drivers/block/zram/zram_drv.c
+++ b/drivers/block/zram/zram_drv.c
@@ -535,23 +535,8 @@ static void zram_reset_device(struct zram *zram, bool reset_capacity)
 
 static void zram_init_device(struct zram *zram, struct zram_meta *meta)
 {
-	if (zram->disksize > 2 * (totalram_pages << PAGE_SHIFT)) {
-		pr_info(
-		"There is little point creating a zram of greater than "
-		"twice the size of memory since we expect a 2:1 compression "
-		"ratio. Note that zram uses about 0.1%% of the size of "
-		"the disk when not in use so a huge zram is "
-		"wasteful.\n"
-		"\tMemory Size: %lu kB\n"
-		"\tSize you selected: %llu kB\n"
-		"Continuing anyway ...\n",
-		(totalram_pages << PAGE_SHIFT) >> 10, zram->disksize >> 10
-		);
-	}
-
 	/* zram devices sort of resembles non-rotational disks */
 	queue_flag_set_unlocked(QUEUE_FLAG_NONROT, zram->disk->queue);
-
 	zram->meta = meta;
 	pr_debug("Initialization done!\n");
 }
-- 
1.8.5.3.493.gb139ac2


^ permalink raw reply related	[flat|nested] 22+ messages in thread

* Re: [PATCHv3 3/8] zram: remove good and bad compress stats
  2014-01-16 13:12 ` [PATCHv3 3/8] zram: remove good and bad compress stats Sergey Senozhatsky
@ 2014-01-16 13:44   ` Jerome Marchand
  2014-01-17  6:48   ` Minchan Kim
  1 sibling, 0 replies; 22+ messages in thread
From: Jerome Marchand @ 2014-01-16 13:44 UTC (permalink / raw)
  To: Sergey Senozhatsky; +Cc: Andrew Morton, Minchan Kim, Nitin Gupta, linux-kernel

On 01/16/2014 02:12 PM, Sergey Senozhatsky wrote:
> Remove `good' and `bad' compressed sub-requests stats. RW request may
> cause a number of RW sub-requests. zram used to account `good' compressed
> sub-queries (with compressed size less than 50% of original size), `bad'
> compressed sub-queries (with compressed size greater that 75% of original
> size), leaving sub-requests with compression size between 50% and 75% of
> original size not accounted and not reported. zram already accounts each
> sub-request's compression size so we can calculate real device compression
> ratio.
> 
> Signed-off-by: Sergey Senozhatsky <sergey.senozhatsky@gmail.com>
Acked-by: Jerome Marchand <jmarchan@redhat.com>

> ---
>  drivers/block/zram/zram_drv.c | 11 -----------
>  drivers/block/zram/zram_drv.h |  2 --
>  2 files changed, 13 deletions(-)
> 
> diff --git a/drivers/block/zram/zram_drv.c b/drivers/block/zram/zram_drv.c
> index b0bcb7e..c7c7789 100644
> --- a/drivers/block/zram/zram_drv.c
> +++ b/drivers/block/zram/zram_drv.c
> @@ -293,7 +293,6 @@ static void zram_free_page(struct zram *zram, size_t index)
>  {
>  	struct zram_meta *meta = zram->meta;
>  	unsigned long handle = meta->table[index].handle;
> -	u16 size = meta->table[index].size;
>  
>  	if (unlikely(!handle)) {
>  		/*
> @@ -307,14 +306,8 @@ static void zram_free_page(struct zram *zram, size_t index)
>  		return;
>  	}
>  
> -	if (unlikely(size > max_zpage_size))
> -		atomic_dec(&zram->stats.bad_compress);
> -
>  	zs_free(meta->mem_pool, handle);
>  
> -	if (size <= PAGE_SIZE / 2)
> -		atomic_dec(&zram->stats.good_compress);
> -
>  	atomic64_sub(meta->table[index].size, &zram->stats.compr_size);
>  	atomic_dec(&zram->stats.pages_stored);
>  
> @@ -478,7 +471,6 @@ static int zram_bvec_write(struct zram *zram, struct bio_vec *bvec, u32 index,
>  	}
>  
>  	if (unlikely(clen > max_zpage_size)) {
> -		atomic_inc(&zram->stats.bad_compress);
>  		clen = PAGE_SIZE;
>  		src = NULL;
>  		if (is_partial_io(bvec))
> @@ -518,9 +510,6 @@ static int zram_bvec_write(struct zram *zram, struct bio_vec *bvec, u32 index,
>  	/* Update stats */
>  	atomic64_add(clen, &zram->stats.compr_size);
>  	atomic_inc(&zram->stats.pages_stored);
> -	if (clen <= PAGE_SIZE / 2)
> -		atomic_inc(&zram->stats.good_compress);
> -
>  out:
>  	if (locked)
>  		mutex_unlock(&meta->buffer_lock);
> diff --git a/drivers/block/zram/zram_drv.h b/drivers/block/zram/zram_drv.h
> index e81e9cd..2f173cb 100644
> --- a/drivers/block/zram/zram_drv.h
> +++ b/drivers/block/zram/zram_drv.h
> @@ -78,8 +78,6 @@ struct zram_stats {
>  	atomic64_t notify_free;	/* no. of swap slot free notifications */
>  	atomic_t pages_zero;		/* no. of zero filled pages */
>  	atomic_t pages_stored;	/* no. of pages currently stored */
> -	atomic_t good_compress;	/* % of pages with compression ratio<=50% */
> -	atomic_t bad_compress;	/* % of pages with compression ratio>=75% */
>  };
>  
>  struct zram_meta {
> 


^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCHv3 7/8] zram: drop not used table `count' member
  2014-01-16 13:12 ` [PATCHv3 7/8] zram: drop not used table `count' member Sergey Senozhatsky
@ 2014-01-16 13:52   ` Jerome Marchand
  2014-01-16 14:17     ` Sergey Senozhatsky
  2014-01-17  7:03   ` Minchan Kim
  1 sibling, 1 reply; 22+ messages in thread
From: Jerome Marchand @ 2014-01-16 13:52 UTC (permalink / raw)
  To: Sergey Senozhatsky; +Cc: Andrew Morton, Minchan Kim, Nitin Gupta, linux-kernel

On 01/16/2014 02:12 PM, Sergey Senozhatsky wrote:
> struct table `count' member is not used.

What was the intended use of this counter anyway?

> 
> Signed-off-by: Sergey Senozhatsky <sergey.senozhatsky@gmail.com>
> ---
>  drivers/block/zram/zram_drv.h | 1 -
>  1 file changed, 1 deletion(-)
> 
> diff --git a/drivers/block/zram/zram_drv.h b/drivers/block/zram/zram_drv.h
> index 58d4ac5..1d5b1f5 100644
> --- a/drivers/block/zram/zram_drv.h
> +++ b/drivers/block/zram/zram_drv.h
> @@ -64,7 +64,6 @@ enum zram_pageflags {
>  struct table {
>  	unsigned long handle;
>  	u16 size;	/* object size (excluding header) */
> -	u8 count;	/* object ref count (not yet used) */
>  	u8 flags;

I noticed that your earlier version of this patch changed the
flags size to 16. What has changed?

Jerome

>  } __aligned(4);
>  
> 


^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCHv3 4/8] zram: use atomic64_t for all zram stats
  2014-01-16 13:12 ` [PATCHv3 4/8] zram: use atomic64_t for all zram stats Sergey Senozhatsky
@ 2014-01-16 13:58   ` Jerome Marchand
  2014-01-17  6:54   ` Minchan Kim
  1 sibling, 0 replies; 22+ messages in thread
From: Jerome Marchand @ 2014-01-16 13:58 UTC (permalink / raw)
  To: Sergey Senozhatsky; +Cc: Andrew Morton, Minchan Kim, Nitin Gupta, linux-kernel

On 01/16/2014 02:12 PM, Sergey Senozhatsky wrote:
> This is a preparation patch for stats code duplication removal.
> 
> 1) use atomic64_t for `pages_zero' and `pages_stored' zram stats.
> 2) `compr_size' and `pages_zero' struct zram_stats members did not
> follow the existing device attr naming scheme: zram_stats.ATTR has
> ATTR_show() function. rename them:
> -- compr_size -> compr_data_size
> -- pages_zero -> zero_pages
> 
> Minchan Kim's note:
>  If we really have trouble with atomic stat operation, we could
>  change it with percpu_counter so that it could solve atomic overhead and
>  unnecessary memory space by introducing unsigned long instead of 64bit
>  atomic_t.
> 
> Signed-off-by: Sergey Senozhatsky <sergey.senozhatsky@gmail.com>
Acked-by: Jerome Marchand <jmarchan@redhat.com>

> ---
>  drivers/block/zram/zram_drv.c | 18 +++++++++---------
>  drivers/block/zram/zram_drv.h |  6 +++---
>  2 files changed, 12 insertions(+), 12 deletions(-)
> 
> diff --git a/drivers/block/zram/zram_drv.c b/drivers/block/zram/zram_drv.c
> index c7c7789..0035d39 100644
> --- a/drivers/block/zram/zram_drv.c
> +++ b/drivers/block/zram/zram_drv.c
> @@ -109,7 +109,7 @@ static ssize_t zero_pages_show(struct device *dev,
>  {
>  	struct zram *zram = dev_to_zram(dev);
>  
> -	return sprintf(buf, "%u\n", atomic_read(&zram->stats.pages_zero));
> +	return sprintf(buf, "%llu\n", (u64)atomic64_read(&zram->stats.zero_pages));
>  }
>  
>  static ssize_t orig_data_size_show(struct device *dev,
> @@ -118,7 +118,7 @@ static ssize_t orig_data_size_show(struct device *dev,
>  	struct zram *zram = dev_to_zram(dev);
>  
>  	return sprintf(buf, "%llu\n",
> -		(u64)(atomic_read(&zram->stats.pages_stored)) << PAGE_SHIFT);
> +		(u64)(atomic64_read(&zram->stats.pages_stored)) << PAGE_SHIFT);
>  }
>  
>  static ssize_t compr_data_size_show(struct device *dev,
> @@ -127,7 +127,7 @@ static ssize_t compr_data_size_show(struct device *dev,
>  	struct zram *zram = dev_to_zram(dev);
>  
>  	return sprintf(buf, "%llu\n",
> -			(u64)atomic64_read(&zram->stats.compr_size));
> +			(u64)atomic64_read(&zram->stats.compr_data_size));
>  }
>  
>  static ssize_t mem_used_total_show(struct device *dev,
> @@ -301,15 +301,15 @@ static void zram_free_page(struct zram *zram, size_t index)
>  		 */
>  		if (zram_test_flag(meta, index, ZRAM_ZERO)) {
>  			zram_clear_flag(meta, index, ZRAM_ZERO);
> -			atomic_dec(&zram->stats.pages_zero);
> +			atomic64_dec(&zram->stats.zero_pages);
>  		}
>  		return;
>  	}
>  
>  	zs_free(meta->mem_pool, handle);
>  
> -	atomic64_sub(meta->table[index].size, &zram->stats.compr_size);
> -	atomic_dec(&zram->stats.pages_stored);
> +	atomic64_sub(meta->table[index].size, &zram->stats.compr_data_size);
> +	atomic64_dec(&zram->stats.pages_stored);
>  
>  	meta->table[index].handle = 0;
>  	meta->table[index].size = 0;
> @@ -452,7 +452,7 @@ static int zram_bvec_write(struct zram *zram, struct bio_vec *bvec, u32 index,
>  		zram_set_flag(meta, index, ZRAM_ZERO);
>  		write_unlock(&zram->meta->tb_lock);
>  
> -		atomic_inc(&zram->stats.pages_zero);
> +		atomic64_inc(&zram->stats.zero_pages);
>  		ret = 0;
>  		goto out;
>  	}
> @@ -508,8 +508,8 @@ static int zram_bvec_write(struct zram *zram, struct bio_vec *bvec, u32 index,
>  	write_unlock(&zram->meta->tb_lock);
>  
>  	/* Update stats */
> -	atomic64_add(clen, &zram->stats.compr_size);
> -	atomic_inc(&zram->stats.pages_stored);
> +	atomic64_add(clen, &zram->stats.compr_data_size);
> +	atomic64_inc(&zram->stats.pages_stored);
>  out:
>  	if (locked)
>  		mutex_unlock(&meta->buffer_lock);
> diff --git a/drivers/block/zram/zram_drv.h b/drivers/block/zram/zram_drv.h
> index 2f173cb..58d4ac5 100644
> --- a/drivers/block/zram/zram_drv.h
> +++ b/drivers/block/zram/zram_drv.h
> @@ -69,15 +69,15 @@ struct table {
>  } __aligned(4);
>  
>  struct zram_stats {
> -	atomic64_t compr_size;	/* compressed size of pages stored */
> +	atomic64_t compr_data_size;	/* compressed size of pages stored */
>  	atomic64_t num_reads;	/* failed + successful */
>  	atomic64_t num_writes;	/* --do-- */
>  	atomic64_t failed_reads;	/* should NEVER! happen */
>  	atomic64_t failed_writes;	/* can happen when memory is too low */
>  	atomic64_t invalid_io;	/* non-page-aligned I/O requests */
>  	atomic64_t notify_free;	/* no. of swap slot free notifications */
> -	atomic_t pages_zero;		/* no. of zero filled pages */
> -	atomic_t pages_stored;	/* no. of pages currently stored */
> +	atomic64_t zero_pages;		/* no. of zero filled pages */
> +	atomic64_t pages_stored;	/* no. of pages currently stored */
>  };
>  
>  struct zram_meta {
> 


^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCHv3 7/8] zram: drop not used table `count' member
  2014-01-16 13:52   ` Jerome Marchand
@ 2014-01-16 14:17     ` Sergey Senozhatsky
  0 siblings, 0 replies; 22+ messages in thread
From: Sergey Senozhatsky @ 2014-01-16 14:17 UTC (permalink / raw)
  To: Jerome Marchand; +Cc: Andrew Morton, Minchan Kim, Nitin Gupta, linux-kernel

On (01/16/14 14:52), Jerome Marchand wrote:
> On 01/16/2014 02:12 PM, Sergey Senozhatsky wrote:
> > struct table `count' member is not used.
> 
> What was the intended use of this counter anyway?
> 
> > 
> > Signed-off-by: Sergey Senozhatsky <sergey.senozhatsky@gmail.com>
> > ---
> >  drivers/block/zram/zram_drv.h | 1 -
> >  1 file changed, 1 deletion(-)
> > 
> > diff --git a/drivers/block/zram/zram_drv.h b/drivers/block/zram/zram_drv.h
> > index 58d4ac5..1d5b1f5 100644
> > --- a/drivers/block/zram/zram_drv.h
> > +++ b/drivers/block/zram/zram_drv.h
> > @@ -64,7 +64,6 @@ enum zram_pageflags {
> >  struct table {
> >  	unsigned long handle;
> >  	u16 size;	/* object size (excluding header) */
> > -	u8 count;	/* object ref count (not yet used) */
> >  	u8 flags;
> 
>

> I noticed that your earlier version of this patch changed the
> flags size to 16. What has changed?
> 

nothing really. I was going to extend flags, but changed my plans. so it was
a leftover.

	-ss

> Jerome
> 
> >  } __aligned(4);
> >  
> > 
> 

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCHv3 3/8] zram: remove good and bad compress stats
  2014-01-16 13:12 ` [PATCHv3 3/8] zram: remove good and bad compress stats Sergey Senozhatsky
  2014-01-16 13:44   ` Jerome Marchand
@ 2014-01-17  6:48   ` Minchan Kim
  1 sibling, 0 replies; 22+ messages in thread
From: Minchan Kim @ 2014-01-17  6:48 UTC (permalink / raw)
  To: Sergey Senozhatsky
  Cc: Andrew Morton, Jerome Marchand, Nitin Gupta, linux-kernel

On Thu, Jan 16, 2014 at 04:12:11PM +0300, Sergey Senozhatsky wrote:
> Remove `good' and `bad' compressed sub-requests stats. RW request may
> cause a number of RW sub-requests. zram used to account `good' compressed
> sub-queries (with compressed size less than 50% of original size), `bad'
> compressed sub-queries (with compressed size greater that 75% of original
> size), leaving sub-requests with compression size between 50% and 75% of
> original size not accounted and not reported. zram already accounts each
> sub-request's compression size so we can calculate real device compression
> ratio.
> 

Most important thing is that we have accounted them but have not used
so it doesn't break anything and we could add them again if someone need them
later. :)

> Signed-off-by: Sergey Senozhatsky <sergey.senozhatsky@gmail.com>

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

-- 
Kind regards,
Minchan Kim

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCHv3 4/8] zram: use atomic64_t for all zram stats
  2014-01-16 13:12 ` [PATCHv3 4/8] zram: use atomic64_t for all zram stats Sergey Senozhatsky
  2014-01-16 13:58   ` Jerome Marchand
@ 2014-01-17  6:54   ` Minchan Kim
  1 sibling, 0 replies; 22+ messages in thread
From: Minchan Kim @ 2014-01-17  6:54 UTC (permalink / raw)
  To: Sergey Senozhatsky
  Cc: Andrew Morton, Jerome Marchand, Nitin Gupta, linux-kernel

On Thu, Jan 16, 2014 at 04:12:12PM +0300, Sergey Senozhatsky wrote:
> This is a preparation patch for stats code duplication removal.
> 
> 1) use atomic64_t for `pages_zero' and `pages_stored' zram stats.
> 2) `compr_size' and `pages_zero' struct zram_stats members did not
> follow the existing device attr naming scheme: zram_stats.ATTR has
> ATTR_show() function. rename them:
> -- compr_size -> compr_data_size
> -- pages_zero -> zero_pages
> 
> Minchan Kim's note:
>  If we really have trouble with atomic stat operation, we could
>  change it with percpu_counter so that it could solve atomic overhead and
>  unnecessary memory space by introducing unsigned long instead of 64bit
>  atomic_t.
> 
> Signed-off-by: Sergey Senozhatsky <sergey.senozhatsky@gmail.com>
Acked-by: Minchan Kim <minchan@kernel.org>

-- 
Kind regards,
Minchan Kim

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCHv3 5/8] zram: remove zram stats code duplication
  2014-01-16 13:12 ` [PATCHv3 5/8] zram: remove zram stats code duplication Sergey Senozhatsky
@ 2014-01-17  6:54   ` Minchan Kim
  0 siblings, 0 replies; 22+ messages in thread
From: Minchan Kim @ 2014-01-17  6:54 UTC (permalink / raw)
  To: Sergey Senozhatsky
  Cc: Andrew Morton, Jerome Marchand, Nitin Gupta, linux-kernel

On Thu, Jan 16, 2014 at 04:12:13PM +0300, Sergey Senozhatsky wrote:
> Introduce ZRAM_ATTR_RO macro that generates device_attribute
> and default ATTR show() function for existing atomic64_t zram
> stats.
> 
> Signed-off-by: Sergey Senozhatsky <sergey.senozhatsky@gmail.com>
Acked-by: Minchan Kim <minchan@kernel.org>

-- 
Kind regards,
Minchan Kim

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCHv3 7/8] zram: drop not used table `count' member
  2014-01-16 13:12 ` [PATCHv3 7/8] zram: drop not used table `count' member Sergey Senozhatsky
  2014-01-16 13:52   ` Jerome Marchand
@ 2014-01-17  7:03   ` Minchan Kim
  1 sibling, 0 replies; 22+ messages in thread
From: Minchan Kim @ 2014-01-17  7:03 UTC (permalink / raw)
  To: Sergey Senozhatsky
  Cc: Andrew Morton, Jerome Marchand, Nitin Gupta, linux-kernel

On Thu, Jan 16, 2014 at 04:12:15PM +0300, Sergey Senozhatsky wrote:
> struct table `count' member is not used.
> 
> Signed-off-by: Sergey Senozhatsky <sergey.senozhatsky@gmail.com>

There is no gain with this patch in memory space ponit of view but it
might reveal the lurking bug.

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

> ---
>  drivers/block/zram/zram_drv.h | 1 -
>  1 file changed, 1 deletion(-)
> 
> diff --git a/drivers/block/zram/zram_drv.h b/drivers/block/zram/zram_drv.h
> index 58d4ac5..1d5b1f5 100644
> --- a/drivers/block/zram/zram_drv.h
> +++ b/drivers/block/zram/zram_drv.h
> @@ -64,7 +64,6 @@ enum zram_pageflags {
>  struct table {
>  	unsigned long handle;
>  	u16 size;	/* object size (excluding header) */
> -	u8 count;	/* object ref count (not yet used) */
>  	u8 flags;
>  } __aligned(4);
>  
> -- 
> 1.8.5.3.493.gb139ac2
> 
> --
> 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/

-- 
Kind regards,
Minchan Kim

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCHv3 8/8] zram: move zram size warning to documentation
  2014-01-16 13:12 ` [PATCHv3 8/8] zram: move zram size warning to documentation Sergey Senozhatsky
@ 2014-01-17  7:08   ` Minchan Kim
  0 siblings, 0 replies; 22+ messages in thread
From: Minchan Kim @ 2014-01-17  7:08 UTC (permalink / raw)
  To: Sergey Senozhatsky
  Cc: Andrew Morton, Jerome Marchand, Nitin Gupta, linux-kernel

On Thu, Jan 16, 2014 at 04:12:16PM +0300, Sergey Senozhatsky wrote:
> Move zram warning about disksize and size of memory correlation
> to zram documentation.
> 
> Signed-off-by: Sergey Senozhatsky <sergey.senozhatsky@gmail.com>
Acked-by: Minchan Kim <minchan@kernel.org>

-- 
Kind regards,
Minchan Kim

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCHv3 0/8] zram stats rework and code cleanup
  2014-01-16 13:12 [PATCHv3 0/8] zram stats rework and code cleanup Sergey Senozhatsky
                   ` (7 preceding siblings ...)
  2014-01-16 13:12 ` [PATCHv3 8/8] zram: move zram size warning to documentation Sergey Senozhatsky
@ 2014-01-17  7:09 ` Minchan Kim
  2014-01-20  4:42   ` Minchan Kim
  8 siblings, 1 reply; 22+ messages in thread
From: Minchan Kim @ 2014-01-17  7:09 UTC (permalink / raw)
  To: Sergey Senozhatsky
  Cc: Andrew Morton, Jerome Marchand, Nitin Gupta, linux-kernel

On Thu, Jan 16, 2014 at 04:12:08PM +0300, Sergey Senozhatsky wrote:
> This patchset includes zram stats clean up and enhancements.
> 
> Sergey Senozhatsky (8):
>   zram: drop `init_done' struct zram member
>   zram: do not pass rw argument to __zram_make_request()
>   zram: remove good and bad compress stats
>   zram: use atomic64_t for all zram stats
>   zram: remove zram stats code duplication
>   zram: report failed read and write stats
>   zram: drop not used table `count' member
>   zram: move zram size warning to documentation
> 
>  Documentation/blockdev/zram.txt |   5 ++
>  drivers/block/zram/zram_drv.c   | 175 +++++++++++++---------------------------
>  drivers/block/zram/zram_drv.h   |  10 +--
>  3 files changed, 64 insertions(+), 126 deletions(-)

Great clean up.

Sergey, Thanks!
I am looking forward to your next step!

-- 
Kind regards,
Minchan Kim

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCHv3 0/8] zram stats rework and code cleanup
  2014-01-17  7:09 ` [PATCHv3 0/8] zram stats rework and code cleanup Minchan Kim
@ 2014-01-20  4:42   ` Minchan Kim
  0 siblings, 0 replies; 22+ messages in thread
From: Minchan Kim @ 2014-01-20  4:42 UTC (permalink / raw)
  To: Sergey Senozhatsky
  Cc: Andrew Morton, Jerome Marchand, Nitin Gupta, linux-kernel

On Fri, Jan 17, 2014 at 04:09:34PM +0900, Minchan Kim wrote:
> On Thu, Jan 16, 2014 at 04:12:08PM +0300, Sergey Senozhatsky wrote:
> > This patchset includes zram stats clean up and enhancements.
> > 
> > Sergey Senozhatsky (8):
> >   zram: drop `init_done' struct zram member
> >   zram: do not pass rw argument to __zram_make_request()
> >   zram: remove good and bad compress stats
> >   zram: use atomic64_t for all zram stats
> >   zram: remove zram stats code duplication
> >   zram: report failed read and write stats
> >   zram: drop not used table `count' member
> >   zram: move zram size warning to documentation
> > 
> >  Documentation/blockdev/zram.txt |   5 ++
> >  drivers/block/zram/zram_drv.c   | 175 +++++++++++++---------------------------
> >  drivers/block/zram/zram_drv.h   |  10 +--
> >  3 files changed, 64 insertions(+), 126 deletions(-)
> 
> Great clean up.
> 
> Sergey, Thanks!
> I am looking forward to your next step!

Hello Sergey,

I guess Andrew wouldn't pick this patchset until closing merge window
so I picked up in my zram tree so let's do further work based on it.

https://git.kernel.org/cgit/linux/kernel/git/minchan/linux.git/log/?h=zram-next

Thanks.

-- 
Kind regards,
Minchan Kim

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCHv3 6/8] zram: report failed read and write stats
  2014-01-16 13:12 ` [PATCHv3 6/8] zram: report failed read and write stats Sergey Senozhatsky
@ 2014-02-04 22:18   ` Andrew Morton
  2014-02-05  9:52     ` Sergey Senozhatsky
  0 siblings, 1 reply; 22+ messages in thread
From: Andrew Morton @ 2014-02-04 22:18 UTC (permalink / raw)
  To: Sergey Senozhatsky
  Cc: Minchan Kim, Jerome Marchand, Nitin Gupta, linux-kernel

On Thu, 16 Jan 2014 16:12:14 +0300 Sergey Senozhatsky <sergey.senozhatsky@gmail.com> wrote:

> zram accounted but did not report numbers of failed read
> and write queries. make these stats available as failed_reads
> and failed_writes attrs.
> 
> Acked-by: Minchan Kim <minchan@kernel.org>
> Acked-by: Jerome Marchand <jmarchan@redhat.com>
> Signed-off-by: Sergey Senozhatsky <sergey.senozhatsky@gmail.com>
> ---
>  drivers/block/zram/zram_drv.c | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/drivers/block/zram/zram_drv.c b/drivers/block/zram/zram_drv.c
> index e7102f1..9d67fbf 100644
> --- a/drivers/block/zram/zram_drv.c
> +++ b/drivers/block/zram/zram_drv.c
> @@ -728,6 +728,8 @@ static DEVICE_ATTR(mem_used_total, S_IRUGO, mem_used_total_show, NULL);
>  
>  ZRAM_ATTR_RO(num_reads);
>  ZRAM_ATTR_RO(num_writes);
> +ZRAM_ATTR_RO(failed_reads);
> +ZRAM_ATTR_RO(failed_writes);
>  ZRAM_ATTR_RO(invalid_io);
>  ZRAM_ATTR_RO(notify_free);
>  ZRAM_ATTR_RO(zero_pages);
> @@ -739,6 +741,8 @@ static struct attribute *zram_disk_attrs[] = {
>  	&dev_attr_reset.attr,
>  	&dev_attr_num_reads.attr,
>  	&dev_attr_num_writes.attr,
> +	&dev_attr_failed_reads.attr,
> +	&dev_attr_failed_writes.attr,
>  	&dev_attr_invalid_io.attr,
>  	&dev_attr_notify_free.attr,
>  	&dev_attr_zero_pages.attr,

Documentation/blockdev/zram.txt and
Documentation/ABI/testing/sysfs-block-zram need updating...


^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCHv3 6/8] zram: report failed read and write stats
  2014-02-04 22:18   ` Andrew Morton
@ 2014-02-05  9:52     ` Sergey Senozhatsky
  0 siblings, 0 replies; 22+ messages in thread
From: Sergey Senozhatsky @ 2014-02-05  9:52 UTC (permalink / raw)
  To: Andrew Morton; +Cc: Minchan Kim, Jerome Marchand, Nitin Gupta, linux-kernel

On (02/04/14 14:18), Andrew Morton wrote:
> Date: Tue, 4 Feb 2014 14:18:49 -0800
> From: Andrew Morton <akpm@linux-foundation.org>
> To: Sergey Senozhatsky <sergey.senozhatsky@gmail.com>
> Cc: Minchan Kim <minchan@kernel.org>, Jerome Marchand
>  <jmarchan@redhat.com>, Nitin Gupta <ngupta@vflare.org>,
>  linux-kernel@vger.kernel.org
> Subject: Re: [PATCHv3 6/8] zram: report failed read and write stats
> X-Mailer: Sylpheed 3.2.0beta5 (GTK+ 2.24.10; x86_64-pc-linux-gnu)
> 
> On Thu, 16 Jan 2014 16:12:14 +0300 Sergey Senozhatsky <sergey.senozhatsky@gmail.com> wrote:
> 
> > zram accounted but did not report numbers of failed read
> > and write queries. make these stats available as failed_reads
> > and failed_writes attrs.
> > 
> > Acked-by: Minchan Kim <minchan@kernel.org>
> > Acked-by: Jerome Marchand <jmarchan@redhat.com>
> > Signed-off-by: Sergey Senozhatsky <sergey.senozhatsky@gmail.com>
> > ---
> >  drivers/block/zram/zram_drv.c | 4 ++++
> >  1 file changed, 4 insertions(+)
> > 
> > diff --git a/drivers/block/zram/zram_drv.c b/drivers/block/zram/zram_drv.c
> > index e7102f1..9d67fbf 100644
> > --- a/drivers/block/zram/zram_drv.c
> > +++ b/drivers/block/zram/zram_drv.c
> > @@ -728,6 +728,8 @@ static DEVICE_ATTR(mem_used_total, S_IRUGO, mem_used_total_show, NULL);
> >  
> >  ZRAM_ATTR_RO(num_reads);
> >  ZRAM_ATTR_RO(num_writes);
> > +ZRAM_ATTR_RO(failed_reads);
> > +ZRAM_ATTR_RO(failed_writes);
> >  ZRAM_ATTR_RO(invalid_io);
> >  ZRAM_ATTR_RO(notify_free);
> >  ZRAM_ATTR_RO(zero_pages);
> > @@ -739,6 +741,8 @@ static struct attribute *zram_disk_attrs[] = {
> >  	&dev_attr_reset.attr,
> >  	&dev_attr_num_reads.attr,
> >  	&dev_attr_num_writes.attr,
> > +	&dev_attr_failed_reads.attr,
> > +	&dev_attr_failed_writes.attr,
> >  	&dev_attr_invalid_io.attr,
> >  	&dev_attr_notify_free.attr,
> >  	&dev_attr_zero_pages.attr,
> 
> Documentation/blockdev/zram.txt and
> Documentation/ABI/testing/sysfs-block-zram need updating...
> 

Hello,
sure, will update.

	-ss

^ permalink raw reply	[flat|nested] 22+ messages in thread

end of thread, other threads:[~2014-02-05  9:55 UTC | newest]

Thread overview: 22+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2014-01-16 13:12 [PATCHv3 0/8] zram stats rework and code cleanup Sergey Senozhatsky
2014-01-16 13:12 ` [PATCHv3 1/8] zram: drop `init_done' struct zram member Sergey Senozhatsky
2014-01-16 13:12 ` [PATCHv3 2/8] zram: do not pass rw argument to __zram_make_request() Sergey Senozhatsky
2014-01-16 13:12 ` [PATCHv3 3/8] zram: remove good and bad compress stats Sergey Senozhatsky
2014-01-16 13:44   ` Jerome Marchand
2014-01-17  6:48   ` Minchan Kim
2014-01-16 13:12 ` [PATCHv3 4/8] zram: use atomic64_t for all zram stats Sergey Senozhatsky
2014-01-16 13:58   ` Jerome Marchand
2014-01-17  6:54   ` Minchan Kim
2014-01-16 13:12 ` [PATCHv3 5/8] zram: remove zram stats code duplication Sergey Senozhatsky
2014-01-17  6:54   ` Minchan Kim
2014-01-16 13:12 ` [PATCHv3 6/8] zram: report failed read and write stats Sergey Senozhatsky
2014-02-04 22:18   ` Andrew Morton
2014-02-05  9:52     ` Sergey Senozhatsky
2014-01-16 13:12 ` [PATCHv3 7/8] zram: drop not used table `count' member Sergey Senozhatsky
2014-01-16 13:52   ` Jerome Marchand
2014-01-16 14:17     ` Sergey Senozhatsky
2014-01-17  7:03   ` Minchan Kim
2014-01-16 13:12 ` [PATCHv3 8/8] zram: move zram size warning to documentation Sergey Senozhatsky
2014-01-17  7:08   ` Minchan Kim
2014-01-17  7:09 ` [PATCHv3 0/8] zram stats rework and code cleanup Minchan Kim
2014-01-20  4:42   ` Minchan Kim

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).