Re: [PATCH v4 7/7] zram: writeback throttle

2018-12-02 Thread Sergey Senozhatsky
On (12/03/18 15:02), Sergey Senozhatsky wrote:
> Seems like I misread writeback_limit_store() a bit.
> 
> So, if I want to, say, let only 10M of writteback pages, I need to
> do
> 
>   echo 0 > writeback_limit
>   echo 10M > writeback_limit_store// memparse format is for
>   // simplicity only; I know
>   // it should be in 4K units.
> 
> every day. How about dropping the "echo 0" and ->stop_writeback?

Ah, this breaks the unlimited writeback.
So, nevermind my comment.

-ss


Re: [PATCH v4 7/7] zram: writeback throttle

2018-12-02 Thread Sergey Senozhatsky
On (12/03/18 15:02), Sergey Senozhatsky wrote:
> Seems like I misread writeback_limit_store() a bit.
> 
> So, if I want to, say, let only 10M of writteback pages, I need to
> do
> 
>   echo 0 > writeback_limit
>   echo 10M > writeback_limit_store// memparse format is for
>   // simplicity only; I know
>   // it should be in 4K units.
> 
> every day. How about dropping the "echo 0" and ->stop_writeback?

Ah, this breaks the unlimited writeback.
So, nevermind my comment.

-ss


Re: [PATCH v4 7/7] zram: writeback throttle

2018-12-02 Thread Sergey Senozhatsky
On (12/03/18 14:50), Sergey Senozhatsky wrote:
> On (12/03/18 11:40), Minchan Kim wrote:
> [..]
> > +   down_read(>init_lock);
> > +   atomic64_set(>stats.bd_wb_limit, val);
> > +   if (val == 0)
> > +   zram->stop_writeback = false;
> > +   up_read(>init_lock);
> 
> [..]
> 
> > +   if (zram->stop_writeback) {
> > +   ret = -EIO;
> > +   break;
> > +   }
> > +
> > if (!blk_idx) {
> > blk_idx = alloc_block_bdev(zram);
> > if (!blk_idx) {
> > @@ -694,6 +732,11 @@ static ssize_t writeback_store(struct device *dev,
> > zram_set_element(zram, index, blk_idx);
> > blk_idx = 0;
> > atomic64_inc(>stats.pages_stored);
> > +   if (atomic64_add_unless(>stats.bd_wb_limit,
> > +   -1 << (PAGE_SHIFT - 12), 0)) {
> > +   if (atomic64_read(>stats.bd_wb_limit) == 0)
> > +   zram->stop_writeback = true;
> > +   }
> 
> Do we need ->stop_writeback? It should be identical to
> 
>   atomic64_read(>stats.bd_wb_limit) == 0

Seems like I misread writeback_limit_store() a bit.

So, if I want to, say, let only 10M of writteback pages, I need to
do

echo 0 > writeback_limit
echo 10M > writeback_limit_store// memparse format is for
// simplicity only; I know
// it should be in 4K units.

every day. How about dropping the "echo 0" and ->stop_writeback?
So then we can just do

echo 10M > writeback_limit_store

every day: if we have ->bd_wb_limit budget then we writeback,
   otherwise we don't.

-ss


Re: [PATCH v4 7/7] zram: writeback throttle

2018-12-02 Thread Sergey Senozhatsky
On (12/03/18 14:50), Sergey Senozhatsky wrote:
> On (12/03/18 11:40), Minchan Kim wrote:
> [..]
> > +   down_read(>init_lock);
> > +   atomic64_set(>stats.bd_wb_limit, val);
> > +   if (val == 0)
> > +   zram->stop_writeback = false;
> > +   up_read(>init_lock);
> 
> [..]
> 
> > +   if (zram->stop_writeback) {
> > +   ret = -EIO;
> > +   break;
> > +   }
> > +
> > if (!blk_idx) {
> > blk_idx = alloc_block_bdev(zram);
> > if (!blk_idx) {
> > @@ -694,6 +732,11 @@ static ssize_t writeback_store(struct device *dev,
> > zram_set_element(zram, index, blk_idx);
> > blk_idx = 0;
> > atomic64_inc(>stats.pages_stored);
> > +   if (atomic64_add_unless(>stats.bd_wb_limit,
> > +   -1 << (PAGE_SHIFT - 12), 0)) {
> > +   if (atomic64_read(>stats.bd_wb_limit) == 0)
> > +   zram->stop_writeback = true;
> > +   }
> 
> Do we need ->stop_writeback? It should be identical to
> 
>   atomic64_read(>stats.bd_wb_limit) == 0

Seems like I misread writeback_limit_store() a bit.

So, if I want to, say, let only 10M of writteback pages, I need to
do

echo 0 > writeback_limit
echo 10M > writeback_limit_store// memparse format is for
// simplicity only; I know
// it should be in 4K units.

every day. How about dropping the "echo 0" and ->stop_writeback?
So then we can just do

echo 10M > writeback_limit_store

every day: if we have ->bd_wb_limit budget then we writeback,
   otherwise we don't.

-ss


Re: [PATCH v4 7/7] zram: writeback throttle

2018-12-02 Thread Sergey Senozhatsky
On (12/03/18 11:40), Minchan Kim wrote:
[..]
> + down_read(>init_lock);
> + atomic64_set(>stats.bd_wb_limit, val);
> + if (val == 0)
> + zram->stop_writeback = false;
> + up_read(>init_lock);

[..]

> + if (zram->stop_writeback) {
> + ret = -EIO;
> + break;
> + }
> +
>   if (!blk_idx) {
>   blk_idx = alloc_block_bdev(zram);
>   if (!blk_idx) {
> @@ -694,6 +732,11 @@ static ssize_t writeback_store(struct device *dev,
>   zram_set_element(zram, index, blk_idx);
>   blk_idx = 0;
>   atomic64_inc(>stats.pages_stored);
> + if (atomic64_add_unless(>stats.bd_wb_limit,
> + -1 << (PAGE_SHIFT - 12), 0)) {
> + if (atomic64_read(>stats.bd_wb_limit) == 0)
> + zram->stop_writeback = true;
> + }

Do we need ->stop_writeback? It should be identical to

atomic64_read(>stats.bd_wb_limit) == 0


Otherwise, looks good!

-ss


Re: [PATCH v4 7/7] zram: writeback throttle

2018-12-02 Thread Sergey Senozhatsky
On (12/03/18 11:40), Minchan Kim wrote:
[..]
> + down_read(>init_lock);
> + atomic64_set(>stats.bd_wb_limit, val);
> + if (val == 0)
> + zram->stop_writeback = false;
> + up_read(>init_lock);

[..]

> + if (zram->stop_writeback) {
> + ret = -EIO;
> + break;
> + }
> +
>   if (!blk_idx) {
>   blk_idx = alloc_block_bdev(zram);
>   if (!blk_idx) {
> @@ -694,6 +732,11 @@ static ssize_t writeback_store(struct device *dev,
>   zram_set_element(zram, index, blk_idx);
>   blk_idx = 0;
>   atomic64_inc(>stats.pages_stored);
> + if (atomic64_add_unless(>stats.bd_wb_limit,
> + -1 << (PAGE_SHIFT - 12), 0)) {
> + if (atomic64_read(>stats.bd_wb_limit) == 0)
> + zram->stop_writeback = true;
> + }

Do we need ->stop_writeback? It should be identical to

atomic64_read(>stats.bd_wb_limit) == 0


Otherwise, looks good!

-ss


[PATCH v4 7/7] zram: writeback throttle

2018-12-02 Thread Minchan Kim
If there are lots of write IO with flash device, it could have a
wearout problem of storage. To overcome the problem, admin needs
to design write limitation to guarantee flash health
for entire product life.

This patch creates a new knob "writeback_limit" on zram.

writeback_limit's default value is 0 so that it doesn't limit
any writeback. If admin want to measure writeback count in a
certain period, he could know it via /sys/block/zram0/bd_stat's
3rd column.

If admin want to limit writeback as per-day 400M, he could do it
like below.

MB_SHIFT=20
4K_SHIFT=12
echo $((400<>4K_SHIFT)) > \
/sys/block/zram0/writeback_limit.

If admin want to allow further write again, he could do it like below

echo 0 > /sys/block/zram0/writeback_limit

If admin want to see remaining writeback budget,

cat /sys/block/zram0/writeback_limit

The writeback_limit count will reset whenever you reset zram(e.g.,
system reboot, echo 1 > /sys/block/zramX/reset) so keeping how many of
writeback happened until you reset the zram to allocate extra writeback
budget in next setting is user's job.

Signed-off-by: Minchan Kim 
---

I removed Reviewed-by from Sergey and Joey because I modified interface
since they had reviewed.

 Documentation/ABI/testing/sysfs-block-zram |  9 
 Documentation/blockdev/zram.txt| 31 +
 drivers/block/zram/zram_drv.c  | 52 --
 drivers/block/zram/zram_drv.h  |  2 +
 4 files changed, 91 insertions(+), 3 deletions(-)

diff --git a/Documentation/ABI/testing/sysfs-block-zram 
b/Documentation/ABI/testing/sysfs-block-zram
index 65fc33b2f53b..9d2339a485c8 100644
--- a/Documentation/ABI/testing/sysfs-block-zram
+++ b/Documentation/ABI/testing/sysfs-block-zram
@@ -121,3 +121,12 @@ Contact:   Minchan Kim 
The bd_stat file is read-only and represents backing device's
statistics (bd_count, bd_reads, bd_writes) in a format
similar to block layer statistics file format.
+
+What:  /sys/block/zram/writeback_limit
+Date:  November 2018
+Contact:   Minchan Kim 
+Description:
+   The writeback_limit file is read-write and specifies the maximum
+   amount of writeback ZRAM can do. The limit could be changed
+   in run time and "0" means disable the limit.
+   No limit is the initial state.
diff --git a/Documentation/blockdev/zram.txt b/Documentation/blockdev/zram.txt
index 906df97527a7..436c5e98e1b6 100644
--- a/Documentation/blockdev/zram.txt
+++ b/Documentation/blockdev/zram.txt
@@ -164,6 +164,8 @@ reset WOtrigger device reset
 mem_used_max  WOreset the `mem_used_max' counter (see later)
 mem_limit WOspecifies the maximum amount of memory ZRAM can use
 to store the compressed data
+writeback_limit   WOspecifies the maximum amount of write IO zram can
+   write out to backing device as 4KB unit
 max_comp_streams  RWthe number of possible concurrent compress operations
 comp_algorithmRWshow and change the compression algorithm
 compact   WOtrigger memory compaction
@@ -275,6 +277,35 @@ Admin can request writeback of those idle pages at right 
timing via
 
 With the command, zram writeback idle pages from memory to the storage.
 
+If there are lots of write IO with flash device, potentially, it has
+flash wearout problem so that admin needs to design write limitation
+to guarantee storage health for entire product life.
+To overcome the concern, zram supports "writeback_limit".
+The "writeback_limit"'s default value is 0 so that it doesn't limit
+any writeback. If admin want to measure writeback count in a certain
+period, he could know it via /sys/block/zram0/bd_stat's 3rd column.
+
+If admin want to limit writeback as per-day 400M, he could do it
+like below.
+
+MB_SHIFT=20
+4K_SHIFT=12
+echo $((400<>4K_SHIFT)) > \
+   /sys/block/zram0/writeback_limit.
+
+If admin want to allow further write again, he could do it like below
+
+echo 0 > /sys/block/zram0/writeback_limit
+
+If admin want to see remaining writeback budget since he set,
+
+cat /sys/block/zram0/writeback_limit
+
+The writeback_limit count will reset whenever you reset zram(e.g.,
+system reboot, echo 1 > /sys/block/zramX/reset) so keeping how many of
+writeback happened until you reset the zram to allocate extra writeback
+budget in next setting is user's job.
+
 = memory tracking
 
 With CONFIG_ZRAM_MEMORY_TRACKING, user can know information of the
diff --git a/drivers/block/zram/zram_drv.c b/drivers/block/zram/zram_drv.c
index f1832fa3ba41..33c5cc879f24 100644
--- a/drivers/block/zram/zram_drv.c
+++ b/drivers/block/zram/zram_drv.c
@@ -330,6 +330,39 @@ static ssize_t idle_store(struct device *dev,
 }
 
 #ifdef CONFIG_ZRAM_WRITEBACK
+static ssize_t writeback_limit_store(struct device *dev,
+   

[PATCH v4 7/7] zram: writeback throttle

2018-12-02 Thread Minchan Kim
If there are lots of write IO with flash device, it could have a
wearout problem of storage. To overcome the problem, admin needs
to design write limitation to guarantee flash health
for entire product life.

This patch creates a new knob "writeback_limit" on zram.

writeback_limit's default value is 0 so that it doesn't limit
any writeback. If admin want to measure writeback count in a
certain period, he could know it via /sys/block/zram0/bd_stat's
3rd column.

If admin want to limit writeback as per-day 400M, he could do it
like below.

MB_SHIFT=20
4K_SHIFT=12
echo $((400<>4K_SHIFT)) > \
/sys/block/zram0/writeback_limit.

If admin want to allow further write again, he could do it like below

echo 0 > /sys/block/zram0/writeback_limit

If admin want to see remaining writeback budget,

cat /sys/block/zram0/writeback_limit

The writeback_limit count will reset whenever you reset zram(e.g.,
system reboot, echo 1 > /sys/block/zramX/reset) so keeping how many of
writeback happened until you reset the zram to allocate extra writeback
budget in next setting is user's job.

Signed-off-by: Minchan Kim 
---

I removed Reviewed-by from Sergey and Joey because I modified interface
since they had reviewed.

 Documentation/ABI/testing/sysfs-block-zram |  9 
 Documentation/blockdev/zram.txt| 31 +
 drivers/block/zram/zram_drv.c  | 52 --
 drivers/block/zram/zram_drv.h  |  2 +
 4 files changed, 91 insertions(+), 3 deletions(-)

diff --git a/Documentation/ABI/testing/sysfs-block-zram 
b/Documentation/ABI/testing/sysfs-block-zram
index 65fc33b2f53b..9d2339a485c8 100644
--- a/Documentation/ABI/testing/sysfs-block-zram
+++ b/Documentation/ABI/testing/sysfs-block-zram
@@ -121,3 +121,12 @@ Contact:   Minchan Kim 
The bd_stat file is read-only and represents backing device's
statistics (bd_count, bd_reads, bd_writes) in a format
similar to block layer statistics file format.
+
+What:  /sys/block/zram/writeback_limit
+Date:  November 2018
+Contact:   Minchan Kim 
+Description:
+   The writeback_limit file is read-write and specifies the maximum
+   amount of writeback ZRAM can do. The limit could be changed
+   in run time and "0" means disable the limit.
+   No limit is the initial state.
diff --git a/Documentation/blockdev/zram.txt b/Documentation/blockdev/zram.txt
index 906df97527a7..436c5e98e1b6 100644
--- a/Documentation/blockdev/zram.txt
+++ b/Documentation/blockdev/zram.txt
@@ -164,6 +164,8 @@ reset WOtrigger device reset
 mem_used_max  WOreset the `mem_used_max' counter (see later)
 mem_limit WOspecifies the maximum amount of memory ZRAM can use
 to store the compressed data
+writeback_limit   WOspecifies the maximum amount of write IO zram can
+   write out to backing device as 4KB unit
 max_comp_streams  RWthe number of possible concurrent compress operations
 comp_algorithmRWshow and change the compression algorithm
 compact   WOtrigger memory compaction
@@ -275,6 +277,35 @@ Admin can request writeback of those idle pages at right 
timing via
 
 With the command, zram writeback idle pages from memory to the storage.
 
+If there are lots of write IO with flash device, potentially, it has
+flash wearout problem so that admin needs to design write limitation
+to guarantee storage health for entire product life.
+To overcome the concern, zram supports "writeback_limit".
+The "writeback_limit"'s default value is 0 so that it doesn't limit
+any writeback. If admin want to measure writeback count in a certain
+period, he could know it via /sys/block/zram0/bd_stat's 3rd column.
+
+If admin want to limit writeback as per-day 400M, he could do it
+like below.
+
+MB_SHIFT=20
+4K_SHIFT=12
+echo $((400<>4K_SHIFT)) > \
+   /sys/block/zram0/writeback_limit.
+
+If admin want to allow further write again, he could do it like below
+
+echo 0 > /sys/block/zram0/writeback_limit
+
+If admin want to see remaining writeback budget since he set,
+
+cat /sys/block/zram0/writeback_limit
+
+The writeback_limit count will reset whenever you reset zram(e.g.,
+system reboot, echo 1 > /sys/block/zramX/reset) so keeping how many of
+writeback happened until you reset the zram to allocate extra writeback
+budget in next setting is user's job.
+
 = memory tracking
 
 With CONFIG_ZRAM_MEMORY_TRACKING, user can know information of the
diff --git a/drivers/block/zram/zram_drv.c b/drivers/block/zram/zram_drv.c
index f1832fa3ba41..33c5cc879f24 100644
--- a/drivers/block/zram/zram_drv.c
+++ b/drivers/block/zram/zram_drv.c
@@ -330,6 +330,39 @@ static ssize_t idle_store(struct device *dev,
 }
 
 #ifdef CONFIG_ZRAM_WRITEBACK
+static ssize_t writeback_limit_store(struct device *dev,
+